New Issue: review of of draft-lior-radius-attribute-type-extension-01

[Alan DeKok <aland@nitros9.org>]

Review of the document
Submitter name: Alan DeKok
Submitter email address: aland@nitros9.org
Date first submitted: March 28, 2007
Reference:
Document: Extended attributes
Comment type: E
Priority: S
Section: Many
Rationale/Explanation of issue:

Section 1.1: Should to be completed, as is noted in the document.  I
suggest using the usual NAS, RADIUS server, and proxy server from the
other drafts and/or RFC's.  I don't think much more is necessary.

Section 2:

...
In the real world, certain vendors have grabbed
   attribute type codes that they shouldn't have.
...

  I suggest "certain vendors have not followed the recommended practice
to obtain an IANA allocation, and have not used VSA's."

  I also suggest adding stronger comments about this.  e.g. "We
re-iterate that vendors MUST NOT allocate attributes out of the RFC
attribute space.  Those attributes can only be allocated by IANA.
Vendors performing such allocations cause inter-operability problems,
heart-aches, and great gnashing of teeth".

  Or something like that.  When told, some vendors fix their software,
some don't understand what the problem is, and some don't care.

Section 2:

...
Therefore, the proposed scheme NUST provide a
   mechanism to group attributes.
...

  Typo "MUST".

Section 2:

...
In recent years, attribute sizes are threatening the limit of 253
   octets.
...

  "threatening" sounds harsh.  Maybe "pushing the limits of the current
RADIUS attributes."

Section 3:

...
We allocate RADIUS the Vendor-Id of zero (0).
...

  As noted earlier, I have compatibility concerns about using vendor Id
of zero.  If possible, a non-zero ID could be used.  However, the IETF
does not currently have a private enterprise code (last I checked).
Adding one just for RADIUS may have ripple effects through other WG's
and other protocols.

Section 3:

...
Vendor Specific attributes are encoded by using the Vendor-Specific
   (26) type code, followed by Length (1 octet), a 4 octet Vendor-Id
   identified using 3 octets (4 octets really)
...

  "Really"?   I suggest just saying "4 octet Vendor-Id."

Section 4:

  The ASCII art is in a different format than the attribute definitions
in the existing RADIUS RFC's.  It should be consistent.

  The ASCII art also has two "Length" fields.  The second should be
"Extended-Length".  The following text also has "Length >= 10", and a
second "Length >= 4".  The second Length should be Extended-Length.

  I suggest also noting that "Extended-Length = Length - 6".  Otherwise,
it becomes theoretically possible for them to have conflicting values.

Section 4:
...
Values XXXX-YYYY are reserved.
...

  I suggest reserving the same number range as in RFC 2865.

Section 4:
...
Fragmentation (F):

   When set(1) the attribute is fragmented.  When clear (0), the
   attribute is not fragmented.  When the value portion of an extended
   attribute exceeds 246 octets the value is fragmented over one or more
   extended attribute.  In this case the Fragmentation Flag is set on
   the attribute(s) that contain the value fragments.
...

  I suggest noting that the "Extended-Length" field is the length of
THIS PORTION of the fragmented attribute.  The real length of the
attribute is found by summing the Extended-Lengths of adjacent
attributes with the "F" bit set, AND where the Extended-Type is the
same, AND where the Tag is the same.

  Likewise, the value of the extended attribute is formed by
concatenating the values of adjacent attributes with the "F" bit set,
AND where the Extended-Type is the same, AND where the Tag is the same.

  This text seems to indicate that the "F" bit is set if an attribute is
fragmented... does that mean that the first fragmented attributes has
"F" set?  The examples in Section 5 indicate that it doesn't.  Either
way, the text & examples need to be clarified and made consistent.

  On one hand, setting the "F" bit on the first fragment helps the
receiver know immediately that the attribute is fragmented.  Otherwise,
it has to look at the *next* attribute to see the fragmentation status
of the current attribute.  But if the "F" bit is set for all fragments,
then the receiver has to compare the Extended-Types && Tags to see if
the current fragment is part of the previous attribute.

  I think I prefer the first approach, where "F" is set on all
fragments.  It also avoids issues.  e.g. One implementor decides to
fragment at non-246 octet boundaries.  Another implementor decides that
with "F" zero, and "Extended-Length" less than 246, the attribute
*cannot* be fragmented, so two attributes are created where the sender
intended to send one.

  Also, attributes generated by a client or server MUST NOT be
fragmented at boundaries other than 246 octets.  Doing so would decrease
the efficiency of packing.  Robust implementations SHOULD support
receiving attributes fragmented at non-246 octet boundaries.

  The Tag field could be renamed, to avoid conflict with the existing
Tag field.  They seem to be a little different...

  There should be more text explaining what the meaning of the grouping
is for the Tag field.  e.g. use-cases.  Guidelines to implementation
authors help significantly with comprehension of the document, and in
creating inter-operable implementations.

  The document should probably have a section 4.1, explaining all of
these issues.

Section 5:

  The ASCII art should again be the same format as in RFC 2865 et al.  I
also suggest full hex dumps of the attributes, in addition to the ASCII art.

  Using "0xDEADDEAD" is interesting.  Maybe something more innocuous,
like "0x12345678" ?

  The document introduces a new syntax to refer to extended attributes
without defining it.  e.g. Foo(0,10).  It looks like the "0" refers to
the Tag, and "10" to the Extended-Type.  If we follow the convention of
previous documents, we do not need to mention the Tag if it is zero.  We
also do not need to mention the value of the Extended-Type.


Section 6:

  I don't think there are any security considerations.  The document
doesn't define any new packets, protocol exchanges, encryption types, or
integrity checks.


Section 7:

  If we're going to allocate a private enterprise code for the IETF, it
should be something other than zero.  Otherwise, I would silently avoid
the idea that we're allocating a vendor ID of 0 to the IETF.

  Alan DeKok.

--
to unsubscribe send a message to radiusext-request@ops.ietf.org with
the word 'unsubscribe' in a single line as the message text body.
archive: <http://psg.com/lists/radiusext/>