Subject: [OPSAWG] implementing from reading draft-ietf-opsawg-tacacs-05

From: Alexander Clouter <[email-masked]>
Date: Wed, 2 Nov 2016 23:14:46 +0000

Hello,

I am working on implementing the TACACS+ protocol.  I do of course have a 
reference (pam_tacplus) to crib from and a pile of lab equipment, but thought 
as the draft your have authored seem aims to be a living standard you might be 
interested in hearing some feedback from the trenches of someone right now 
trying to implement from it.

Below are my thoughts on various sections, though no doubt might follow up 
later as my work progresses and other questions arise.

Thanks!

> 3.4.  The TACACS+ Packet Header
>
>   ...
>
>   seq_no
>
>   This is the sequence number of the current packet for the current
>   session.  The first packet in a session MUST have the sequence number
>   1 and each subsequent packet will increment the sequence number by
>   one.  Thus clients only send packets containing odd sequence numbers,
>   and TACACS+ servers only send packets containing even sequence
>   numbers.

If a client or server observes this condition not being met, is it okay to tear 
down the connection immediately?  As a receiver of these packets, should I 
enforce this draft, or be flexible?

>   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
>   [RFC1750] )

What is the attack vector here?  Why does this need to be cryptographically 
strong and how does it compromise the protocol's security?  As it is present in 
the clear in the header and the TCP session its-self is wrapped in a 
(supposedly) suitable number, I can not see what I am protecting against here.

Is there any problems treating this as an opaque bitfield as it seems to only 
be used in the MD5 XOR or is there some undocumented authentication mechanism 
that makes use of this I will find in the field?

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

If I see padding, do I kill the connection?  Is this a SHOULD NOT?

> 3.6.  Encryption
>
>   In this case, the packet body is encrypted by XOR-ing it byte-wise
>   with a pseudo random pad.
>
>   ENCRYPTED {data} == data ^ pseudo_pad

I know it, you know it, everyone on OPSAWG probably knows it, but the poor 
fresh out of university intern is going to really appreciate if you add:

:   The packet can then be decrypted by repeating the same process but
:   on the encrypted data.
:
:   data == ENCRYPTED {data} ^ pseudo_pad

:)

>   The version number is the one byte concatenation of the major and
>   minor version numbers.
>
>   The session id is used in network byte order.

Easier to simply state 'as is in the header'?

>   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.
>
>   Commonly such failures are seen when the keys are mismatched between
>   the client and the TACACS+ server.

If the keys are mis-matched, how could any of the other sessions be valid?  
Also keeping the connection open surely is just going to produce a stream of 
errors?  Is it not safer to simply tear down the connection here; after sending 
the terminating message?

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

Wow, this is well hidden!  Would this not be better around section 3.4?  I 
cannot see how the packet would become valid after the XORing process, maybe I 
am missing something here?

> 4.4.1.  Version Behaviour
>
>   The TACACS+ protocol is versioned to allow revisions while
>   maintaining backwards compatibility.  The version number is in every
>   packet header.  The changes between minor_version 0 and 1 apply only
>   to the authentication process, and all deal with the way that CHAP
>   and PAP authentications are handled. minor_version 1 may only be used
>   for authentication kinds that explicitly call for it in the table
>   below:
>
>		LOGIN    CHPASS   SENDAUTH
>   ASCII          v0         v0       -
>   PAP            v1         -        v1
>   CHAP           v1         -        v1
>   MS-CHAPv1/2    v1         -        v1
>
>
>   The '-' symbol represents that the option is not valid.
>
>   When a server receives a packet with a minor_version that it does not
>   support, it will return an ERROR status with the minor_version set to
>   the closest supported value.

Is it okay to tear down the connection if a non-valid but technically could be 
supported combination is requested?

>   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.  The removal of SENDPASS
>   was prompted by security concerns, and is no longer considered part
>   of the TACACS+ protocol.

Where can I read about SENDPASS?  Will I see it out in the field?  Though it is 
not considered part of the protocol (vendor extension?) should it be supported 
still?

> 4.4.2.  Common Authentication Flows
>
>   Inbound CHAP login
>
>   ...
>
>   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).

"note that unlike in RFC 1334, the length is not included in the value and is 
derived instead from...".  If you are pointing people at RFC's then for me it 
helps me to have it explicitly stated where those deviations are.  Maybe this 
is just me though.

>   Inbound MS-CHAP v2 login
>
>   ...
>
>   To perform the authentication, the server will use a the algorithm
>   specified RFC2759 [RFC2759] on the user's secret and challenge and
>   then compare the resulting value with the response.  The REPLY from
>   the server MUST be a PASS or FAIL.

nit: '...will use -a- the algorithm specified...'

> 4.4.3.  Aborting an Authentication Session
>
> ...
>
>   When the status equals TAC_PLUS_AUTHEN_STATUS_FOLLOW the packet
>   indicates that the TACACS+ server requests that authentication is
>   performed with an alternate server.  The data field MUST contain
>   ASCII text describing one or more servers.  A server description
>   appears like this:
>
>   [@<protocol>@]<host>>[@<key>]
>
>   If more than one host is specified, they MUST be separated into rows
>   by an ASCII Carriage Return (0x0D).
>
>   Use of the hosts in a TAC_PLUS_AUTHEN_STATUS_FOLLOW packet is at the
>   discretion of the TACACS+ client.  It may choose to use any one, all
>   or none of these hosts.  If it chooses to use none, then it MUST
>   treat the authentication as if the return status was
>   TAC_PLUS_AUTHEN_STATUS_FAIL.
>
>   While the order of hosts in this packet indicates a preference, but
>   the client is not obliged to use that ordering.

This does not tell me what to actually do when I receive a list of more than 
one host.  Is this a client configuration which is coupled by the administrator 
to the server configuration?

If multiple hosts are actually for failover purposes, what do I do if I get an 
TAC_PLUS_AUTHEN_STATUS_FAIL from the first one?  Do I take it as gospel and not 
bother trying the other targets, or do I assume that it is broken and I should 
retry against the other systems to seek a TAC_PLUS_AUTHEN_STATUS_PASS?

>   If the status equals TAC_PLUS_AUTHEN_STATUS_ERROR, then the host is
>   indicating that it is experiencing an unrecoverable error and the
>   authentication will proceed as if that host could not be contacted.
>   The data field may contain a message to be printed on an
>   administrative console or log.

...and TAC_PLUS_AUTHEN_STATUS_FAIL could come back as the backend DB is bust so 
"try other hosts too"?

> 5.  Authorization
> 6.  Accounting

I have not gotten this far in my development work so I'll get back to you.

> 9.  TACACS+ Security Considerations
>
>   ...
>
>      To use IPsec where available.

...boo! :)

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

Where can I find out about the TLS approach?  I saw in an earlier draft 
TAC_PLUS_START_TLS but then in the OPSAWG mailing list archives it looks like 
you are going for a dedicated port instead?  Does TLS actually exist yet 
anywhere or do I just ignore it for now?

---

From: "Douglas Gash (dcmgash)" <[email-masked]>
Date: Thu, 3 Nov 2016 13:12:13 +0000

Thanks Alexander.

We have had a good set of feedback also from other members (Thanks in
particular to Alan DeKok) and will be uploading a new version shortly.

We will also mix in the comments from yourself.

Please see some comments inline, feel free to shout if you think I¹ve
misunderstood or just plain to something askew.

>enforce this draft, or be flexible?

The problem you will have if you get an even seq_no from a client is that,
if you increment in your response, you¹ll need to contravene the spec in
sending an odd number response.
So the recommendation is to reject in this case. I would not be flexible
here.

As to dropping the connection, really the only reason you may not do that
is in single connection mode if there are other sessions that are well
behaved on the connection, if they are already in progress. So the next
upload will propose a compromise: if there is some error that would close
the connection in non single connect mode, then we MUST NOT accept any
more connections, but established sessions that are legal may complete,
the connection should be closed then.

>If I see padding, do I kill the connection?  Is this a SHOULD NOT?

Padding may lead to make the Encryption even easier to decrypt, to the
recommendation at this time would be to reject. However, Realistically,
your data may have values that just happen to look like padding, so I
suspect no one enforces this. This is one item that will get obsoleted by
secure TACACS.

>:   data == ENCRYPTED {data} ^ pseudo_pad

Good catch ;-)

>Easier to simply state 'as is in the header'?

Agreed.

>If the keys are mis-matched, how could any of the other sessions be

This point has been made by other commentors. As mentioned above, revision
will be to close connection after any legit sessions already established
complete.

>errors?  Is it not safer to simply tear down the connection here; after

Yup, see above.

>Wow, this is well hidden!  Would this not be better around section 3.4?

Agreed.

>  I 

Sure, with provisos above. In your case I assume you mean, a condition
that would not be helped if same request was sent to any other method.
(I.e. Bad password).

>not considered part of the protocol (vendor extension?) should it be

It should be deprecated. You can read about it in the Draft spec. I¹ve
stopped supporting it in last couple of versions of out T+ server, since
2007.

>helps me to have it explicitly stated where those deviations are.  Maybe

Agreed.

>nit: '...will use -a- the algorithm specified...'

Thanks.

>to the server configuration?

I view this as an optional feature these days. I suspect it was a bit of
an experiment in the first draft.

>retry against the other systems to seek a TAC_PLUS_AUTHEN_STATUS_PASS?

The next upload will have more detail on FAIL vs ERROR.

FAIL means: the process worked but the result was negative, so accept the
FAIL and stop processing
ERROR: means: the process broke for some reason, so try alternatives if
you have them (I.e. As if this server was to contactable).

>...and TAC_PLUS_AUTHEN_STATUS_FAIL could come back as the backend DB is

In real world situations, you¹d probably want to make it tunable as to
whether you returned FAIL or ERROR if backend DB is bust, but in most
situations I think you¹d default to ERROR here.

>>      To use IPsec where available.

Tell me ;-)
>anywhere or do I just ignore it for now?

We have agreed to make an informational note for Opsawg of old protocol
first, Secure TACACS+ will be hot on its heels.

---

From: Alexander Clouter <[email-masked]>
Date: Fri, 4 Nov 2016 17:22:56 +0000

Thanks for the reply, just two or three notes on your reply.

Thanks!

>>(supposedly) suitable number, I can not see what I am protecting against

The padding bit I knew, its why the session_id needs to come from a 
cryptographically strong source that I cannot understand the reasoning behind.

>>If multiple hosts are actually for failover purposes, what do I do if I
>>get an TAC_PLUS_AUTHEN_STATUS_FAIL from the first one?  Do I take it as 
>>gospel and not bother trying the other targets, or do I assume that it is 
>>broken and I should retry against the other systems to seek a 
>>TAC_PLUS_AUTHEN_STATUS_PASS?
>

So multiple hosts here mean "only one needs to succeed to authenticate the use" 
as its purpose is for resilence?

>>...and TAC_PLUS_AUTHEN_STATUS_FAIL could come back as the backend DB is
>>bust so "try other hosts too"?
>

My thinking is more tolerance of buggy third party equipment rather than how I 
should respond.

---

From: "Douglas Gash (dcmgash)" <[email-masked]>
Date: Tue, 8 Nov 2016 11:49:01 +0000

Hi,

Please see inline...

>cryptographically strong source that I cannot understand the reasoning

 This is an interesting point. We may have gone overboard there, after
all, as you say, the session id is in plaintext anyway.

>as its purpose is for resilence?

Yes. Just like on device method lists they may have a set of servers they
try until they get a PASS or FAIL.

>My thinking is more tolerance of buggy third party equipment rather than

that’s what I thought, which is why in real world you may want to make it
tuneable. In this case are you meaning the client which needs to deal with
a server that is sending back FAIL rather thabn ERROR in case a backend DB
is down (where I’d say it should  send ERROR), or a server determining how
best to deal with a backend DB is down (normally ERROR, but would consider
to make it tuneable).

---

From: Alexander Clouter <[email-masked]>
Date: Wed, 23 Nov 2016 23:24:08 +0000

Hi,

On Wed, Nov 02, 2016 at 11:14:46PM +0000, Alexander Clouter wrote:
> [snipped]

In the Accounting REQUEST packet, the status field is 8 bits, and the table 
only describes considering the first 3 bits.  It is clear what to do about the 
INVALID sections, but nothing seems to be mentions about what to do if any of 
the other 5 bits are set.

Should I treat these bits also as INVALID and send the client a status error 
message, or are they reserved and should be just ignored?

Thanks

---

From: Alexander Clouter <[email-masked]>
Date: Wed, 30 Nov 2016 16:43:59 +0000

Hello,

I cannot find the guidance in the draft, but I am looking on how to handle 
authentication session state.

Of course the session_id is part of it but the question is should the source IP 
and maybe also the TCP port make up that tuple?  Nothing about the definition 
of the sequence_id hints at if different TCP connections from the same client 
could carry *different* authentications with the *same* session_id.

For example, is it valid to have in a SINGLE CONNECT situation multiple TCP 
connections from the same client to the server?   Should I consider it expected 
behaviour that the following CONTINUE packets can vary over which TCP 
connection they traverse over (ie. should the port be part of the tuple)?

If TCP port to be part of the tuple, what happens if the TCP connection is 
killed (network reasons rather that protocol)?  Should any state associated 
with it also be destroyed or is the following sequence of events on the same 
session_id valid:

TCP connection 1 opened
seq 1 -> start
seq 2 -> reply
TCP connection 1 RST
TCP connection 2 opened
seq 3 -> cont

Should an authentication with multiple continuation packets be forcible made to 
do this operation over the same TCP connection or have to restart afresh if 
there is an interruption?

Thanks

On Wed, Nov 02, 2016 at 11:14:46PM +0000, Alexander Clouter wrote:
>
>I am working on implementing the TACACS+ protocol.  I do of course 
>have a reference (pam_tacplus) to crib from and a pile of lab 
>equipment, but thought as the draft your have authored seem aims to be 
>a living standard you might be interested in hearing some feedback 
>from the trenches of someone right now trying to implement from it.
>
>Below are my thoughts on various sections, though no doubt might 
>follow up later as my work progresses and other questions arise.
>
>> [snipped]

---

From: "Douglas Gash (dcmgash)" <[email-masked]>
Date: Wed, 30 Nov 2016 17:32:20 +0000

Hi,

Please see inline, this is my interpretation, would welcome other points
of view.

I think this is all determinable from the doc but will check/update if it
is a little hidden.

>could carry *different* authentications with the *same* session_id.

The session_id and TCP connection must be sufficient to uniquely identity
every session.

>connections from the same client to the server?

A SINGLE CONNECT does not link multiple TCP sessions, it is a facet of a
single TCP connection (I.e. A TCP connection can be upgraded to SINGLE
CONNECT).

A client can have multiple connections to a server, and those connections
may be SIGNLE CONNECT or NOT.

The session_id only identifies a session within a connection.

>   Should I consider it expected

Sessions are ³inside² connectionsŠ they cannot span multiple TCP
connections.

>there is an interruption?

I think that is n AND rather than or ;-)

Šauthentication with multiple continuation packets MUST be on the same TCP
connection AND client has to restart afresh if
there is an interruptionŠ so server should clear its sessions to
accommodate that.