[radext] Mohamed Boucadair's Yes on draft-ietf-radext-radiusdtls-bis-15: (with COMMENT)

[Mohamed Boucadair via Datatracker <[e-mail]>]

Mohamed Boucadair has entered the following ballot position for
draft-ietf-radext-radiusdtls-bis-15: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ 
for more information about how to handle DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-radext-radiusdtls-bis/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Hi Jan-Frederik, Margaret, and Stefan,

Special thanks for the effort put into this important piece of work.

Thanks to Jürgen Schönwälder for the OPSDIR review and to Margaret for the
follow-up.

The document is dense with a comprehensive list of implementation and
operational considerations through the document. Some of these guidance would
be better exposed to operators if clearly separated from the protocol
machinery, but as Jürgen said let’s hope those who will deploy will tag all
those.

Please find below some few comments, fwiw:

# Source port number selection

CURRENT (3.1):
   The client source port used for
   RadSec connections is not fixed -- it is typically an ephemeral port
   picked by the client Operating System.

Can this also mention the mechanism in RFC6056?

If randomization is followed, then this would help with this:

Section 6.5.2

   RADIUS/DTLS clients SHOULD NOT send both RADIUS/UDP and RADIUS/DTLS
   packets to different servers from the same source socket.

# keepalive

CURRENT:
   RadSec implementations MUST utilize the existence of a TCP, TLS or
   DTLS connection where applicable in addition to the application-layer
   watchdog defined in [RFC3539], Section 3.4 when determining the
   liveness of each connection.

I guess by “utilize the existence” you meant implement some
heartbeat/keepalives.

For TCP, please note that rfc9293#3.8.4 says:

   Implementers MAY include "keep-alives" in their TCP implementations
   (MAY-5), although this practice is not universally accepted.

I don’t see an issue with the behavior in the spec given that 3539 requires
anyway the following:

   AAA protocols MUST support
   an application layer watchdog message.

# Logging

Some events are better logged for operational needs. For example, the following
events (and similar) should be logged

CURRENT:
   That is, the implementation SHOULD send a TLS close
   notification and, in the case of RADIUS/TLS, the underlying TCP
   connection MUST be closed if any of the following circumstances are
   seen:

# server IP?

OLD:
   *  server IP,

   *  server port.

NEW:
   *  server IP address,

   *  server port number.

OLD:
   Where a server accepts packets on multiple different 3-tuples
   (protocol, server IP, server port),

NEW:
   Where a server accepts packets on multiple different 3-tuples
   (protocol, server IP address, server port number),

# Same behavior

Section 3.1
   RadSec endpoints MUST NOT use the old RADIUS/UDP or RADIUS/TCP ports
   for RADIUS/DTLS or RADIUS/TLS.

Section 3.12
   Implementations MUST NOT exchange both insecure and secure traffic on
   the same UDP or TCP port.  It is RECOMMENDED that implementations
   make it impossible for such a configuration to be created.

These are covering the same points. May be consider having this discussion in
one single place.

Alternatively, consider linking both such as:

NEW
   RadSec endpoints MUST NOT use the old RADIUS/UDP or RADIUS/TCP ports
   for RADIUS/DTLS or RADIUS/TLS. See also Section 3.12.

# packets, records, and datagrams

CURRENT:
   RADIUS/DTLS endpoints MUST send exactly one RADIUS packet per DTLS
   record.  This ensures that the RADIUS packets do not get fragmented
   at a point where a re-ordering of UDP packets would result in
   decoding failures.  The DTLS specification mandates that a DTLS
   record must not span multiple UDP datagrams.  We note that a single
   UDP datagram may, however, contain multiple DTLS records.  RADIUS/
   DTLS endpoints MAY use this behavior to send multiple RADIUS packets
   in one UDP packet.

Maybe add a pointer to rfc9147#section-4.3?

I would delete “we note”.

Cheers,
Med