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

[Alan DeKok <[e-mail]>]

Barry Leiba wrote:
> 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.

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

> Right, I understand that.  And so...  what should an implementation
> 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.

> 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.

  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.

> 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?

  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.
---

> 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.

  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.

>> Many other RADIUS RFCs have been
>> published which add new data types
> 
> 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.

>>> 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.
> 
> 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).

  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.

>>> 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.
> 
> 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.

  Yes.  It will be a MUST.

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

  OK.

>>> 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.
> 
> 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.

  OK.  I'll add some text.

> 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.

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

>  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.

  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".

>>> 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".
> 
> 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?

  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".

   * Where a group of TLVs is loosely defined, or is expected to change,
     they SHOULD request allocation from the "long 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.