From a732bb3496cf24e62b607293de7c5ae49b3891ce Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 11 Apr 2013 09:43:24 +0200 Subject: Fix decode failure when AVP Length < 8 Such a length caused decode of a message with valid (24-bit) length to fail. Note that the error detected is wrong: it should be 5014 (INVALID_AVP_LENGTH), not 3009 (INVALID_AVP_BITS). This will be dealt with by OTP-11007. --- lib/diameter/test/diameter_traffic_SUITE.erl | 53 +++++++++++++++++++++------- 1 file changed, 40 insertions(+), 13 deletions(-) (limited to 'lib/diameter/test') diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl index 781ed234cc..5ccfd4bb6b 100644 --- a/lib/diameter/test/diameter_traffic_SUITE.erl +++ b/lib/diameter/test/diameter_traffic_SUITE.erl @@ -49,7 +49,9 @@ send_unsupported_app/1, send_error_bit/1, send_unsupported_version/1, - send_invalid_avp_bits/1, + send_long_avp_length/1, + send_short_avp_length/1, + send_zero_avp_length/1, send_invalid_avp_length/1, send_invalid_reject/1, send_long/1, @@ -268,7 +270,9 @@ tc() -> send_unsupported_app, send_error_bit, send_unsupported_version, - send_invalid_avp_bits, + send_long_avp_length, + send_short_avp_length, + send_zero_avp_length, send_invalid_avp_length, send_invalid_reject, send_long, @@ -481,13 +485,20 @@ send_unsupported_version(Config) -> ['STA', _SessionId, {'Result-Code', ?UNSUPPORTED_VERSION} | _] = call(Config, Req). -%% Send a request containing an incorrect AVP length. -send_invalid_avp_bits(Config) -> +%% Send a request containing an AVP length > data size. +send_long_avp_length(Config) -> Req = ['STR', {'Termination-Cause', ?LOGOUT}], ?answer_message(?INVALID_AVP_BITS) = call(Config, Req). +%% Send a request containing an AVP length < data size. +send_short_avp_length(Config) -> + Req = ['STR', {'Termination-Cause', ?LOGOUT}], + + ['STA', _SessionId, {'Result-Code', ?INVALID_AVP_LENGTH} | _] + = call(Config, Req). + %% Send a request containing an AVP length that doesn't match the %% AVP's type. send_invalid_avp_length(Config) -> @@ -496,6 +507,13 @@ send_invalid_avp_length(Config) -> ['STA', _SessionId, {'Result-Code', ?INVALID_AVP_LENGTH} | _] = call(Config, Req). +%% Send a request containing an AVP whose advertised length is < 8. +send_zero_avp_length(Config) -> + Req = ['STR', {'Termination-Cause', ?LOGOUT}], + + ?answer_message(?INVALID_AVP_BITS) + = call(Config, Req). + %% Send a request containing 5xxx errors that the server rejects with %% 3xxx. send_invalid_reject(Config) -> @@ -804,25 +822,32 @@ log(#diameter_packet{bin = Bin} = P, T) %% prepare/4 -prepare(Pkt, Caps, send_invalid_avp_bits, #group{client_dict0 = Dict0} - = Group) -> +prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) + when N == send_long_avp_length; + N == send_short_avp_length; + N == send_zero_avp_length -> Req = prepare(Pkt, Caps, Group), - %% Last AVP in our STR is Termination-Cause of type Unsigned32: - %% set its length improperly. + %% Second last AVP in our STR is Auth-Application-Id of type + %% Unsigned32: set AVP Length to a value other than 12. #diameter_packet{header = #diameter_header{length = L}, bin = B} = E = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), - Offset = L - 7, %% to AVP Length - <> = B, - E#diameter_packet{bin = <>}; + Offset = L - 7 - 12, %% to AVP Length + <> = B, %% assert + AL = case N of + send_long_avp_length -> 13; + send_short_avp_length -> 11; + send_zero_avp_length -> 0 + end, + E#diameter_packet{bin = <>}; prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) when N == send_invalid_avp_length; N == send_invalid_reject -> Req = prepare(Pkt, Caps, Group), %% Second last AVP in our STR is Auth-Application-Id of type - %% Unsigned32: Send a value of length 8. + %% Unsigned32: send data of length 8. #diameter_packet{header = #diameter_header{length = L}, bin = B0} = E @@ -944,7 +969,9 @@ answer(Pkt, Req, _Peer, Name, #group{client_dict0 = Dict0}) -> [Dict:rec2msg(R) | Vs]. answer(Rec, [_|_], N) - when N == send_invalid_avp_bits; + when N == send_long_avp_length; + N == send_short_avp_length; + N == send_zero_avp_length; N == send_invalid_avp_length; N == send_invalid_reject -> Rec; -- cgit v1.2.3 From 9b1d1935b707f5fd19693500dfa6368580cc93e8 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 23 Apr 2013 11:49:29 +0200 Subject: Correct AVP Length error testcases To return what RFC 6733 says. 3588 says less so follow 6733, even though the extra specification of 6733 means that it isn't strictly backwards compatible. In particular, 6733 says to send a zero'd payload or none at all while 3588 says to send the offending AVP, despite the fact that the peer will likely have equal difficulty in decoding it. The testcases now fail, which will be remedied in subsequent commits. --- lib/diameter/test/diameter_traffic_SUITE.erl | 44 ++++++++++++++++------------ 1 file changed, 25 insertions(+), 19 deletions(-) (limited to 'lib/diameter/test') diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl index 5ccfd4bb6b..1d5627af5e 100644 --- a/lib/diameter/test/diameter_traffic_SUITE.erl +++ b/lib/diameter/test/diameter_traffic_SUITE.erl @@ -487,31 +487,31 @@ send_unsupported_version(Config) -> %% Send a request containing an AVP length > data size. send_long_avp_length(Config) -> - Req = ['STR', {'Termination-Cause', ?LOGOUT}], - - ?answer_message(?INVALID_AVP_BITS) - = call(Config, Req). + send_invalid_avp_length(Config). %% Send a request containing an AVP length < data size. send_short_avp_length(Config) -> - Req = ['STR', {'Termination-Cause', ?LOGOUT}], + send_invalid_avp_length(Config). - ['STA', _SessionId, {'Result-Code', ?INVALID_AVP_LENGTH} | _] - = call(Config, Req). +%% Send a request containing an AVP whose advertised length is < 8. +send_zero_avp_length(Config) -> + send_invalid_avp_length(Config). %% Send a request containing an AVP length that doesn't match the %% AVP's type. send_invalid_avp_length(Config) -> Req = ['STR', {'Termination-Cause', ?LOGOUT}], - ['STA', _SessionId, {'Result-Code', ?INVALID_AVP_LENGTH} | _] - = call(Config, Req). - -%% Send a request containing an AVP whose advertised length is < 8. -send_zero_avp_length(Config) -> - Req = ['STR', {'Termination-Cause', ?LOGOUT}], - - ?answer_message(?INVALID_AVP_BITS) + ['STA', _SessionId, + {'Result-Code', ?INVALID_AVP_LENGTH}, + _OriginHost, + _OriginRealm, + _UserName, + _Class, + _ErrorMessage, + _ErrorReportingHost, + {'Failed-AVP', [#'diameter_base_Failed-AVP'{'AVP' = [_]}]} + | _] = call(Config, Req). %% Send a request containing 5xxx errors that the server rejects with @@ -828,19 +828,25 @@ prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) N == send_zero_avp_length -> Req = prepare(Pkt, Caps, Group), %% Second last AVP in our STR is Auth-Application-Id of type - %% Unsigned32: set AVP Length to a value other than 12. + %% Unsigned32: set AVP Length to a value other than 12 and place + %% it last in the message (so as not to mess with Termination-Cause). #diameter_packet{header = #diameter_header{length = L}, bin = B} = E = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), - Offset = L - 7 - 12, %% to AVP Length - <> = B, %% assert + Offset = L - 24, %% to Auth-Application-Id + <> + = B, AL = case N of send_long_avp_length -> 13; send_short_avp_length -> 11; send_zero_avp_length -> 0 end, - E#diameter_packet{bin = <>}; + E#diameter_packet{bin = <>}; prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) when N == send_invalid_avp_length; -- cgit v1.2.3 From 4ce2d3a67488811be96853f9b83ef8da2712b5e7 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Fri, 5 Apr 2013 11:04:07 +0200 Subject: Fix recognition of 5014 (INVALID_AVP_LENGTH) errors Invalid lengths come in two flavours: ones that correctly point at the end of an AVP's payload but don't agree with its type, and ones that point elsewhere. The former are relatively harmless but the latter leave no way to recover AVP boundaries, which typically results in failure to decode subsequent AVP's in the message in question. In the case that AVP Length points past the end of the message, diameter incorrectly regarded the error as 5009, INVALID_AVP_BITS: not right since the error has nothing to do with AVP Flags. Ditto if the length was less than 8, a minimal header length. Only in the remaining case was the detected error 5014, INVALID_AVP_LENGTH. However, in this case it slavishly followed RFC 3588 in suggesting the undecodable AVP as Failed-AVP, thereby passing the woeful payload back to the sender to have equal difficulty decoding. Now follow RFC 6733 and suggest an AVP with a zero-filled payload. --- lib/diameter/test/diameter_3xxx_SUITE.erl | 46 +++++++++++++------------------ 1 file changed, 19 insertions(+), 27 deletions(-) (limited to 'lib/diameter/test') diff --git a/lib/diameter/test/diameter_3xxx_SUITE.erl b/lib/diameter/test/diameter_3xxx_SUITE.erl index 89c78d8b57..0ec0d5020f 100644 --- a/lib/diameter/test/diameter_3xxx_SUITE.erl +++ b/lib/diameter/test/diameter_3xxx_SUITE.erl @@ -40,7 +40,7 @@ send_unknown_application/1, send_unknown_command/1, send_ok/1, - send_invalid_avp_bits/1, + send_invalid_hdr_bits/1, send_missing_avp/1, send_ignore_missing_avp/1, send_double_error/1, @@ -136,7 +136,7 @@ tc() -> [send_unknown_application, send_unknown_command, send_ok, - send_invalid_avp_bits, + send_invalid_hdr_bits, send_missing_avp, send_ignore_missing_avp, send_double_error, @@ -216,27 +216,26 @@ send_ok([_,_]) -> send_ok(Config) -> send_ok(?group(Config)). -%% send_invalid_avp_bits/1 +%% send_invalid_hdr_bits/1 %% -%% Send a request with an incorrect length on the optional -%% Origin-State-Id that a callback ignores. +%% Send a request with an incorrect E-bit that a callback ignores. %% Callback answers. -send_invalid_avp_bits([callback, _]) -> +send_invalid_hdr_bits([callback, _]) -> #diameter_base_STA{'Result-Code' = 2001, %% SUCCESS 'Failed-AVP' = [], 'AVP' = []} = call(); %% diameter answers. -send_invalid_avp_bits([_,_]) -> - #'diameter_base_answer-message'{'Result-Code' = 3009, %% INVALID_AVP_BITS +send_invalid_hdr_bits([_,_]) -> + #'diameter_base_answer-message'{'Result-Code' = 3008, %% INVALID_HDR_BITS 'Failed-AVP' = [], 'AVP' = []} = call(); -send_invalid_avp_bits(Config) -> - send_invalid_avp_bits(?group(Config)). +send_invalid_hdr_bits(Config) -> + send_invalid_hdr_bits(?group(Config)). %% send_missing_avp/1 %% @@ -282,8 +281,7 @@ send_ignore_missing_avp(Config) -> %% send_double_error/1 %% -%% Send a request with both an incorrect length on the optional -%% Origin-State-Id and a missing AVP. +%% Send a request with both an invalid E-bit and a missing AVP. %% Callback answers with STA. send_double_error([callback, _]) -> @@ -294,8 +292,8 @@ send_double_error([callback, _]) -> %% diameter answers with answer-message. send_double_error([_,_]) -> - #'diameter_base_answer-message'{'Result-Code' = 3009, %% INVALID_AVP_BITS - 'Failed-AVP' = [_], + #'diameter_base_answer-message'{'Result-Code' = 3008, %% INVALID_HDR_BITS + 'Failed-AVP' = [], 'AVP' = []} = call(); @@ -392,20 +390,16 @@ prepare(Pkt, Caps, T) T == send_5xxx -> sta(Pkt, Caps); -prepare(Pkt0, Caps, send_invalid_avp_bits) -> - Req0 = sta(Pkt0, Caps), - %% Append an Origin-State-Id with an incorrect AVP Length in order - %% to force 3009. - Req = Req0#diameter_base_STR{'Origin-State-Id' = [7]}, - #diameter_packet{bin = Bin} +prepare(Pkt0, Caps, send_invalid_hdr_bits) -> + Req = sta(Pkt0, Caps), + %% Set the E-bit to force 3008. + #diameter_packet{bin = <>} = Pkt = diameter_codec:encode(?DICT, Pkt0#diameter_packet{msg = Req}), - Offset = size(Bin) - 12 + 5, - <> = Bin, - Pkt#diameter_packet{bin = <>}; + Pkt#diameter_packet{bin = <>}; prepare(Pkt0, Caps, send_double_error) -> - dehost(prepare(Pkt0, Caps, send_invalid_avp_bits)); + dehost(prepare(Pkt0, Caps, send_invalid_hdr_bits)); prepare(Pkt, Caps, T) when T == send_missing_avp; @@ -480,9 +474,7 @@ request(send_3xxx, _Req, _Caps) -> request(send_5xxx, _Req, _Caps) -> {answer_message, 5999}; -request(send_invalid_avp_bits, Req, Caps) -> - #diameter_base_STR{'Origin-State-Id' = []} - = Req, +request(send_invalid_hdr_bits, Req, Caps) -> %% Default errors field but a non-answer-message and only 3xxx %% errors detected means diameter sets neither Result-Code nor %% Failed-AVP. -- cgit v1.2.3 From 38a75600cbd5b51ae61921df78cc02348e563564 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Fri, 3 May 2013 01:38:12 +0200 Subject: Detect all 5005 (MISSING_AVP) errors and don't reverse errors The previous commit ensures that only one will be reported in an answer message when diameter itself sets Result-Code/Failed-AVP. The order of errors in #diameter_packet.errors is that in which they're detected, not the reverse as previously. --- lib/diameter/test/diameter_codec_SUITE_data/diameter_test_unknown.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/diameter/test') diff --git a/lib/diameter/test/diameter_codec_SUITE_data/diameter_test_unknown.erl b/lib/diameter/test/diameter_codec_SUITE_data/diameter_test_unknown.erl index bce3d78a37..49f2158b1a 100644 --- a/lib/diameter/test/diameter_codec_SUITE_data/diameter_test_unknown.erl +++ b/lib/diameter/test/diameter_codec_SUITE_data/diameter_test_unknown.erl @@ -71,6 +71,6 @@ dec('AR', #diameter_packet dec('BR', #diameter_packet {msg = #recv_BR{'Origin-Host' = ?HOST, 'Origin-Realm' = ?REALM}, - errors = [{5008, ?NOT_MANDATORY_YYY}, - {5001, ?MANDATORY_XXX}]}) -> + errors = [{5001, ?MANDATORY_XXX}, + {5008, ?NOT_MANDATORY_YYY}]}) -> ok. -- cgit v1.2.3 From 066544fa0c85e2603f5f0ebfdd8b2f07a8e4a4a7 Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Tue, 14 May 2013 13:11:32 +0200 Subject: Fix recognition of 5001 on mandatory AVP's An AVP setting the M-bit was not regarded as erroneous if it was defined in the dictionary in question and its container (message or Grouped AVP) had an 'AVP' field. It's now regarded as a 5001 error (AVP_UNSUPPORTED), as in the case that the AVP is not defined. --- lib/diameter/test/diameter_traffic_SUITE.erl | 29 ++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) (limited to 'lib/diameter/test') diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl index 1d5627af5e..42cb4b6844 100644 --- a/lib/diameter/test/diameter_traffic_SUITE.erl +++ b/lib/diameter/test/diameter_traffic_SUITE.erl @@ -54,6 +54,7 @@ send_zero_avp_length/1, send_invalid_avp_length/1, send_invalid_reject/1, + send_unrecognized_mandatory/1, send_long/1, send_nopeer/1, send_noapp/1, @@ -275,6 +276,7 @@ tc() -> send_zero_avp_length, send_invalid_avp_length, send_invalid_reject, + send_unrecognized_mandatory, send_long, send_nopeer, send_noapp, @@ -420,13 +422,14 @@ send_protocol_error(Config) -> ?answer_message(?TOO_BUSY) = call(Config, Req). -%% Send an ASR with an arbitrary AVP and expect success and the same -%% AVP in the reply. +%% Send an ASR with an arbitrary non-mandatory AVP and expect success +%% and the same AVP in the reply. send_arbitrary(Config) -> - Req = ['ASR', {'AVP', [#diameter_avp{name = 'Class', value = "XXX"}]}], + Req = ['ASR', {'AVP', [#diameter_avp{name = 'Product-Name', + value = "XXX"}]}], ['ASA', _SessionId, {'Result-Code', ?SUCCESS} | Avps] = call(Config, Req), - {'AVP', [#diameter_avp{name = 'Class', + {'AVP', [#diameter_avp{name = 'Product-Name', value = "XXX"}]} = lists:last(Avps). @@ -522,6 +525,14 @@ send_invalid_reject(Config) -> ?answer_message(?TOO_BUSY) = call(Config, Req). +%% Send an STR containing a known AVP, but one that's not allowed and +%% sets the M-bit. +send_unrecognized_mandatory(Config) -> + Req = ['STR', {'Termination-Cause', ?LOGOUT}], + + ['STA', _SessionId, {'Result-Code', ?AVP_UNSUPPORTED} | _] + = call(Config, Req). + %% Send something long that will be fragmented by TCP. send_long(Config) -> Req = ['STR', {'Termination-Cause', ?LOGOUT}, @@ -868,6 +879,16 @@ prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) 0:32/integer, T/binary>>}; +prepare(Pkt, Caps, send_unrecognized_mandatory, #group{client_dict0 = Dict0} + = Group) -> + Req = prepare(Pkt, Caps, Group), + #diameter_packet{bin = <>} + = E + = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), + {Code, Flags, undefined} = Dict0:avp_header('Proxy-State'), + Avp = <>, + E#diameter_packet{bin = <>}; + prepare(Pkt, Caps, send_unsupported, #group{client_dict0 = Dict0} = Group) -> Req = prepare(Pkt, Caps, Group), #diameter_packet{bin = <>} -- cgit v1.2.3 From 0b5dbb28332012c22b915364b55ce31843a26bdf Mon Sep 17 00:00:00 2001 From: Anders Svensson Date: Thu, 16 May 2013 18:40:13 +0200 Subject: Remove redundant integer type specifiers from binaries --- lib/diameter/test/diameter_codec_test.erl | 10 +++++----- lib/diameter/test/diameter_traffic_SUITE.erl | 19 +++++++------------ 2 files changed, 12 insertions(+), 17 deletions(-) (limited to 'lib/diameter/test') diff --git a/lib/diameter/test/diameter_codec_test.erl b/lib/diameter/test/diameter_codec_test.erl index 0baac59c1a..24d4c7665e 100644 --- a/lib/diameter/test/diameter_codec_test.erl +++ b/lib/diameter/test/diameter_codec_test.erl @@ -265,7 +265,7 @@ arity(M, Name, AvpName, Rec) -> %% enum/3 enum(M, Name, {_,E}) -> - B = <>, + B = <>, B = M:avp(encode, E, Name), E = M:avp(decode, B, Name). @@ -322,15 +322,15 @@ values('Unsigned64') -> values('Float32') -> E = (1 bsl 8) - 2, F = (1 bsl 23) - 1, - <> = <<0:1/integer, E:8/integer, F:23/integer>>, - <> = <<1:1/integer, E:8/integer, F:23/integer>>, + <> = <<0:1, E:8, F:23>>, + <> = <<1:1, E:8, F:23>>, {[0.0, infinity, '-infinity', Mx, Mn], [0]}; values('Float64') -> E = (1 bsl 11) - 2, F = (1 bsl 52) - 1, - <> = <<0:1/integer, E:11/integer, F:52/integer>>, - <> = <<1:1/integer, E:11/integer, F:52/integer>>, + <> = <<0:1, E:11, F:52>>, + <> = <<1:1, E:11, F:52>>, {[0.0, infinity, '-infinity', Mx, Mn], [0]}; values('Address') -> diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl index 42cb4b6844..21a9625fa1 100644 --- a/lib/diameter/test/diameter_traffic_SUITE.erl +++ b/lib/diameter/test/diameter_traffic_SUITE.erl @@ -847,7 +847,7 @@ prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), Offset = L - 24, %% to Auth-Application-Id <> = B, AL = case N of @@ -857,7 +857,7 @@ prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) end, E#diameter_packet{bin = <>}; + Hdr/binary, AL:24, Data/binary>>}; prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) when N == send_invalid_avp_length; @@ -870,14 +870,9 @@ prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group) = E = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), Offset = L - 7 - 12, %% to AVP Length - <> = B0, - <> = H0, %% assert - E#diameter_packet{bin = <>}; + <> = B0, + <> = H0, %% assert + E#diameter_packet{bin = <>}; prepare(Pkt, Caps, send_unrecognized_mandatory, #group{client_dict0 = Dict0} = Group) -> @@ -894,7 +889,7 @@ prepare(Pkt, Caps, send_unsupported, #group{client_dict0 = Dict0} = Group) -> #diameter_packet{bin = <>} = E = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), - E#diameter_packet{bin = <>}; + E#diameter_packet{bin = <>}; prepare(Pkt, Caps, send_unsupported_app, #group{client_dict0 = Dict0} = Group) -> @@ -902,7 +897,7 @@ prepare(Pkt, Caps, send_unsupported_app, #group{client_dict0 = Dict0} #diameter_packet{bin = <>} = E = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}), - E#diameter_packet{bin = <>}; + E#diameter_packet{bin = <>}; prepare(Pkt, Caps, send_error_bit, Group) -> #diameter_packet{header = Hdr} = Pkt, -- cgit v1.2.3