Subject: Re: [radext] Issues with draft-ietf-radext-radius-extensions-09

From: Alan DeKok <[email-masked]>
Date: Fri, 01 Feb 2013 10:50:03 -0500

I've taken the liberty of responding to both messages here.

  I've updated the draft as per the comments, and submitted a new
version.  Diffs should be available shortly from:

http://tools.ietf.org/html/draft-ietf-radext-radius-extensions

> === COMMENT ===
> -- Section 1.1.1 --
...
> It's actually entirely possible to deprecate items in a registry, so
> if you still want to handle this.  The document could add a column to
> the table, perhaps called "Status", which would have the values
> "current" (or blank to mean "current") or "deprecated" (or "obsolete",
> or whatever).

  I think the simplest thing to do is just to delete that text.  The
  standard space is almost completely full, so the issue will shortly
  be meaningless.

> -- Section 2.1 --
>
> === COMMENT ===
> In the description of "Type":
>
> I suggest adding a forward reference to make an important
clarification here:
>
> NEW
>       This field is identical to the Type field of the Attribute format
>       defined in [RFC2865] Section 5.  It will contain one of the Extended
>       Type values defined in Section 3, below.
> END

  I'm not sure why that's necessary.  The Type field always contains a
valid RADIUS attribute type.  The Attribute Types are managed by IANA,
and new attributes of "Extended Type" can be assigned at any time.

  The intention here is to define the format, independent of value.
IANA can then assign certain values as having a particular format.

  For example, RFC 2865 Section 5 defines Type as:

   Type

      The Type field is one octet.  Up-to-date values of the RADIUS Type
      field are specified in the most recent "Assigned Numbers" RFC [6].
      Values 192-223 are reserved for experimental use, values 224-240
      are reserved for implementation-specific use, and values 241-255
      are reserved and should not be used.

  There's no need to say "it contains one of the RADIUS attribute
types defined in Sections 5.1 through 5.X of this document".

> === COMMENT ===
> In the description of "Length":
>
>       The Length field is one octet, and indicates the length of this
>       Attribute including the Type, Length, Extended-Type, and Value
>       fields.  Permitted values are between 4 and 255.  If a client or
>       server receives an Extended Attribute with a Length of 2 or 3,
>       then that Attribute MUST be considered to be an "invalid
>       attribute", and handled as per Section 2.8, below.
>
> Curious: What does a client or server do when it receives an Extended
> Attribute with a length of 0 or 1?  (Same comment goes for Section
> 2.2.)

  This is covered in the definition of RADIUS attributes, in RFC 2865,
Section 5, top of page 25.  The Length field includes the "Type" and
"Length" fields, so Lengths of 0 and 1 are invalide.

  The idea behind this specification is to work *within* existing
RADIUS practices.  We don't re-defined RADIUS at all.

> === DISCUSS ===
> Also, this doesn't make sense to me.  Section 2.8 starts with this:
>
>    The term "invalid attribute" is new to this specification.  It is
>    defined to mean that the Length field of an Attribute is valid (as
>    per [RFC2865], Section 5, top of page 25).
>
> These two appear to be contradictory: the first says that if the
> length is invalid, it's an "invalid attribute", and the second says
> that "invalid attribute" means that the length field is valid (but
> there are other problems).  This comment also applies to "TLV-Length"
> in Section 2.3.

  The intent here is that the *contents* of the attribute are invalid.
The next sentence (elided above) says:

   However, the Value field of the attribute does
   not follow the format required by the data type defined for that
   Attribute.  e.g. an Attribute of type "address" which encapsulates
   more than four, or less than four, octets of data.

So an attribute which carries a 2-byte "IPv4" address is invalid.  The
RADIUS packet itself can be properly parsed (validated, signed, etc.).
But the data carried by a particular attribute is meaningless.

  RADIUS already has the concept of "malformed" packets.  These are
packets which do not pass validation as having the correct format.
This is again independent of interpretation of the contents of the
packet.  e.g. an Accounting-Request sent to an authentication port is
correctly formed, but is not valid for that port.

  Re-using the word "malformed" to describe the attribute would cause
the confusion you highlight above.

> === DISCUSS ===
> In the description of "Value":
>
>       This field is similar to the Value field of the Attribute format
>       defined in [RFC2865] Section 5.  The format of the data SHOULD be
>       a valid RADIUS data type.
>
>       The addition of the Extended-Type field decreases the maximum
>       length for attributes of type "text" or "string" from 253 to 252
>       octets.  Where an Attribute needs to carry more than 252 octets of
>       data, the "Long Extended Type" format SHOULD be used.
>
> The first SHOULD makes a subtle, but very significant change to the
> RADIUS protocol, and I want to make sure what you're doing is what you
> mean to do.  2865 says, "The format of the value field is one of five
> data types." It has to be one of those types, and can't be anything
> else -- nothing else is defined.

  The text in RFC 2865 isn't a MUST.  Many other RADIUS RFCs have been
published which add new data types, *without* defining them as was
done in 2865.  e.g. 3162 defines "Address" to be IPv6 addresses, even
though 2865 says "address" is IPv4.

  6158 notes this issue, and defines more data types, but doesn't name them,
I just published an individual draft yesterday to address this issue:

https://datatracker.ietf.org/doc/draft-dekok-radext-datatypes/

> But you are now allowing an
> implementor who understands the implications of doing so to use a
> different data type.

  That has *always* been the RADIUS practice.  This document changes
nothing.

> 1. Do you really mean to allow that?

  Yes.

> 2. If "yes" to (1), what other data types exist that might be used?

  Lots.

> Where are their formats defined?

  6158, and my individual draft.

> 3. If "yes" to (1), what *are* the implications of using other data types?

  Pretty much nothing.  Implementations define many non-standard data
types, such as 8-bit integer, 32-bit integer, "polymorphic"
attributes, etc.

> That comment also applies to "Value" in Section 2.2 and to "TLV-Value"
> in Section 2.3.

  The same responses as above apply.

> For the second SHOULD, there's a similar issue: What are the
> alternatives to using the "Long Extended Type" format?  What are the
> implications of using those alternatives?

  The alternatives are to use "ad hoc" methods.  An attribute is
defined to have the magical property that multiple attributes mean
fragmented data.  This has to be specified for each attribute needing
this property, and has to be implemented in an ad-hoc way in any
client or server.

  i.e. there is *no* standard way to transport more than 253 octets of
data.  That's why we're writing this draft.

> -- Section 2.2 --
>
> === COMMENT ===
> In the description of "Type":
>
> As in 2.1, I suggest adding a forward reference to make an important
> clarification here:
>
> NEW
>       This field is identical to the Type field of the Attribute format
>       defined in [RFC2865] Section 5.  It will contain one of the Long
>       Extended Type values defined in Section 3, below.
> END

  The same comments as above apply.  I'm not sure why a forward
reference is necessary.

> === DISCUSS ===
> In the description of the "M" bit:
>
> 1.  The first two paragraphs contradict each other.  The first says
> that M MUST be 0 if length < 255.  The second says, "When the More
> field is set (1), the attribute SHOULD have a Length field of value
> 255".  According to the first paragraph, that needs to be MUST, not
> SHOULD. And then with that change, the next clause (about value 4)
> can be removed.

>  The "that is" at the beginning of the third sentence
> should be removed, because the ordering has not been mentioned before.

  I'm not sure why.  Where else would this be mentioned?

> 2.  The last sentence of the second paragraph ends, "and the last
> attribute in a packet MUST NOT have the More field set (1)." That's
> certainly true, but isn't what you really want to say that all pieces
> of the entire fragmented attribute MUST be included in one packet?

  Sure.  But RADIUS doesn't support inter-packet fragmentation.  I'll
add some clarifying text.

> 3.  The third paragraph, which contradicts the first, should be removed.

  OK.

> In the description of "Value":
>
> === DISCUSS ===
>       This field is similar to the Value field of the Attribute format
>       defined in [RFC2865] Section 5.  It may contain a complete set of
>       data (when the Length field has value less than 255), or it may
>       contain a fragment of data (when the More field is set).
>
> Isn't it possible for the Length field to be equal to 255 and to have
> M=0 (if the last fragment of the fragmented attribute takes up the
> whole 255 bytes, for instance)?  And doesn't the last fragment of a
> fragmented attribute contain a fragment of data, even though the More
> field is 0?

  Yes.

> === DISCUSS ===
>       We note that the maximum size of a fragmented attribute is limited
>       only by the RADIUS packet length limitation (i.e. 4096 octets, not
>       counting various headers and overhead).  Implementations SHOULD be
>       able to handle fragmented attributes of 4096 octets.
>
> This is incorrect: the RADIUS packet length is limited to 4096 octets
> *including* the various headers and overhead.  That puts a maximum
> size of fragmented attributes as something less than 4096.  As I
> compute it:
>
> The RADIUS header is 20 octets, so that's 4096-20 = 4076 octets left
> for attributes.  That will hold 15 fragments of 255 octets each, and a
> 16th fragment of 231 octets.  And that results in a total attribute
> length (after headers are removed and the fragments are assembled) of
> 15*251+227 = 3992.

  I think that's a little detailed for the document, but sure.  I'll
write some clarifying text.

> === DISCUSS ===
> The last paragraph of Section 2.2 says this:
>
>    Implementations MUST be able to process non-
>    contiguous fragments.  That is, fragments which are mixed together
>    with other attributes of a different Type.
>
> That contradicts what you said in the description of the "M" bit:

  Sort of.  The text above suggests that implementations be liberal in
what they accept.  The other text mandates that implementations be
conservative in what they generate.

> === DISCUSS ===
> -- Section 2.3.1 --
>
>    The depth of TLV nesting is limited only by the restrictions on the
>    TLV-Length field.  The limit of 253 octets of data results in a limit
>    of 126 levels of nesting.  However, nesting depths of more than 4 are
>    NOT RECOMMENDED.
>
> Again: Why is it NOT RECOMMENDED?

  The recommendation above came out of discussion in the RADEXT WG.
There are existing non-IETF specifications which uses nested TLVs.
They haven't seen a need for a nesting of more than 4 levels deep.

  The WG consensus was that we had no idea *why* anyone would need
more than 4 levels of nesting.  And if a specification tried to use
that deep of a nesting, it would cause discussion, and likely a
re-design.

> How would I know how to evaluate the implications of going for more
> than 4 levels?

  The WG has no idea.  That's why the text recommends against it, but
without detailed reasons.

> === COMMENT ===
> -- Section 2.7.2 --
>
> The last paragraph seems out of place, as the following section
> describes the TLV naming, basically repeating the last paragraph of
> 2.7.2.  I suggest correcting that organizational oddness.

  OK.

> === COMMENT ===
> -- Section 2.7.3 --
>
> At the end of the first paragraph, I think "replied" should be "repeated".

  OK.
> === COMMENT ===
> -- Section 2.7.4 --
>
> In the first sentence of the fourth paragraph, it seems as though "few
> restrictions" is meant to be "a few restrictions", yes?

  The intention is to state that a naive approach would require 8-bit
numbers at each level of the "dotted number" notation.  However,
various portions of the notation can be 32 bits or 16 bits.  The only
way to know is to examine the numbers themselves.

  e.g. as discussed in the document, 26.429.32768 is allowed, but
26.428.32768 is not.  We don't want to get into the gritty details of
defining the entire set of allowed values.  Instead, we note that many
values exist, with few restrictions.

> What does the example in paragraph 5 tell me that the example in
> paragraph 3 doesn't?  That is, why is the second example needed?

  It's a duplicate.  It can be removed.

> === COMMENT ===
> -- Section 2.8 --
>
>    For Attributes of type "Long Extended Type", an Attribute is
>    considered to be an "invalid attribute" when does not match the
>    criteria set out in Section 2.2, above.
>
> 1.  Typo: "when *it* does not match".

  OK.

> 2.  I think it would be useful to remind implementors that one bad
> apple spoils the whole bunch: that if one fragment of a fragmented
> attribute is invalid, the entire attribute (all fragments) needs to be
> considered invalid.

  That's phrased a little awkwardly.  See Section 2.2:

    Any interpretation of the resulting data MUST occur after the
    fragments have been reassembled.

  i.e. you can't have one fragment that's invalid.  Either the entire
attribute is invalid, or none of it is.

  I think a better phrasing would be to update that text in Section
2.2 with the following:

    If the reassembled data does not match the expected format, all
    fragments MUST be treated as "invalid attributes", and the reassembled
    data MUST be discarded.

> === COMMENT ===
> -- Sections 3 and 4 subsections --
>
> It somewhat bothers me to have the diagrams and some information
> repeated here (repeated repeatedly, in fact).  Is it not possible to
> refer back to Sections 2.1 and 2.2 even more than you already do?

  It's the practice with RADIUS specifications.  RFC 2865 defines an
attribute format, data types, but then repeats the ASCII art for each
attribute.  It could instead have said "1 is User-Name, data type
text".

> You might then also add the information in these two paragraphs to the
> descriptions of "Value" in Sections 2.1 and 2.2:
>
>       The Value field is one or more octets.  Implementations not
>       supporting this specification SHOULD support the field as
>       undistinguished octets.
>
>       Implementations supporting this specification MUST use the
>       Identifier of "Type.Extended-Type" to determine the interpretation
>       of the Value field.

  OK.

> === DISCUSS ===
> -- Section 2.1 --
>
> In the description of Extended Type:
>
>    Values 241-255 are reserved and SHOULD NOT be used.
>
> Why is this SHOULD NOT, rather than MUST NOT?  Isn't this another
> aspect of potential "antisocial behaviour"?

  I'll change it to MUST NOT.

> === DISCUSS ===
> -- Section 2.8 --
>
>    When an implementation receives an "invalid attribute", it SHOULD be
>    silently discarded.  If it is not discarded, it MUST NOT be handled
>    in the same manner as a well-formed attribute.
>
> This is contradicted by the discussion of proxies in Section 5.2:

  OK.

>    This practice is NOT RECOMMENDED.  Proxy servers SHOULD forward
>    attributes, even ones which they do not understand, or which are not
>    in a local dictionary.  When forwarded, these attributes SHOULD be
>    sent verbatim, with no modifications or changes.  This requirement
>    includes "invalid attributes", as there may be some other system in
>    the network which understands them.
>
> I know that the point in Section 2.8 is not intended to apply to
> proxies.  I think you need to make that clear, to avoid implementation
> errors.  This should work, I think:
>
> NEW
>    When an implementation receives an "invalid attribute", it SHOULD be
>    silently discarded, except when the implementation is acting as a
>    proxy (see Section 5.2 for discussion of proxy servers).  If it is
>    not discarded, it MUST NOT be handled in the same manner as a
>    well-formed attribute.
> END

  OK.  I'll use that text.

> === DISCUSS ===
> -- Section 6.2 --
>
>       Complex data types MAY be used where they reduce complexity in
>       non-RADIUS systems, though this practice is NOT RECOMMENDED.  The
>       complex data type exceptions of [RFC6158] Section 3.2.4 still
>       apply.  In all other situations, complex data types MUST NOT be
>       used.  Instead, complex data types MUST be decomposed into TLVs.
...

> NEW
>    [RFC6158], Section 3.2.4, describes situations in which complex
>    data types might be appropriate.  They SHOULD NOT be used even
>    in those situations, without careful consideration of the
>    described limitations.  In all other cases, not covered by the
>    complex data type exceptions, complex data types MUST NOT be
>    used.  Instead, complex data types MUST be decomposed into TLVs.
> END

  That looks good.  I'll use it.

> === DISCUSS ===
> -- Section 6.4 --
>
>    * The data type "tlv" SHOULD NOT be used for standard RADIUS
>      attributes.  This format was NOT RECOMMENDED by [RFC6158],
>      but their utility is now evident.
>
> I don't understand what this means to say.  "SHOULD NOT" and "NOT
> RECOMMENDED" are synonyms.  Are you just repeating what 6158 says, or
> are you intending to change it?  (In contrast, what you say a couple
> of bullets down about integer64 *is* correct, changing a SHOULD NOT
> from 6158 to a MAY here.)

  I'll just delete the second sentence from the bullet point.

> === DISCUSS ===
> -- Section 6.5 and 6.6 --
>
> From 6.5:
>    * Where a group of TLVs is loosely defined, or is expected to change,
>      they SHOULD request allocation from the "long extended space".
>
> From 6.6:
>    * Specifications of an attribute which always encode less than 253
>      octets of data MUST NOT request allocation from the long extended
>      space.  The standard space, or the short extended space MUST be
>      used instead.
>
> Isn't this a contradiction?  If I have a loosely defined group of TLVs
> that I expect to have change, but which will always be less than 253
> octets, which space do I get it from?

  The text says "short extended space".

> === COMMENT ===
> -- Section 6.6 --
>    * When the extended space is nearing exhaustion, a new specification
>      SHOULD be written which requests allocation of one or more RADIUS
>      Attributes from the "Reserved" portion of the standard space
>
> Very small point, but: it's quite odd in a protocol standard to use
> 2119 language to try to give the IETF an action item for later.  Do
> you mind losing the "SHOULD", and maybe making it "will have to be"?

  OK.  I'll make the change.

> === DISCUSS ===
> -- Section 9 --
>
> Your grammar doesn't allow for the whitespace that's in your examples.
>  Being strict about it, your first two examples should be like this,
> according to the grammar:
>
>      241.1"bob"
>      241.2{12345}
>
> That's clearly not what you want.  You'll need to change the last
> three items in your grammar like this:
>
> NEW
>    TLV = "{" SP 1*DIGIT SP DATA SP "}"
>
>    DATA = (HEXCHAR *(SP HEXCHAR)) / (TLV *(SP TLV)) / STRING
>
>    LINE = Identifier SP DATA
> OLD

  OK.

> And in Section 9.2, your long data portions lack the required
quotation marks.

  OK.

> === COMMENT ===
> I gather that you've been exchanging messages with IANA about the IANA
> Considerations.  There's nothing that looks amiss here, but I strongly
> encourage you to be very sure that IANA correctly understands what you
> want them to do AND that you update the document to reflect anything
> that comes up in the discussion.  It can be a total pain later if this
> gets done wrong.

  Done.
<

---

From: Barry Leiba <[email-masked]>
Date: Fri, 1 Feb 2013 19:40:17 -0500

Hi, Alan.

>   I've taken the liberty of responding to both messages here.

Great; thanks for merging them.  And thanks for the quick reply.  I'm
travelling (Japan & China), but I'll try to be as responsive.

Just the things that need further comment...

> IANA can then assign certain values as having a particular format.

Understood...  and this is a non-blocking comment, so I won't pursue
it beyond this response.  I just thought it would be useful to clarify
that *this* format is *only* used with the extended type values, and
not with any others.  If you think it's not necessary to say that,
then we're done here.  But perhaps it's a harmless and useful thing to
add.

> "Length" fields, so Lengths of 0 and 1 are invalide.

Right, I understand that.  And so...  what should an implementation
that receives an EA with a length of 1 do?

If the answer is, "Well, treat it as an invalid attribute, of course,"
then why shouldn't the text above just say, "If a client or server
receives an Extended attribute with a length less than 4, then..."?  I
don't see the point in calling out 2 and 3 specifically.

But, again, this one is non-blocking, so this is all I'll say about
that.  Do what you think is best.

>   The intent here is that the *contents* of the attribute are invalid.

Yes, I get that.

You are saying (in 2.8) that "invalid attribute" is a situation when
the length is valid and the content is not.

But you are also saying (in 2.1) that an EA is considered "invalid
attribute" when its length is invalid.

Do you not see those two as contradictory?

>   The text in RFC 2865 isn't a MUST.

I think we understand normative language differently (perhaps we
should loop Pete Resnick in on this point).  When a spec says "X is Y"
or "X contains integer values from 1 to 9", or similar things, it's
making a normative statement that that's the way it is, and there are
no other choices.  It really is equivalent to MUST, in a situation
where using the word MUST is not necessary.

> Many other RADIUS RFCs have been
> published which add new data types

And that's fine: they have expanded the definition of the field.  In
an example where the base spec says "X is an integer that has values
from 1 to 9", we can put out another spec that updates the base and
says, "valid values for X are expanded to include 10 through 66." The
result is that X can now be 1 to 66.  But 88 is still invalid (MUST
NOT be used, if you put it in 2119 language).

If you then say, in another update document, "X SHOULD have values
from 1 to 75", you have made a significant change: you're not only
expanding the valid range to 75, but you're *also* saying that 88 is
OK now, if the implementor fully understands the implications.

Before, 88 was simply invalid.  Now, 88 is valid, but NOT RECOMMENDED.
 There's a difference, and I want to be sure that difference is
intended, not accidental.

>   That has *always* been the RADIUS practice.  This document changes

That's simply not correct.  It may well be that this document does not
change actual practice.  But it's *certainly* true that this document
changes what is *valid* according to the specification.

>From this discussion, though, it seems that the change *is* what you
mean to do.  But it disturbs me that you don't see that you're
changing anything (and, by extension, that the WG perhaps does not
understand that either).

>> 1. Do you really mean to allow that?

OK

>   6158, and my individual draft.

OK, and I understand that the RADIUS community is well aware of this.
I have no concern about that now; thanks.

> types, such as 8-bit integer, 32-bit integer, "polymorphic"

Then what's the point of the SHOULD?  If it doesn't matter -- if there
are no interoperability implications, or whatnot -- then you shouldn't
be saying SHOULD.

> data.  That's why we're writing this draft.

Understood.  Then why is this not MUST?

>   I'm not sure why.  Where else would this be mentioned?

I think you misunderstand what I'm asking -- it's a minor point of
English usage.  We usually use "that is" to talk about something
that's already been said, but we need to highlight it or say it
another way.  In this case, you haven't said anything at all about
ordering yet, so I think you should just take the words "that is" out.
 That's all.  No big deal, and this part is certainly non-blocking!

> write some clarifying text.

Yes, sure...  I didn't mean that I expected you to put the computation
in the document!  Thanks.

> conservative in what they generate.

OK, but then I think this text should make that clear.  Something like this:

   Implementations MUST be able to process non-
   contiguous fragments -- that is, fragments which are mixed together
   with other attributes of a different Type.  This will allow them to
   accept errors in the sending of the atrtibutes.

Or some such.  Otherwise, it leaves the contradiction in question.

> that deep of a nesting, it would cause discussion, and likely a

Understood.  But from the point of view of RFC 2119 (Section 6):

   Imperatives of the type defined in this memo must be used with care
   and sparingly.  In particular, they MUST only be used where it is
   actually required for interoperation or to limit behavior which has
   potential for causing harm (e.g., limiting retransmisssions)  For
   example, they must not be used to try to impose a particular method
   on implementors where the method is not required for
   interoperability.

If there is no reason to limit the nesting depth other than that you
think it's unnecessary to allow more than 4 levels, then you shouldn't
be using 2119 key words for it.  Maybe, "However, nesting depths of
more than 4 have not been shown to be necessary, may be indicative of
implementation errors, and ought not be used." Or something like that.
 Doesn't have to go into any detail, but just not say that something
is an interoperability issue if it's not.

...
> Instead, we note that many values exist, with few restrictions.

Ack.  Then it's correct as it stands.

...
...
>     fragments MUST be treated as "invalid attributes", and the reassembled

Perfect.

> attribute.

OK.  No problem.

>   The text says "short extended space".

The text in 6.6 says that, yes.  But the text in 6.5 says that it
SHOULD come from "long extended space".  I think they contradict each
other.  Don't they?

---
Ha... a much smaller list remains!

Barry

---

From: Sam Hartman <[email-masked]>
Date: Fri, 01 Feb 2013 20:32:23 -0500

>>>>> "Barry" == Barry Leiba <[email-masked]> writes:
esn't make sense to me.  Section
    >>> 2.8 starts with this:
    >>> 
    >>> The term "invalid attribute" is new to this specification.  It
    >>> is defined to mean that the Length field of an Attribute is
    >>> valid (as per [RFC2865], Section 5, top of page 25).
    >>> 
    >>> length is invalid, it's an "invalid attribute", and the second
    >>> says that "invalid attribute" means that the length field is
    >>> valid (but there are other problems).  This comment also applies
    >>> to "TLV-Length" in Section 2.3.
    >> 

    Barry> Yes, I get that.

    Barry> You are saying (in 2.8) that "invalid attribute" is a
    Barry> situation when the length is valid and the content is not.

    Barry> But you are also saying (in 2.1) that an EA is considered
    Barry> "invalid attribute" when its length is invalid.

If I may, I think this is a layering issue.

You have the base radius layer, which requires length be at least 2.
If your length isn't 2, you fall off the parse path in RFC 2865; it's a
low level problem.

If your length is at least 2 you get into an attribute-type-specific
abstraction.  One outcome of this abstraction is "invalid attribute," a
new concept introduced in this spec.  It means well-formed at the
2865-level but semantically garbage at an attribute-type-specific level
introduced in this spec.  And the attribute-type-specific layer for
extended types introduces the length must be 4 rule.

So, it's kind of like saying that a different chunk of code in the
abstraction will return an error depending on which length range things
fall in. 0-1: something like BaseRadius.WayTooShort; 2: if it's an
extended attribute extendedattribute.InvalidAttribute

The above is just an analogy to explain why I don't think it's
contradictory.  I'm not entirely convinced that distinction in
abstraction is useful.  I.E. the spec might be improved by addressing
your concern, and the wording might not be good enough for you to see a
reading that isn't contradictory, but I don't see any internal
consistency problem.

--Sam

---

From: Alan DeKok <[email-masked]>
Date: Fri, 01 Feb 2013 20:34:02 -0500

> then we're done here.  But perhaps it's a harmless and useful thing to

  I'll take a look at coming up with some phrasing.

> that receives an EA with a length of 1 do?

  It won't.  The *packet* is malformed, and will be discarded.  So any
*attribute* parsing will always see a Length of 2 or more.

> don't see the point in calling out 2 and 3 specifically.

  Because 2 and 3 are values which allow the packet to be "well formed",
but result in the attribute being invalid.  Therefore, they need to be
called out as invalid for the attribute.

> Do you not see those two as contradictory?

  Superficially, yes.  It's the distinction between packets and
attributes that makes them consistent.

  Perhaps the text could better say:

---
The term "invalid attribute" is new to this specification.  It is
defined to mean that the Length field of an Attribute permits the
packet to be accepted as not being "malformed".  However, the Value
field of the attribute does not follow the format required by the data
type defined for that Attribute, and therefore the attribute is
"malformed".  In order to distinguish the two cases, we refer to
"malformed" packets, and "invalid attributes".

For example, an implementation receives a packet which is well-formed.
That packet contains an Attribute allegedly of data type "address",
but which has Length not equal to four.  In that situation, the packet
is well formed, but the attribute is not.  Therefore, it is an
"invalid attribute".

A similar analysis can be performed when an attribute carries TLVs.
The encapsulating attribute may be well formed, but the TLV may be an
"invalid attribute".  The existence of an "invalid attribute" in a
packet or attribute MUST NOT result in the implementation discarding
the entire packet, or treating the packet as a negative
acknowledgment.  Instead, only the "invalid attribute" is treated
specially.
---

> where using the word MUST is not necessary.

  Yes.  However, this document isn't setting out to correct two decades
of RADIUS mis-steps.  I don't want to see this document held up to
correct ambiguities raised in other specifications.

> And that's fine: they have expanded the definition of the field.

  That's the point.  They haven't.  The "data types" defined in RFC 2865
were *never* expanded.  Later RFCs used many new data types, without
naming or defining them.  They therefore fail to meet the requirement of
2865 that only 5 data types are valid.

> changing anything (and, by extension, that the WG perhaps does not

  Of course we're changing RADIUS by adding the new formats.  That much
is obvious.  What I object to is the idea that we can't add *new* data
types to the ones listed in 2865.  Why?  This document updates 2865,
which is part of the IETF process.

  Much effort went into ensuring that *existing* implementations
wouldn't break when they saw packets using the new data types.  For
those implementations, nothing changes.  They see attributes which were
unallocated when the implementation was written.  Therefore, they ignore
them.

> are no interoperability implications, or whatnot -- then you shouldn't

  Yes.  It will be a MUST.

  OK.

> Or some such.  Otherwise, it leaves the contradiction in question.

  OK.  I'll add some text.

> be using 2119 key words for it.

  The consensus was also that allowing nesting of more than 4 levels
deep would make implementations much more complex.

>  Maybe, "However, nesting depths of

  How about this:

The depth of TLV nesting is limited only by the restrictions on the
TLV-Length field.  The limit of 253 octets of data results in a limit
of 126 levels of nesting.  However, nesting depths of more than 4 are
NOT RECOMMENDED.  They have not been demonstrated to be necessary in
practice, and they appear to make implementations more complex.
Reception of packets with such deeply nest TLVs may indicate
implementation errors or denial of service attacks.  Where
implementations do not support deep nesting of TLVs, it is RECOMMENDED
that the unsupported layers are treated as "invalid attributes".

> SHOULD come from "long extended space".  I think they contradict each

  The text in 6.5 says:

   * Where a group of TLVs is strictly defined, and not expected to
     change, and totals less than 247 octets of data, they SHOULD
     request allocation from the "short extended space".

  So I think it's OK.

> Ha... a much smaller list remains!

  Quick turn-around helps.  I'll issue another draft now.

  Alan DeKok.

---

From: Barry Leiba <[email-masked]>
Date: Sat, 2 Feb 2013 01:44:14 -0500

> *attribute* parsing will always see a Length of 2 or more.

Ack.  Between Sam's answer and yours, I now understand this.  I still
think it would be better to just say "less than 4", which would cover
all cases without confusing anyone (uh, like me)...  but this issue is
answered, and if you really want to say "2 or 3" rather than "less
than 4", I understand.

> "malformed" packets, and "invalid attributes".
...etc...

That'd be lovely, and thanks.  I accept that this might already be
clear to RADIUS folks ("RADII"?).  On the other hand, there are often
people implementing our specs who don't start off knowing what they're
doing, and clarity for outsiders can really help them.

> correct ambiguities raised in other specifications.

No, I agree with you.  I'm certainly not aiming to do that, and I'll
consider my comments in that light.

> which is part of the IETF process.

Ah, no, you still misunderstand.  You *absolutely* can do that, and
should.  That's not my question at all.  My question was just about
the SHOULD.  The original spec said that there were certain valid data
types, and everything else was invalid.  Other specs added new data
types, but everything else that wasn't specified somewhere was still
invalid.  By putting that SHOULD in, you're saying that other data
types, ones that aren't specified in any specs, ARE valid, but just
SHOULD NOT be used.  That's different to what the spec said before.

You have told me that this change is reflecting actual deployment, and
is intentional.  I accept that, so we're actually OK on this point
now.  I wish this had been specified differently, but we are where we
are.

So I think we can let this point rest, and thanks for working with me on it.

> that the unsupported layers are treated as "invalid attributes".

Groovy.

>      they SHOULD request allocation from the "long extended space".

This is the only one that remains, and I'm happy to downgrade it to
COMMENT level, given the discussion.  It is now officially
non-blocking.

It's that second bullet you quote that I wonder about.  Is it possible
for a group of TLVs to be loosely defined or expected to change (and
thus fit under the second bullet there), and yet always encode less
than 253 octets (and this fit into the "short extended space MUST be
used" bullet in 6.6?

Perhaps that's not possible, so there's no issue at all.  But if it is
possible, I think there's a conflict between the last bullet in 6.5
and the 6th bullet in 6.6, *in that specific situation*.

I'll go through the diffs between -09 and -11 now, while I wait for my
flight to HKG, and double-check that we're all set on the other
points.

b

---

From: Barry Leiba <[email-masked]>
Date: Sat, 2 Feb 2013 01:53:48 -0500

> flight to HKG, and double-check that we're all set on the other

Verified.  Please make any further changes that my last message moves
you to, and then make sure the working group is OK with the changes.

And, again, thanks very much for working through this with me!

Barry

---

From: Barry Leiba <[email-masked]>
Date: Sun, 3 Feb 2013 11:49:17 +0900

> you to, and then make sure the working group is OK with the changes.

FWIW, my suggestion:

1. Alan posts a -12 version if he has further changes.
2. Alan posts a message to the radext list, saying that he's made
quite a bunch of changes in response to IESG Evaluation, and the WG
should review them.  Include a URL for the diff between -09 and -12
(or -11).
3. The chairs post a message to the radext list asking for any
objections to the changes to be posted by, say, 15 Feb.  The chairs
might specifically note that this is a call for review of the changes
only.
4. Benoit sticks the document on the 21 Feb telechat agenda (and
remember to go back afterward and remove the "returning item" flag).
I suggest doing this now; no need to wait.
5. When 15 Feb rolls around, Benoit changes the status back to "IESG
Evaluation".

Of course, this all assumes no significant objections from the WG, but
you all know how to deal with that if it happens.  :-)

Barry

---

From: Alan DeKok <[email-masked]>
Date: Sun, 03 Feb 2013 10:11:18 -0500

> 1. Alan posts a -12 version if he has further changes.

  I don't at this time.

> should review them.  Include a URL for the diff between -09 and -12

http://tools.ietf.org/rfcdiff?url2=draft-ietf-radext-radius-extensions-10.txt
http://tools.ietf.org/rfcdiff?url2=draft-ietf-radext-radius-extensions-11.txt

  I hope there will be little issue with the diffs.  They are
clarifications, with no change of function or meaning.

  Alan DeKok.

---

From: Sam Hartman <[email-masked]>
Date: Sun, 03 Feb 2013 10:28:58 -0500

>>>>> "Alan" == Alan DeKok <[email-masked]> writes:

    >> 1. Alan posts a -12 version if he has further changes.

    Alan>   I don't at this time.

    >> quite a bunch of changes in response to IESG Evaluation, and the
    >> WG should review them.  Include a URL for the diff between -09
    >> and -12 (or -11).

    Alan> http://tools.ietf.org/rfcdiff?url2=draft-ietf-radext-radius-extensions-10.txt
    Alan> http://tools.ietf.org/rfcdiff?url2=draft-ietf-radext-radius-extensions-11.txt

    Alan>   I hope there will be little issue with the diffs.  They are
    Alan> clarifications, with no change of function or meaning.

I have not reviewed the diffs, but from what I've read of barry's
comments and Alan's responses, this all sounds quite minor.

--Sam

---

From: Benoit Claise <[email-masked]>
Date: Tue, 05 Feb 2013 15:48:58 +0100

Thanks Barry,

I can see how the document improved thanks to your review.
See in line.
> should review them.  Include a URL for the diff between -09 and -12
https://www.ietf.org/rfcdiff?url1=draft-ietf-radext-radius-extensions-09&difftype=--html&submit=Go!&url2=draft-ietf-radext-radius-extensions-11
> might specifically note that this is a call for review of the changes
Chairs, please do so.
Done.
> 5. When 15 Feb rolls around, Benoit changes the status back to "IESG
Will do.

Regards, Benoit
> _______________________________________________
> radext mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/radext
>
>

---

From: Jouni Korhonen <[email-masked]>
Date: Wed, 6 Feb 2013 18:42:08 +0200

Folks,

Everybody has already seen the proposal from Barry - thanks for it ;)

So lets make it official. Since we all have seen the changes and the
discussion that lead to those, we should be OK with a one week review
time. Every WG member is encouraged to verify the latest -11 version
of the I-D whether the _changes_ are from -09. This is not a place for
new material etc.

Express your OK/NOK/opinion by 13th Feb. If everything is fine, our
AD can do the required steps regarding IESG telechat agenda and we
are one step closer to publication.

- Jouni & Mauricio

---

From: Jouni Korhonen <[email-masked]>
Date: Wed, 13 Feb 2013 19:53:58 +0200

Folks,

I take the silence as a sign of agreement. The document is good to
move forward on the IESG process.

- Jouni

---

From: Benoit Claise <[email-masked]>
Date: Thu, 14 Feb 2013 00:08:51 +0100

Jouni,

Thanks for letting me know.
The document status justt moved from IESG evaluation/AD follow up to 
IESG evaluation, and the draft is on the IESG agenda for Feb 21st.
Let me point the YES ballot from Barry, along with his comment.

    *Comment (2013-02-05)*

    I had an excellent set of exchanges with Alan Dekok, in which he sorted out all
    my issues (reflected in the -11) version.  There remain a couple of minor
    things we disagree on, but that will happen, and they are minor.  Thanks very
    much to Alan for the quick turnaround and good work.

Regards, Benoit