Re: [radext] Review of draft-ietf-radext-radiusdtls

[Jan-Frederik Rieckers <[e-mail]>]

On 11.03.24 23:57, Alan DeKok wrote:
>    Now that I have a little bit of time, I'll do a detailed review of this document.  I still need to compare it to the previous docs and correlate missing / copied / added text, but that can wait a little bit.
> 
>    I'll leave nits for later.
> 
> 1.1
> 
> 	 ... where RADIUS packets need to be transferred through different administrative domains and untrusted, potentially hostile networks.
> 
>    The "and" here should be "or" instead.  Perhaps even just
> 
> 	... where RADIUS packets need to be sent across insecure or untrusted networks.

Integrated.

> 
> 1.2
> 
> 	... 	• RFC6614 marked TLSv1.1 or later as mandatory, this specification requires TLSv1.2 as minimum and recommends usage of TLSv1.3
> 
>    We should mandate that RADIUS servers use TLS 1.3.  If we don't do that, then there is little incentive for clients to upgrade, because servers won't support it.

Well, that's a policy decision up for the WG. The latest consensus IIRC 
was that this document should be as backward-compatible as possible, and 
since the original spec was written before TLS1.3, with TLS 1.2 we get 
the best backward compatibility, while still adhering to other IETF 
documents (mainly the "Deprecating TLS 1.0 and TLS 1.1")

> 	... 	• RFC6614 allowed usage of TLS compression, this document forbids it.
> 
>    Good, but why?  Perhaps "it was not found to be useful", or "no one implemented it".

I'd put it in the Design Decisions section.

To put it somewhere where I can copy the text for this if we decide to 
add this section:
For this specifically, my rationale was that compression is only allowed 
if there is never the possibility of chosen-plaintext (even partial), 
because that would potentially reveal other plaintext through length of 
the record.
And since we have some plaintext that an attacker could potentially 
choose (Username, MAC Address, EAP-Message) it is safer to just forbid 
compression.

> 2.
> 
> 	... Client implementations SHOULD implement both, but MUST only implement one of RADIUS/TLS or RADIUS/DTLS.
> 
>    I'm not sure why clients can't support both?  Perhaps
> 
> 	...  but MUST implement at least one of RADIUS/TLS or RADIUS/DTLS.

Ok, I see the confusion. (or vs xor, and bad placement of the word "only")

Fixed.

> 
> 3.1
> 
> 	... The requirement that RADIUS remain largely unchanged ensures the simplest possible implementation and widest interoperability of the specification.
> 
>    It would be worth repeating here that MD5 is bad, and people should be aware of security issues.  See "Security Considerations" section, and the ALPN doc.

I've added two sentences, probably worth looking over and doing a bit of 
wordsmithing there.


> 3.2
> 
> 	... RADIUS/(D)TLS does not use separate ports for authentication, accounting and dynamic authorization changes. The source port is arbitrary.
> 
>    It would be worth noting here that clients still have an 8-bit ID limitation, and that this issue is addressed by ALPN.
> 
> 	... RADIUS/TLS servers MUST immediately start the TLS negotiation when a new connection is opened. They MUST close the connection and discard any data sent if the connecting client does not start a TLS negotiation.
> 
>    Add "or if the TLS negotiation fails at any point".

Added.
> 
> 	... RADIUS/(D)TLS peers MUST NOT use the old RADIUS/UDP or RADIUS/TCP ports for RADIUS/DTLS or RADIUS/TLS.
> 
>    This seems to contradict the first paragraph in 3.2?

I think this might be a result of different wording in 6614/7360. Would 
have to check. I've added a TODO note.

> 3.3
> 
> 	... RADIUS/(D)TLS clients MUST mark a connection DOWN if one or more of the following conditions are met: * The administrator has marked the connection administrative DOWN. * The network stack indicates that the connection is no longer viable. * The application-layer watchdog algorithm has marked it DOWN.
> 
>    formatting off bullet points

*sigh* markdown being markdown.
Hopefully fixed, will check.

> 
> 	... If a RADIUS/(D)TLS client has multiple connection to a server, it MUST NOT decide to mark the whole server as DOWN until all connections to it have been marked DOWN.
> 
>    Maybe add a discussion of what, exactly, is a "server".  Destination IP?  How is this affected by RFC 7585 dynamic DNS lookups?

I've added a TODO for after the IETF.


> 	... For RADIUS/TLS, the peers MAY send TCP keepalives
> 
>    Perhaps change this to SHOULD, and explain why.  Experience shows that many people put firewalls between critical network services.  And those firewalls then discard TCP session state for sessions they think are "dead".  And then the critical network services can no longer communicate.
> 
>    It may also be worth adding a note that such practices are likely to cause network outages.  Especially when the firewall team is separate from the RADIUS team, and the two don't talk to each other.
> 
> 
> 4.1
> 
> 	... Implementations MUST follow the recommendations given in [RFC9325].
> 
>    Which are...?  Why are we following these recommendations?

I've added a TODO for after the meeting.

> 
> 	... 	• support for TLS 1.3 [RFC8446] / DTLS 1.3 [RFC9147] or higher is RECOMMENDED.
> 
>     I'd make this mandatory for servers.
(see above, compatibility with old spec)
> 
> 
> 4.2
> 
> 	... Allowing anonymous clients would ensure privacy for RADIUS/(D)TLS traffic, but would negate all other security aspects of the protocol
> 
>    add: plus, the use of a fixed shared secret would negate all security if peers were not mutually authenticated.

I've added a sentence.

> 4.2.1.
> 
> 	... 	• Implementations MUST allow the configuration of a list of trusted Certificate Authorities for new TLS sessions.
> 
>    Perhaps also note that this list should be specific to the application, and SHOULD NOT use the default system certificate store.

Why? If the decision is to use WebPKI because it's easier and we have 
well-known hostnames, then this is a perfectly good use case.
I would not per-se label this as less favorable than using a private CA 
(which we would do if we make this a SHOULD NOT)


> 	... Implementations SHOULD indicate their trusted Certification authorities (CAs).
> 
>    Do RADUUS/TLS implementations do this today?
See comment from Fabian.


> 	... For clients configured by name
> 
>    DNS name?
Fixed.


I'll address the other comments in a later mail.

Cheers,
Janfred
-- 
Herr Jan-Frederik Rieckers
Security, Trust & Identity Services

E-Mail: [e-mail] | Fon: [phone] | Fax: [phone]
Pronomen: er/sein | Pronouns: he/him
__________________________________________________________________________________

DFN - Deutsches Forschungsnetz | German National Research and Education 
Network
Verein zur Förderung eines Deutschen Forschungsnetzes e.V.
Alexanderplatz 1 | 10178 Berlin
https://www.dfn.de

Vorstand: Prof. Dr.-Ing. Stefan Wesner | Prof. Dr. Helmut Reiser | 
Christian Zens
Geschäftsführung: Dr. Christian Grimm | Jochem Pattloch
VR AG Charlottenburg 7729B | USt.-ID. DE 136623822