Subject: [radext] Comments on draft-dekok-radext-datatypes

From: "Jim Schaad" <[email-masked]>
Date: Tue, 5 Feb 2013 13:29:01 -0600

Alan,

First let me say that I am probably not yet truly competent to review this
draft so you can take some of this as probably wrong.

1.  In the text in section 2. Where you are talking about the tag "Value",
you should make clear that this does not apply when you are using the string
"value".

2.  While I understand the desire to have upper case strings in the section
headers, having different names that vary only by capitalization (for
example IPv4addr and ipv4addr) seems to add confusion rather than removing
it.

3.  For IPv4addr, can the sub-set consist of multiple ranges of addresses or
is it supposed to be a single range?   Is there any benefit in saying how
the ranges should be specified for inclusion in a dictionary file? 

I guess that the multiple ranges would apply to all of the types specified
that can be restricted.

4.  For Time, I think you probably need to add the standard verbiage about
not encoding leap seconds at this point.

5.  For Text, why is an "absent" text string not allowed?  This would mean a
length of 0 octets.

6.  For Text, probably not necessary - but is there an advantage to saying
it is not null terminated?

7.  Why should the String type not have the standard verbiage copied from
Text about ranges of lengths?

8.  Ditto comment on zero length octet string

9.  For IPv6addr - ditto the question about defining standard way of
specifying the range for inclusion in dictionary files.  (I would be
surprised if the answer was any different.)

10.  For TLV, is there a reason that a TLV-Data cannot be zero bytes long?
Just putting in the type might be enough to signal what needs to be said.

11.  For Long-Extended, It would seem that the use of the More bit is
problematic with the use of fragmented packaging as it is currently defined.
If a proxy looks at this type (for some reason), I would expect it to start
tossing packets in which either 1) the next attribute is not the same type
and extended type or 2) it is the last attribute in a packet.  This means
that the fragment draft may want to change the M flag to a T flag and back
again.

12.  For Long-Extended, what does it mean if the content is not a valid
RADIUS data type?  What is the SHOULD doing in this statement.  I would
think that the content is going to be based on how the attribute is defined.
Following the SHOULD says that I need to add a String wrapper on my SAML
statement when sending it.  Is this what is intended?

13.  In section 3.1 - you should re-iterate the fact that the Value field is
an artifact of the registry and does not have any one-the wire meaning.

Typos
s/thes/these/
s/consisten/consistent/
s/longths/lengths/
s/.././
s/completely fill/completely fills/

jim

---

From: Alan DeKok <[email-masked]>
Date: Tue, 05 Feb 2013 15:03:50 -0500

> draft so you can take some of this as probably wrong.

  Reviews are always welcome.  Sometimes getting outside reviews
clarifies things which were obvious to people on the inside.

> you should make clear that this does not apply when you are using the string

  OK.  I'll try to come up with some clarifying text.

> example IPv4addr and ipv4addr) seems to add confusion rather than removing

  Good point.  I'll fix it to use lowercase for the section headers.

> is it supposed to be a single range? 

  It could be multiple ranges.  I'll poke the text.

>  Is there any benefit in saying how

  I'd avoid talking about dictionary files.  They've been used for 20
years, but they haven't previously been defined in RFCs.

> I guess that the multiple ranges would apply to all of the types specified

  Yes.  I'll go fix that.

> not encoding leap seconds at this point.

  Ouch, yes.

> 5.  For Text, why is an "absent" text string not allowed?  This would mean a

  Zero length attributes are not allowed in RADIUS.

> it is not null terminated?

  Sure.  RFC 2865 says the same thing.

> Text about ranges of lengths?

  Sure.  I'll go fix that.

> 8.  Ditto comment on zero length octet string

  Done.

> surprised if the answer was any different.)

  It's the same answer.

> Just putting in the type might be enough to signal what needs to be said.

  Same answer as above for text and string.

> that the fragment draft may want to change the M flag to a T flag and back

  Yes, there are interaction effects.  I'll have to think about that.

> RADIUS data type?

  It's a "string"

>  What is the SHOULD doing in this statement.

  It's changed to a MUST.

>  I would

  If the SAML stuff is printable text, it's "text" format.  If the SAML
stuff is a binary blob, it's a "string" format.  So yes, data types
which are opaque to RADIUS MUST be data type "string".

  I'll add some text clarifying this.  For example CHAP-Password is also
of type "string", even though it has a complex structure which is parsed
by RADIUS implementations.

> an artifact of the registry and does not have any one-the wire meaning.

  OK.

> s/completely fill/completely fills/

  Thanks.

  Alan DeKok.