aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnders Svensson <anders@erlang.org>2012-11-18 19:39:04 +0100
committerAnders Svensson <anders@erlang.org>2012-11-18 19:39:04 +0100
commit8134b7447050d9a6bd6b00fa30f4f7162bb342a9 (patch)
treee67805288cb370515e4e2fb337bd45f9b16b5a72
parent66776eeb6efaaddc013b763aa7a218cb16fc7db3 (diff)
parent81e2c30e1dd83974e6a41f0988124501da1e8ad6 (diff)
downloadotp-8134b7447050d9a6bd6b00fa30f4f7162bb342a9.tar.gz
otp-8134b7447050d9a6bd6b00fa30f4f7162bb342a9.tar.bz2
otp-8134b7447050d9a6bd6b00fa30f4f7162bb342a9.zip
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
-rw-r--r--lib/diameter/src/base/diameter_peer_fsm.erl57
-rw-r--r--lib/diameter/src/base/diameter_watchdog.erl8
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