Re: [OPSAWG] draft-ietf-opsawg-tacacs-06 Contributions, Status and Plans

[Alan DeKok <[e-mail]>]

On May 20, 2017, at 8:24 AM, Douglas Gash (dcmgash) <[e-mail]> wrote:
>> If the field is unused, the spec should say the field is ignored, and
>> treated as if it did not exist.
> 
> Agreed, though I¹m not sure how an unused field would not be ignored,
> almost by definition,. The only circumstance I can think of where a field
> has data but is unused may not be ignored is for logging, are you thinking
> we should preclude such?

  No.  Nut the draft needs to say what to do.

  If the protocol has provisions for something to happen, then the draft needs to say what implementations should do when that thing happens.

  In this case, it should state that unused fields are ignored, and treated as if they were not present.

>> The original RADIUS RFC used a secret key, but didn't forbid a key of
>> zero length.  I ran into at least one vendor who didn't allow for the
>> entry of secret keys, and always set it to a zero-length string.  The
>> next rev of the RFC forbade that...
> 
> So I think the specification of secrets for obfuscated option is, as you
> say, critical, and will provide some guidance such as non-empty. Also will
> make sure we include that servers MUST support capability of secret per
> client-server tuple (see next comment).

  And it should suggest a minimum secret length.

> But I¹m not so sure what we should say about storage.

  "secret keys need to be kept secret". 

>> OK. And if there's still no username?
> 
> Do you mean, if client replies to the request to
> TAC_PLUS_AUTHEN_STATUS_GETUSER with an empty username. I would simply
> repeat the request in case it was a user error. We can document this. The
> protocol has a built-in limit of the number of interactions due to the
> seq_no, but implementations can limit interactions.
> 
> We can add this detail.

  Thanks.  My $0.02 is that the username SHOULD be in the first packet.  If not, it MUST be in the second packet.  If not, the session is broken, and the server returns ERROR.

  i.e. relying on the 256 packet limit for authentication is probably too lax.

...  RE: CHAP challenges and length
>> That reply make implementation-specific assumptions.  There are TACACS+
>> implementations which don't get the challenge from a third-party.
> 
> Just to clarify my comment: it is not an aspect of the processing of the
> T+ protocol that chooses the challenge. The protocol will get the
> challenge and response as pre-packaged unit.

   If the protocol transports data without looking at it, it's fine to treat that data as an opaque blob.  If the protocol looks at the data, the draft should have requirements on the data format.

   For an example, see:

https://tools.ietf.org/html/rfc2865#section-5.40

  The CHAP-Challenge attribute in RADIUS is defined to have at least 5 octets of challenge data.

> So I think we can demand that challenges be generated according to the
> spec for CHAP. But reaching into that spec to add our own criteria would,
> I think, be a mixing of concerns.

  Nope.  The TACACS+ server which authenticates the user should be secure by design.  This means rejecting zero-length challenges, and challenges which "too short".

  I would suggest that the challenges are a minimum of 8 octets in length.

... RE: password changes.
> Ok, I see what you mean. We can add the SHOULD not clause here.
> 
> Interesting though, as this is a significant use case for T+. Will be
> insteresting to see what the main objects were for why it was removed from
> RADIUS.

  It was insecure, and no one liked it.  The other reasons have been largely lost to time.

>>>   The START and STOP flags are mutually exclusive.  When the WATCHDOG
>>>   flag is set along with the START flag, it indicates that the update
>>>   record is a duplicate of the original START record.
>>> 
>>> * why would an update duplicate the original START record?  This seems
>>> redundant.
>>> [Well, here we are constrained by a clear definition form original
>>> draft.]
>> 
>> What does it mean, tho?  This document should say.  i.e. portions of
>> the protocol should not be left as "we have no idea what this means".
> 
> I think it is more a case of: it is redundant, but it was specified, so we
> cannot choose to change it to make it more efficient.

  It's not about being "more efficient".  It's about specifying what this means.

  I have no idea what it means to duplicate a START record.  Is the data in the second START the same?  Is it different?  What is done in either case?

  Again, you need to document the protocol.  I'm not sure this point is getting across.  If you don't know what a portion of the protocol means, then either find out and document it, or document that "this exists, but we have no idea what it is, what it does, and we don't recommend that people use it".

  Leaving portions of the protocol entirely undefined and implementation-defined is bad.

  In RADIUS, I've authored or co-authored about 4 RFCs fixing things which were missed or undefined, and which cause interoperability issues.  And I'm still finding things which are broken in the specs.  Two in the last month, in fact...

  If TACACS is to be a standard, it should be clear what portions are well defined, and what portions are "I dunno...".  That gives guidance for future drafts as to what works, and what needs fixing.

>>> * how often do the updates get sent?  Saying "implementation defined" is
>>> OK, but some recommendations would be good.  i.e. not more than 100x per
>>> second, and not less than 1 day apart (as extreme examples)
>>> [so lets stick with implementation defined, the range of T+ devices we
>>> see is so diverse and use cases are such that I would not like to
>>> constrain.]
>> 
>> It would be good to have recommendations.  e.g. "updates more than once
>> per second are NOT RECOMMEND, updates more than an hour apart are NOT
>> RECOMMENDED".
>> 
>> The goal is to guide implementors to choosing values which won't cause
>> problems for everyone else.
> 
> Sure, will recommend minimum of 5 seconds, but I¹m not sure we can specify
> an upper limit as it is not mandatory.

   It's always good to recommend best practices, even if the behaviour isn't mandatory.

  Alan DeKok.