diff options
Diffstat (limited to 'lib/diameter')
-rw-r--r-- | lib/diameter/doc/src/diameter.xml | 12 | ||||
-rw-r--r-- | lib/diameter/include/diameter_gen.hrl | 56 | ||||
-rw-r--r-- | lib/diameter/src/base/diameter_codec.erl | 21 | ||||
-rw-r--r-- | lib/diameter/src/base/diameter_peer_fsm.erl | 7 | ||||
-rw-r--r-- | lib/diameter/src/base/diameter_watchdog.erl | 7 | ||||
-rw-r--r-- | lib/diameter/test/diameter_relay_SUITE.erl | 44 |
6 files changed, 105 insertions, 42 deletions
diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml index ea175a58b8..854bc5b432 100644 --- a/lib/diameter/doc/src/diameter.xml +++ b/lib/diameter/doc/src/diameter.xml @@ -794,14 +794,6 @@ Messages larger than the specified number of bytes are discarded.</p> Defaults to <c>16777215</c>, the maximum value of the 24-bit Message Length field in a Diameter Header.</p> -<warning> -<p> -This option should be set to as low a value as is sufficient for the -Diameter applications and peers in question, since decoding incoming -messages from a malicious peer can otherwise generate significant -load.</p> -</warning> - </item> <tag><c>{restrict_connections, false @@ -1231,9 +1223,7 @@ is not the length of the message in question, as received over the transport interface documented in &man_transport;.</p> <p> -If <c>exit</c> then a warning report is emitted and the parent of the -transport process in question exits, which causes the transport -process itself to exit as described in &man_transport;. +If <c>exit</c> then the transport process in question exits. If <c>handle</c> then the message is processed as usual, a resulting &app_handle_request; or &app_handle_answer; callback (if one takes place) indicating the <c>5015</c> error (DIAMETER_INVALID_MESSAGE_LENGTH). diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl index e8ffe7f92c..ebe9e6363d 100644 --- a/lib/diameter/include/diameter_gen.hrl +++ b/lib/diameter/include/diameter_gen.hrl @@ -185,9 +185,10 @@ decode_avps(Name, Recs) -> = lists:foldl(fun(T,A) -> decode(Name, T, A) end, {[], {newrec(Name), []}}, Recs), - {Rec, Avps, Failed ++ missing(Rec, Name)}. -%% Append 5005 errors so that a 5014 for the same AVP will take -%% precedence in a Result-Code/Failed-AVP setting. + {Rec, Avps, Failed ++ missing(Rec, Name, Failed)}. +%% Append 5005 errors so that errors are reported in the order +%% encountered. Failed-AVP should typically contain the first +%% encountered error accordg to the RFC. newrec(Name) -> '#new-'(name2rec(Name)). @@ -200,20 +201,36 @@ newrec(Name) -> %% Failed-AVP AVP SHOULD be included in the message. The Failed-AVP %% AVP MUST contain an example of the missing AVP complete with the %% Vendor-Id if applicable. The value field of the missing AVP -%% should be of correct minimum length and contain zeroes. - -missing(Rec, Name) -> - [{5005, empty_avp(F)} || F <- '#info-'(element(1, Rec), fields), - A <- [avp_arity(Name, F)], - false <- [have_arity(A, '#get-'(F, Rec))]]. +%% should be of correct minimum length and contain zeros. + +missing(Rec, Name, Failed) -> + Avps = lists:foldl(fun({_, #diameter_avp{code = C, vendor_id = V}}, A) -> + sets:add_element({C,V}, A) + end, + sets:new(), + Failed), + [{5005, A} || F <- '#info-'(element(1, Rec), fields), + not has_arity(avp_arity(Name, F), '#get-'(F, Rec)), + #diameter_avp{code = C, vendor_id = V} + = A <- [empty_avp(F)], + not sets:is_element({C,V}, Avps)]. %% Maximum arities have already been checked in building the record. -have_arity({Min, _}, L) -> - Min =< length(L); -have_arity(N, V) -> +has_arity({Min, _}, L) -> + has_prefix(Min, L); +has_arity(N, V) -> N /= 1 orelse V /= undefined. +%% Compare a non-negative integer and the length of a list without +%% computing the length. +has_prefix(0, _) -> + true; +has_prefix(_, []) -> + false; +has_prefix(N, L) -> + has_prefix(N-1, tl(L)). + %% empty_avp/1 empty_avp(Name) -> @@ -581,14 +598,17 @@ pack(undefined, 1, FieldName, Avp, Acc) -> %% AVP MUST be included and contain a copy of the first instance of %% the offending AVP that exceeded the maximum number of occurrences %% + pack(_, 1, _, Avp, {Rec, Failed}) -> {Rec, [{5009, Avp} | Failed]}; -pack(L, {_, Max}, _, Avp, {Rec, Failed}) - when length(L) == Max -> - {Rec, [{5009, Avp} | Failed]}; - -pack(L, _, FieldName, Avp, Acc) -> - p(FieldName, fun(V) -> [V|L] end, Avp, Acc). +pack(L, {_, Max}, FieldName, Avp, Acc) -> + case '*' /= Max andalso has_prefix(Max, L) of + true -> + {Rec, Failed} = Acc, + {Rec, [{5009, Avp} | Failed]}; + false -> + p(FieldName, fun(V) -> [V|L] end, Avp, Acc) + end. %% p/4 diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl index bf2fe8e7ca..2ad971a422 100644 --- a/lib/diameter/src/base/diameter_codec.erl +++ b/lib/diameter/src/base/diameter_codec.erl @@ -655,16 +655,23 @@ split_data(Bin, Len) -> %% The normal case here is data as an #diameter_avp{} list or an %% iolist, which are the cases that generated codec modules use. The -%% other case is as a convenience in the relay case in which the +%% other cases are a convenience in the relay case in which the %% dictionary doesn't know about specific AVP's. -%% Grouped AVP whose components need packing ... -pack_avp([#diameter_avp{} = A | Avps]) -> - pack_avp(A#diameter_avp{data = Avps}); -pack_avp(#diameter_avp{data = [#diameter_avp{} | _] = Avps} = A) -> - pack_avp(A#diameter_avp{data = encode_avps(Avps)}); +%% Decoded Grouped AVP with decoded components: ignore components +%% since they're already encoded in the Grouped AVP. +pack_avp([#diameter_avp{} = Grouped | _Components]) -> + pack_avp(Grouped); -%% ... data as a type/value tuple ... +%% Grouped AVP whose components need packing. It's intentional that +%% this isn't equivalent to [Grouped | Components]: here the +%% components need to be encoded before wrapping with the Grouped AVP, +%% and the list is flat, nesting being accomplished in the data +%% fields. +pack_avp(#diameter_avp{data = [#diameter_avp{} | _] = Components} = Grouped) -> + pack_avp(Grouped#diameter_avp{data = encode_avps(Components)}); + +%% Data as a type/value tuple ... pack_avp(#diameter_avp{data = {Type, Value}} = A) when is_atom(Type) -> pack_avp(A#diameter_avp{data = diameter_types:Type(encode, Value)}); diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl index 2255d0a76b..a9ee4940a3 100644 --- a/lib/diameter/src/base/diameter_peer_fsm.erl +++ b/lib/diameter/src/base/diameter_peer_fsm.erl @@ -319,7 +319,7 @@ handle_info(T, #state{} = State) -> ?LOG(stop, Reason), {stop, {shutdown, Reason}, State}; stop -> - ?LOG(stop, T), + ?LOG(stop, truncate(T)), {stop, {shutdown, T}, State} catch exit: {diameter_codec, encode, T} = Reason -> @@ -355,6 +355,11 @@ code_change(_, State, _) -> %% --------------------------------------------------------------------------- %% --------------------------------------------------------------------------- +truncate({'DOWN' = T, _, process, Pid, _}) -> + {T, Pid}; +truncate(T) -> + T. + putr(Key, Val) -> put({?MODULE, Key}, Val). diff --git a/lib/diameter/src/base/diameter_watchdog.erl b/lib/diameter/src/base/diameter_watchdog.erl index de9c4bca33..55c303dec2 100644 --- a/lib/diameter/src/base/diameter_watchdog.erl +++ b/lib/diameter/src/base/diameter_watchdog.erl @@ -245,11 +245,16 @@ handle_info(T, #watchdog{} = State) -> event(T, State, S), %% before 'watchdog' {noreply, S}; stop -> - ?LOG(stop, T), + ?LOG(stop, truncate(T)), event(T, State, State#watchdog{status = down}), {stop, {shutdown, T}, State} end. +truncate({'DOWN' = T, _, process, Pid, _}) -> + {T, Pid}; +truncate(T) -> + T. + close({'DOWN', _, process, TPid, {shutdown, Reason}}, #watchdog{transport = TPid, parent = Pid}) -> diff --git a/lib/diameter/test/diameter_relay_SUITE.erl b/lib/diameter/test/diameter_relay_SUITE.erl index 7142239bbb..5f7837e879 100644 --- a/lib/diameter/test/diameter_relay_SUITE.erl +++ b/lib/diameter/test/diameter_relay_SUITE.erl @@ -333,13 +333,39 @@ realm(Host) -> call(Server) -> Realm = realm(Server), + %% Include some arbitrary AVPs to exercise encode/decode, that + %% are received back in the STA. + Avps = [#diameter_avp{code = 111, + data = [#diameter_avp{code = 222, + data = <<222:24>>}, + #diameter_avp{code = 333, + data = <<333:16>>}]}, + #diameter_avp{code = 444, + data = <<444:24>>}, + #diameter_avp{code = 555, + data = [#diameter_avp{code = 666, + data = [#diameter_avp + {code = 777, + data = <<7>>}]}, + #diameter_avp{code = 888, + data = <<8>>}, + #diameter_avp{code = 999, + data = <<9>>}]}], + Req = ['STR', {'Destination-Realm', Realm}, {'Destination-Host', [Server]}, {'Termination-Cause', ?LOGOUT}, - {'Auth-Application-Id', ?APP_ID}], + {'Auth-Application-Id', ?APP_ID}, + {'AVP', Avps}], + #diameter_base_STA{'Result-Code' = ?SUCCESS, 'Origin-Host' = Server, - 'Origin-Realm' = Realm} + 'Origin-Realm' = Realm, + %% Unknown AVPs can't be decoded as Grouped since + %% types aren't known. + 'AVP' = [#diameter_avp{code = 111}, + #diameter_avp{code = 444}, + #diameter_avp{code = 555}]} = call(Req, [{filter, realm}]). call(Req, Opts) -> @@ -433,9 +459,18 @@ request(_Pkt, #diameter_caps{origin_host = {OH, _}}) request(#diameter_packet{msg = #diameter_base_STR{'Session-Id' = SId, 'Origin-Host' = Host, 'Origin-Realm' = Realm, - 'Route-Record' = Route}}, + 'Route-Record' = Route, + 'AVP' = Avps}}, #diameter_caps{origin_host = {OH, _}, origin_realm = {OR, _}}) -> + + %% Payloads of unknown AVPs aren't decoded, so we don't know that + %% some types here are Grouped. + [#diameter_avp{code = 111, vendor_id = undefined}, + #diameter_avp{code = 444, vendor_id = undefined, data = <<444:24>>}, + #diameter_avp{code = 555, vendor_id = undefined}] + = Avps, + %% The request should have the Origin-Host/Realm of the original %% sender. R = realm(?CLIENT), @@ -446,4 +481,5 @@ request(#diameter_packet{msg = #diameter_base_STR{'Session-Id' = SId, {reply, #diameter_base_STA{'Result-Code' = ?SUCCESS, 'Session-Id' = SId, 'Origin-Host' = OH, - 'Origin-Realm' = OR}}. + 'Origin-Realm' = OR, + 'AVP' = Avps}}. |