Merge branch 'anders/diameter/hardening/OTP-11721' into maint

* anders/diameter/hardening/OTP-11721:
  Simplify example server
  Make example server answer unsupported requests with 3001
  Make example code quiet
  Don't count messages on arbitrary keys
  Replace traffic-related log reports with no-op function calls

1 files changed, 80 insertions, 50 deletions

diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index f2ac745053..5fac61f416 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -32,7 +32,8 @@
 -export([receive_message/4]).
 
 %% towards diameter_peer_fsm and diameter_watchdog
--export([incr_error/3,
+-export([incr/4,
+         incr_error/4,
          incr_rc/4]).
 
 %% towards diameter_service
@@ -48,6 +49,8 @@
 -include_lib("diameter/include/diameter.hrl").
 -include("diameter_internal.hrl").
 
+-define(LOGX(Reason, T), begin ?LOG(Reason, T), x({Reason, T}) end).
+
 -define(RELAY, ?DIAMETER_DICT_RELAY).
 -define(BASE,  ?DIAMETER_DICT_COMMON).  %% Note: the RFC 3588 dictionary
 
@@ -113,38 +116,52 @@ peer_down(TPid) ->
     failover(TPid).
 
 %% ---------------------------------------------------------------------------
-%% incr_error/3
+%% incr/4
 %% ---------------------------------------------------------------------------
 
-%% A decoded message with errors.
-incr_error(Dir, #diameter_packet{header = H, errors = [_|_]}, TPid) ->
-    incr_error(Dir, H, TPid);
+incr(Dir, #diameter_packet{header = H}, TPid, Dict) ->
+    incr(Dir, H, TPid, Dict);
 
-%% An encoded message with errors and an identifiable header ...
-incr_error(Dir, {_, _, #diameter_header{} = H}, TPid) ->
-    incr_error(Dir, H, TPid);
+incr(Dir, #diameter_header{} = H, TPid, Dict) ->
+    incr(TPid, {msg_id(H, Dict), Dir}).
 
-%% ... or not.
-incr_error(Dir, {_,_}, TPid) ->
-    incr(TPid, {unknown, Dir, error});
+%% ---------------------------------------------------------------------------
+%% incr_error/4
+%% ---------------------------------------------------------------------------
 
-incr_error(Dir, #diameter_header{} = H, TPid) ->
-    incr_error(Dir, diameter_codec:msg_id(H), TPid);
+%% Decoded message without errors.
+incr_error(recv, #diameter_packet{errors = []}, _, _) ->
+    ok;
 
-incr_error(Dir, {_,_,_} = Id, TPid) ->
-    incr(TPid, {Id, Dir, error});
+incr_error(recv = D, #diameter_packet{header = H}, TPid, Dict) ->
+    incr_error(D, H, TPid, Dict);
 
-incr_error(_, _, _) ->
-    false.
+%% Encoded message with errors and an identifiable header ...
+incr_error(send = D, {_, _, #diameter_header{} = H}, TPid, Dict) ->
+    incr_error(D, H, TPid, Dict);
+
+%% ... or not.
+incr_error(send = D, {_,_}, TPid, _) ->
+    incr_error(D, unknown, TPid);
 
+incr_error(Dir, #diameter_header{} = H, TPid, Dict) ->
+    incr_error(Dir, msg_id(H, Dict), TPid);
+
+incr_error(Dir, Id, TPid, _) ->
+    incr_error(Dir, Id, TPid).
+
+incr_error(Dir, Id, TPid) ->
+    incr(TPid, {Id, Dir, error}).
+    
 %% ---------------------------------------------------------------------------
 %% incr_rc/4
 %% ---------------------------------------------------------------------------
 
--spec incr_rc(send|recv, #diameter_packet{}, TPid, Dict0)
+-spec incr_rc(send|recv, Pkt, TPid, Dict0)
    -> {Counter, non_neg_integer()}
     | Reason
- when TPid :: pid(),
+ when Pkt :: #diameter_packet{},
+      TPid :: pid(),
       Dict0 :: module(),
       Counter :: {'Result-Code', integer()}
                | {'Experimental-Result', integer(), integer()},
@@ -154,9 +171,8 @@ incr_rc(Dir, Pkt, TPid, Dict0) ->
     try
         incr_rc(Dir, Pkt, Dict0, TPid, Dict0)
     catch
-        exit: {invalid_error_bit = E, _} ->
-            E;
-        exit: no_result_code = E ->
+        exit: {E,_} when E == no_result_code;
+                         E == invalid_error_bit ->
             E
     end.
 
@@ -234,7 +250,7 @@ spawn_request(TPid, Pkt, Dict0, Opts, RecvData) ->
         spawn_opt(fun() -> recv_request(TPid, Pkt, Dict0, RecvData) end, Opts)
     catch
         error: system_limit = E ->  %% discard
-            ?LOG({error, E}, now())
+            ?LOG(error, E)
     end.
 
 %% ---------------------------------------------------------------------------
@@ -263,8 +279,9 @@ recv_R({#diameter_app{id = Id, dictionary = Dict} = App, Caps},
        Pkt0,
        Dict0,
        RecvData) ->
+    incr(recv, Pkt0, TPid, Dict),
     Pkt = errors(Id, diameter_codec:decode(Id, Dict, Pkt0)),
-    incr_error(recv, Pkt, TPid),
+    incr_error(recv, Pkt, TPid, Dict),
     {Caps, Pkt, App, recv_R(App, TPid, Dict0, Caps, RecvData, Pkt)};
 %% Note that the decode is different depending on whether or not Id is
 %% ?APP_ID_RELAY.
@@ -336,23 +353,25 @@ rc(N) ->
 %%      This error is returned when a request is received with an invalid
 %%      message length.
 
-errors(_, #diameter_packet{header = #diameter_header{length = Len},
+errors(_, #diameter_packet{header = #diameter_header{length = Len} = H,
                            bin = Bin,
                            errors = Es}
           = Pkt)
   when Len < 20;
        0 /= Len rem 4;
        8*Len /= bit_size(Bin) ->
+    ?LOG(invalid_message_length, {H, bit_size(Bin)}),
     Pkt#diameter_packet{errors = [5015 | Es]};
 
 %%   DIAMETER_UNSUPPORTED_VERSION       5011
 %%      This error is returned when a request was received, whose version
 %%      number is unsupported.
 
-errors(_, #diameter_packet{header = #diameter_header{version = V},
+errors(_, #diameter_packet{header = #diameter_header{version = V} = H,
                            errors = Es}
           = Pkt)
   when V /= ?DIAMETER_VERSION ->
+    ?LOG(unsupported_version, H),
     Pkt#diameter_packet{errors = [5011 | Es]};
 
 %%   DIAMETER_COMMAND_UNSUPPORTED       3001
@@ -360,12 +379,13 @@ errors(_, #diameter_packet{header = #diameter_header{version = V},
 %%      recognize or support.  This MUST be used when a Diameter node
 %%      receives an experimental command that it does not understand.
 
-errors(Id, #diameter_packet{header = #diameter_header{is_proxiable = P},
+errors(Id, #diameter_packet{header = #diameter_header{is_proxiable = P} = H,
                             msg = M,
                             errors = Es}
            = Pkt)
   when ?APP_ID_RELAY /= Id, undefined == M;  %% don't know the command
        ?APP_ID_RELAY == Id, not P ->         %% command isn't proxiable
+    ?LOG(command_unsupported, H),
     Pkt#diameter_packet{errors = [3001 | Es]};
 
 %%   DIAMETER_INVALID_HDR_BITS          3008
@@ -374,9 +394,11 @@ errors(Id, #diameter_packet{header = #diameter_header{is_proxiable = P},
 %%      inconsistent with the command code's definition.
 
 errors(_, #diameter_packet{header = #diameter_header{is_request = true,
-                                                     is_error = true},
+                                                     is_error = true}
+                                  = H,
                             errors = Es}
           = Pkt) ->
+    ?LOG(invalid_hdr_bits, H),
     Pkt#diameter_packet{errors = [3008 | Es]};
 
 %% Green.
@@ -532,7 +554,6 @@ answer_message(RC,
                               origin_realm = {OR,_}},
                Dict0,
                Pkt) ->
-    ?LOG({error, RC}, Pkt),
     {Dict0, answer_message(OH, OR, RC, Dict0, Pkt)}.
 
 %% resend/7
@@ -651,6 +672,7 @@ reply(Msg, Dict, TPid, Dict0, Fs, ReqPkt) ->
                  TPid,
                  reset(make_answer_packet(Msg, ReqPkt), Dict, Dict0),
                  Fs),
+    incr(send, Pkt, TPid, Dict),
     incr_rc(send, Pkt, Dict, TPid, Dict0),  %% count outgoing
     send(TPid, Pkt).
 
@@ -1035,21 +1057,29 @@ incr_rc(Dir, Pkt, Dict, TPid, Dict0) ->
                      errors = Es}
         = Pkt,
 
-    Id = diameter_codec:msg_id(Hdr),
+    Id = msg_id(Hdr, Dict),
 
     %% Count incoming decode errors.
-    recv /= Dir orelse [] == Es orelse incr_error(Dir, Id, TPid),
+    recv /= Dir orelse [] == Es orelse incr_error(Dir, Id, TPid, Dict),
 
     %% Exit on a missing result code.
     T = rc_counter(Dict, Msg),
-    T == false andalso x(no_result_code, answer, [Dir, Pkt]),
+    T == false andalso ?LOGX(no_result_code, {Dict, Dir, Hdr}),
     {Ctr, RC} = T,
 
     %% Or on an inappropriate value.
     is_result(RC, E, Dict0)
-        orelse x({invalid_error_bit, RC}, answer, [Dir, Pkt]),
+        orelse ?LOGX(invalid_error_bit, {Dict, Dir, Hdr, RC}),
 
-    incr(TPid, {Id, Dir, Ctr}).
+    incr(TPid, {Id, Dir, Ctr}),
+    Ctr.
+
+%% Only count on known keeps so as not to be vulnerable to attack:
+%% there are 2^32 (application ids) * 2^24 (command codes) * 2 (R-bits)
+%% = 2^57 Ids for an attacker to choose from.
+msg_id(Hdr, Dict) ->
+    {_ApplId, Code, R} = Id = diameter_codec:msg_id(Hdr),
+    choose('' == Dict:msg_name(Code, 0 == R), unknown, Id).
 
 %% No E-bit: can't be 3xxx.
 is_result(RC, false, _Dict0) ->
@@ -1067,8 +1097,8 @@ is_result(RC, true, _) ->
 
 %% incr/2
 
-incr(TPid, {_, _, T} = Counter) ->
-    {T, diameter_stats:incr(Counter, TPid, 1)}.
+incr(TPid, Counter) ->
+    diameter_stats:incr(Counter, TPid, 1).
 
 %% rc_counter/2
 
@@ -1112,13 +1142,6 @@ int(N)
 int(_) ->
     undefined.
 
--spec x(any(), atom(), list()) -> no_return().
-
-%% Warn and exit request process on errors in an incoming answer.
-x(Reason, F, A) ->
-    diameter_lib:warning_report(Reason, {?MODULE, F, A}),
-    x(Reason).
-
 x(T) ->
     exit(T).
 
@@ -1425,12 +1448,14 @@ handle_answer(SvcName,
 %% want to examine the answer?
 
 handle_A(Pkt, SvcName, Dict, Dict0, App, #request{transport = TPid} = Req) ->
+    incr(recv, Pkt, TPid, Dict),
+
     try
         incr_rc(recv, Pkt, Dict, TPid, Dict0) %% count incoming
     of
         _ -> answer(Pkt, SvcName, App, Req)
     catch
-        exit: no_result_code ->
+        exit: {no_result_code, _} ->
             %% RFC 6733 requires one of Result-Code or
             %% Experimental-Result, but the decode will have detected
             %% a missing AVP. If both are optional in the dictionary
@@ -1462,11 +1487,16 @@ a(#diameter_packet{errors = Es}
        callback == AE ->
     cb(ModX, handle_answer, [Pkt, msg(P), SvcName, {TPid, Caps}]);
 
-a(Pkt, SvcName, _, report, Req) ->
-    x(errors, handle_answer, [SvcName, Req, Pkt]);
+a(Pkt, SvcName, _, AE, _) ->
+    a(Pkt#diameter_packet.header, SvcName, AE).
+
+a(Hdr, SvcName, report) ->
+    MFA = {?MODULE, handle_answer, [SvcName, Hdr]},
+    diameter_lib:warning_report(errors, MFA),
+    a(Hdr, SvcName, discard);
 
-a(Pkt, SvcName, _, discard, Req) ->
-    x({errors, handle_answer, [SvcName, Req, Pkt]}).
+a(Hdr, SvcName, discard) ->
+    x({answer_errors, {SvcName, Hdr}}).
 
 %% Note that we don't check that the application id in the answer's
 %% header is what we expect. (TODO: Does the rfc says anything about
@@ -1544,7 +1574,7 @@ encode(Dict, TPid, #diameter_packet{bin = undefined} = Pkt) ->
         diameter_codec:encode(Dict, Pkt)
     catch
         exit: {diameter_codec, encode, T} = Reason ->
-            incr_error(send, T, TPid),
+            incr_error(send, T, TPid, Dict),
             exit(Reason)
     end;
 
@@ -1652,7 +1682,7 @@ resend_request(Pkt0,
                        packet = Pkt0,
                        caps = Caps},
 
-    ?LOG(retransmission, Req),
+    ?LOG(retransmission, Pkt#diameter_packet.header),
     TRef = send_request(TPid, Pkt, Req, SvcName, Tmo),
     {TRef, Req}.