|
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. #1 - Waiting on resolution of Issue 272 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. Fixed by removing the referenced text Section 2: ... Therefore, the proposed scheme NUST provide a mechanism to group attributes. ... Typo "MUST". Fixed. 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." Fixed. BTW, I'm somewhat surprised by the delicacy of your
sensibilities ;-). 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. OK 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. Don't understand: ripple effects? 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." Fixed Section 4: The ASCII art is in a different format than the
attribute definitions in the existing RADIUS RFC's. It should be
consistent. Fixed 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. Fixed 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. This doesn't make much sense to me, given that the two types are
(currently) different sizes. Why reserve a hunk out of the middle? 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. A textual contribution would be welcomed. Section 5: The ASCII art should again be the same format as
in RFC 2865 et al. Fixed I also suggest full hex dumps of the attributes, in
addition to the ASCII art. Care to generate some? 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. Fine |