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

Alan DeKok <aland@freeradius.org> Tue, 09 August 2016 16:38 UTC

Return-Path: <aland@freeradius.org>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 7F48712D844; Tue, 9 Aug 2016 09:38:19 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.901
X-Spam-Level:
X-Spam-Status: No, score=-1.901 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id kq-PjjXvajMs; Tue, 9 Aug 2016 09:38:17 -0700 (PDT)
Received: from mail.networkradius.com (mail.networkradius.com [62.210.147.122]) by ietfa.amsl.com (Postfix) with ESMTP id ADDEA12D850; Tue, 9 Aug 2016 09:38:15 -0700 (PDT)
Received: from localhost (localhost [127.0.0.1]) by mail.networkradius.com (Postfix) with ESMTP id DA850199F; Tue, 9 Aug 2016 16:38:14 +0000 (UTC)
Received: from mail.networkradius.com ([127.0.0.1]) by localhost (mail-server.vmhost2.networkradius.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id spsY5353PsCP; Tue, 9 Aug 2016 16:38:14 +0000 (UTC)
Received: from [10.188.40.108] (ppp-seco11pa2-46-193-134-192.wb.wifirst.net [46.193.134.192]) by mail.networkradius.com (Postfix) with ESMTPSA id 0C547B91; Tue, 9 Aug 2016 16:38:13 +0000 (UTC)
Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
Content-Type: multipart/signed; boundary="Apple-Mail=_CDEE4BBC-A3FB-49EE-8003-C44F9403BA41"; protocol="application/pgp-signature"; micalg="pgp-sha256"
X-Pgp-Agent: GPGMail 2.6b2
From: Alan DeKok <aland@freeradius.org>
In-Reply-To: <9904FB1B0159DA42B0B887B7FA8119CA75267BC5@AZ-FFEXMB04.global.avaya.com>
Date: Tue, 09 Aug 2016 18:38:12 +0200
Message-Id: <0945BC67-129B-4B91-97FE-99D95367CB46@freeradius.org>
References: <9904FB1B0159DA42B0B887B7FA8119CA75267BC5@AZ-FFEXMB04.global.avaya.com>
To: "Romascanu, Dan (Dan)" <dromasca@avaya.com>
X-Mailer: Apple Mail (2.3124)
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/19pMABdnrx7-uqWITtrP2wS3BGE>
Cc: "radext@ietf.org" <radext@ietf.org>, General Area Review Team <gen-art@ietf.org>, "draft-ietf-radext-datatypes.all@tools.ietf.org" <draft-ietf-radext-datatypes.all@tools.ietf.org>
Subject: Re: [Gen-art] Gen-ART LC review of draft-ietf-radext-datatypes-04.txt
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 09 Aug 2016 16:38:19 -0000

On Aug 9, 2016, at 5:28 PM, Romascanu, Dan (Dan) <dromasca@avaya.com> 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 reminder of the RADIUS Attribute Type registry policy would also help.

  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.