On Jun 25, 2024, at 9:16 AM, Claudio Allocchio via Datatracker <noreply@xxxxxxxx> wrote: > 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. The intent is more to explain the history as to why we're here. i.e. instead of just going "do something... just because", the introduction gives motivation. It also addresses historic reasons for staying with MD5. I'll do some rewording to remove repetition, but I think it's useful to leave motivation there. > 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. It doesn't hurt, I think. I'm a big fan of spoon-feeding the reader information. Since it took me a bit to go through ALPN and figure out exactly what's going on, I see it as useful to give a summary for the reader. i.e. I've found it useful to err on the side of too much information rather than too little. > 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." The recommendation in Section 3.3. is to be backwards compatible. The feedback from other reviewers was that it was useful to allow systems to *not* be backwards compatible. That is entirely a choice to be made by local administrators, and should be allowed. If systems are configured to be not backwards compatible, then administrators will discover issues very quickly, when their network goes down. > 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. I don't see it as re-stating the concept, but rather as explaining what to do in all of the corner cases. I've run into many situations where implementors do something ridiculous, and then claim "It's allowed by the specification". In almost every situation, it's not allowed. Instead, the RFC doesn't forbid it. So experience shows that telling people to "do the right thing" results in interoperability issues, and increased costs to administrators trying to use products. In contrast, being a little more prescriptive in the document has little additional cost. > 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. Sure. > 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? I think it's worth rearranging Section 6 and Section 7 to make it clearer what are changes for ALPN, and what impacts legacy RADIUS: 6. Other considerations when using ALPN 7. Other RADIUS considerations > 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. I'll move that to a renamed section 7. > 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. That's addressed by making a new explicatt section 7. > For all the rest, the document is ok and ready to go forward. Thanks for the feedback. I'll issue an updated draft shortly. Alan DeKok. -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
