Skip to main content

Last Call Review of draft-ietf-radext-radiusv11-07
review-ietf-radext-radiusv11-07-artart-lc-allocchio-2024-06-25-00

Request Review of draft-ietf-radext-radiusv11
Requested revision No specific revision (document currently at 11)
Type Last Call Review
Team ART Area Review Team (artart)
Deadline 2024-06-26
Requested 2024-06-12
Authors Alan DeKok
I-D last updated 2024-06-25
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 Claudio Allocchio
State Completed
Request Last Call review on draft-ietf-radext-radiusv11 by ART Area Review Team Assigned
Posted at https://mailarchive.ietf.org/arch/msg/art/T6s2PIOiL_d1QM6k-Z0wcDTyHrE
Reviewed revision 07 (document currently at 11)
Result Ready w/nits
Completed 2024-06-25
review-ietf-radext-radiusv11-07-artart-lc-allocchio-2024-06-25-00
Hello,

Here is my list of observations on this document.

nits: in the 1. Introduction section you repeat a large number of times that
this experimental specification requires only a small set of modifications to
existing implementations, and then finally you list the only 3 needed. I may
suggest just stating this "need for small set of modification" just once, where
you list them. You also repeat a number of times that MD5 use is deprecated,
and shared secret should not be used anymore: same comment as above... just
state it once.In the Introduction this sort of "excessive over-statement of
concepts" happens also elsewhere. As this is a technical specifications, I
suggest avoiding these repetitions, "as if you need to strongly defend the
idea" that it is not going to make any harm.

nits: sections 3.2 Do we really need this digest of RFC7303 here? it makes no
harm, but it seems a bit odd this "support for readers not familiar" in a
specification.

issue (maybe?): section 3.3. here you make it possible for an implementation
not to be backward compatible and just support radius/1.1 (even if not
recommended). This is against the principle "be liberal in what you accept",
and could create non interoperable situations. Are we sure we really want this
to happen? how can we be sure that "all (or nearly all) RADIUS clients have
been updated to support RADIUS/1.1."

nits: section 3.5 again the text re-states the concept a number of time. Maybe
it is just a matter of style, which pervdes the whole specification.

suggestion: 4.2.1 the proposed method to generate an initial Token value has of
course a 1/(2^32) possibility to generate the same initial value. This has no
effect of course (besides the rare chance that a debugging ends into 2
overlapping series f tokens. Why not also add this to the explanation? this is
already partially there in the explanation, tough... so just a suggestion.

Session 6 nits:

nit: session 6.1 describes and tries to give a fix to a known problem which is
not specific to the new extension, but general. Shall we make it more explicit
somewhere n the text that here we are trying to retro-fit a general issue?

nit: also 6.2 gives recommendations to update historic implementations, which
is fine, but maybe "hidden", "not visible" in a specification like this which
in the end is a "profile" (and experimental for now). 6.5 does the same for
another issue.

Suggestion: make evident the attempt of all section 6 to fix existing issues,
besides describing them here, or implementers may risk to ignore what's written
in section 6.

For all the rest, the document is ok and ready to go forward.

all the best!