Subject: [OPSAWG] Start of WGLC for TACACS+ document.

From: Warren Kumari <[email-masked]>
Date: Mon, 3 Oct 2016 17:02:05 -0400

Dear OpsAWG WG,

The authors of draft-ietf-opsawg-tacacs-05 have indicated that they
believe that the document is ready, and have asked for Working Group
Last Call.

This WGLC ends Mon 17-Oct-2016.

Please review this draft to see if you think it is ready for
publication and send comments to the list, clearly stating your view.
Please note that "Yeah, I reviewed it, it looks great!" is a useful
comment (well, more useful than silence).

Please note that this document is primarily describing an existing
protocol, *NOT* designing a new one.  This means that comments of the
form "Ick, this uses MD5?! You *so* should replace it with
[SHA-512|BLAKE2|GOST|SWIFFT|<etc>]" or "flags should be 16bits, not 8,
what if we want to extend this?" are not helpful.

Also, we already know that this protocol has security issues. The plan
is to publish this document, finally documenting (after 19 years) one
of the core protocols used by network people. Once this is done, there
will be a new document published, extending this to be more secure.
There is some history behind this decision...

The draft is available here (for easy clickin'):
https://datatracker.ietf.org/doc/draft-ietf-opsawg-tacacs-05/

In addition, to satisfy RFC 6702 ("Promoting Compliance with
Intellectual Property Rights (IPR)"):
Are you personally aware of any IPR that applies to
draft-ietf-opsawg-tacacs-05?  If so, has this IPR been disclosed in
compliance with IETF IPR rules? (See RFCs 3979, 4879, 3669, and 5378
for more details.)

Thanks,
Warren Kumari
(as OpsAWG WG co-chair)

-- 
I don't think the execution is relevant when it was obviously a bad
idea in the first place.
This is like putting rabid weasels in your pants, and later expressing
regret at having chosen those particular rabid weasels and that pair
of pants.
   ---maf

---

From: Alan DeKok <[email-masked]>
Date: Wed, 5 Oct 2016 12:15:46 -0400

Some comments.  Mostly little nits and clarifications.

  The major points for me are related to security.  The "Security Considerations" section is almost empty, and will certainly not pass a review from the security area.  They will in all likelihood block publication until substantive comments have been added.

 ------ 

3.3.  Single Connect Mode

   The packet header (see below) contains a flag to allow sessions to be
   multiplexed on a connection.

* it would be good to name the flag here.

   If the server sets this flag in the first reply packet in response to
   the first packet from a client, this indicates its willingness to
   support single-connection over the current connection.  The server
   may set this flag even if the client does not set it, but the client
   is under no obligation to honor it.

* what happens if the client does not honour the flag?

   The flag is only relevant for the first two packets on a connection,
   to allow the client and server to establish single connection mode.
   The flag MUST be ignored after these two packets since the single-
   connect status of a connection, once established, must not be
   changed.  The connection must instead be closed and a new connection
   opened, if required.

  Why would it be required to change the mode?  It would be good to explain the use-cases, and why (or not) they make sense.

   TAC_PLUS_UNENCRYPTED_FLAG := 0x01

   If this flag is set, then body encryption is not used.  If this flag
   is cleared, the packet is encrypted.  Unencrypted packets are
   intended for testing, and are not recommended for normal use.

* it would be good to have a pointer to the security considerations, with a discussion of the implications.

*  Also, it's odd that a completely insecure mode is just "not recommended for normal use".  I would suggest more panic-inducing phrasing.  e.g. ."Unencrypted packets offer a complete compromise of all security related to the protocol and MUST NOT be used outside of a testing environment"

   TAC_PLUS_SINGLE_CONNECT_FLAG := 0x04

   This flag is ussequernt, ed to allow a client and server to agree whether
   multiple sessions may be multiplexed onto a single connection.

* I find the "agree" phrasing odd.  I've mentioned before that the tone of the document is often philosophical.  Perhaps saying instead "this flag signals whether the client and server are capable of performing session multiplexing"

* i.e. the flag is a signal, not a capability for the systems to agree.

   session_id

   The Id for this TACACS+ session.  The session id is to be selected
   randomly.  This field does not change for the duration of the TACACS+
   session.  (If this value is not a cryptographically strong random
   number, it will compromise the protocol's security, see  RFC 1750

* it would be good to update the paragraph to be more prescriptive.  i.e.

   The Id for this TACACS+ session.  The session id MUST be taken
   fro a cryptographically strong random number generation.  If not,
   the protocol's security will be compromised.  See  RFC 1750
   for further information

...

   length

   The total length of the packet body (not including the header).  This
   value is in network byte order.  Packets are never padded beyond this
   length.

* Why is the text on "padding" there?  The TACACS+ packets are sent over TCP, not UDP.  So any "padding" would be outside of the scope of the current packet, and be seen by the other end as a subsequent, but invalid, TACACS+ packet.

3.5 ...

      - Unused fixed length fields SHOULD have values of zero.

* what happens when they don't?  I suggest adding text saying that the fields are ignored.

     - There will be no padding in any of the fields or at the end of a
      packet.

* I wonder again what "padding" means here.

3.6.  Encryption

   The body of packets may be encrypted.   ...

* Which is not a standard encryption method.  Security people will complain here.  I suggest (as before) using the term "obfuscated".  With a note that for historical reasons, the flags / fields / etc. refer to TACACS+ "encryption".

 ...  This document does not discuss the management and storage of
   those keys.  ...

* there should be some discussion in the Security Considerations section about this.  Otherwise, this subject will be brought up in the security review.

  ...  It is an implementation detail of the server and client,
   as to whether they will maintain only one key, or a different key for
   each client or server with which they communicate.  For security
   reasons, the latter options MUST be available, but it is a site
   dependent decision as to whether the use of separate keys is
   appropriate.

* those sentences seem to disagree with each other.  It's an "implementation detail", but it MUST be supported.  I suggest requiring the capability of per-server and per-client keys.

  ...   The session id is used in network byte order.

* nit: if the session ID is a 32-bit opaque token, it's not in any byte order.  Maybe just "as taken from the packet header" ?

   When a server detects that the secret(s) it has configured for the
   device mismatch, it MUST return ERROR.  The handling of the TCP
   connection by the server is implementation independent.

* I repeat my objection here.  There is just no reason for the TCP connection to remain open after the connection has been determined to be fraudulent.  This subject also came up in the RADIUS over TCP drafts.  The consensus there was the same: if it's not a real / known / good client, there is every reason to close the connection, and zero reasons to keep it open.

   After a packet body is decrypted, the lengths of the component values
   in the packet are summed.  If the sum is not identical to the
   cleartext datalength value from the header, the packet MUST be
   discarded, and an error signalled.  The underlying TCP connection MAY
   also be closed, if it is not being used for other sessions in single-
   connect mode.

* This last sentence is confusing.  Are there different secrets for different packets in one TCP stream?  If so, nothing in the document says so.  If not, then there is every reason to believe that the secret is mismatched. And, the *next* packet will decrypt incorrectly.  Which means (again) you might as well just close the TCP connection.

   If an error must be declared but the type of the incoming packet
   cannot be determined, a packet with the identical cleartext header
   but with a sequence number incremented by one and the length set to
   zero MUST be returned to indicate an error.

* this mix of clear-text and encrypted signalling opens up the protocol to an attacker.  Since this "NAK'" is not protected, an attacker can forge the NAK at any time, and force the connection to be closed.

* this attack (along with all others) should be noted in the Security Considerations section.

...
   Packet fields are as follows:

   action

   This describes the authentication action to be performed.  Legal
   values are:

* what happens if a client/server receives values outside of the legal range?  I suggest sending ERROR, and closing the TCP connection.

* similar comments apply for other fields with "legal values".  While it's good to define what happens when things go as planned, it's also good to define what happens when things *don't* go according to plan.

* review of 4.2 and subsequent sections will follow in a later message.

---

From: Alan DeKok <[email-masked]>
Date: Wed, 5 Oct 2016 16:54:45 -0400

Continuing...

  Summary: the spec is still fairly descriptive / philosophical instead of prescriptive.  Many optional parts of the protocol are described, "A and B are allowed", but there is often no description of what to *do* when either A or B is present.  I've suggested descriptive text, or places where descriptive text can be added.

 The larger worry here is what happens if the descriptive text conflicts with existing implementations?  This is a topic which should be discussed in the "Security Considerations" section.

  i.e. the spec documents the existing protocol, with recommendations for security and inter-operability.  As the protocol has historically been poorly specified, implementations are likely to follow the broad requirements of this specification.  They are also likely to not follow many of the newly-clarified behaviours / requirements.  The hope is that this specification can guide implementors towards a common understanding of the protocol.

  Also, vague requirements and open options in a protocol spec are ripe for implementation bugs and security issues.  That worry drives many of my comments.

-----

4.2 ...

server_msg, server_msg_len

   c A message to be displayed to the user.  This field is optional.  If
   it exists, it is intended to be presented to the user.  US-ASCII
   charset MUST be used.  The server_msg_len MUST indicate the length of
   the server_msg field, in bytes.

* The last sentence is atypical of RFC 2119 language.  Then language is typically about system behaviour or inter-field comparisons.  I'm not sure what it means that a field MUST indicate a length.

* i.e. the "server_msg_len" field is *defined* to encode the length of the "server_msg" field.  No "MUST" is necessary.

* similar comments apply to many of the other "foo_len" fields, elsewhere in the document

 rem_addr, rem_addr_len

   An US-ASCII string this is a "best effort" description of the remote
   location from which the user has connected to the client. 

* perhaps instead of "best effort", just re-use language from the "port" description.  The contents of this field are the clients description of the remote location...

4.4 ...

   When the REPLY status equals TAC_PLUS_AUTHEN_STATUS_GETDATA,
   TAC_PLUS_AUTHEN_STATUS_GETUSER or TAC_PLUS_AUTHEN_STATUS_GETPASS,
   then authentication continues and the SHOULD provide server_msg

* editorial: the WHAT should provide...

   ... quest for more information to flexibly support future
   requirements.  If the TAC_PLUS_REPLY_FLAG_NOECHO flag is set in the
   REPLY, then the user response must not be echoed as it is entered.

* the implicit assumption here is that there is a person who is entering text on a terminal.  It would be good to have a bit more explanation of this expected use-case.

  NOTE: Only those requests which have changed from their minor_version
   0 implementation (i.e.  CHAP, MS-CHAP and PAP authentications) will
   use the new minor_version number of 1.  All other requests (i.e.  all
   authorisation and accounting and ASCII authentication) MUST continue
   to use the same minor_version number of 0.

* that's an odd phrasing of the requirement.  Perhaps something more prescriptive would be useful.  e.g.:

  NOTE: Authentication requests which use ASCII login or chpass MUST use minor version 0.  Authentication requests using PAP, CHAP, or MS-CHAP MUST
  use minor version 1.  All authorization and accounting packets MUST use minor version 0.

4.4.2...

   This section describes common authentication flows.  If the options
   are implemented, they MUST follow the description.  If the server
   does not implement an option, it will respond with
   TAC_PLUS_AUTHEN_STATUS_ERROR.

* It's not typically required to say that implementations MUST follow the spec.

* also, perspective terminology is better than descriptive. Not "will do", but "MUST do".

*  I suggest re-phrasing:

  This section describes common authentication flows.  If the server
   does not implement an option, it MUST respond with
   TAC_PLUS_AUTHEN_STATUS_ERROR.

   This is a standard ASCII authentication.  The START packet may
   contain the username, but need not do so.  

* what happens when the username is / is-not included?  i.e. what do implementations *do*?

* if this is a valid protocol flow, then it should be described.  e.g. "Any username in the START packet is ignored", or "any username in the START packet MUST match the username in subsequent packets"

   ... The data fields in both
   the START and CONTINUE packets are not used for ASCII logins. 

* what happens if there is data in the fields?  Is it ignored?

* RFC 2119 language is recommended here.  "any data (if present) MUST be ignored", or "the data field MUST NOT be present"

* I have no idea how to make these requirements compatible with existing implementations.  Sorry...

  CHAP:

   The length of the challenge value can be determined from the length
   of the data field minus the length of the id (always 1 octet) and the
   length of the response field (always 16 octets).

* it would be good to recommend a minimum length for challenge.  1 octet is clearly insufficient, but is (silently) allowed by the spec.  I suggest at least 8 octets/

* also, the challenge MUST be derived from a cryptographically strong pseudo-random number generator, and MUST change on every CHAP request.  Otherwise, the spec allows (also silently) a fixes challenge.

   To perform the authentication, the server will run MD5 over the id,
   the user's secret and the challenge, as defined in the PPP
   Authentication RFC 1334 and then compare that value
   with the response.

* again "server WILL run".  As opposed to "calculates..."

MSCHAPv1 / MSCHAPv2:

   The length of the challenge value can be determined from the length
   of the data field minus the length of the id (always 1 octet) and the
   length of the response field (always 49 octets).

*  RFC 2433 says that the challenge is 8 octets.  RFC 2759 says that the challenge is 16 octets.  It would be good to re-iterate that here.

* i.e. smaller challenges are insecure, and should be forbidden

   ... To perform the authentication, the server will use a the algorithm
   specified  RFC2759 

* editorial: "specified IN RFC ..."

 ... ASCII change password request

* I suspect that the security area review will recommend that this be removed.  Even RADIUS had "change password" removed at the behest of the security people. 

4.4.3.  Aborting an Authentication Session

   The client may prematurely terminate a session by setting the
   TAC_PLUS_CONTINUE_FLAG_ABORT flag in the CONTINUE message.  If this
   flag is set, the data portion of the message may contain an ASCII
   message explaining the reason for the abort. 

* again, what happens if the field is / is-not present?

* what does the server do with this message?  Should it be logged somewhere?

 ... The session is
   terminated and no REPLY message is sent.

* what happens to the TCP connection?  Does it remain open?  It may be good to say "The session is terminated, and the session Id is no longer valid.  However, other sessions on the same TCP connection may continue..."

   The host is specified as either a fully qualified domain name, or an
   ASCII numeric IPV4 address specified as octets separated by dots
   ('.'), or IPV6 address test representation defined in  RFC 4291.

* editorial ".. text .. "representation, not "test"

   If a key is supplied, the client MAY use the key in order to
   authenticate to that host.  The client may use a preconfigured key
   for the host if it has one. 

* so the server is pushing keys to clients?  The security implications of this should be discussed

* what happens if the client gets pushed a key, and then also has a pre-configured key?  Which one takes precedence?  Why?

 ... that's it for today.  More later.

---

From: t.petch <[email-masked]>
Date: Thu, 6 Oct 2016 11:00:05 +0100

Um

Alan is right to pick up on the style - philosophical - and the
security - lack of.

But do we want to change it all at this time?

This is an Informational document describing the state of play as of
some time past, perhaps not as far back as 1997 but not for 2016.  It
would require many changes to make it a 2016 Standards Track document
but that is not what I see us doing except that is how I take Alan's
comments.

The analogy I have in mind is when SSL v3 was published, long after it
had been superseded by anyone who took security seriously, but was
needed as an RFC to refer to, although it would not pass muster because
the security thereof was too weak.  It would not have met the standards
of the day but was published  despite that.

Tom Petch

----- Original Message -----
From: "Alan DeKok" <[email-masked]>
To: "Warren Kumari" <[email-masked]>
Cc: "[email-masked]" <[email-masked]>;
<[email-masked]>;
"[email-masked]" <OpsAWG-

---

From: Alan DeKok <[email-masked]>
Date: Thu, 6 Oct 2016 09:56:46 -0400

> But do we want to change it all at this time?

  Please show where I'm trying to change the protocol.

> but that is not what I see us doing except that is how I take Alan's

  Then you're not reading my comments.

  I would like to implement historical TACACS+.  I have *NO IDEA* what to do for huge swaths of the protocol.

  I would like to deploy historical TACACS+. I have *NO IDEA* what the security implications are of using it.

> of the day but was published  despite that.

  I'm not asking that the protocol be *fixed* in this document.  I'm asking that it be *documented*.  That shouldn't be hard to understand.  I've been saying it for about a year now, for anyone who bothers to read my messages.

  I'll note that RFC 6101 is "Category: Historic", and has substantial text about the security (or lack thereof) of the protocol.  It has substantial text about how the historical protocol works. I'm suggesting we do the same here.

  I'm suggesting the the TACACS+ protocol be documented as designed, in sufficient detail that someone can read the document and create an inter-operable implementation.  I'm suggesting that the  TACACS+ protocols security (or lack thereof) be documented.

  Which is (so far as I'm aware) still IETF practice for informational specifications.

  If the goal for the document is something else, fine.  Update the document to say that.   Something like:

  "This document attempts to specify the historical TACACS+ protocol.  However, there are many portions of the protocol which are under-specified or unspecified.  We cannot second-guess twenty years of practice here.  As a result, this specification is incomplete, under-specified, insecure, and should not be used by anyone to implement anything.  Please wait for the Standards track document to get the actual TACACS+ specification that people can implement".

  If the document can be updated with such text, I'll withdraw all of my review comments.  But I predict that the document won't pass security area review.  And they'll make all of the same comments as I've made here, with a recommendation that the document not be published until it's fixed.

  Alan DeKok.

---

From: "Douglas Gash (dcmgash)" <[email-masked]>
Date: Thu, 6 Oct 2016 15:06:12 +0000

Hi Alan,

Thank you for your detailed review so far. When we get the last
instalment, we¹ll categorise the comments and respond.

Best regards,

Doug.

On 06/10/2016 14:56, "Alan DeKok" <[email-masked]> wrote:

>  I'm not asking that the protocol be *fixed* in this document.  I'm
>asking that it be *documented*.  That shouldn't be hard to understand.
>I've been saying it for about a year now, for anyone who bothers to read
>my messages.
>
>  I'll note that RFC 6101 is "Category: Historic", and has substantial
>text about the security (or lack thereof) of the protocol.  It has
>substantial text about how the historical protocol works. I'm suggesting
>we do the same here.
>
>  I'm suggesting the the TACACS+ protocol be documented as designed, in
>sufficient detail that someone can read the document and create an
>inter-operable implementation.  I'm suggesting that the  TACACS+
>protocols security (or lack thereof) be documented.
>
>  "This document attempts to specify the historical TACACS+ protocol.
>However, there are many portions of the protocol which are
>under-specified or unspecified.  We cannot second-guess twenty years of
>practice here.  As a result, this specification is incomplete,
>under-specified, insecure, and should not be used by anyone to implement
>anything.  Please wait for the Standards track document to get the actual
>TACACS+ specification that people can implement".
>
>  If the document can be updated with such text, I'll withdraw all of my
>review comments.  But I predict that the document won't pass security
>area review.  And they'll make all of the same comments as I've made
>here, with a recommendation that the document not be published until it's
>fixed.
>
>  Alan DeKok.
>

---

From: t.petch <[email-masked]>
Date: Thu, 6 Oct 2016 17:19:19 +0100

----- Original Message -----
From: "Alan DeKok" <[email-masked]>
Sent: Thursday, October 06, 2016 2:56 PM

<tp>

Alan

You are not, and I did not mean to imply that you were.  I mean that
your changes seem to me reflect a different approach, a different style
which I do not see as that of the authors.  I see the existing style as
unusual, but not wrong, and am content to leave it as is for an
Informational RFC which documents what is, or perhaps what has been. You
used the word 'philosophical' and I agreed.

So I would keep changes to the minimum, omissions or contradictions and
those required  by the publication process (such as splitting References
into Informative and Normative).  It is, after all, WGLC.

On Security, I would invite the chairs to ask for an early review by the
Security Directorate, explaining the context of this document, asking
them how much more is needed and how it should be expressed.  It might
not be very much although my experience is that I never know in advance
what they are going to come up with.

Tom Petch

  I would like to implement historical TACACS+.  I have *NO IDEA* what
to do for huge swaths of the protocol.

  I would like to deploy historical TACACS+. I have *NO IDEA* what the
security implications are of using it.

  Which is (so far as I'm aware) still IETF practice for informational
specifications.

  If the goal for the document is something else, fine.  Update the
document to say that.   Something like:

  "This document attempts to specify the historical TACACS+ protocol.
However, there are many portions of the protocol which are
under-specified or unspecified.  We cannot second-guess twenty years of
practice here.  As a result, this specification is incomplete,
under-specified, insecure, and should not be used by anyone to implement
anything.  Please wait for the Standards track document to get the
actual TACACS+ specification that people can implement".

  If the document can be updated with such text, I'll withdraw all of my
review comments.  But I predict that the document won't pass security
area review.  And they'll make all of the same comments as I've made
here, with a recommendation that the document not be published until
it's fixed.

  Alan DeKok.

---

From: Alan DeKok <[email-masked]>
Date: Thu, 6 Oct 2016 12:35:30 -0400

> used the word 'philosophical' and I agreed.

  I see the existing draft as not documenting the protocol.  That's my main concern.

> into Informative and Normative).  It is, after all, WGLC.

  This doesn't address my concerns.

  Again, as an implementor, I have *no idea* what to do for huge swathes of the protocol.

  On top of that, the current draft is silent on serious security topics.

  e.g. Length of CHAP challenges is implied from the context.  OK, that's fine, but what are the *limits* on CHAP challenge lengths?

 A: none.  CHAP challenges can be omitted entirely!

> what they are going to come up with.

  I've been a member of SAAG for a while now, and have done reviews, and read many more.  My prediction is that the current document will give SAAG a heart attack, and they will refuse publication.

  There is a huge push to get the draft published.  That's fine.  What's *not* fine is that it seems to me at least... that many people simply don't care what's in the document.  They don't care if the protocol is badly specified.  Or if it's insecure.  In fact, these "features" could be beneficial.  Because it means that everyone can claim compliance.  And no one has to re-examine their implementation.

  That may sound harsh, but I just don't see any other explanation.  If we are going to document the protocol, then let's document the protocol.  If we're going to agree that we don't need to document the protocol then put a huge disclaimer at the top of the draft saying so.

  Giving lip service to "document the protocol", while opposing attempts to do so is unproductive.

  Alan DeKok.

---

From: "Douglas Gash (dcmgash)" <[email-masked]>
Date: Thu, 6 Oct 2016 17:34:47 +0000

Hi Alan,

Regarding the security concern, the approach we have with the current
document is:

1) A statement that the protocol does not provide security in a modern
environment. 
2) The two common approaches to support T+ so that it may be used.
3) An indication that a version with built-in security is to follow, which
will not require the mitigations in 2) (actually I suspect it may precede
the info document ;-))

We need to make sure there is no ambiguity for 1) for any security review.
No soft-peddalling. T+ provides the function, not the security.

SoŠ An approach we could employ is to enumerate all the ways in which 1)
is true, including an analysis of each of the other components of the rest
of the protocol, and their contribution to 1).

My question would be: would this dissection of vulnerabilities and
exploration of the attack vectors would help anyone from a security
perspective, when we have established that the approaches 2) are required?

I suspect that we¹d probably miss some, and that may give implementors
more comfort than is afforded by the clear and simple statement.

Therefore, for security review, I¹d suggest we keep the statement simple,
as defined above. However, if this statement is not clear enough/correct
in the document, then certainly, it should be clarified.

Best Regards,

Doug.

On 06/10/2016 17:35, "Alan DeKok" <[email-masked]> wrote:

>  e.g. Length of CHAP challenges is implied from the context.  OK, that's
>fine, but what are the *limits* on CHAP challenge lengths?
>
>  I've been a member of SAAG for a while now, and have done reviews, and
>read many more.  My prediction is that the current document will give
>SAAG a heart attack, and they will refuse publication.
>
>  There is a huge push to get the draft published.  That's fine.  What's
>*not* fine is that it seems to me at least... that many people simply
>don't care what's in the document.  They don't care if the protocol is
>badly specified.  Or if it's insecure.  In fact, these "features" could
>be beneficial.  Because it means that everyone can claim compliance.  And
>no one has to re-examine their implementation.
>
>  That may sound harsh, but I just don't see any other explanation.  If
>we are going to document the protocol, then let's document the protocol.
>If we're going to agree that we don't need to document the protocol then
>put a huge disclaimer at the top of the draft saying so.
>

---

From: "Blumenthal, Uri - 0553 - MITLL" <[email-masked]>
Date: Thu, 6 Oct 2016 18:00:07 +0000

A protocol description needs to include: (a) what threats/attacks become possible as a result of its introduction, (b) how the protocol defends against [at least some of] them, (c) how the remaining attacks can be mitigated, (d) what are the remaining risks.

Yes, dissection of vulnerabilities and exploration of the attack vectors would help, especially if you provide ways to defend/mitigate. If you just say “when you deploy X, anybody would be able to do Y and totally screw you up”, somehow I doubt that such a protocol would be warmly welcomed. If you don’t say anything – it simply shows that you did not think of the security angle, and therefore the protocol is not ready even for the draft publication.

Why do you think anyone at SAAG would agree/permit to “bless”/publish a protocol that “does not provide security in a modern Environment”? I’d think that everybody realized by now that in today’s environment a protocol that “provides the function, not the security” has no right to live, and ideally should be stamped out – but in any case not rewarded by receiving a “standard” status.

So hold the publication until your (3) comes along. In the meanwhile, enjoy the vulnerabilities of your deployed implementations.

-- 

Regards,

Uri Blumenthal

On 10/6/16, 13:34 , "OPSAWG on behalf of Douglas Gash (dcmgash)" <[email-masked] on behalf of [email-masked]> wrote:

Hi Alan,

document is:

environment. 

the info document ;-))

 

Best Regards,

Doug.

  I see the existing draft as not documenting the protocol.  That's my

main concern.

  Again, as an implementor, I have *no idea* what to do for huge swathes

of the protocol.

  Giving lip service to "document the protocol", while opposing attempts

to do so is unproductive.

  Alan DeKok.

_______________________________________________

OPSAWG mailing list

[email-masked]

https://www.ietf.org/mailman/listinfo/opsawg

---

From: "Douglas Gash (dcmgash)" <[email-masked]>
Date: Thu, 6 Oct 2016 18:11:55 +0000

Agreed, to provide the new version of the protocol with the security was the original plan. In fact, that was how this process started.

Reverting to that plan, or a modified for of that plan, would always be an option, if preferred by the group.

From: "Blumenthal, Uri - 0553 - MITLL" <[email-masked]<mailto:[email-masked]>>
Date: Thursday, 6 October 2016 19:00
To: Douglas Gash <[email-masked]<mailto:[email-masked]>>, Alan DeKok <[email-masked]<mailto:[email-masked]>>, "t.petch" <[email-masked]<mailto:[email-masked]>>
Cc: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
Subject: Re: [OPSAWG] Start of WGLC for TACACS+ document.

On 10/6/16, 13:34 , "OPSAWG on behalf of Douglas Gash (dcmgash)" <[email-masked]<mailto:[email-masked]> on behalf of [email-masked]<mailto:[email-masked]>> wrote:

Hi Alan,

Best Regards,

Doug.

On 06/10/2016 17:35, "Alan DeKok" <[email-masked]<mailto:[email-masked]>> wrote:

On Oct 6, 2016, at 12:19 PM, t.petch <[email-masked]<mailto:[email-masked]>> wrote:

  Alan DeKok.

_______________________________________________
OPSAWG mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/opsawg

---

From: Alan DeKok <[email-masked]>
Date: Thu, 6 Oct 2016 14:51:21 -0400

> 1) A statement that the protocol does not provide security in a modern

  I'm happy with that.

> 2) The two common approaches to support T+ so that it may be used.

  OK...

> will not require the mitigations in 2) (actually I suspect it may precede

  Except that the current document doesn't even document CHAP securely.

  RFC 1994 (published in 1996) says:
 
   Each challenge value SHOULD be unique, since repetition of a
   challenge value in conjunction with the same secret would permit an
   attacker to reply with a previously intercepted response.  Since it
   is expected that the same secret MAY be used to authenticate with
   servers in disparate geographic regions, the challenge SHOULD exhibit
   global and temporal uniqueness.

  Tho I'll also note that RFC 1994 also doesn't specify a minimum length for the challenge.

  That is, the document doesn't even meet the security requirements in place in 1994.

> No soft-peddalling. T+ provides the function, not the security.

  I'm fine with that.

  I want to document historical TACACS+.  This means the document needs to answer the following questions:

a) can people implement it?

b) can people gain the maximum amount of security available in the historical protocol?

c) can people be aware of the security concerns and trade-offs with using the historical protocol?

  The push-back so far shows that the answers are:

a) no

b) no

c) no

  I suggest these are the wrong answers.  *All* of them are the wrong answers.

> perspective, when we have established that the approaches 2) are required?

  Yes.  See any of the RADIUS RFCs in the last 5 years.  The SAAG reviews require discussion not only of the issues with the new standards, but also of the problems with the historical protocol.

> more comfort than is afforded by the clear and simple statement.

  The argument that we can't be perfect isn't a reason to give up entirely.

  If the statement is anything other than "no one should use this protocol for anything", we might as well *try* to document security issues.

> in the document, then certainly, it should be clarified.

  Then the introduction and/or abstract should say:

  It shouldn't be a contentious issue.  Either document the protocol, or admit we're not going to document the protocol.

  Alan DeKok.

---

From: Dan Romascanu <[email-masked]>
Date: Fri, 7 Oct 2016 08:50:57 +0300

FWIW - I agree with much of Alan's observation and with his approach here.
Documenting an existing protocol does not mean changing it. Asking for
changes in the content of the document (including more details about
implementation of parts of the protocols, and security implications of
deploying this technology in today's Internet) does not mean changing the
protocol. These are legitimate comments and IMO necessary changes to ask at
WGLC phase. Keeping 'changes to the minimum, omissions or contradictions
and those required  by the publication process' is not a WGLC goal.

Regards,

Dan

---

From: Randy Bush <[email-masked]>
Date: Sat, 08 Oct 2016 09:52:56 +0900

> But do we want to change it all at this time?

yes.  a proper sec cons does not change the protocol but advises as to
its use.

randy

---

From: Alan DeKok <[email-masked]>
Date: Tue, 11 Oct 2016 10:02:52 -0400

Continuing with 4.4.3

     While the order of hosts in this packet indicates a preference, but
     the client is not obliged to use that ordering.

* the use of "obliged" is unusual here.  Perhaps "the order used by the client is implementation specific, and does not have to follow the order sent by the server"

   If the status equals TAC_PLUS_AUTHEN_STATUS_RESTART, then the
   authentication sequence is restarted with a new START packet from the
   client. ...

* nit: it may be good to say that the next session does not have to use the current TCP connection

* "restart" may also be ambiguous.  Does the session ID change for the next session?  It may be good to say instead "If the status equals TAC_PLUS_AUTHEN_STATUS_RESTART, then the client re-authenticates with a new session, beginning with a new START packet"

 ...   This REPLY packet indicates that the current authen_type
   value (as specified in the START packet) is not acceptable for this
   session, but that others may be.

* Which others "may" be acceptable?  And why "acceptable for this session"?   is the client free to re-try the same authentication type in a different session?  i.e. what does the client *do*?

* perhaps say "the REPLY indicates a negative acknowledgement that the current authen_type is not accepted by the server.  The client SHOULD start a new session with a different authen_type"

   If a client chooses not to accept the TAC_PLUS_AUTHEN_STATUS_RESTART
   packet, then it is TREATED as if the status was
   TAC_PLUS_AUTHEN_STATUS_FAIL.

* why uppercase "TREATED" ?

* why "chooses not to accept" ?  again, prescriptive is better than philosophical

* Perhaps "If the client does not start a new session after receiving a TAC_PLUS_AUTHEN_STATUS_RESTART, it MUST behave as if it received a TAC_PLUS_AUTHEN_STATUS_FAIL."

5.  Authorization
...

   The authorization REQUEST message contains a fixed set of fields that
   indicate how the user was authenticated or processed

* nit: remove "or processed"

* from a security point of view, it's unusual for a client to indicate to a server how a user was authenticated.  This allows the client to claim authentication when none existed.

...
     This field matches the port field in the authentication section
    (Section 4) above

* what is meant by "matches"?  Perhaps "The definition of this field is the same as in Section 4".  Or does the field in the authorization session have to contain the same data as the similar field from the authentication session?

* similar comments apply for other uses of "matches"

   It is not legal for an attribute name to contain either of the
   separators.  It is legal for attribute values to contain the
   separators.

* what are the legal characters for an attribute name?  The previous paragraph says US-ASCII.  So is "($#@!" a legal attribute name?

* a reference to Section 7 would be useful here.

   Optional arguments are ones that may be disregarded by either client
   or server.  Mandatory arguments require that the receiving side
   understands the attribute and will act on it.  If the client receives
   a mandatory argument that it cannot oblige or does not understand, it
   MUST consider the authorization to have failed.

* "oblige" is an unusual word to use here.   Perhaps "If the client receives a mandatory argument it cannot implement, it MUST consider the authorization to have failed"

5.2 The Authorization RESPONSE Packet Body
...
   server_msg, server_msg_len

   This is an US-ASCII string that may be presented to the user.  The
   decision to present this message is client specific.

* nit: perhaps "This field is a message from the server to the client.  The client MAY display it to the user"

* saying "decision is client specific" is a long way of saying "the client MAY do it", but doesn't always have to

...

   The attribute-value pair that describes the authorization to be
   performed.  (see below), 

* That's not clear to me.  Perhaps "if present, the list of additional authorizations permitted the user"

    If the status equals TAC_PLUS_AUTHOR_STATUS_FAIL, then the
   appropriate action is to deny the user action.

* again, a bit more philosophical than prescriptive.  Perhaps "If the status equals TAC_PLUS_AUTHOR_STATUS_FAIL, then the requested action MUST be denied"

   If the status equals TAC_PLUS_AUTHOR_STATUS_PASS_ADD, then the
   arguments specified in the request are authorized and the arguments
   in the response are to be used IN ADDITION to those arguments.

* uppercase "IN ADDITION" is unusual.  Perhaps:

   If the status equals TAC_PLUS_AUTHOR_STATUS_PASS_ADD, then the
   arguments specified in the request are authorized and the client MUST
   also enforce the authorization attribute-value pairs (if any) in the response.

* similar comments apply here:

   If the status equals TAC_PLUS_AUTHOR_STATUS_PASS_REPL then the
   arguments in the request are to be completely replaced by the
   arguments in the response.

* perhaps

   If the status equals TAC_PLUS_AUTHOR_STATUS_PASS_REPL then the
   client MUST use the authorization attribute-value pairs (if any) in the response,
   instead of any authorization attribute-value pairs from the request.

...

    If the intended action is to approve the authorization with no
   modifications, then the status is set to
   TAC_PLUS_AUTHOR_STATUS_PASS_ADD and the arg_cnt is set to 0.

* should be prescriptive.  Perhaps:

   Servers can signal to client that the authorization is approved with no
    modifications by setting the status to
   TAC_PLUS_AUTHOR_STATUS_PASS_ADD and the arg_cnt is set to 0.

...

   A status of TAC_PLUS_AUTHOR_STATUS_ERROR indicates an error occurred
   on the server.

* what does the client do then?  Perhaps "the client MAY show an error to the user (how???), and MUST treat the response as TAC_PLUS_AUTHOR_STATUS_FAIL"

* Also, the last sentence of the subsequent paragraph also talks about ERROR, and seems misplaced.

6.1.  The Account REQUEST Packet Body

   TACACS+ accounting is very similar to authorization.  The packet
   format is also similar. 

* nit: that seems redundant.  All of the packets use a common format.

  ...   There is a fixed portion and an extensible
   portion.  The extensible portion uses all the same attribute-value
   pairs that authorization uses, and adds several more.

* except that the authorization section didn't list any attribute-value pairs.  So it's hard to say that those (non-existent) pairs are re-used here

   All other fields are defined in the authorization and authentication
   sections above and have the same semantics.

* Can an accounting packet indicate an authen_type and authen_service?  If so, what does that mean?

6.2.  The Accounting REPLY Packet Body

   The response to an accounting message is used to indicate that the
   accounting function on the server has completed.

* what does that mean?  Do servers always implement "functions"?  As background, RFC 2866 (RADIUS accounting) says:

...
     Upon receipt of an Accounting-Request, the server MUST transmit an
      Accounting-Response reply if it successfully records the
      accounting packet, and MUST NOT transmit any reply if it fails to
      record the accounting packet.
...

* perhaps similar text could be used here.

   ... The server will
   reply with success only when the record has been committed to the
   required level of security,

* what is a "required level of security"?  Is that indicated in the packet?  Elsewhere?

* my recommendation is just to delete that...

   ... relieving the burden on the client from
   ensuring any better form of accounting is required.

* I find that statement awkward.  Perhaps re-using / re-phrasing text from RFC 2866 would be good here.

6.2.  The Accounting REPLY Packet Body

* there's more duplication definition of fields here.  It may be useful to just say "server_msg has the same meaning and behaviour as defined in Section 4.2"

  ...    The server MUST terminate the session after sending a REPLY.

* this is the only reference to a session being terminated in the document.  What does it mean?  Perhaps that the next REQUEST packet has to have a different session ID?  If so, it would be good to say that in the REQUEST section.

   The TAC_PLUS_ACCT_FLAG_START flag indicates that this is a start
   accounting message.  Start messages will only be sent once when a
   task is started.  The TAC_PLUS_ACCT_FLAG_STOP indicates that this is
   a stop record and that the task has terminated.  The
   TAC_PLUS_ACCT_FLAG_WATCHDOG flag means that this is an update record.
   Update records are sent at the client's discretion when the task is
   still running.

* what's a "task" ?  This is the first reference in the document to a "task".  Perhaps more explanation here as to the expected use-case

   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.

  ... If the START
   flag is not set, then this indicates a minimal record indicating only
   that task is still running.

* nit: perhaps delete "a minimal record indicating only".

* Some more questions...

* does the client retry an accounting packet if it doesn't get a response from the server?

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

* I have similar questions about the TCP connection.  What happens if a client can't connect to a server?  How often should it retry?  Should it treat "unable to connect" as "authentication / authorization fail"?  Should the user be forbidden *all* access if the client cannot connect to the server?   Should the user be given minimal access in order to be able to diagnose server connection problems?

7.  Attribute-Value Pairs

   It is recommended that hosts be specified as a numeric address so as
   to avoid any ambiguities.

* Is this IPv6 safe?  I suspect not... but it should be stated

   Absolute times are specified in seconds since the epoch, 12:00am Jan
   1 1970.  The timezone MUST be UTC unless a timezone attribute is
   specified.

* nit: perhaps "a decimal number representing seconds since..." except what about time zones?

* how is a time zone specified?  what format is the time?  Examples would help here.

* nit: the section defines numbers, booleans, hosts, and time.  But no "text" type.  RADIUS is just in the process of fixing this (20+ years later...), which is why I noticed

* adding a data type to each attribute definition would be useful

* also, it would be good to say which attributes are mandatory, and which are optional

   Attributes may be submitted with no value, in which case they consist
   of the name and the mandatory or optional separator.

* this sounds like the separator is optional.  Perhaps "... consist of the name and separator only"

7.1.  Authorization Attributes

   ...
   a protocol that is a subset of a service.  An example would be any
   PPP NCP. 

* nit: expand acronyms on first use.  What's a "PPP NCP" ?

   cmd

   a shell (exec) command.  This indicates the command name for a shell
   command that is to be run.  This attribute MUST be specified if
   service equals "shell".  If no value is present then the shell itself
   is being referred to.

* perhaps a bit more explanation of the use-case here. What is a "shell" in reference to an administrative management protocol?

   routing

   A boolean. ...

* note that "routing" is defined to be a boolean, but the other attributes don't have data types defined.  I *think* "cmd" is a textual attribute, but it's not defined that way

   timeout

   an absolute timer for the connection (in minutes).  A value of zero
   indicates no timeout.

* probably "numeric" data type?

   autocmd

   an auto-command to run.  Used only when service=shell and cmd=NULL

* it's confusing to have an explicit NULL in "cmd".  Perhaps quotes would clarify it here.  e.g.

  n auto-command to run.  Used only when the packet contains attribute-values of "service=shell" and "cmd="

   noescape

   Boolean.  ...

* nit: consistent terminology.  "Boolean" ? or "A boolean ..." ??

   callback-dialstring

   Indicates that callback is to be done.  Value is NULL, or a
   dialstring.  A NULL value indicates that the service MAY choose to
   get the dialstring through other means.

* nit: explicit NULL is confusing.  Perhaps "value is a dial string, or when omitted, indicates that the server may choose to get the value via other means"

* nit: what's a "dialstring" ?

* what's a callback?  How does that work?

7.2.  Accounting Attributes

...

   The following new attributes are defined for TACACS+ accounting only.
   When these attribute-value pairs are included in the argument list,
   they will precede any attribute-value pairs that are defined in the
   authorization section (Section 5) above.

* "will" precede?  Perhaps "MUST precede"

   task_id

   Start and stop records for the same event MUST have matching task_id
   attribute values.  The client must not reuse a specific task_id in a
   start record until it has sent a stop record for that task_id.

* how big is the task_id?  Since values can be omitted, is it OK to have a zero-length task_id?  or 1 octet?

* are there recommendations for the content of task_id? e.g. incrementing numbers, strings, etc.

* is task_id mandatory to occur in accounting packets?

   start_time

   The time the action started ().

* nit: what are the brackets for?

   stop_time

   The time the action stopped (in seconds since the epoch.)

* which attributes are allowed in which packets?  I presume it's not recommended to have a START which contains "stop_time"

   elapsed_time

   The elapsed time in seconds for the action.  Useful when the device
   does not keep real time.

* I'd recommend removing that last sentence.  it also indicates that the device may not have accurate time, notwithstanding the definition of "absolute times" at the start of Section 7.

   bytes_in

   The number of input bytes transferred by this action

* input to where?  This was poorly specified in RADIUS, and some implementations got it wrong...

* this number might be large.  Should implementations allow 16 bit numbers? 32-bit?  64-bit?  What are the expected / recommended limits?

* i.e. implementations MUST be able to parse numbers up to 2^32, and SHOULD be able to parse numbers up to 2^64

   status

   The numeric status value associated with the action.  This is a
   signed four (4) byte word in network byte order. 0 is defined as
   success.  Negative numbers indicate errors.  Positive numbers
   indicate non-error failures.  The exact status values may be defined
   by the client.

* This is the first (and only) time that a field is defined as non-ASCII.  That's very off.

* nit: values "may" be defined by the client?  Perhaps just saying "values are implementation specific" would be better

---

From: Alan DeKok <[email-masked]>
Date: Wed, 12 Oct 2016 11:07:39 -0400

Continuing with Section 8

   ...  Privilege Levels are ordered values from 0 to 15
   with each level representing a privilege level that is a superset of
   the next lower value. 

* nit: perhaps this could be "each level is defined to be a superset of the next lower value", instead of "representing".

   If a client uses a different privilege level scheme, then it must map
   the privilege level to scheme above.

* editorial :"... to A scheme..."

* and how is mapping done?  It might be better to just omit that sentence.

   Privilege Levels are applied in two ways in the TACACS+ protocol:

* nit: perhaps "used" instead of "applied"

      - As an argument in authorization EXEC phase (when service=shell
      and cmd=NULL), where it is primarily used to set the initial
      privilege level for the EXEC session.

* how does that work?  And what does it mean to set an initial privilege level?

      - In the packet headers for Authentication, Authorization and
      Accounting.  The privilege level in the header is primarily
      significant in the Authentication phase for enable authentication
      where a different privilege level is required.

* again, how does that work?  Additional text would be useful.  e.g.

In typical use-cases, an administrator can perform a series of commands at a low privilege level.  When additional privileges are required, the administrator can authenticate at the desired level, perform a series of commands, and the drop privilege level to the lower one.  This methodology minimizes possible errors by using high privilege levels only when necessary, and using low privilege levels for most commands.

* expanding on the use-case and work flow would be of great benefit.

   The use of Privilege levels to determine session-based access to
   commands and resources is not mandatory for clients, but it is in
   common use so SHOULD be supported by servers.

* and clients, presumably?  Perhaps instead, say:

It is RECOMMENDED that clients and servers use Privilege levels to signal
and control session-based access.  It is RECOMMENDED that clients permit
users to change their Privilege levels, so as to ensure that commands are
executed with the minimum Privilege level required.

9.  TACACS+ Security Considerations

* there is a lot which can be said here.  My proposal here is a substantial amount of text.  I'll include it in another message

---

From: Alan DeKok <[email-masked]>
Date: Wed, 12 Oct 2016 17:35:50 -0400

My $0.02 on the contents of the Security Considerations section.   I'm sure I've missed things.

  Please also comment if the suggestions here are wrong or unworkable.

----
9. Security Considerations

The protocol described in this document has been in widespread use
since specified in "The Draft" (1998).  However it does not meet
modern security standards, and faces vulnerabilities with privacy and
authenticity.

This section describes issues with the TACACS+ specification, then the
protocol, deployments, followed by recommendations for implementations.

Because of the security issues with TACACS+, the authors intend to
follow up this document with an enhanced specification of the
protocol employing modern security mechanisms.

9.1 Security of The Draft Specification

TACACS+ was originally specified in "The Draft" (1998).  However, this
draft is incomplete, and leaves key points unspecified.  As a result,
software authors have had to make implementation choices about what
should, or should not, be done in certain situations.  These
implementation choices are somewhat constrained by ad hoc
interoperability tests.  That is, all TACACS+ clients and servers
interoperate, so there is a rough consensus on how the protocol works.

While the authors have made every effort to maintain compatibility,
there is no guarantee, however, that this specification matches the
behavior of current or historical implementations.  We believe that
any incompatibilites are due to modern security requirements.  We
therefore suggest that implementors should read this document, and
verify that their implementations follow the security practices
recommended here.

9.2 Security of The Protocol

The major security issue with the TACACS+ protocol is the ad hoc
"encryption" defined in Section 3.6.  This "encryption" is more
properly called "obfuscation", and has had no security analysis.  That
is, the security or insecurity of the protocol is entirely unknown.

Attacks on cryptographic hashes are well known and are getting better
with time, as discussed in [RFC4270].  The security of the TACACS+
protocol is dependent on MD5 [RFC1321], which has security issues as
discussed in [RFC6151].  It is not known if the issues described in
[RFC6151] apply to TACACS+.  <<AD: lifted from RFC 6929 Section 11 >>

The choice of encrypting the body but not the packet header is
unusual.  The lack of security on the packet header means that an
attacker can modify the header without detection.

For example, a "session_id" can be replaced by an alternate one, which
could allow an unprivileged administrator to "steal" the authorization
from a session for a privileged administrator.  An attacker could also
update the "flags" field to indicate that one or the other end of a
connection requires TAC_PLUS_UNENCRYPTED_FLAG, which could remove what
little security is available.

What little security that exists is dependent on implementations
limiting access to known clients, and on the strenght of the secret
key.  Attacks who can guess the key or break the obfuscastion method
can gain unrestricted and undetected access to all TACACS+ traffic.
This access permits the attacker to execute any command on any system,
leaving the attacker in complete control of the network.  The negative
side effects of such a successful attack cannot be overstated.

9.2.1 Security of Authentication Sessions

There are a number of security issues with authentication sessions.
As the sessions enable the exchange of clear-text passwords, an
attacker capable of observing the unencrypted traffic or breakign the
encrypted traffic, can gain complete network access.  Similarly,
MSCHAPv1 and MS-CHAPv2 offer little additional security, while ARAP is
insecure and MUST NOT be used.  As of the publication of this
document, there has been no similar attacks on the CHAP protocol.

Section 4.4.3 permits the redirection of a session to another server
via the TAC_PLUS_AUTHEN_STATUS_FOLLOW mechanism.  As part of this
process, the secret key for a new server can be sent to the client.
This public exchange of secret keys means that once one session is
broken, it may be possible to leverage that attack to attacking
connections to other servers.  Section 9.4, below, has recommendations
for securing this portion of the protocol.

9.2.2 Security of Authorization Sessions

The primary issue with authorization sessions is that unauthenticated
authorization is allowed.  Almost as bad, authorization is permitted
where authentication was done by a third party, such as with
TAC_PLUS_AUTHEN_METH_KRB5.  This third party authentication is based
entirely on the client claiming that the user has been authenticated,
and the server trusting that request.  This practice is entirely
insecure.

There is also no way to cryptographically tie an authorization session
to a particular authentication session.  That is, when the client
indicates "authen_method" in the packet header, the authentication and
authorization sessions are tied together implicitly by the contents of
the other fields, such as "use", "port", "rem_addr", etc.

<<AD: it would be good for Section 5.1 to contain recommendations on
how the server can use these fields to tie together authentication /
authorization sessions.  Perhaps recommend that all AAA include a
"task_id"? >>

The specification allows for the exchange of attribute-value pairs.
While a few such attributes are defined here, the protocol is
extensible, and vendors can define their own attributes.  There is no
registry for such attributes, and in the absence of a published
specification, no way for a client or server to know the meaning of a
new attribute.

As a result, vendors SHOULD NOT define new attribute-value pairs,
unless such pairs are used only to communicate between client and
server implementations both written by the same vendor.

The "cmd" and "cmd-arg" attributes define shell commands and
arguments, but does not specify what these are.  The specific
commands, therefore, are undefined.

9.2.3 Security of Accounting Sessions

The security considerations for accounting sessions are largely the
same as for authorization sessions.  This section describes additional
issues specific to accounting sessions.

There is no way in TACACS+ to signal that accounting is required.
There is no way for a server to signal a client how often accounting
is required.  The accounting packets are received solely at the
clients discretion.  Adding such functionality would assist with
auditing of user actions.

The "task_id" field is defined only for accounting packets, and not
for authentication or authorization packets.  As such, it is difficult
to correlate accounting data with a previous authentication or
authorization request.

<<AD: re-use the recommendations suggested for Section 5.1, above? >>

9.4 Security of Deployments

Due to the above concerns with the protocol, it is critical that it be
deployed in a secure manner.

Systems using TACACS+ MUST be configured so that TACACS+ traffic is
secured.  Any user or service who is not interacting with the TACACS+
protocol MUST NOT be able to observe TACACS+ traffic, even when
TACACS+ "encryption" is used.

When used inside of enterprises, TACACS+ traffic MUST be sent over
management networks.  When used on the wider internet, TACACS+ traffic
MUST be secured via a modern security protocol such as TLS or IPSec.
As this specification describes the historic protocol, recommendations
on TLS and IPSec parameters are outside of the scope of this document.

9.4 Recommendations for Implementations

The following recommendations are for clients and servers which
implement the TACACS+ protocol.  These recommendations describe end
user interaction or user interface design.  While not part of the
protocol itself, good designs can prevent insecure configurations.

Clients and servers MUST default to rejecting all connections which
have the TAC_PLUS_UNENCRYPTED_FLAG set.  Clients and servers MAY
permit such connections when a debugging or test flag is set.  Clients
and servers MUST NOT run with this flag set in normal environments.

Similarly, the default for both clients and servers MUST be to require
a secret key when configuring a new server or client.  The secret key
MUST NOT be empty, and SHOULD be at least eight (8) characters in
length.  Clients and servers SHOULD permit the user of keys unique to
each server or client which they are connecting to.  They SHOULD warn
the administrator if a secret key is re-used.  Documentation for
implementations SHOULD recommend the use of strong secret keys.

Servers should maintain list of known clients, and reject connections
which originate from other systems.  Servers SHOULD NOT permit the
configuration of wildcard clients.  e.g. clients which map to a
network range.

Where a client or server detects that the secret key is incorrect, it
SHOULD terminate the underlying TCP connection, and log a message to
the system administrator.  There is no good reason to keep a
connection open when the systems are unable to communicate in a
meaningful fashion.

Implementions MUST default to using TAC_PLUS_AUTHEN_TYPE_CHAP for
authentication.  We recognize, however, that this authentication
method is incompatible with many password storage systems.  As a
result, other authentication methods are still in the protocol, and
can be used so long as administrators recognize the security issues
with their use.

Clients MUST default to treating TAC_PLUS_AUTHEN_STATUS_FOLLOW as if
TAC_PLUS_AUTHEN_STATUS_FAIL had been received instead.  Clients
SHOULD allow administrators to explicitly enable the
TAC_PLUS_AUTHEN_STATUS_FOLLOW functionality.  When the functionality
is enabled, clients SHOULD permit administrators to specify ranges of
addresses, and/or sets of hosts, along with corresponding secret keys,
for which redirection will be permitted.  Clients SHOULD ignore
redirects to hosts which are outside of the pre-configured range or
list.  Clients SHOULD ignore any key provided via
TAC_PLUS_AUTHEN_STATUS_FOLLOW, and SHOULD instead use a preconfigured
key for that host.

For authorization, servers SHOULD default to requiring
TAC_PLUS_AUTHEN_METH_TACACSPLUS.  Servers SHOULD track authentication
sessions via methods "to be described" in Section 5.1.

Where a server receives an unknown attribute-value pair, it SHOULD
reply with TAC_PLUS_AUTHOR_STATUS_FAIL.  Where a server receives an
unrecognized values in authorization attributes such as "cmd",
"cmd-arg", "acl", etc., the server SHOULD reply with
TAC_PLUS_AUTHOR_STATUS_FAIL.  When a client receives an unknown
authorization attribute, it SHOULD behave as if it had received
TAC_PLUS_AUTHOR_STATUS_FAIL.

---

From: "Douglas Gash (dcmgash)" <[email-masked]>
Date: Tue, 18 Oct 2016 07:11:58 +0000

Many thanks Alan for the thorough review.

We¹ll collate all your comments and respond shortly.