From 9321b4b3aa332f140268ef8f3251f6314a4984fe Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Mon, 23 Mar 2015 07:57:26 +0100 Subject: Fix ordering of AVPs in relayed messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 6.1.9 of RFC 6733 states this: A relay or proxy agent MUST append a Route-Record AVP to all requests forwarded. The AVP was inserted as the head of the AVP list, not appended, since the entire AVP list was reversed relative to the received order. Thanks to Andrzej TrawiƄski. --- lib/diameter/src/base/diameter_codec.erl | 40 ++++++++++++++++++++++++------ lib/diameter/src/base/diameter_traffic.erl | 2 +- 2 files changed, 33 insertions(+), 9 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index a2b04bfd63..07ad5f97d7 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -90,7 +90,7 @@ encode(Mod, Msg) -> msg = Msg}). e(_, #diameter_packet{msg = [#diameter_header{} = Hdr | As]} = Pkt) -> - try encode_avps(As) of + try encode_avps(reorder(As)) of Avps -> Length = size(Avps) + 20, @@ -183,26 +183,50 @@ values(Avps) -> %% Message as a list of #diameter_avp{} ... encode_avps(_, _, [#diameter_avp{} | _] = Avps) -> - encode_avps(reorder(Avps, [], Avps)); + encode_avps(reorder(Avps)); %% ... or as a tuple list or record. encode_avps(Mod, MsgName, Values) -> Mod:encode_avps(MsgName, Values). %% reorder/1 +%% +%% Reorder AVPs for the relay case using the index field of +%% diameter_avp records. Decode populates this field in collect_avps +%% and presents AVPs in reverse order. A relay then sends the reversed +%% list with a Route-Record AVP prepended. The goal here is just to do +%% lists:reverse/1 in Grouped AVPs and the outer list, but only in the +%% case there are indexed AVPs at all, so as not to reverse lists that +%% have been explicilty sent (unindexed, in the desired order) as a +%% diameter_avp list. The effect is the same as lists:keysort/2, but +%% only on the cases we expect, not a general sort. + +reorder(Avps) -> + case reorder(Avps, []) of + false -> + Avps; + Sorted -> + Sorted + end. -reorder([#diameter_avp{index = 0} | _] = Avps, Acc, _) -> +%% reorder/3 + +%% In case someone has reversed the list already. (Not likely.) +reorder([#diameter_avp{index = 0} | _] = Avps, Acc) -> Avps ++ Acc; -reorder([#diameter_avp{index = N} = A | Avps], Acc, _) +%% Assume indexed AVPs are in reverse order. +reorder([#diameter_avp{index = N} = A | Avps], Acc) when is_integer(N) -> lists:reverse(Avps, [A | Acc]); -reorder([H | T], Acc, Avps) -> - reorder(T, [H | Acc], Avps); +%% An unindexed AVP. +reorder([H | T], Acc) -> + reorder(T, [H | Acc]); -reorder([], Acc, _) -> - Acc. +%% No indexed members. +reorder([], _) -> + false. %% encode_avps/1 diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 3b62afca47..3717e43e4a 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -592,7 +592,7 @@ resend(false, Route = #diameter_avp{data = {Dict0, 'Route-Record', OH}}, Seq = diameter_session:sequence(Mask), Hdr = Hdr0#diameter_header{hop_by_hop_id = Seq}, - Msg = [Hdr, Route | Avps], + Msg = [Hdr, Route | Avps], %% reordered at encode resend(send_request(SvcName, App, Msg, Opts), Caps, Dict0, Pkt). %% The incoming request is relayed with the addition of a %% Route-Record. Note the requirement on the return from call/4 below, -- cgit v1.2.3 From 5228b6e5e3906a7a26e2e730b9f0213844e967a8 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 21 Mar 2015 10:56:28 +0100 Subject: Be lenient with errors in incoming DPR To avoid having the peer interpret the error as meaning the connection shouldn't be closed, which probably does more harm than ignoring syntactic errors in the DPR. Note that RFC 6733 says this about incoming DPR, in 5.4 Disconnecting Peer Connections: Upon receipt of the message, the Disconnect-Peer-Answer message is returned, which SHOULD contain an error if messages have recently been forwarded, and are likely in flight, which would otherwise cause a race condition. The race here is presumably between answers to forwarded requests and the outgoing DPA, but we have no handling for this: whether or not there are pending answers is irrelevant to how DPR is answered. It's questionable that a peer should be able to prevent disconnection in any case: it has to be the node sending DPR that decides if it's approriate, and the peer should take it as an indication of what's coming. Incoming DPA is already treated leniently: the only error that's not ignored is mismatching End-to-End and Hop-by-Hop Identifiers, since there's no distinguishing an erroneous value from an unsolicited DPA. This mismatch could also be ignored, which is the case for DWA for example, but this problem is already dealt with by dpa_timeout, which causes a connection to be closed even when the expected DPA isn't received. --- lib/diameter/src/base/diameter_peer_fsm.erl | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 9ff6845ab7..8bfdb3ae39 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -642,7 +642,9 @@ rcv('DPA' = N, diameter_peer:close(TPid), {stop, N}; -%% Ignore anything else, an unsolicited DPA in particular. +%% Ignore anything else, an unsolicited DPA in particular. Note that +%% dpa_timeout deals with the case in which the peer sends the wrong +%% identifiers in DPA. rcv(N, #diameter_packet{header = H}, _) when N == 'CER'; N == 'CEA'; @@ -820,7 +822,7 @@ build_answer(Type, errors = Es} = Pkt, S) -> - {RC, FailedAVP} = result_code(H, Es), + {RC, FailedAVP} = result_code(Type, H, Es), {answer(Type, RC, FailedAVP, S), post(Type, RC, Pkt, S)}. inband_security([]) -> @@ -890,6 +892,19 @@ set(['answer-message' | _] = Ans, FailedAvp) -> set([_|_] = Ans, FailedAvp) -> Ans ++ FailedAvp. +%% result_code/3 + +%% Be lenient with errors in DPR since there's no reason to be +%% otherwise. Rejecting may cause the peer to missinterpret the error +%% as meaning that the connection should not be closed, which may well +%% lead to more problems than any errors in the DPR. + +result_code('DPR', _, _) -> + {2001, []}; + +result_code('CER', H, Es) -> + result_code(H, Es). + %% result_code/2 result_code(#diameter_header{is_error = true}, _) -> -- cgit v1.2.3 From 9f496aaba6e8a63180eca3a77d01205ca0fcbff7 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 21 Mar 2015 10:11:51 +0100 Subject: Add transport_opt() dpr_timeout To cause a peer connection to be closed following an outgoing DPA, in case the peer fails to do so. It is the recipient of DPA that should close the connection according to RFC 6733. --- lib/diameter/src/base/diameter.erl | 1 + lib/diameter/src/base/diameter_config.erl | 1 + lib/diameter/src/base/diameter_peer_fsm.erl | 46 ++++++++++++++++++++++------- 3 files changed, 37 insertions(+), 11 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index a45d84f95b..da2695e1be 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -343,6 +343,7 @@ call(SvcName, App, Message) -> | {capabilities_cb, evaluable()} | {capx_timeout, 'Unsigned32'()} | {disconnect_cb, evaluable()} + | {dpr_timeout, 'Unsigned32'()} | {dpa_timeout, 'Unsigned32'()} | {length_errors, exit | handle | discard} | {connect_timer, 'Unsigned32'()} diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index aa4d6e5a20..a89831aa5e 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -534,6 +534,7 @@ opt({capabilities, Os}) -> opt({K, Tmo}) when K == capx_timeout; + K == dpr_timeout; K == dpa_timeout -> ?IS_UINT32(Tmo); diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 8bfdb3ae39..8d67f0aa90 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -63,7 +63,8 @@ %% Keys in process dictionary. -define(CB_KEY, cb). %% capabilities callback -define(DPR_KEY, dpr). %% disconnect callback --define(DPA_KEY, dpa). %% timeout for DPA reception +-define(DPA_KEY, dpa). %% timeout for incoming DPA, or shutdown after + %% outgoing DPA -define(REF_KEY, ref). %% transport_ref() -define(Q_KEY, q). %% transport start queue -define(START_KEY, start). %% start of connected transport @@ -83,18 +84,26 @@ N == ?GOAWAY; N == goaway; N == ?BUSY; N == busy). -%% RFC 3588: +%% RFC 6733: %% %% Timeout An application-defined timer has expired while waiting %% for some event. %% --define(EVENT_TIMEOUT, 10000). + %% Default timeout for reception of CER/CEA. +-define(CAPX_TIMEOUT, 10000). -%% Default timeout for DPA in response to DPR. A bit short but the -%% timeout used to be hardcoded. (So it could be worse.) +%% Default timeout for DPA to be received in response to an outgoing +%% DPR. A bit short but the timeout used to be hardcoded. (So it could +%% be worse.) -define(DPA_TIMEOUT, 1000). +%% Default timeout for the connection to be closed by the peer +%% following an outgoing DPA in response to an incoming DPR. It's the +%% recipient of DPA that should close the connection according to the +%% RFC. +-define(DPR_TIMEOUT, 5000). + -type uint32() :: diameter:'Unsigned32'(). -record(state, @@ -189,9 +198,10 @@ i({Ack, WPid, {M, Ref} = T, Opts, {Mask, Nodes, Dict0, Svc}}) -> putr(?REF_KEY, Ref), putr(?SEQUENCE_KEY, Mask), putr(?RESTRICT_KEY, Nodes), - putr(?DPA_KEY, proplists:get_value(dpa_timeout, Opts, ?DPA_TIMEOUT)), + putr(?DPA_KEY, {proplists:get_value(dpr_timeout, Opts, ?DPR_TIMEOUT), + proplists:get_value(dpa_timeout, Opts, ?DPA_TIMEOUT)}), - Tmo = proplists:get_value(capx_timeout, Opts, ?EVENT_TIMEOUT), + Tmo = proplists:get_value(capx_timeout, Opts, ?CAPX_TIMEOUT), OnLengthErr = proplists:get_value(length_errors, Opts, exit), {TPid, Addrs} = start_transport(T, Rest, Svc), @@ -416,7 +426,8 @@ transition({shutdown, Pid, Reason}, #state{parent = Pid, dpr = false} = S) -> transition({shutdown, Pid, _}, #state{parent = Pid}) -> ok; -%% DPA reception has timed out. +%% DPA reception has timed out, or peer has not closed the connection +%% as a result of outgoing DPA. transition(dpa_timeout, _) -> stop; @@ -840,7 +851,7 @@ cea(CEA, RC, Dict0) -> post('CER' = T, RC, Pkt, S) -> {T, caps(S), {RC, Pkt}}; post('DPR', _, _, #state{parent = Pid}) -> - [fun(S) -> inform_dpr(Pid), S end]. + [fun(S) -> dpr_timer(), inform_dpr(Pid), S end]. inform_dpr(Pid) -> Pid ! {'DPR', self()}. %% tell watchdog to die with us @@ -1247,11 +1258,24 @@ dpa_timer(Tmo) -> dpa_timeout() -> dpa_timeout(getr(?DPA_KEY)). -dpa_timeout(undefined) -> +dpa_timeout({_, Tmo}) -> + Tmo; +dpa_timeout(undefined) -> %% set in old code ?DPA_TIMEOUT; -dpa_timeout(Tmo) -> +dpa_timeout(Tmo) -> %% ditto Tmo. +dpr_timer() -> + dpa_timer(dpr_timeout()). + +dpr_timeout() -> + dpr_timeout(getr(?DPA_KEY)). + +dpr_timeout({Tmo, _}) -> + Tmo; +dpr_timeout(_) -> %% set in old code + ?DPR_TIMEOUT. + %% register_everywhere/1 %% %% Register a term and ensure it's not registered elsewhere. Note that -- cgit v1.2.3 From e541c3d17c7b8295201cd7d72e876c1c67d0fc50 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Sat, 21 Mar 2015 12:07:32 +0100 Subject: Discard incoming/outgoing requests after incoming DPR With the same motivation as in commits 5bd2d72 and b1fd629. As in the latter, incoming DPR is the only exception. --- lib/diameter/src/base/diameter_peer_fsm.erl | 38 +++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 10 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 8d67f0aa90..4ad4c346f4 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -117,9 +117,14 @@ transport :: pid(), %% transport process dictionary :: module(), %% common dictionary service :: #diameter_service{}, - dpr = false :: false | {uint32(), uint32()} %% set in old code - | {boolean(), uint32(), uint32()}, - %% | hop by hop and end to end identifiers + dpr = false :: false + | true %% DPR received, DPA sent + | {uint32(), uint32()} %% set in old code + | {boolean(), uint32(), uint32()}, + %% hop by hop and end to end identifiers in + %% outgoing DPR; boolean says whether or not + %% the request was sent explicitly with + %% diameter:call/4. length_errors :: exit | handle | discard}). %% There are non-3588 states possible as a consequence of 5.6.1 of the @@ -550,13 +555,19 @@ recv(Bin, S) -> %% recv1/3 -%% Incoming request after DPR has been sent: discard. Don't discard -%% DPR, so both ends don't do so when sending simultaneously. +%% Incoming request after outgoing DPR: discard. Don't discard DPR, so +%% both ends don't do so when sending simultaneously. recv1(Name, #diameter_packet{header = #diameter_header{is_request = true} = H}, #state{dpr = {_,_,_}}) when Name /= 'DPR' -> - invalid(false, recv_after_dpr, H); + invalid(false, recv_after_outgoing_dpr, H); + +%% Incoming request after incoming DPR: discard. +recv1(_, + #diameter_packet{header = #diameter_header{is_request = true} = H}, + #state{dpr = true}) -> + invalid(false, recv_after_incoming_dpr, H); %% DPA with identifier mismatch, or in response to a DPR initiated by %% the service. @@ -707,8 +718,10 @@ outgoing(#diameter_packet{header = #diameter_header{application_id = 0, if T == false -> inform_dpr(Pid), send_dpr(true, Pkt, dpa_timeout(), S); + T == true -> + invalid(false, dpr_after_dpa, H); %% DPA sent: discard true -> - invalid(false, dpr_after_dpr, H) %% already sent: discard + invalid(false, dpr_after_dpr, H) %% DPR sent: discard end; %% Explict CER or DWR: discard. These are sent by us. @@ -851,7 +864,12 @@ cea(CEA, RC, Dict0) -> post('CER' = T, RC, Pkt, S) -> {T, caps(S), {RC, Pkt}}; post('DPR', _, _, #state{parent = Pid}) -> - [fun(S) -> dpr_timer(), inform_dpr(Pid), S end]. + [fun(S) -> dpr_timer(), inform_dpr(Pid), dpr(S) end]. + +dpr(#state{dpr = false} = S) -> %% not awaiting DPA + S#state{dpr = true}; %% DPR received +dpr(S) -> %% DPR already sent or received + S. inform_dpr(Pid) -> Pid ! {'DPR', self()}. %% tell watchdog to die with us @@ -1144,7 +1162,7 @@ close(Reason) -> %% dpr/2 %% -%% The RFC isn't clear on whether DPR should be send in a non-Open +%% The RFC isn't clear on whether DPR should be sent in a non-Open %% state. The Peer State Machine transitions it documents aren't %% exhaustive (no Stop in Wait-I-CEA for example) so assume it's up to %% the implementation and transition to Closed (ie. die) if we haven't @@ -1160,7 +1178,7 @@ dpr(Reason, #state{state = 'Open', Peer = {self(), Caps}, dpr(CBs, [Reason, Ref, Peer], S); -%% Connection is open, DPR already sent. +%% Connection is open, DPR already sent or received. dpr(_, #state{state = 'Open'}) -> ok; -- cgit v1.2.3 From 75ee72b4e1d288b1d96194d0e352eb0c73db4a6f Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 12 Feb 2015 11:18:52 +0100 Subject: Strip potentially large terms when sending outgoing Diameter messages Both incoming and outgoing Diameter messages pass through two or three processes, depending on whether they're incoming or outgoing: the transport process and corresponding peer_fsm process and (for incoming) watchdog processes. Since terms other than binary are copied when passing process boundaries, large terms lead to copying that can be problematic, if frequent enough. Since only the bin and transport_data fields of a diameter_packet record are needed by the transport process, discard others when sending outgoing messages. Strictly speaking, the statement that only the aforementioned fields are needed by the transport process depends on the transport process. It's true of those implemented by diameter (in diameter_tcp and diameter_sctp), but an implementation that makes use of other fields is assuming more than the documentation in diameter_transport(3) promises. --- lib/diameter/src/base/diameter_peer.erl | 21 +++++++++++++++------ lib/diameter/src/base/diameter_traffic.erl | 12 +++++++++--- lib/diameter/src/base/diameter_watchdog.erl | 11 +++++++++-- 3 files changed, 33 insertions(+), 11 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_peer.erl b/lib/diameter/src/base/diameter_peer.erl index ea326dd03e..89b63c8a92 100644 --- a/lib/diameter/src/base/diameter_peer.erl +++ b/lib/diameter/src/base/diameter_peer.erl @@ -232,12 +232,22 @@ recv(Pid, Pkt) -> %% # send/2 %% --------------------------------------------------------------------------- -send(Pid, #diameter_packet{transport_data = undefined, - bin = Bin}) -> - send(Pid, Bin); +send(Pid, Msg) -> + ifc_send(Pid, {send, strip(Msg)}). -send(Pid, Pkt) -> - ifc_send(Pid, {send, Pkt}). +%% Send only binary when possible. +strip(#diameter_packet{transport_data = undefined, + bin = Bin}) -> + Bin; + +%% Strip potentially large message terms. +strip(#diameter_packet{transport_data = T, + bin = Bin}) -> + #diameter_packet{transport_data = T, + bin = Bin}; + +strip(Msg) -> + Msg. %% --------------------------------------------------------------------------- %% # close/1 @@ -326,7 +336,6 @@ code_change(_OldVsn, State, _Extra) -> {ok, State}. %% --------------------------------------------------------- -%% INTERNAL FUNCTIONS %% --------------------------------------------------------- %% ifc_send/2 diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 0b503338a6..18c1965f77 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2013-2014. All Rights Reserved. +%% Copyright Ericsson AB 2013-2015. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -1679,8 +1679,14 @@ recv(TPid, Pid, TRef, Ref) -> %% send/2 -send(Pid, Pkt) -> - Pid ! {send, Pkt}. +send(Pid, Pkt) -> %% Strip potentially large message terms. + #diameter_packet{header = H, + bin = Bin, + transport_data = T} + = Pkt, + Pid ! {send, #diameter_packet{header = H, + bin = Bin, + transport_data = T}}. %% retransmit/4 diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index 67715906e8..8223b7df98 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -572,11 +572,18 @@ rcv('DWR', Pkt, #watchdog{transport = TPid, DPkt = diameter_codec:decode(Dict0, Pkt), diameter_traffic:incr(recv, DPkt, TPid, Dict0), diameter_traffic:incr_error(recv, DPkt, TPid, Dict0), - EPkt = encode(dwa, Dict0, Pkt), + #diameter_packet{header = H, + transport_data = T, + bin = Bin} + = EPkt + = encode(dwa, Dict0, Pkt), diameter_traffic:incr(send, EPkt, TPid, Dict0), diameter_traffic:incr_rc(send, EPkt, TPid, Dict0), - send(TPid, {send, EPkt}), + %% Strip potentially large message terms. + send(TPid, {send, #diameter_packet{header = H, + transport_data = T, + bin = Bin}}), ?LOG(send, 'DWA'); rcv('DWA', Pkt, #watchdog{transport = TPid, -- cgit v1.2.3 From 1590920c910c030369fbf871b63f6836b988e90a Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 12 Feb 2015 11:41:59 +0100 Subject: Add service_opt() string_decode To control whether stringish Diameter types are decoded to string or left as binary. The motivation is the same as in the parent commit: to avoid large strings being copied when incoming Diameter messages are passed between processes; or *if* in the case of messages destined for handle_request and handle_answer callbacks, since these are decoded in the dedicated processes that the callbacks take place in. It would be possible to do something about other messages without requiring an option, but disabling the decode is the most effective. The value is a boolean(), true being the default for backwards compatibility. Setting false causes both diameter_caps records and decoded messages to contain binary() in relevant places that previously had string(): diameter_app(3) callbacks need to be prepared for the change. The Diameter types affected are OctetString and the derived types that can contain arbitrarily large values: OctetString, UTF8String, DiameterIdentity, DiameterURI, IPFilterRule, and QoSFilterRule. Time and Address are unaffected. The DiameterURI decode has been redone using re(3), which both simplifies and does away with a vulnerability resulting from the conversion of arbitrary strings to atom. The solution continues the use and abuse of the process dictionary for encode/decode purposes, last seen in commit 0f9cdba. --- lib/diameter/src/base/diameter.erl | 3 +- lib/diameter/src/base/diameter_capx.erl | 40 +++++++-- lib/diameter/src/base/diameter_codec.erl | 39 ++++++++- lib/diameter/src/base/diameter_config.erl | 15 +++- lib/diameter/src/base/diameter_peer_fsm.erl | 16 +++- lib/diameter/src/base/diameter_service.erl | 52 +++++++---- lib/diameter/src/base/diameter_traffic.erl | 57 ++++++++---- lib/diameter/src/base/diameter_types.erl | 130 ++++++++++++++-------------- lib/diameter/src/base/diameter_watchdog.erl | 40 +++++---- 9 files changed, 264 insertions(+), 128 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl index 1bbdf6e34d..a3c259c651 100644 --- a/lib/diameter/src/base/diameter.erl +++ b/lib/diameter/src/base/diameter.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2014. All Rights Reserved. +%% Copyright Ericsson AB 2010-2015. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -306,6 +306,7 @@ call(SvcName, App, Message) -> | {restrict_connections, restriction()} | {sequence, sequence() | evaluable()} | {share_peers, remotes()} + | {string_decode, boolean()} | {use_shared_peers, remotes()} | {spawn_opt, list()}. diff --git a/lib/diameter/src/base/diameter_capx.erl b/lib/diameter/src/base/diameter_capx.erl index 93548ecafd..7dc61f229f 100644 --- a/lib/diameter/src/base/diameter_capx.erl +++ b/lib/diameter/src/base/diameter_capx.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2013. All Rights Reserved. +%% Copyright Ericsson AB 2010-2015. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -50,7 +50,8 @@ -export([build_CER/2, recv_CER/3, recv_CEA/3, - make_caps/2]). + make_caps/2, + binary_caps/1]). -include_lib("diameter/include/diameter.hrl"). -include("diameter_internal.hrl"). @@ -115,7 +116,8 @@ mk_caps(Caps0, Opts) -> -define(SC(K,F), set_cap({K, Val}, {Caps, #diameter_caps{F = false} = C}) -> - {Caps#diameter_caps{F = cap(K, Val)}, C#diameter_caps{F = true}}). + {Caps#diameter_caps{F = cap(K, copy(Val))}, + C#diameter_caps{F = true}}). ?SC('Origin-Host', origin_host); ?SC('Origin-Realm', origin_realm); @@ -375,10 +377,10 @@ capx_to_caps(CEX, Dict) -> 'Firmware-Revision', 'AVP'], CEX), - #diameter_caps{origin_host = OH, - origin_realm = OR, + #diameter_caps{origin_host = copy(OH), + origin_realm = copy(OR), vendor_id = VId, - product_name = PN, + product_name = copy(PN), origin_state_id = OSI, host_ip_address = IP, supported_vendor_id = SV, @@ -389,6 +391,32 @@ capx_to_caps(CEX, Dict) -> firmware_revision = FR, avp = X}. +%% Copy binaries to avoid retaining a reference to a large binary +%% containing AVPs we aren't interested in. +copy(B) + when is_binary(B) -> + binary:copy(B); + +copy(T) -> + T. + +%% binary_caps/1 +%% +%% Encode stringish capabilities with {string_decode, false}. + +binary_caps(Caps) -> + lists:foldl(fun bcaps/2, Caps, [#diameter_caps.origin_host, + #diameter_caps.origin_realm, + #diameter_caps.product_name]). + +bcaps(N, Caps) -> + case element(N, Caps) of + undefined -> + Caps; + V -> + setelement(N, Caps, iolist_to_binary(V)) + end. + %% --------------------------------------------------------------------------- %% --------------------------------------------------------------------------- diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index b4ecb63961..2de0dcf373 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2014. All Rights Reserved. +%% Copyright Ericsson AB 2010-2015. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -22,6 +22,8 @@ -export([encode/2, decode/2, decode/3, + setopts/1, + getopt/1, collect_avps/1, decode_header/1, sequence_numbers/1, @@ -58,6 +60,41 @@ %% | AVPs ... %% +-+-+-+-+-+-+-+-+-+-+-+-+- +%%% --------------------------------------------------------------------------- +%%% # setopts/1 +%%% # getopt/1 +%%% --------------------------------------------------------------------------- + +%% These functions are a compromise in the same vein as the use of the +%% process dictionary in diameter_gen.hrl in generated codec modules. +%% Instead of rewriting the entire dictionary generation to pass +%% encode/decode options around, the calling process sets them by +%% calling setopts/1. At current, the only option is whether or not to +%% decode binaries as strings, which is used by diameter_types. + +setopts(Opts) + when is_list(Opts) -> + lists:foreach(fun setopt/1, Opts). + +%% Decode stringish types to string()? The default true is for +%% backwards compatibility. +setopt({string_decode = K, false = B}) -> + setopt(K, B); + +setopt(_) -> + ok. + +setopt(Key, Value) -> + put({diameter, Key}, Value). + +getopt(Key) -> + case get({diameter, Key}) of + undefined when Key == string_decode -> + true; + V -> + V + end. + %%% --------------------------------------------------------------------------- %%% # encode/2 %%% --------------------------------------------------------------------------- diff --git a/lib/diameter/src/base/diameter_config.erl b/lib/diameter/src/base/diameter_config.erl index c0a4f7df69..e446f7c479 100644 --- a/lib/diameter/src/base/diameter_config.erl +++ b/lib/diameter/src/base/diameter_config.erl @@ -642,13 +642,23 @@ make_config(SvcName, Opts) -> {false, monitor}, {?NOMASK, sequence}, {nodes, restrict_connections}, + {true, string_decode}, {[], spawn_opt}]), + D = proplists:get_value(string_decode, SvcOpts, true), + #service{name = SvcName, rec = #diameter_service{applications = Apps, - capabilities = Caps}, + capabilities = binary_caps(Caps, D)}, options = SvcOpts}. +binary_caps(Caps, true) -> + Caps; +binary_caps(Caps, false) -> + diameter_capx:binary_caps(Caps). + +%% make_opts/2 + make_opts(Opts, Defs) -> Known = [{K, get_opt(K, Opts, D)} || {D,K} <- Defs], Unknown = Opts -- Known, @@ -667,7 +677,8 @@ opt(K, false = B) opt(K, true = B) when K == share_peers; - K == use_shared_peers -> + K == use_shared_peers; + K == string_decode -> B; opt(restrict_connections, T) diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index ee6e7dd89e..cbf601f6f5 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2014. All Rights Reserved. +%% Copyright Ericsson AB 2010-2015. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -138,7 +138,8 @@ %% # start/3 %% --------------------------------------------------------------------------- --spec start(T, [Opt], {diameter:sequence(), +-spec start(T, [Opt], {[diameter:service_opt()] + | diameter:sequence(), %% from old code [node()], module(), #diameter_service{}}) @@ -177,10 +178,15 @@ init(T) -> proc_lib:init_ack({ok, self()}), gen_server:enter_loop(?MODULE, [], i(T)). -i({Ack, WPid, {M, Ref} = T, Opts, {Mask, Nodes, Dict0, Svc}}) -> +i({Ack, WPid, T, Opts, {{_,_} = Mask, Nodes, Dict0, Svc}}) -> %% from old code + i({Ack, WPid, T, Opts, {[{sequence, Mask}], Nodes, Dict0, Svc}}); + +i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) -> erlang:monitor(process, WPid), wait(Ack, WPid), diameter_stats:reg(Ref), + diameter_codec:setopts(SvcOpts), + {_,_} = Mask = proplists:get_value(sequence, SvcOpts), {[Cs,Ds], Rest} = proplists:split(Opts, [capabilities_cb, disconnect_cb]), putr(?CB_KEY, {Ref, [F || {_,F} <- Cs]}), putr(?DPR_KEY, [F || {_, F} <- Ds]), @@ -699,6 +705,8 @@ build_answer('CER', = Pkt, #state{dictionary = Dict0} = S) -> + diameter_codec:setopts([{string_decode, false}]), + {SupportedApps, RCaps, CEA} = recv_CER(CER, S), [RC, IS] = Dict0:'#get-'(['Result-Code', 'Inband-Security-Id'], CEA), @@ -886,6 +894,8 @@ handle_CEA(#diameter_packet{header = H} = DPkt = diameter_codec:decode(Dict0, Pkt), + diameter_codec:setopts([{string_decode, false}]), + RC = result_code(incr_rc(recv, DPkt, Dict0)), {SApps, IS, RCaps} = recv_CEA(DPkt, S), diff --git a/lib/diameter/src/base/diameter_service.erl b/lib/diameter/src/base/diameter_service.erl index 04401a3d87..a01bcdd4e7 100644 --- a/lib/diameter/src/base/diameter_service.erl +++ b/lib/diameter/src/base/diameter_service.erl @@ -130,7 +130,8 @@ :: [{sequence, diameter:sequence()} %% sequence mask | {share_peers, diameter:remotes()} %% broadcast to | {use_shared_peers, diameter:remotes()} %% use from - | {restrict_connections, diameter:restriction()}]}). + | {restrict_connections, diameter:restriction()} + | {string_decode, boolean()}]}). %% shared_peers reflects the peers broadcast from remote nodes. %% Record representing an RFC 3539 watchdog process implemented by @@ -261,16 +262,22 @@ whois(SvcName) -> %% --------------------------------------------------------------------------- -spec pick_peer(SvcName, AppOrAlias, Opts) - -> {{TPid, Caps, App}, Mask} - | false - | {error, term()} + -> {{TPid, Caps, App}, Mask, SvcOpts} + | false %% no selection + | {error, no_service} when SvcName :: diameter:service_name(), - AppOrAlias :: {alias, diameter:app_alias()} | #diameter_app{}, - Opts :: tuple(), + AppOrAlias :: #diameter_app{} + | {alias, diameter:app_alias()}, + Opts :: {fun((Dict :: module()) -> [term()]), + diameter:peer_filter(), + Xtra :: list()}, TPid :: pid(), Caps :: #diameter_caps{}, App :: #diameter_app{}, - Mask :: diameter:sequence(). + Mask :: diameter:sequence(), + SvcOpts :: [diameter:service_opt()]. +%% Extract Mask in the returned tuple so that diameter_traffic doesn't +%% need to know about the ordering of SvcOpts used here. pick_peer(SvcName, App, Opts) -> pick(lookup_state(SvcName), App, Opts). @@ -287,10 +294,10 @@ pick(#state{service = #diameter_service{applications = Apps}} Opts) -> %% initial call from diameter:call/4 pick(S, find_outgoing_app(Alias, Apps), Opts); -pick(_, false, _) -> - false; +pick(_, false = No, _) -> + No; -pick(#state{options = [{_, Mask} | _]} +pick(#state{options = [{_, Mask} | SvcOpts]} = S, #diameter_app{module = ModX, dictionary = Dict} = App0, @@ -299,7 +306,7 @@ pick(#state{options = [{_, Mask} | _]} [_,_] = RealmAndHost = diameter_lib:eval([DestF, Dict]), case pick_peer(App, RealmAndHost, Filter, S) of {TPid, Caps} -> - {{TPid, Caps, App}, Mask}; + {{TPid, Caps, App}, Mask, SvcOpts}; false = No -> No end. @@ -690,7 +697,8 @@ service_options(Opts) -> {restrict_connections, proplists:get_value(restrict_connections, Opts, ?RESTRICT)}, - {spawn_opt, proplists:get_value(spawn_opt, Opts, [])}]. + {spawn_opt, proplists:get_value(spawn_opt, Opts, [])}, + {string_decode, proplists:get_value(string_decode, Opts, true)}]. %% The order of options is significant since we match against the list. mref(false = No) -> @@ -802,10 +810,13 @@ start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT, when Type == connect; Type == accept -> #diameter_service{applications = Apps} - = Svc + = Svc1 = merge_service(Opts, Svc0), - {_,_} = Mask = proplists:get_value(sequence, SvcOpts), - RecvData = diameter_traffic:make_recvdata([SvcName, PeerT, Apps, Mask]), + Svc = binary_caps(Svc1, proplists:get_value(string_decode, SvcOpts, true)), + RecvData = diameter_traffic:make_recvdata([SvcName, + PeerT, + Apps, + SvcOpts]), T = {{spawn_opts([Opts, SvcOpts]), RecvData}, Opts, SvcOpts, Svc}, Rec = #watchdog{type = Type, ref = Ref, @@ -816,8 +827,13 @@ start(Ref, Type, Opts, N, #state{watchdogT = WatchdogT, [], N). +binary_caps(Svc, true) -> + Svc; +binary_caps(#diameter_service{capabilities = Caps} = Svc, false) -> + Svc#diameter_service{capabilities = diameter_capx:binary_caps(Caps)}. + wd(Type, Ref, T, WatchdogT, Rec) -> - Pid = wd(Type, Ref, T), + Pid = start_watchdog(Type, Ref, T), insert(WatchdogT, Rec#watchdog{pid = Pid}), Pid. @@ -831,7 +847,7 @@ spawn_opts(Optss) -> T /= link, T /= monitor]. -wd(Type, Ref, T) -> +start_watchdog(Type, Ref, T) -> {_MRef, Pid} = diameter_watchdog:start({Type, Ref}, T), Pid. @@ -852,7 +868,7 @@ ms({applications, As}, #diameter_service{applications = Apps} = S) %% The fact that all capabilities can be configured on the transports %% means that the service doesn't necessarily represent a single -%% locally implemented Diameter peer as identified by Origin-Host: a +%% locally implemented Diameter node as identified by Origin-Host: a %% transport can configure its own Origin-Host. This means that the %% service little more than a placeholder for default capabilities %% plus a list of applications that individual transports can choose diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index 18c1965f77..a9dc46ea31 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -77,7 +77,8 @@ {peerT :: ets:tid(), service_name :: diameter:service_name(), apps :: [#diameter_app{}], - sequence :: diameter:sequence()}). + sequence :: diameter:sequence(), + codec :: list()}). %% Record stored in diameter_request for each outgoing request. -record(request, @@ -92,11 +93,16 @@ %% # make_recvdata/1 %% --------------------------------------------------------------------------- -make_recvdata([SvcName, PeerT, Apps, Mask | _]) -> +make_recvdata([SvcName, PeerT, Apps, {_,_} = Mask | _]) -> %% from old code + make_recvdata([SvcName, PeerT, Apps, [{sequence, Mask}]]); + +make_recvdata([SvcName, PeerT, Apps, SvcOpts | _]) -> + {_,_} = Mask = proplists:get_value(sequence, SvcOpts), #recvdata{service_name = SvcName, peerT = PeerT, apps = Apps, - sequence = Mask}. + sequence = Mask, + codec = [T || {K,_} = T <- SvcOpts, K == string_decode]}. %% --------------------------------------------------------------------------- %% peer_up/1 @@ -270,8 +276,11 @@ recv_request(TPid, #diameter_packet{header = #diameter_header{application_id = Id}} = Pkt, Dict0, - #recvdata{peerT = PeerT, apps = Apps} + #recvdata{peerT = PeerT, + apps = Apps, + codec = Opts} = RecvData) -> + diameter_codec:setopts(Opts), send_A(recv_R(diameter_service:find_incoming_app(PeerT, TPid, Id, Apps), TPid, Pkt, @@ -279,7 +288,13 @@ recv_request(TPid, RecvData), TPid, Dict0, - RecvData). + RecvData); + +recv_request(TPid, Pkt, Dict0, RecvData) -> %% from old code + recv_request(TPid, + Pkt, + Dict0, + #recvdata{} = erlang:append_element(RecvData, [])). %% recv_R/5 @@ -1225,10 +1240,9 @@ answer_rc(_, _, Sent) -> send_R(SvcName, AppOrAlias, Msg, Opts, Caller) -> case pick_peer(SvcName, AppOrAlias, Msg, Opts) of - {{_,_,_} = Transport, Mask} -> + {Transport, Mask, SvcOpts} -> + diameter_codec:setopts(SvcOpts), send_request(Transport, Mask, Msg, Opts, Caller, SvcName); - false -> - {error, no_connection}; {error, _} = No -> No end. @@ -1290,6 +1304,8 @@ send_request({TPid, Caps, App} SvcName, []). +%% send_R/7 + send_R({send, Msg}, Pkt, Transport, Opts, Caller, SvcName, Fs) -> send_R(make_request_packet(Msg, Pkt), Transport, @@ -1550,7 +1566,9 @@ a(Hdr, SvcName, discard) -> %% timer value is ignored. This means that an answer could be accepted %% from a peer after timeout in the case of failover. -retransmit({{_,_,App} = Transport, _Mask}, Req, Opts, SvcName, Timeout) -> +%% retransmit/5 + +retransmit({{_,_,App} = Transport, _, _}, Req, Opts, SvcName, Timeout) -> try retransmit(Transport, Req, SvcName, Timeout) of T -> recv_A(Timeout, SvcName, App, Opts, T) catch @@ -1571,17 +1589,26 @@ pick_peer(SvcName, pick_peer(SvcName, App, Msg, Opts#options{extra = []}); pick_peer(_, _, undefined, _) -> - false; + {error, no_connection}; pick_peer(SvcName, AppOrAlias, Msg, #options{filter = Filter, extra = Xtra}) -> - diameter_service:pick_peer(SvcName, - AppOrAlias, - {fun(D) -> get_destination(D, Msg) end, - Filter, - Xtra}). + pick(diameter_service:pick_peer(SvcName, + AppOrAlias, + {fun(D) -> get_destination(D, Msg) end, + Filter, + Xtra})). + +pick({{_,_,_} = Transport, Mask}) -> %% from old code; dialyzer complains + {Transport, Mask, []}; %% about this + +pick(false) -> + {error, no_connection}; + +pick(T) -> + T. %% handle_error/4 diff --git a/lib/diameter/src/base/diameter_types.erl b/lib/diameter/src/base/diameter_types.erl index 442d90c98b..28a0635c57 100644 --- a/lib/diameter/src/base/diameter_types.erl +++ b/lib/diameter/src/base/diameter_types.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2010-2013. All Rights Reserved. +%% Copyright Ericsson AB 2010-2015. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -90,7 +90,12 @@ 'OctetString'(decode, Bin) when is_binary(Bin) -> - binary_to_list(Bin); + case diameter_codec:getopt(string_decode) of + true -> + binary_to_list(Bin); + _ -> + Bin + end; 'OctetString'(decode, B) -> ?INVALID_LENGTH(B); @@ -298,17 +303,19 @@ 'OctetString'(M, lists:duplicate(0,7)); 'DiameterURI'(encode, #diameter_uri{type = Type, - fqdn = D, - port = P, + fqdn = DN, + port = PN, transport = T, - protocol = Prot} - = U) -> - S = lists:append([atom_to_list(Type), "://", D, - ":", integer_to_list(P), + protocol = P}) + when (Type == 'aaa' orelse Type == 'aaas'), + is_integer(PN), + 0 =< PN, + (T == tcp orelse T == sctp orelse T == udp), + (P == diameter orelse P == radius orelse P == 'tacacs+') -> + iolist_to_binary([atom_to_list(Type), "://", DN, + ":", integer_to_list(PN), ";transport=", atom_to_list(T), - ";protocol=", atom_to_list(Prot)]), - U = scan_uri(S), %% assert - list_to_binary(S); + ";protocol=", atom_to_list(P)]); 'DiameterURI'(encode, Str) -> Bin = iolist_to_binary(Str), @@ -321,7 +328,6 @@ 'IPFilterRule'(encode = M, zero) -> 'OctetString'(M, lists:duplicate(0,33)); -%% TODO: parse grammar. 'IPFilterRule'(M, X) -> 'OctetString'(M, X). @@ -331,7 +337,6 @@ 'QoSFilterRule'(encode = M, zero = X) -> 'IPFilterRule'(M, X); -%% TODO: parse grammar. 'QoSFilterRule'(M, X) -> 'OctetString'(M, X). @@ -339,7 +344,13 @@ 'UTF8String'(decode, Bin) when is_binary(Bin) -> - tl([0|_] = unicode:characters_to_list([0, Bin])); %% assert list return + case diameter_codec:getopt(string_decode) of + true -> + %% assert list return + tl([0|_] = unicode:characters_to_list([0, Bin])); + false -> + <<_/binary>> = unicode:characters_to_binary(Bin) + end; 'UTF8String'(decode, B) -> ?INVALID_LENGTH(B); @@ -507,55 +518,42 @@ msb(false) -> ?TIME_2036. %% %% aaa-protocol = ( "diameter" / "radius" / "tacacs+" ) -scan_uri(Bin) - when is_binary(Bin) -> - scan_uri(binary_to_list(Bin)); -scan_uri("aaa://" ++ Rest) -> - scan_fqdn(Rest, #diameter_uri{type = aaa}); -scan_uri("aaas://" ++ Rest) -> - scan_fqdn(Rest, #diameter_uri{type = aaas}). - -scan_fqdn(S, U) -> - {[_|_] = F, Rest} = lists:splitwith(fun is_fqdn/1, S), - scan_opt_port(Rest, U#diameter_uri{fqdn = F}). - -scan_opt_port(":" ++ S, U) -> - {[_|_] = P, Rest} = lists:splitwith(fun is_digit/1, S), - scan_opt_transport(Rest, U#diameter_uri{port = list_to_integer(P)}); -scan_opt_port(S, U) -> - scan_opt_transport(S, U). - -scan_opt_transport(";transport=" ++ S, U) -> - {P, Rest} = transport(S), - scan_opt_protocol(Rest, U#diameter_uri{transport = P}); -scan_opt_transport(S, U) -> - scan_opt_protocol(S, U). - -scan_opt_protocol(";protocol=" ++ S, U) -> - {P, ""} = protocol(S), - U#diameter_uri{protocol = P}; -scan_opt_protocol("", U) -> - U. - -transport("tcp" ++ S) -> - {tcp, S}; -transport("sctp" ++ S) -> - {sctp, S}; -transport("udp" ++ S) -> - {udp, S}. - -protocol("diameter" ++ S) -> - {diameter, S}; -protocol("radius" ++ S) -> - {radius, S}; -protocol("tacacs+" ++ S) -> - {'tacacs+', S}. - -is_fqdn(C) -> - is_digit(C) orelse is_alpha(C) orelse C == $. orelse C == $-. - -is_alpha(C) -> - ($a =< C andalso C =< $z) orelse ($A =< C andalso C =< $Z). - -is_digit(C) -> - $0 =< C andalso C =< $9. +scan_uri(Bin) -> + RE = "^(aaas?)://" + "([-a-zA-Z0-9.]+)" + "(:([0-9]+))?" + "(;transport=(tcp|sctp|udp))?" + "(;protocol=(diameter|radius|tacacs\\+))?$", + {match, [A, DN, PN, T, P]} = re:run(Bin, + RE, + [{capture, [1,2,4,6,8], binary}]), + #diameter_uri{port = PN0, + transport = T0, + protocol = P0} + = #diameter_uri{}, + #diameter_uri{type = to_atom(A), + fqdn = from_bin(DN), + port = to_int(PN, PN0), + transport = to_atom(T, T0), + protocol = to_atom(P, P0)}. + +from_bin(B) -> + case diameter_codec:getopt(string_decode) of + true -> + binary_to_list(B); + false -> + B + end. + +to_int(<<>>, N) -> + N; +to_int(B, _) -> + binary_to_integer(B). + +to_atom(<<>>, A) -> + A; +to_atom(B, _) -> + to_atom(B). + +to_atom(B) -> + binary_to_atom(B, latin1). diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index 8223b7df98..d14ddb758b 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -124,14 +124,15 @@ i({Ack, T, Pid, {RecvData, wait(Ack, Pid), {_, Seed} = diameter_lib:seed(), random:seed(Seed), - putr(restart, {T, Opts, Svc}), %% save seeing it in trace - putr(dwr, dwr(Caps)), %% + putr(restart, {T, Opts, Svc, SvcOpts}), %% save seeing it in trace + putr(dwr, dwr(Caps)), %% + diameter_codec:setopts([{string_decode, false}]), {_,_} = Mask = proplists:get_value(sequence, SvcOpts), Restrict = proplists:get_value(restrict_connections, SvcOpts), Nodes = restrict_nodes(Restrict), Dict0 = common_dictionary(Apps), #watchdog{parent = Pid, - transport = start(T, Opts, Mask, Nodes, Dict0, Svc), + transport = start(T, Opts, SvcOpts, Nodes, Dict0, Svc), tw = proplists:get_value(watchdog_timer, Opts, ?DEFAULT_TW_INIT), @@ -166,11 +167,11 @@ config({okay, N}, Rec) when ?IS_NATURAL(N) -> Rec#config{okay = N}. -%% start/5 +%% start/6 -start(T, Opts, Mask, Nodes, Dict0, Svc) -> +start(T, Opts, SvcOpts, Nodes, Dict0, Svc) -> {_MRef, Pid} - = diameter_peer_fsm:start(T, Opts, {Mask, Nodes, Dict0, Svc}), + = diameter_peer_fsm:start(T, Opts, {SvcOpts, Nodes, Dict0, Svc}), Pid. %% common_dictionary/1 @@ -320,7 +321,7 @@ code_change(_, State, _) -> %% expiry; or another watchdog is saying the same after reestablishing %% a connection previously had by this one. transition(close, #watchdog{}) -> - {{accept, _}, _, _} = getr(restart), %% assert + {accept, _} = role(), %% assert stop; %% Service is asking for the peer to be taken down gracefully. @@ -369,7 +370,7 @@ transition({open, TPid, Hosts, _} = Open, restrict = {_,R}, config = #config{suspect = OS}} = S) -> - case okay(getr(restart), Hosts, R) of + case okay(role(), Hosts, R) of okay -> set_watchdog(S#watchdog{status = okay, num_dwa = OS}); @@ -423,7 +424,7 @@ transition({'DOWN', _, process, TPid, _Reason} = D, = S0) -> S = S0#watchdog{pending = false, transport = undefined}, - {{M,_}, _, _} = getr(restart), + {M,_} = role(), %% Close an accepting watchdog immediately if there's no %% restriction on the number of connections to the same peer: the @@ -490,7 +491,7 @@ encode(dwa, Dict0, #diameter_packet{header = H, transport_data = TD} %% okay/3 -okay({{accept, Ref}, _, _}, Hosts, Restrict) -> +okay({accept, Ref}, Hosts, Restrict) -> T = {?MODULE, connection, Ref, Hosts}, diameter_reg:add(T), if Restrict -> @@ -501,7 +502,7 @@ okay({{accept, Ref}, _, _}, Hosts, Restrict) -> %% Register before matching so that at least one of two registering %% processes will match the other. -okay({{connect, _}, _, _}, _, _) -> +okay({connect, _}, _, _) -> okay. %% okay/2 @@ -516,6 +517,11 @@ okay(C) -> [_|_] = [send(P, close) || {_,P} <- C, self() /= P], reopen. +%% role/0 + +role() -> + element(1, getr(restart)). + %% set_watchdog/1 set_watchdog(#watchdog{tw = TwInit, @@ -801,26 +807,28 @@ restart(S) -> %% reconnect has won race with timeout %% state down rather then initial when receiving notification of an %% open connection. -restart({{connect, _} = T, Opts, Svc}, +restart({T, Opts, Svc}, S) -> %% put in old code + restart({T, Opts, Svc, []}, S); + +restart({{connect, _} = T, Opts, Svc, SvcOpts}, #watchdog{parent = Pid, - sequence = Mask, restrict = {R,_}, dictionary = Dict0} = S) -> send(Pid, {reconnect, self()}), Nodes = restrict_nodes(R), - S#watchdog{transport = start(T, Opts, Mask, Nodes, Dict0, Svc), + S#watchdog{transport = start(T, Opts, SvcOpts, Nodes, Dict0, Svc), restrict = {R, lists:member(node(), Nodes)}}; %% No restriction on the number of connections to the same peer: just %% die. Note that a state machine never enters state REOPEN in this %% case. -restart({{accept, _}, _, _}, #watchdog{restrict = {_, false}}) -> +restart({{accept, _}, _, _, _}, #watchdog{restrict = {_, false}}) -> stop; %% 'DOWN' was in old code: 'close' was not sent %% Otherwise hang around until told to die, either by the service or %% by another watchdog. -restart({{accept, _}, _, _}, S) -> +restart({{accept, _}, _, _, _}, S) -> S. %% Don't currently use Opts/Svc in the accept case. -- cgit v1.2.3 From 35f564094033ea2eb4c5b01d0d0b1c0d629ea5b1 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Fri, 20 Mar 2015 02:03:31 +0100 Subject: Reject transport=udp;protocol=diameter at DiameterURI encode Both RFC 3588 and 6733 disallow the combination. Make its encode fail. --- lib/diameter/src/base/diameter_types.erl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_types.erl b/lib/diameter/src/base/diameter_types.erl index 28a0635c57..8497329c20 100644 --- a/lib/diameter/src/base/diameter_types.erl +++ b/lib/diameter/src/base/diameter_types.erl @@ -311,7 +311,12 @@ is_integer(PN), 0 =< PN, (T == tcp orelse T == sctp orelse T == udp), - (P == diameter orelse P == radius orelse P == 'tacacs+') -> + (P == diameter orelse P == radius orelse P == 'tacacs+'), + (P /= diameter orelse T /= udp) -> + #diameter_uri{port = PN0, + transport = T0, + protocol = P0} + = #diameter_uri{}, iolist_to_binary([atom_to_list(Type), "://", DN, ":", integer_to_list(PN), ";transport=", atom_to_list(T), -- cgit v1.2.3 From b8a7df45c9e57a832f7db9b9b875b31d0ab7d29c Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Fri, 20 Mar 2015 02:18:06 +0100 Subject: Adapt to changed DiameterURI defaults in RFC 6733 Despite claims of full backwards compatibility, the text of RFC 6733 changes the interpretation of unspecified values in a DiameterURI. In particular, 3588 says that the default port and transport are 3868 and sctp respectively, while 6733 says it's either 3868/tcp (aaa) or 5658/tcp (aaas). The 3588 defaults were used regardless, but now use them only if the common dictionary is diameter_gen_base_rfc3588. The 6733 defaults are used otherwise. This kind of change in the standard can lead to interop problems, since a node has to know which RFC its peer is following to know that it will properly interpret missing URI components. Encode of a URI includes all components to avoid such confusion. That said, note that the defaults in the diameter_uri record have *not* been changed. This avoids breaking code that depends on them, but the risk is that such code sends inappropriate values. The record defaults may be changed in a future release, to force values to be explicitly specified. --- lib/diameter/src/base/diameter_codec.erl | 9 ++++ lib/diameter/src/base/diameter_peer_fsm.erl | 2 +- lib/diameter/src/base/diameter_traffic.erl | 2 +- lib/diameter/src/base/diameter_types.erl | 68 ++++++++++++++++++++++++----- lib/diameter/src/base/diameter_watchdog.erl | 3 +- 5 files changed, 70 insertions(+), 14 deletions(-) (limited to 'lib/diameter/src/base') diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index cc0953f5d3..15a4c5e86f 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -81,6 +81,13 @@ setopts(Opts) setopt({string_decode = K, false = B}) -> setopt(K, B); +%% Regard anything but the generated RFC 3588 dictionary as modern. +%% This affects the interpretation of defaults during the decode +%% of values of type DiameterURI, this having changed from RFC 3588. +%% (So much for backwards compatibility.) +setopt({common_dictionary, diameter_gen_base_rfc3588}) -> + setopt(rfc, 3588); + setopt(_) -> ok. @@ -91,6 +98,8 @@ getopt(Key) -> case get({diameter, Key}) of undefined when Key == string_decode -> true; + undefined when Key == rfc -> + 6733; V -> V end. diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index af0fce57a8..0ee3986b97 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -187,7 +187,7 @@ i({Ack, WPid, {M, Ref} = T, Opts, {SvcOpts, Nodes, Dict0, Svc}}) -> erlang:monitor(process, WPid), wait(Ack, WPid), diameter_stats:reg(Ref), - diameter_codec:setopts(SvcOpts), + diameter_codec:setopts([{common_dictionary, Dict0} | SvcOpts]), {_,_} = Mask = proplists:get_value(sequence, SvcOpts), {[Cs,Ds], Rest} = proplists:split(Opts, [capabilities_cb, disconnect_cb]), putr(?CB_KEY, {Ref, [F || {_,F} <- Cs]}), diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl index c1c5a35531..784f9ca08f 100644 --- a/lib/diameter/src/base/diameter_traffic.erl +++ b/lib/diameter/src/base/diameter_traffic.erl @@ -280,7 +280,7 @@ recv_request(TPid, apps = Apps, codec = Opts} = RecvData) -> - diameter_codec:setopts(Opts), + diameter_codec:setopts([{common_dictionary, Dict0} | Opts]), send_A(recv_R(diameter_service:find_incoming_app(PeerT, TPid, Id, Apps), TPid, Pkt, diff --git a/lib/diameter/src/base/diameter_types.erl b/lib/diameter/src/base/diameter_types.erl index 8497329c20..fe7613541c 100644 --- a/lib/diameter/src/base/diameter_types.erl +++ b/lib/diameter/src/base/diameter_types.erl @@ -313,18 +313,19 @@ (T == tcp orelse T == sctp orelse T == udp), (P == diameter orelse P == radius orelse P == 'tacacs+'), (P /= diameter orelse T /= udp) -> - #diameter_uri{port = PN0, - transport = T0, - protocol = P0} - = #diameter_uri{}, iolist_to_binary([atom_to_list(Type), "://", DN, ":", integer_to_list(PN), ";transport=", atom_to_list(T), ";protocol=", atom_to_list(P)]); +%% Don't omit defaults since they're dependent on whether RFC 3588 or +%% 6733 is being followed. For one, we don't know this at encode; for +%% two (more importantly), we don't know how the peer will interpret +%% defaults, so it's best to be explicit. Interpret defaults on decode +%% since there's no choice. 'DiameterURI'(encode, Str) -> Bin = iolist_to_binary(Str), - #diameter_uri{} = scan_uri(Bin), %% type check + #diameter_uri{} = scan_uri(Bin), %% assert Bin. %% -------------------- @@ -523,6 +524,45 @@ msb(false) -> ?TIME_2036. %% %% aaa-protocol = ( "diameter" / "radius" / "tacacs+" ) +%% RFC 6733, 4.3.1, changes the defaults: +%% +%% "aaa://" FQDN [ port ] [ transport ] [ protocol ] +%% +%% ; No transport security +%% +%% "aaas://" FQDN [ port ] [ transport ] [ protocol ] +%% +%% ; Transport security used +%% +%% FQDN = < Fully Qualified Domain Name > +%% +%% port = ":" 1*DIGIT +%% +%% ; One of the ports used to listen for +%% ; incoming connections. +%% ; If absent, the default Diameter port +%% ; (3868) is assumed if no transport +%% ; security is used and port 5658 when +%% ; transport security (TLS/TCP and DTLS/SCTP) +%% ; is used. +%% +%% transport = ";transport=" transport-protocol +%% +%% ; One of the transports used to listen +%% ; for incoming connections. If absent, +%% ; the default protocol is assumed to be TCP. +%% ; UDP MUST NOT be used when the aaa-protocol +%% ; field is set to diameter. +%% +%% transport-protocol = ( "tcp" / "sctp" / "udp" ) +%% +%% protocol = ";protocol=" aaa-protocol +%% +%% ; If absent, the default AAA protocol +%% ; is Diameter. +%% +%% aaa-protocol = ( "diameter" / "radius" / "tacacs+" ) + scan_uri(Bin) -> RE = "^(aaas?)://" "([-a-zA-Z0-9.]+)" @@ -532,15 +572,21 @@ scan_uri(Bin) -> {match, [A, DN, PN, T, P]} = re:run(Bin, RE, [{capture, [1,2,4,6,8], binary}]), - #diameter_uri{port = PN0, - transport = T0, - protocol = P0} - = #diameter_uri{}, - #diameter_uri{type = to_atom(A), + Type = to_atom(A), + {PN0, T0} = defaults(diameter_codec:getopt(rfc), Type), + #diameter_uri{type = Type, fqdn = from_bin(DN), port = to_int(PN, PN0), transport = to_atom(T, T0), - protocol = to_atom(P, P0)}. + protocol = to_atom(P, diameter)}. + +%% Choose defaults based on the RFC, since 6733 has changed them. +defaults(3588, _) -> + {3868, sctp}; +defaults(6733, aaa) -> + {3868, tcp}; +defaults(6733, aaas) -> + {5658, tcp}. from_bin(B) -> case diameter_codec:getopt(string_decode) of diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index fec6fb6107..de9c4bca33 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -126,11 +126,12 @@ i({Ack, T, Pid, {RecvData, random:seed(Seed), putr(restart, {T, Opts, Svc, SvcOpts}), %% save seeing it in trace putr(dwr, dwr(Caps)), %% - diameter_codec:setopts([{string_decode, false}]), {_,_} = Mask = proplists:get_value(sequence, SvcOpts), Restrict = proplists:get_value(restrict_connections, SvcOpts), Nodes = restrict_nodes(Restrict), Dict0 = common_dictionary(Apps), + diameter_codec:setopts([{common_dictionary, Dict0}, + {string_decode, false}]), #watchdog{parent = Pid, transport = start(T, Opts, SvcOpts, Nodes, Dict0, Svc), tw = proplists:get_value(watchdog_timer, -- cgit v1.2.3