Add service_opt() incoming_maxlen

To bound the length of incoming messages that will be decoded. A message
longer than the specified number of bytes is discarded. An
incoming_maxlen_exceeded counter is incremented to make note of the
occurrence.

The motivation is to prevent a sufficiently malicious peer from
generating significant load by sending long messages with many AVPs for
diameter to decode. The 24-bit message length header accomodates

  (16#FFFFFF - 20) div 12 = 1398099

Unsigned32 AVPs for example, which the current record-valued decode is
too slow with in practice. A bound of 16#FFFF bytes allows for 5461
small AVPs, which is probably more than enough for the majority of
applications, but the default is the full 16#FFFFFF.

8 files changed, 77 insertions, 8 deletions

diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index 37e67d8630..6e41b01c44 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -783,6 +783,27 @@ be matched by corresponding &capability; configuration, of
 
 </item>
 
+<marker id="incoming_maxlen"/>
+<tag><c>{incoming_maxlen, 0..16777215}</c></tag>
+<item>
+<p>
+Bound on the expected size of incoming Diameter messages.
+Messages larger than the specified number of bytes are discarded.</p>
+
+<p>
+Defaults to <c>16777215</c>, the maximum value of the 24-bit Message
+Length field in a Diameter Header.</p>
+
+<warning>
+<p>
+This option should be set to as low a value as is sufficient for the
+Diameter applications and peers in question, since decoding incoming
+messages from a malicious peer can otherwise generate significant
+load.</p>
+</warning>
+
+</item>
+
 <tag><c>{restrict_connections, false
                              | node
                              | nodes
diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl
index 67dfc7bdbf..010f977b97 100644
--- a/lib/diameter/src/base/diameter.erl
+++ b/lib/diameter/src/base/diameter.erl
@@ -45,6 +45,7 @@
 
 -export_type([evaluable/0,
               restriction/0,
+              message_length/0,
               remotes/0,
               sequence/0,
               app_alias/0,
@@ -298,6 +299,9 @@ call(SvcName, App, Message) ->
     | [node()]
     | evaluable().
 
+-type message_length()
+   :: 0..16#FFFFFF.
+
 %% Options passed to start_service/2
 
 -type service_opt()
@@ -307,6 +311,7 @@ call(SvcName, App, Message) ->
     | {sequence, sequence() | evaluable()}
     | {share_peers, remotes()}
     | {string_decode, boolean()}
+    | {incoming_maxlen, message_length()}
     | {use_shared_peers, remotes()}
     | {spawn_opt, list()}.
 
diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl
index edfb91dbf4..8ac3b9d6ca 100644
--- a/lib/diameter/src/base/diameter_config.erl
+++ b/lib/diameter/src/base/diameter_config.erl
@@ -159,7 +159,8 @@ stop_service(SvcName) ->
 %% # add_transport/2
 %% --------------------------------------------------------------------------
 
--spec add_transport(diameter:service_name(), {connect|listen, [diameter:transport_opt()]})
+-spec add_transport(diameter:service_name(),
+                    {connect|listen, [diameter:transport_opt()]})
    -> {ok, diameter:transport_ref()}
     | {error, term()}.
 
@@ -645,6 +646,7 @@ make_config(SvcName, Opts) ->
                          {false, monitor},
                          {?NOMASK, sequence},
                          {nodes, restrict_connections},
+                         {16#FFFFFF, incoming_maxlen},
                          {true, string_decode},
                          {[], spawn_opt}]),
 
@@ -670,6 +672,10 @@ make_opts(Opts, Defs) ->
 
     [{K, opt(K,V)} || {K,V} <- Known].
 
+opt(incoming_maxlen, N)
+  when 0 =< N, N < 1 bsl 24 ->
+    N;
+
 opt(spawn_opt, L)
   when is_list(L) ->
     L;
diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl
index aac2685514..2255d0a76b 100644
--- a/lib/diameter/src/base/diameter_peer_fsm.erl
+++ b/lib/diameter/src/base/diameter_peer_fsm.erl
@@ -125,7 +125,8 @@
                        %% outgoing DPR; boolean says whether or not
                        %% the request was sent explicitly with
                        %% diameter:call/4.
-         length_errors :: exit | handle | discard}).
+         length_errors :: exit | handle | discard,
+         incoming_maxlen :: integer() | infinity}).
 
 %% There are non-3588 states possible as a consequence of 5.6.1 of the
 %% standard and the corresponding problem for incoming CEA's: we don't
@@ -203,6 +204,7 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) ->
     diameter_stats:reg(Ref),
     diameter_codec:setopts([{common_dictionary, Dict0} | SvcOpts]),
     {_,_} = Mask = proplists:get_value(sequence, SvcOpts),
+    Maxlen = proplists:get_value(incoming_maxlen, SvcOpts, 16#FFFFFF),
     {[Cs,Ds], Rest} = proplists:split(Opts, [capabilities_cb, disconnect_cb]),
     putr(?CB_KEY, {Ref, [F || {_,F} <- Cs]}),
     putr(?DPR_KEY, [F || {_, F} <- Ds]),
@@ -223,7 +225,8 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) ->
            dictionary = Dict0,
            mode = M,
            service = svc(Svc, Addrs),
-           length_errors = OnLengthErr}.
+           length_errors = OnLengthErr,
+           incoming_maxlen = Maxlen}.
 %% The transport returns its local ip addresses so that different
 %% transports on the same service can use different local addresses.
 %% The local addresses are put into Host-IP-Address avps here when
@@ -326,11 +329,14 @@ handle_info(T, #state{} = State) ->
         {?MODULE, Tag, Reason}  ->
             ?LOG(stop, Tag),
             {stop, {shutdown, Reason}, State}
-    end.
+    end;
 %% The form of the throw caught here is historical. It's
 %% significant that it's not a 2-tuple, as in ?FAILURE(Reason),
 %% since these are caught elsewhere.
 
+handle_info(T, S) ->  %% started in old code
+    handle_info(T, #state{} = erlang:append_element(S, infinity)).
+
 %% Note that there's no guarantee that the service and transport
 %% capabilities are good enough to build a CER/CEA that can be
 %% succesfully encoded. It's not checked at diameter:add_transport/2
@@ -561,6 +567,12 @@ recv(Bin, S) ->
 
 %% recv1/3
 
+recv1(_,
+      #diameter_packet{header = H, bin = Bin},
+      #state{incoming_maxlen = M})
+  when M < size(Bin) ->
+    invalid(false, incoming_maxlen_exceeded, {size(Bin), H});
+
 %% Incoming request after outgoing DPR: discard. Don't discard DPR, so
 %% both ends don't do so when sending simultaneously.
 recv1(Name,
diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl
index a01bcdd4e7..86e744dfbe 100644
--- a/lib/diameter/src/base/diameter_service.erl
+++ b/lib/diameter/src/base/diameter_service.erl
@@ -131,7 +131,8 @@
              | {share_peers, diameter:remotes()}       %% broadcast to
              | {use_shared_peers, diameter:remotes()}  %% use from
              | {restrict_connections, diameter:restriction()}
-             | {string_decode, boolean()}]}).
+             | {string_decode, boolean()}
+             | {incoming_maxlen, diameter:message_length()}]}).
 %% shared_peers reflects the peers broadcast from remote nodes.
 
 %% Record representing an RFC 3539 watchdog process implemented by
@@ -698,7 +699,8 @@ service_options(Opts) ->
                                                 Opts,
                                                 ?RESTRICT)},
      {spawn_opt, proplists:get_value(spawn_opt, Opts, [])},
-     {string_decode, proplists:get_value(string_decode, Opts, true)}].
+     {string_decode, proplists:get_value(string_decode, Opts, true)},
+     {incoming_maxlen, proplists:get_value(incoming_maxlen, Opts, 16#FFFFFF)}].
 %% The order of options is significant since we match against the list.
 
 mref(false = No) ->
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 784f9ca08f..538ebeeeba 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -78,7 +78,11 @@
          service_name :: diameter:service_name(),
          apps         :: [#diameter_app{}],
          sequence     :: diameter:sequence(),
-         codec        :: list()}).
+         codec        :: [{string_decode, boolean()}
+                          | {incoming_maxlen, diameter:message_length()}]}).
+%% Note that incoming_maxlen is currently handled in diameter_peer_fsm,
+%% so that any message exceeding the maximum is discarded. Retain the
+%% option in case we want to extend the values and semantics.
 
 %% Record stored in diameter_request for each outgoing request.
 -record(request,
@@ -102,7 +106,9 @@ make_recvdata([SvcName, PeerT, Apps, SvcOpts | _]) ->
               peerT = PeerT,
               apps = Apps,
               sequence = Mask,
-              codec = [T || {K,_} = T <- SvcOpts, K == string_decode]}.
+              codec = [T || {K,_} = T <- SvcOpts,
+                            lists:member(K, [string_decode,
+                                             incoming_maxlen])]}.
 
 %% ---------------------------------------------------------------------------
 %% peer_up/1
@@ -233,6 +239,8 @@ receive_message(TPid, Pkt, Dict0, RecvData)
          Dict0,
          RecvData).
 
+%% recv/6
+
 %% Incoming request ...
 recv(true, false, TPid, Pkt, Dict0, T) ->
     spawn_request(TPid, Pkt, Dict0, T);
@@ -240,6 +248,7 @@ recv(true, false, TPid, Pkt, Dict0, T) ->
 %% ... answer to known request ...
 recv(false, #request{ref = Ref, handler = Pid} = Req, _, Pkt, Dict0, _) ->
     Pid ! {answer, Ref, Req, Dict0, Pkt};
+
 %% Note that failover could have happened prior to this message being
 %% received and triggering failback. That is, both a failover message
 %% and answer may be on their way to the handler process. In the worst
@@ -1693,6 +1702,8 @@ send({TPid, Pkt, #request{handler = Pid} = Req0, SvcName, Timeout, TRef}) ->
     end.
 
 %% recv/4
+%%
+%% Relay an answer from a remote node.
 
 recv(TPid, Pid, TRef, Ref) ->        
     receive
diff --git a/lib/diameter/test/diameter_config_SUITE.erl b/lib/diameter/test/diameter_config_SUITE.erl
index ea77aa3716..bbdf672291 100644
--- a/lib/diameter/test/diameter_config_SUITE.erl
+++ b/lib/diameter/test/diameter_config_SUITE.erl
@@ -85,6 +85,9 @@
          {string_decode,
           [[true], [false]],
           [[0], [x]]},
+         {incoming_maxlen,
+          [[0], [65536], [16#FFFFFF]],
+          [[-1], [1 bsl 24], [infinity], [false]]},
          {spawn_opt,
           [[[]], [[monitor, link]]],
           [[false]]},
diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index 10c58ab6e7..7dd9f39f85 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -59,6 +59,7 @@
          send_unexpected_mandatory_decode/1,
          send_unexpected_mandatory/1,
          send_long/1,
+         send_maxlen/1,
          send_nopeer/1,
          send_noapp/1,
          send_discard/1,
@@ -182,6 +183,7 @@
          {'Acct-Application-Id', [?DIAMETER_APP_ID_ACCOUNTING]},
          {restrict_connections, false},
          {string_decode, Decode},
+         {incoming_maxlen, 1 bsl 21},
          {spawn_opt, [{min_heap_size, 5000}]}
          | [{application, [{dictionary, D},
                            {module, ?MODULE},
@@ -317,6 +319,7 @@ tc() ->
      send_unexpected_mandatory_decode,
      send_unexpected_mandatory,
      send_long,
+     send_maxlen,
      send_nopeer,
      send_noapp,
      send_discard,
@@ -635,6 +638,12 @@ send_long(Config) ->
     ['STA', _SessionId, {'Result-Code', ?SUCCESS} | _]
         = call(Config, Req).
 
+%% Send something longer than the configure incoming_maxlen.
+send_maxlen(Config) ->
+    Req = ['STR', {'Termination-Cause', ?LOGOUT},
+                  {'User-Name', [lists:duplicate(1 bsl 21, $X)]}],
+    {timeout, _} = call(Config, Req).
+
 %% Send something for which pick_peer finds no suitable peer.
 send_nopeer(Config) ->
     Req = ['STR', {'Termination-Cause', ?LOGOUT}],