aboutsummaryrefslogtreecommitdiffstats
path: root/lib/diameter
diff options
context:
space:
mode:
authorAnders Svensson <[email protected]>2014-05-27 09:00:40 +0200
committerAnders Svensson <[email protected]>2014-05-27 09:00:40 +0200
commit316fe2a30714b9eb87e0d11f6bdbbd1eb9fefdc1 (patch)
tree14601c07ecb5101af0d15a3ad36db9b7db004270 /lib/diameter
parent44fc88d806af0b4a5c861e3501d210a8c574533b (diff)
parent58a74f2b6b6bd604d60fa8dca347cdd4ad2e4a3b (diff)
downloadotp-316fe2a30714b9eb87e0d11f6bdbbd1eb9fefdc1.tar.gz
otp-316fe2a30714b9eb87e0d11f6bdbbd1eb9fefdc1.tar.bz2
otp-316fe2a30714b9eb87e0d11f6bdbbd1eb9fefdc1.zip
Merge branch 'anders/diameter/5014/OTP-11946' into maint
* anders/diameter/5014/OTP-11946: Fix handling of AVP length errors (5014) in unknown AVPs Add testcases that send unknown AVPs with a bad AVP Length
Diffstat (limited to 'lib/diameter')
-rw-r--r--lib/diameter/include/diameter_gen.hrl17
-rw-r--r--lib/diameter/src/base/diameter_codec.erl77
-rw-r--r--lib/diameter/test/diameter_traffic_SUITE.erl52
3 files changed, 105 insertions, 41 deletions
diff --git a/lib/diameter/include/diameter_gen.hrl b/lib/diameter/include/diameter_gen.hrl
index 319ad5a783..ebc10b8918 100644
--- a/lib/diameter/include/diameter_gen.hrl
+++ b/lib/diameter/include/diameter_gen.hrl
@@ -412,6 +412,23 @@ pack_avp(_, Arity, Avp, Acc) ->
%% pack_AVP/3
+%% Length failure was induced because of a header/payload length
+%% mismatch. The AVP Length is reset to match the received data if
+%% this AVP is encoded in an answer message, since the length is
+%% computed.
+%%
+%% Data is a truncated header if command_code = undefined, otherwise
+%% payload bytes. The former is padded to the length of a header if
+%% the AVP reaches an outgoing encode in diameter_codec.
+%%
+%% RFC 6733 says that an AVP returned with 5014 can contain a minimal
+%% payload for the AVP's type, but in this case we don't know the
+%% type.
+
+pack_AVP(_, #diameter_avp{data = <<0:1, Data/binary>>} = Avp, Acc) ->
+ {Rec, Failed} = Acc,
+ {Rec, [{5014, Avp#diameter_avp{data = Data}} | Failed]};
+
pack_AVP(Name, #diameter_avp{is_mandatory = M} = Avp, Acc) ->
case pack_arity(Name, M) of
0 ->
diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl
index 84139d7f5e..0ca4a84d21 100644
--- a/lib/diameter/src/base/diameter_codec.erl
+++ b/lib/diameter/src/base/diameter_codec.erl
@@ -237,7 +237,7 @@ rec2msg(Mod, Rec) ->
%% Unsuccessfully decoded AVPs will be placed in #diameter_packet.errors.
--spec decode(module(), #diameter_packet{} | bitstring())
+-spec decode(module(), #diameter_packet{} | binary())
-> #diameter_packet{}.
decode(Mod, Pkt) ->
@@ -271,7 +271,7 @@ decode(_, Mod, #diameter_packet{header = Hdr} = Pkt) ->
decode_avps(MsgName, Mod, Pkt, collect_avps(Pkt));
decode(Id, Mod, Bin)
- when is_bitstring(Bin) ->
+ when is_binary(Bin) ->
decode(Id, Mod, #diameter_packet{header = decode_header(Bin), bin = Bin}).
decode_avps(MsgName, Mod, Pkt, {E, Avps}) ->
@@ -298,7 +298,7 @@ decode_avps(MsgName, Mod, Pkt, Avps) -> %% ... or not
%%% # decode_header/1
%%% ---------------------------------------------------------------------------
--spec decode_header(bitstring())
+-spec decode_header(binary())
-> #diameter_header{}
| false.
@@ -309,7 +309,7 @@ decode_header(<<Version:8,
ApplicationId:32,
HopByHopId:32,
EndToEndId:32,
- _/bitstring>>) ->
+ _/binary>>) ->
<<R:1, P:1, E:1, T:1, _:4>>
= CmdFlags,
%% 3588 (ch 3) says that reserved bits MUST be set to 0 and ignored
@@ -422,7 +422,7 @@ msg_id(#diameter_header{application_id = A,
is_request = R}) ->
{A, C, if R -> 1; true -> 0 end};
-msg_id(<<_:32, Rbit:1, _:7, CmdCode:24, ApplId:32, _/bitstring>>) ->
+msg_id(<<_:32, Rbit:1, _:7, CmdCode:24, ApplId:32, _/binary>>) ->
{ApplId, CmdCode, Rbit}.
%%% ---------------------------------------------------------------------------
@@ -433,17 +433,18 @@ msg_id(<<_:32, Rbit:1, _:7, CmdCode:24, ApplId:32, _/bitstring>>) ->
%% order in the binary. Note also that grouped avp's aren't unraveled,
%% only those at the top level.
--spec collect_avps(#diameter_packet{} | bitstring())
+-spec collect_avps(#diameter_packet{} | binary())
-> [Avp]
| {Error, [Avp]}
when Avp :: #diameter_avp{},
Error :: {5014, #diameter_avp{}}.
collect_avps(#diameter_packet{bin = Bin}) ->
- <<_:20/binary, Avps/bitstring>> = Bin,
+ <<_:20/binary, Avps/binary>> = Bin,
collect_avps(Avps);
-collect_avps(Bin) ->
+collect_avps(Bin)
+ when is_binary(Bin) ->
collect_avps(Bin, 0, []).
collect_avps(<<>>, _, Acc) ->
@@ -473,7 +474,9 @@ collect_avps(Bin, N, Acc) ->
split_avp(Bin) ->
{Code, V, M, P, Len, HdrLen} = split_head(Bin),
- {Data, B} = split_data(Bin, HdrLen, Len - HdrLen),
+
+ <<_:HdrLen/binary, Rest/binary>> = Bin,
+ {Data, B} = split_data(Rest, Len - HdrLen),
{B, #diameter_avp{code = Code,
vendor_id = V,
@@ -483,17 +486,15 @@ split_avp(Bin) ->
%% split_head/1
-split_head(<<Code:32, 1:1, M:1, P:1, _:5, Len:24, V:32, _/bitstring>>) ->
+split_head(<<Code:32, 1:1, M:1, P:1, _:5, Len:24, V:32, _/binary>>) ->
{Code, V, M, P, Len, 12};
-split_head(<<Code:32, 0:1, M:1, P:1, _:5, Len:24, _/bitstring>>) ->
+split_head(<<Code:32, 0:1, M:1, P:1, _:5, Len:24, _/binary>>) ->
{Code, undefined, M, P, Len, 8};
-%% Header is truncated: pack_avp/1 will pad to the minimum header
-%% length.
-split_head(B)
- when is_bitstring(B) ->
- ?THROW({5014, #diameter_avp{data = B}}).
+%% Header is truncated.
+split_head(Bin) ->
+ ?THROW({5014, #diameter_avp{data = Bin}}).
%% 3588:
%%
@@ -528,34 +529,27 @@ split_head(B)
%% split_data/3
-split_data(Bin, HdrLen, Len)
- when 0 =< Len ->
- split_data(Bin, HdrLen, Len, (4 - (Len rem 4)) rem 4);
-
-split_data(_, _, _) ->
- invalid_avp_length().
+split_data(Bin, Len) ->
+ Pad = (4 - (Len rem 4)) rem 4,
-%% split_data/4
+ %% Len might be negative here, but that ensures the failure of the
+ %% binary match.
-split_data(Bin, HdrLen, Len, Pad) ->
case Bin of
- <<_:HdrLen/binary, Data:Len/binary, _:Pad/binary, Rest/bitstring>> ->
+ <<Data:Len/binary, _:Pad/binary, Rest/binary>> ->
{Data, Rest};
_ ->
- invalid_avp_length()
+ %% Header length points past the end of the message. As
+ %% stated in the 6733 text above, it's sufficient to
+ %% return a zero-filled minimal payload if this is a
+ %% request. Do this (in cases that we know the type) by
+ %% inducing a decode failure and letting the dictionary's
+ %% decode (in diameter_gen) deal with it. Here we don't
+ %% know type. If the type isn't known, then the decode
+ %% just strips the extra bit.
+ {<<0:1, Bin/binary>>, <<>>}
end.
-%% invalid_avp_length/0
-%%
-%% AVP Length doesn't mesh with payload. Induce a decode error by
-%% returning a payload that no valid Diameter type can have. This is
-%% so that a known AVP will result in 5014 error with a zero'd
-%% payload. Here we simply don't know how to construct this payload.
-%% (Yes, this solution is an afterthought.)
-
-invalid_avp_length() ->
- {<<0:1>>, <<>>}.
-
%%% ---------------------------------------------------------------------------
%%% # pack_avp/1
%%% ---------------------------------------------------------------------------
@@ -587,17 +581,22 @@ pack_avp(#diameter_avp{data = {Dict, Name, Value}} = A) ->
{Name, Type} = Dict:avp_name(Code, Vid),
pack_avp(A#diameter_avp{data = {Hdr, {Type, Value}}});
+%% ... with a truncated header ...
pack_avp(#diameter_avp{code = undefined, data = B})
- when is_bitstring(B) ->
+ when is_binary(B) ->
%% Reset the AVP Length of an AVP Header resulting from a 5014
%% error. The RFC doesn't explicitly say to do this but the
%% receiver can't correctly extract this and following AVP's
%% without a correct length. On the downside, the header doesn't
%% reveal if the received header has been padded.
Pad = 8*header_length(B) - bit_size(B),
- Len = size(<<H:5/binary, _:24, T/binary>> = <<B/bitstring, 0:Pad>>),
+ Len = size(<<H:5/binary, _:24, T/binary>> = <<B/binary, 0:Pad>>),
<<H/binary, Len:24, T/binary>>;
+%% ... from a dictionary compiled against old code (diameter_gen) ...
+pack_avp(#diameter_avp{data = <<0:1, B/binary>>} = A) ->
+ pack_avp(A#diameter_avp{data = B});
+
%% ... or as an iolist.
pack_avp(#diameter_avp{code = Code,
vendor_id = V,
diff --git a/lib/diameter/test/diameter_traffic_SUITE.erl b/lib/diameter/test/diameter_traffic_SUITE.erl
index a97c54fc04..89592f02ef 100644
--- a/lib/diameter/test/diameter_traffic_SUITE.erl
+++ b/lib/diameter/test/diameter_traffic_SUITE.erl
@@ -1,7 +1,7 @@
%%
%% %CopyrightBegin%
%%
-%% Copyright Ericsson AB 2010-2013. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2014. 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
@@ -43,7 +43,9 @@
send_protocol_error/1,
send_arbitrary/1,
send_unknown/1,
+ send_unknown_short/1,
send_unknown_mandatory/1,
+ send_unknown_short_mandatory/1,
send_noreply/1,
send_unsupported/1,
send_unsupported_app/1,
@@ -266,7 +268,9 @@ tc() ->
send_protocol_error,
send_arbitrary,
send_unknown,
+ send_unknown_short,
send_unknown_mandatory,
+ send_unknown_short_mandatory,
send_noreply,
send_unsupported,
send_unsupported_app,
@@ -447,6 +451,24 @@ send_unknown(Config) ->
data = <<17>>}]}
= lists:last(Avps).
+%% Ditto, and point the AVP length past the end of the message. Expect
+%% 5014.
+send_unknown_short(Config) ->
+ send_unknown_short(Config, false, ?INVALID_AVP_LENGTH).
+
+send_unknown_short(Config, M, RC) ->
+ Req = ['ASR', {'AVP', [#diameter_avp{code = 999,
+ is_mandatory = M,
+ data = <<17>>}]}],
+ ['ASA', _SessionId, {'Result-Code', RC} | Avps]
+ = call(Config, Req),
+ [#'diameter_base_Failed-AVP'{'AVP' = As}]
+ = proplists:get_value('Failed-AVP', Avps),
+ [#diameter_avp{code = 999,
+ is_mandatory = M,
+ data = <<17, _/binary>>}] %% extra bits from padding
+ = As.
+
%% Ditto but set the M flag.
send_unknown_mandatory(Config) ->
Req = ['ASR', {'AVP', [#diameter_avp{code = 999,
@@ -461,6 +483,11 @@ send_unknown_mandatory(Config) ->
data = <<17>>}]
= As.
+%% Ditto, and point the AVP length past the end of the message. Expect
+%% 5014 instead of 5001.
+send_unknown_short_mandatory(Config) ->
+ send_unknown_short(Config, true, ?INVALID_AVP_LENGTH).
+
%% Send an STR that the server ignores.
send_noreply(Config) ->
Req = ['STR', {'Termination-Cause', ?BAD_ANSWER}],
@@ -836,6 +863,26 @@ log(#diameter_packet{bin = Bin} = P, T)
%% prepare/4
prepare(Pkt, Caps, N, #group{client_dict0 = Dict0} = Group)
+ when N == send_unknown_short_mandatory;
+ N == send_unknown_short ->
+ Req = prepare(Pkt, Caps, Group),
+
+ #diameter_packet{header = #diameter_header{length = L},
+ bin = Bin}
+ = E
+ = diameter_codec:encode(Dict0, Pkt#diameter_packet{msg = Req}),
+
+ %% Find the unknown AVP data at the end of the message and alter
+ %% its length header.
+
+ {Padding, [17|_]} = lists:splitwith(fun(C) -> C == 0 end,
+ lists:reverse(binary_to_list(Bin))),
+
+ Offset = L - length(Padding) - 4,
+ <<H:Offset/binary, Len:24, T/binary>> = Bin,
+ E#diameter_packet{bin = <<H/binary, (Len+9):24, T/binary>>};
+
+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 ->
@@ -997,7 +1044,8 @@ answer(Rec, [_|_], N)
N == send_short_avp_length;
N == send_zero_avp_length;
N == send_invalid_avp_length;
- N == send_invalid_reject ->
+ N == send_invalid_reject;
+ N == send_unknown_short_mandatory ->
Rec;
answer(Rec, [], _) ->
Rec.