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

[Alexander Clouter <[e-mail]>]

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?