Subject: [Gen-art] Gen-ART review of draft-ietf-radext-rfc3576bis-10.txt

From: Ben Campbell <[email-masked]>
Date: Tue, 16 Oct 2007 21:29:17 -0500

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-masked]
https://www1.ietf.org/mailman/listinfo/gen-art

---

From: Ben Campbell <[email-masked]>
Date: Tue, 16 Oct 2007 23:34:44 -0500

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

Comments:

Thanks!

Ben.

---

From: Bernard Aboba <[email-masked]>
Date: Wed, 17 Oct 2007 13:29:55 -0700

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

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

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

[BA] OK. 

[BA] OK. 

[BA] Added "each". 

[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-masked]] 
Sent: Tuesday, October 16, 2007 9:35 PM
To: General Area Review Team; [email-masked]; [email-masked];
[email-masked]; [email-masked]; Bernard Aboba;
[email-masked]; Russ Housley
Subject: Gen-ART review of draft-ietf-radext-rfc3576bis-10.txt

Comments:

Thanks!

Ben.

---

From: Ben Campbell <[email-masked]>
Date: Wed, 17 Oct 2007 19:53:11 -0500

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:

> reviewer