From 4ddc8f40399d7c4856d0c8f9463832012c25002f Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 15 Nov 2012 14:57:18 +0100 Subject: Add check of End-to-End and Hop-by-Hop identfiers in received CEA --- lib/diameter/src/base/diameter_peer_fsm.erl | 43 ++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 13 deletions(-) (limited to 'lib/diameter') diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 4acfd8313b..3cf1129d59 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 @@ -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) -> -- cgit v1.2.3 From 62403f98c887a1b2e99e41da576c171c29d59193 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 15 Nov 2012 15:05:26 +0100 Subject: Add check of End-to-End and Hop-by-Hop identfiers in received DPA --- lib/diameter/src/base/diameter_peer_fsm.erl | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'lib/diameter') diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 3cf1129d59..e06dc136ce 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -592,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. -- cgit v1.2.3 From 81e2c30e1dd83974e6a41f0988124501da1e8ad6 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 15 Nov 2012 15:47:02 +0100 Subject: Add comment about lack of identifier checks on DWA --- lib/diameter/src/base/diameter_watchdog.erl | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'lib/diameter') 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 -- cgit v1.2.3