I have been selected as the General Area Review Team (Gen-ART) reviewer
for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).
Please wait for direction from your document shepherd or AD before
posting a new version of the draft.
Document: draft-ietf-radext-rfc3576bis-10
Reviewer: Ben Campbell
Review Date: 16 October 2007
IESG Telechat date: 18 October 2007
Summary:
This draft is basically ready for publication as an informational
RFC, but has nits that should be fixed before publication.
Comments:
I reviewed the 09 version of this draft for IETF last call. If there
was a response to that review, I missed it. Rather than review this
version from scratch, I looked to see if my comments from the 09
review were addressed.
In all fairness, most of my comments are pedantic nits.
Please see imbedded comments.
Thanks!
Ben.
On Sep 20, 2007, at 5:04 PM, Ben Campbell wrote:
I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).
Please resolve these comments along with any other Last Call comments
you may receive.
Document: draft-ietf-radext-rfc3576bis-09.txt
Reviewer: Ben Campbell
Review Date: 9/20/07
IETF LC End Date: 9/24/07
Summary:
This draft is basically ready for publication as an informational
RFC, but has nits that should be fixed before publication.
Comments:
idnits reports two relevant issues:
-- The exact meaning of the all-uppercase expression 'MAY NOT'
is not
defined in RFC 2119. If it is intended as a requirements
expression, it
should be rewritten using one of the combinations defined in
RFC 2119;
otherwise it should not be all-uppercase.
This has been fixed in version 10.
** Obsolete normative reference: RFC 2409 (Obsoleted by RFC 4306)
No change in version 10. It may be that reference to the old IKE RFC
is intentional, in which case I am fine with it. I do not have enough
expertise in IKE to judge that for myself, so perhaps the authors
will comment.
Section 2.2, diagram:
I'm not sure that the term "authz" is well known enough to use
without definition. Consider spelling out "authorization" or adding
"authz" to the terminology list.
This was fixed in version 10.
Section 2.3: Definition of RADIUS code values
This section references RFC3575 for the IANA registrations. RFC3575
references RFC3576, which is being obsoleted by _this_ spec, for
the value definitions. Is that the intent?
No change. I'm okay with this if this was intentional, but would like
to see some comment to that effect.
Section 3, first paragraph:
"All NAS and session identification attributes included in a
CoA-Request or Disconnect-Request packet MUST match at least one
session in order for a Request to be successful; otherwise a
Disconnect-NAK or CoA-NAK MUST be sent."
Is the above text correct? If so, what does it mean for a NAS
identification attribute to match a _session_? I assume we are
using the NAS identifiers as a scope in which the session
identifiers have meaning, so that all sessions could be considered
of having an implied NAS property? A little more explanation might
be helpful.
No apparent change in the text. I think the authors mean by "all" to
say that the combination of NAS and session identification attributes
MUST match a session--but the text still seems ambiguous on this point.
Section 3, 5th paragraph from end:
"Where a Diameter client utilizes the same Session-Id for both
authorization and accounting, inclusion of an Acct-Session-Id
Attribute in a Disconnect-Request or CoA-Request can assist with
Diameter/RADIUS translation, since Diameter RAR and ASR commands
include a Session-Id AVP. An Acct-Session-Id Attribute SHOULD be
included in Disconnect-Request and CoA-Request packets."
I find the above text to be confusing. I _think_ you intend to
compare the difference between an NAC behavior and a Diameter
client behavior, but on the first read I it seemed like you were
proposing suggested RADIUS behavior as conditional based on what a
particular Diameter client does. I propose changing the first few
words to "While Diameter clients... "
No change, but after re-reading, I think I understand the meaning--no
change needed other than perhaps finding a reviewer who can read more
carefully :-)
Section 3.1, last paragraph:
Should the following sentence us normative language?
"Since
Disconnect and CoA responses are authenticated on the entire packet
contents, the stripping of the Proxy-State Attribute invalidates
the
integrity check - so the proxy needs to recompute it."
No change. If I am reading this right, they are saying that if a
proxy does not recompute the integrity check, things will break. That
seems like a good place to say the proxy MUST recompute...
Section 3.2, entire section:
It would be helpful to mention in this section how Authorize Only
is used to ease mapping to Diameter, and reference the Diameter
Considerations section. As it is, I spent some time wondering what
the semantic effect of the resulting Access-Request message was
supposed to be.
I don't find any change related to this comment. However, it is not a
critical comment; it was merely a personal opinion of a way to
improve readability of the spec.
Section 3.5 Paragraph 5:
"When the HMAC-MD5 message integrity check is calculated the
Request Authenticator field and Message-Authenticator Attribute
should be considered to be sixteen octets of zero."
Is that 16 octets _combined_ or _each_? I originally assumed
combined, but the calculation for use in an ACK/NAK the Message-
Authenticator is treated as 16 octets of zero all by itself.
No change in the draft. I think this needs to be more precise, and
could easily be fixed by inserting the word "each" somewhere.
Section 6.2 paragraph 4:
Can a proxy be expected to easily know if it is one-hop away from
the NAS? Is the mechanism for determining this well-known or
documented somewhere that could be referenced here? (Sorry if I am
showing my RADIUS ignorance here.)
No change in the draft. In all fairness, this was a question more
than a comment, but I have not seen a response.
_______________________________________________
Gen-art mailing list
[email protected]
https://www1.ietf.org/mailman/listinfo/gen-art
