Skip to content

Commit

Permalink
Adapt to changed DiameterURI defaults in RFC 6733
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Anders Svensson committed Mar 24, 2015
1 parent 35f5640 commit b8a7df4
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 14 deletions.
9 changes: 9 additions & 0 deletions lib/diameter/src/base/diameter_codec.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion lib/diameter/src/base/diameter_peer_fsm.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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]}),
Expand Down
2 changes: 1 addition & 1 deletion lib/diameter/src/base/diameter_traffic.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
68 changes: 57 additions & 11 deletions lib/diameter/src/base/diameter_types.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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.

%% --------------------
Expand Down Expand Up @@ -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.]+)"
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion lib/diameter/src/base/diameter_watchdog.erl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit b8a7df4

Please sign in to comment.