Merge branch 'anders/diameter/identifier_checks/OTP-10565' into maint

* anders/diameter/identifier_checks/OTP-10565:
  Add comment about lack of identifier checks on DWA
  Add check of End-to-End and Hop-by-Hop identfiers in received DPA
  Add check of End-to-End and Hop-by-Hop identfiers in received CEA

2 files changed, 45 insertions, 20 deletions

diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl
index 4acfd8313b..e06dc136ce 100644
--- a/lib/diameter/src/base/diameter_peer_fsm.erl
+++ b/lib/diameter/src/base/diameter_peer_fsm.erl
@@ -90,15 +90,20 @@
 %% timeout used to be hardcoded. (So it could be worse.)
 -define(DPA_TIMEOUT, 1000).
 
+-type uint32() :: diameter:'Unsigned32'().
+
 -record(state,
         {state = 'Wait-Conn-Ack'   %% state of RFC 3588 Peer State Machine
-              :: 'Wait-Conn-Ack' | recv_CER | 'Wait-CEA' | 'Open',
+              :: 'Wait-Conn-Ack'
+               | recv_CER
+               | 'Wait-CEA' %% old code
+               | {'Wait-CEA', uint32(), uint32()}
+               | 'Open',
          mode :: accept | connect | {connect, reference()},
          parent       :: pid(),  %% watchdog process
          transport    :: pid(),  %% transport process
          service      :: #diameter_service{},
-         dpr = false  :: false | {diameter:'Unsigned32'(),
-                                  diameter:'Unsigned32'()}}).
+         dpr = false  :: false | {uint32(), uint32()}}).
                             %% | hop by hop and end to end identifiers
 
 %% There are non-3588 states possible as a consequence of 5.6.1 of the
@@ -455,8 +460,12 @@ send_CER(#state{mode = {connect, Remote},
         close({already_connected, Remote, LCaps}, S),
     CER = build_CER(S),
     ?LOG(send, 'CER'),
-    send(TPid, encode(CER)),
-    start_timer(S#state{state = 'Wait-CEA'}).
+    #diameter_packet{header = #diameter_header{end_to_end_id = Eid,
+                                               hop_by_hop_id = Hid}}
+        = Pkt
+        = encode(CER),
+    send(TPid, Pkt),
+    start_timer(S#state{state = {'Wait-CEA', Hid, Eid}}).
 
 %% Register ourselves as connecting to the remote endpoint in
 %% question. This isn't strictly necessary since a peer implementing
@@ -487,10 +496,8 @@ encode(Rec) ->
     Hdr = #diameter_header{version = ?DIAMETER_VERSION,
                            end_to_end_id = Seq,
                            hop_by_hop_id = Seq},
-    Pkt = #diameter_packet{header = Hdr,
-                           msg = Rec},
-    #diameter_packet{bin = Bin} = diameter_codec:encode(?BASE, Pkt),
-    Bin.
+    diameter_codec:encode(?BASE, #diameter_packet{header = Hdr,
+                                                  msg = Rec}).
 
 sequence() ->
     case getr(?SEQUENCE_KEY) of
@@ -558,7 +565,14 @@ discard(Reason, F, A) ->
 %% rcv/3
 
 %% Incoming CEA.
-rcv('CEA', Pkt, #state{state = 'Wait-CEA'} = S) ->
+rcv('CEA',
+    #diameter_packet{header = #diameter_header{end_to_end_id = Eid,
+                                               hop_by_hop_id = Hid}}
+    = Pkt,
+    #state{state = {'Wait-CEA' = T, Hid, Eid}}
+    = S) ->
+    handle_CEA(Pkt, S#state{state = T});
+rcv('CEA', Pkt, #state{state = 'Wait-CEA'} = S) ->  %% old code
     handle_CEA(Pkt, S);
 
 %% Incoming CER
@@ -578,16 +592,16 @@ rcv(N, Pkt, S)
        N == 'DPR' ->
     handle_request(N, Pkt, S);
 
-%% DPA even though we haven't sent DPR: ignore.
-rcv('DPA', _Pkt, #state{dpr = false}) ->
-    ok;
-
-%% DPA in response to DPR. We could check the sequence numbers but
-%% don't bother, just close.
-rcv('DPA' = N, _Pkt, #state{transport = TPid}) ->
+%% DPA in response to DPR and with the expected identifiers.
+rcv('DPA' = N,
+    #diameter_packet{header = #diameter_header{end_to_end_id = Eid,
+                                               hop_by_hop_id = Hid}},
+    #state{transport = TPid,
+           dpr = {Hid, Eid}}) ->
     diameter_peer:close(TPid),
     {stop, N};
 
+%% Ignore anything else, an unsolicited DPA in particular.
 rcv(_, _, _) ->
     ok.
 
@@ -1055,13 +1069,16 @@ send_dpr(Reason, Opts, #state{transport = TPid,
                    origin_realm = {OR, _}}
         = Caps,
 
-    Bin = encode(['DPR', {'Origin-Host', OH},
+    #diameter_packet{header = #diameter_header{end_to_end_id = Eid,
+                                               hop_by_hop_id = Hid}}
+        = Pkt
+        = encode(['DPR', {'Origin-Host', OH},
                          {'Origin-Realm', OR},
                          {'Disconnect-Cause', Cause}]),
-    send(TPid, Bin),
+    send(TPid, Pkt),
     dpa_timer(Tmo),
     ?LOG(send, 'DPR'),
-    S#state{dpr = diameter_codec:sequence_numbers(Bin)}.
+    S#state{dpr = {Hid, Eid}}.
 
 opt({timeout, Tmo}, Rec)
   when ?IS_TIMEOUT(Tmo) ->
diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl
index d6a2d2833b..243ad0a986 100644
--- a/lib/diameter/src/base/diameter_watchdog.erl
+++ b/lib/diameter/src/base/diameter_watchdog.erl
@@ -487,6 +487,14 @@ throwaway(S) ->
     throw({?MODULE, throwaway, S}).
 
 %% rcv/2
+%%
+%% The lack of Hop-by-Hop and End-to-End Identifiers checks in a
+%% received DWA is intentional. The purpose of the message is to
+%% demonstrate life but a peer that consistently bungles it by sending
+%% the wrong identifiers causes the connection to toggle between OPEN
+%% and SUSPECT, with failover and failback as result, despite there
+%% being no real problem with connectivity. Thus, relax and accept any
+%% incoming DWA as being in response to an outgoing DWR.
 
 %%   INITIAL       Receive DWA          Pending = FALSE
 %%                                      Throwaway()          INITIAL