Subject: [Gen-art] Gen-ART LC review of draft-ietf-radext-datatypes-04.txt

From: "Romascanu, Dan (Dan)" <[email-masked]>
Date: Tue, 9 Aug 2016 15:28:10 +0000

I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair.  Please treat these comments just like any other last call comments.

For more information, please see the FAQ at

https://trac.tools.ietf.org/area/gen/trac/wiki/GenArtfaq<https://urldefense.proofpoint.com/v2/url?u=https-3A__trac.tools.ietf.org_area_gen_trac_wiki_GenArtfaq&d=CwMFAg&c=BFpWQw8bsuKpl1SgiZH64Q&r=I4dzGxR31OcNXCJfQzvlsiLQfucBXRucPvdrphpBsFA&m=A146UD4syR9uGHj3mNEcdcuht-Sznx47S9gyH87tBfU&s=pbahG3QoEuw2lLVL8Vw1XGd--c3Ak_-FexGmP-_DqyY&e=>

Document: draft-ietf-radext-datatypes-04.txt

Reviewer: Dan Romascanu

Review Date: 8/9/16

IETF LC End Date: 8/17/16

IESG Telechat date: 8/18/16

Summary:

Ready with issues.

This is an important document that tries to bring clarification to a number of different interpretations and recommendations around the RADIUS specifications. Being familiar - up to a certain point - with the history, I believe that it's long due and that it will be very useful. There are however a number of issues which I believe need to be clarified before the document is approved by the IESG.

Major issues:

1.       Although the document makes the claim that it does not impact previous specifications and implementations, I am not sure this is completely the case. For example, the statement in RFC 6158, section 2.1 about 'all other data formats (than the ones defined there) being NOT RECOMMENDED is obsolete by this document. I think that there is a need to make clear what is not longer valid or recommended in specifications that this document updates.

2.       This document creates a new IANA registry and updates another one. There is no mention however about the policy of adding new entries or making other modifications to the new registry. A reminder of the RADIUS Attribute Type registry policy would also help.

Minor issues:

1.       In section 3.5 - there is no mention that the string length is limited to 253 octets. This is obvious if we assume the definition of  "string" sticking with previous RADIUS documents, and clear by the fact that "concat" deals in 3.6 with transport of more than 253 octets, but for clarity I believe that this needs to be explicitly stated.

2.       It is not clear why the Prefix-Length value greater than 128 for ipv6prefix in 3.10 and greater than 32 for ipv4prefix in 3.11 need only SHOULD be treated as "invalid attributes". Why not MUST? If there are exception cases it would be good to explain them.

Nits/editorial comments:

1.       Section 2.1.4: The paragraph that starts with 'The "Value" field should be given ....' needs to use capitalized SHOULD to be consistent with the paragraph that refer to the "Name" and "Format" fields.

---

From: Alan DeKok <[email-masked]>
Date: Tue, 9 Aug 2016 18:38:12 +0200

On Aug 9, 2016, at 5:28 PM, Romascanu, Dan (Dan) <[email-masked]> wrote:
> 1.       Although the document makes the claim that it does not impact previous specifications and implementations, I am not sure this is completely the case.

  As noted in the document, implementations have already chosen data types for all existing attributes.  I haven't seen any conflicting data types.  i.e. all implementations seem to have made the same choices.

  The document updates previous specifications, most notably where the specifications are inconsistent and/or wrong.  So it does impact previous specifications, but only to codify standard corrections.  Which (as noted above) all implementations have already fixed in practice.

> For example, the statement in RFC 6158, section 2.1 about 'all other data formats (than the ones defined there) being NOT RECOMMENDED is obsolete by this document. I think that there is a need to make clear what is not longer valid or recommended in specifications that this document updates

  Maybe this works:

This document updates [RFC6158] to permit the data types defined in
the "Data Type Registry" as "basic data types", as per Section 2.1 of
that document.  The recommendations of [RFC6158] are otherwise
unchanged.

  I think that should clarify the issue.

> 2.       This document creates a new IANA registry and updates another one. There is no mention however about the policy of adding new entries or making other modifications to the new registry. A r…

  The -06 rev has the following text:

4.1.  Create a Data Type Registry

   This section defines a new RADIUS registry, called "Data Type".
   Allocation in this registry requires IETF Review.  The "Registration
   Procedures" for this registry are "Standards Action".

  Which I think satisfies your concern.

> Minor issues:
>
> 1.       In section 3.5 – there is no mention that the string length is limited to 253 octets. This is obvious if we assume the definition of  “string” sticking with previous RADIUS documents, and clear by the fact that “concat” deals in 3.6 with transport of more than 253 octets, but for clarity I believe that this needs to be explicitly stated.

  Yes and no.  RFC 6929 allows for extended attributes to carry more than 253 octets of data.  The same comment applies to attributes of type "text".

  I'll add some text clarifying this.

> 2.       It is not clear why the Prefix-Length value greater than 128 for ipv6prefix in 3.10 and greater than 32 for ipv4prefix in 3.11 need only SHOULD be treated as “invalid attributes”. Why not MUST? If there are exception cases it would be good to explain them.

 The text was mostly taken from previous standards.  I'm OK with it being a MUST.

> Nits/editorial comments:
>
> 1.       Section 2.1.4: The paragraph that starts with ‘The “Value” field should be given ….’ needs to use capitalized SHOULD to be consistent with the paragraph that refer to the “Name” and “Format” fields.

  Fixed, thanks.

  Alan DeKok.

---

From: "Romascanu, Dan (Dan)" <[email-masked]>
Date: Tue, 9 Aug 2016 17:03:12 +0000

Hi Alan,

Thanks for the quick response. The planned edits seem to address all the issues that I raised. There are good chances that I will just say 'I am happy' when I will see the revised I-D.

Regards,

Dan

________________________________
From: Gen-art [[email-masked]] on behalf of Romascanu, Dan (Dan)
Sent: Tuesday, August 09, 2016 6:28 PM
To: General Area Review Team
Cc: [email-masked]; [email-masked]
Subject: [Gen-art] Gen-ART LC review of draft-ietf-radext-datatypes-04.txt

Reviewer: Dan Romascanu

Review Date: 8/9/16

Summary:

Ready with issues.

This is an important document that tries to bring clarification to a number of different interpretations and recommendations around the RADIUS specifications. Being familiar – up to a certain point – with the history, I believe that it’s long due and that it will be very useful. There are however a number of issues which I believe need to be clarified before the document is approved by the IESG.

Major issues:

1.       Although the document makes the claim that it does not impact previous specifications and implementations, I am not sure this is completely the case. For example, the statement in RFC 6158, section 2.1 about ‘all other data formats (than the ones defined there) being NOT RECOMMENDED is obsolete by this document. I think that there is a need to make clear what is not longer valid or recommended in specifications that this document updates.

Minor issues:

Nits/editorial comments:

---

From: Jari Arkko <[email-masked]>
Date: Thu, 18 Aug 2016 16:48:21 +0200

Thanks for the review, Dan! And Alan for edits.

Jari