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