All of my concerns have been either addressed in version 11 of the draft, or answered satisfactorily in this email. (I sent a separate email to Russ saying essentially the same thing.)

Thanks!

Ben.

On Oct 17, 2007, at 3:29 PM, Bernard Aboba wrote:


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.

[BA] Yes, the reference to IKEv1 was intentional.


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.

[BA] Yes, this was intentional.  The values were formally allocated in
RFC 3575.


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.

[BA] The intent was indeed for the combination of NAS and session
Identification attributes to match a session.  We can say
 "The combination of all the NAS..."

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 :-)

[BA] OK.


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...

[BA] OK.

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.

[BA] Added "each".


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.

[BA] In order to route CoA or Disconnect Requests to the NAS, the proxy
will need to be configured with the IP addresses corresponding to the
NAS-Identifier attribute (in case NAS-IPv4-Address or NAS-IPv6- Address
is
Not present), so yes, the proxy will know if the NAS is one hop away or
not.

A -11 document with the fixes included has now been posted to the
archive:
http://www.ietf.org/internet-drafts/draft-ietf-radext- rfc3576bis-11.txt


-----Original Message-----
From: Ben Campbell [mailto:[EMAIL PROTECTED]
Sent: Tuesday, October 16, 2007 9:35 PM
To: General Area Review Team; [EMAIL PROTECTED]; [EMAIL PROTECTED];
[EMAIL PROTECTED]; [EMAIL PROTECTED]; Bernard Aboba;
[EMAIL PROTECTED]; Russ Housley
Subject: Gen-ART review of draft-ietf-radext-rfc3576bis-10.txt


[Oops, sent from wrong address and got moderated. Here it is again.
Apologies for the repeat.]

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

Reply via email to