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.

5 files changed, 70 insertions, 14 deletions

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,