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

[Barry Leiba <[e-mail]>]

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

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.

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

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

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

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

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.

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

Groovy.

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

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