Skip to main content

Last Call Review of draft-ietf-radext-radiusv11-08
review-ietf-radext-radiusv11-08-secdir-lc-leiba-2024-06-26-00

Request Review of draft-ietf-radext-radiusv11
Requested revision No specific revision (document currently at 11)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2024-06-26
Requested 2024-06-12
Authors Alan DeKok
I-D last updated 2024-06-26
Completed reviews Genart Last Call review of -08 by Christer Holmberg (diff)
Artart Last Call review of -07 by Claudio Allocchio (diff)
Secdir Last Call review of -08 by Barry Leiba (diff)
Opsdir Last Call review of -10 by Susan Hares (diff)
Secdir Telechat review of -10 by Barry Leiba (diff)
Assignment Reviewer Barry Leiba
State Completed
Request Last Call review on draft-ietf-radext-radiusv11 by Security Area Directorate Assigned
Posted at https://mailarchive.ietf.org/arch/msg/secdir/1S_Gjj8lhbvv3Opzb3KC9pSLr_Y
Reviewed revision 08 (document currently at 11)
Result Has issues
Completed 2024-06-26
review-ietf-radext-radiusv11-08-secdir-lc-leiba-2024-06-26-00
Thanks for this important update: getting rid of MD5 is quite overdue, and will
make a long-term difference.

The Abstract seems longer and more detailed than necessary.  In particular, I
would strike the second paragraph completely, as it’s detail that doesn’t need
to be in the Abstract.  I also think the last two sentences of the first
paragraph add detail that’s unnecessary in the Abstract.

— Section 1 —

I am concerned that you do not UPDATE 2865 or 6613, yet you say this:

   While this document permits MD5 to be removed when using (D)TLS
   transports, it makes no changes to UDP or TCP transports.  It is
   therefore RECOMMENDED that those transports only be used within
   secure networks, and only used in situations where FIPS compliance is
   not an issue.

Without the update to 2865 and 6613, how are implementors of the UDP and TCP
transports supposed to be aware of the RECOMMENDED statement above?  I think
this document should update 2865 and 6613, and that the statement above should
be at a MUST level, not a SHOULD, as this:

NEW
   While this document permits MD5 to be removed when using (D)TLS
   transports, it makes no changes to UDP or TCP transports.  It
   therefore follows that those transports MUST NOT be used outside of
   secure networks, and MUST NOT be used in situations where FIPS
   compliance is needed.
END

Near the end of the section is this:

   This specification is compatible with all past and future RADIUS
   specifications.

You can’t possibly promise compatibility with all future versions.  Maybe you
could change “future” to “anticipated”, or maybe just reconsider mentioning
that at all.

— Section 3.3 —

   When set, this flag contains the list of permitted ALPN versions in
   humanly readable form.

The flag contains the list of permitted RADIUS versions, not ALPN versions.

I have to say that I find this section to be pretty confusing.  I write it off
to the fact that I’m not generally familiar with RADIUS and the associated
documents, and I hope that RADIUS practitioners do fint it clearer than I do. 
In particular, I don’t get the distinction between the Version flag and the
ALPN name, and why both are needed when they each seem to identify the same
thing.  But again, I’m sure this just reflects my lack of familiarity with this.

— Section 10 —

The one-sentence security considerations doesn’t show any actual consideration
of the security properties of the changes proposed here.  I think you need to
spend some time looking at what happens when someone gets the implementation
wrong, when someone tries to attack this… just as other protocols do when they
write their Security Considerations section.  But I’ll leave that to the
Security ADs to sort out with you when they ballot DISCUSS….

— References —

Nit: the appearance of BCP 14 at the start of the references section is
redundant to the RFC 8174 reference at the end of the section.  You can remove
the former.