Re: [OPSAWG] TACACS+ informational document.

2016-04-22 Thread Joel Jaeggli
Thanks, 

This is useful feedback.

> On Apr 22, 2016, at 05:38, Alan DeKok  wrote:
> 
>  A short summary:
> 
> - many fields are named but not defined
> 
> - structures with multiple fields are described, but field order is not 
> defined
> 
> - terms are used inconsistently
> 
> - the document is silent on critical points
> 
>  - how do user logins map to TCP connections?  1-1?  1-N?  N-M?
> 
>  - can the same session_id be used in multiple TCP connections?
> 
> - the general tone seems philosophical: systems "know" things, not 
> prescriptive: systems "do" things.
> 
> - edge cases are not discussed
> 
> - what happens with zero-length fields?
> 
> - common use-cases aren't described (e.g. inter-site use of the protocol)
> 
> - the security considerations section is minimal
> 
>  - how do the edge cases affect security?
> 
>  - is the TCP connection closed when the key is found to be wrong?  If not, 
> why not?
> 
>  - what are best practice recommendations for deployment?
> 
>  - what impact does inter-site deployment have on security?
> 
> 
>  As an implementor, I would have to guess at large portions of the protocol, 
> or I would have to read the source to existing implementations.  The draft as 
> is stands today can get me ~90% of the way to implementing the protocol, but 
> critical portions are not present.
> 
>  Alan DeKok.
> 
> ___
> OPSAWG mailing list
> OPSAWG@ietf.org
> https://www.ietf.org/mailman/listinfo/opsawg
> 

___
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg


Re: [OPSAWG] TACACS+ informational document.

2016-04-22 Thread Alan DeKok
  A short summary:

- many fields are named but not defined

- structures with multiple fields are described, but field order is not defined

- terms are used inconsistently

- the document is silent on critical points

  - how do user logins map to TCP connections?  1-1?  1-N?  N-M?

  - can the same session_id be used in multiple TCP connections?

- the general tone seems philosophical: systems "know" things, not 
prescriptive: systems "do" things.

- edge cases are not discussed

 - what happens with zero-length fields?

- common use-cases aren't described (e.g. inter-site use of the protocol)

- the security considerations section is minimal

  - how do the edge cases affect security?

  - is the TCP connection closed when the key is found to be wrong?  If not, 
why not?

  - what are best practice recommendations for deployment?

  - what impact does inter-site deployment have on security?


  As an implementor, I would have to guess at large portions of the protocol, 
or I would have to read the source to existing implementations.  The draft as 
is stands today can get me ~90% of the way to implementing the protocol, but 
critical portions are not present.

  Alan DeKok.

___
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg


Re: [OPSAWG] TACACS+ informational document.

2016-04-21 Thread Alan DeKok
  More comments...

4.4.  Description of Authentication Process

   Authentications are classified by the action, authen_type and service
   fields in the START packet of the authentication Session.  The user,
   priv_lvl, service, port and rem_addr in the START packet are all
   provided to help identify the conditions on the client

  What is meant by "conditions on the client"?  It would be best to describe 
what those fields mean instead.

  ... The information necessary to transact the authentication is passed ...

  What is "information necessary to transact the authentication"?  This sounds 
unclear to me.

  ...  A set of standard authentication classifications is defined in this
   document.  

  What is meant by "authentication classifications" ?  It's not clear from the 
context or the rest of the document.  Perhaps it means "authentication 
protocols" ?  i.e. one exchange is done for PAP, another for MS-CHAP, another 
for OTP, etc.

  ... 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 server_msg may be used by the
   client to prompt the user for more information.

  "may be used"?  What happens when the client doesn't use server_msg to prompt 
the user?

   ... All three cause the same action to be performed, but in the case of
   TAC_PLUS_AUTHEN_STATUS_GETUSER, the client can know that the
   information that the user responds with is a username, and for
   TAC_PLUS_AUTHEN_STATUS_GETPASS, that the user response represents a
   password.

  I'm wary of statements such as "the client can know..."   Is that a 
philosophical statement?

  It's generally best to describe actions instead of knowledge.  e..g. "for 
prompts using TAC_PLUS_AUTHEN_STATUS_GETUSER, the users response is interpreted 
by the client as a username, for use in subsequent exchanges".

  ... TAC_PLUS_AUTHEN_STATUS_GETPASS, that the user response represents a
   password.

  Similar comments apply here.

   ... TAC_PLUS_AUTHEN_STATUS_GETDATA is the generic request for
   more information.

  And what happens then with the data?  When is this message used?

4.4.1.  Version Behaviour

   ... in the table below:

  LOGINCHPASS   SENDAUTH
   ASCII  v0 v0   NA
   PAPv1NA   v1
   CHAP   v1NA   v1
   MS-CHAPv1NA   v1


  What does "NA" mean?  I suspect I know, but it's good to be explicit instead 
of assuming that the reader knows the authors intent.

  ... The removal of SENDPASS
   was prompted by security concerns, and is no longer considered part
   of the TACACS+ protocol.

  It may be worth considering the removal of all documentation of SENDPASS.  If 
it's not just deprecated by removed entirely, then it should probably be just 
removed entirely.

  4.4.2.  Common Authentication Flows

   This section describes the authentication flows that should be
   supported.

  What is meant by "should be supported"?  Is this an RFC 2119 statement?  Or 
are these examples of authentication flows which are in common use, and that 
all clients && servers are expected to support?

   ... A PAP authentication only consists of a username and password 
   RFC 1334 [RFC1334] .

  I think referencing a later RFC is preferable.

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

  This would seem to impose limits on the "date len" field.  It would be good 
to state these explicitly.

  Also, to indicate that the challenges MUST have a minimal length, which 
should probably be no less than 8, though 16 is suggested.  The alternative is 
to allow zero-length challenges, which is not good.

  And how are the challenge, ID, and response fields packed into the data 
field?  i.e. in what order are they packed?  ASCII art would be *very* helpful 
here. 

  Similar comments apply for MS-CHAPv1 and MS-CHAPv2 authentication.

 4.4.3

  ... When the status equals TAC_PLUS_AUTHEN_STATUS_FOLLOW the packet
   indicates that the TACACS+ server requests that authentication should
   be performed with an alternate server.  The data field MUST contain
   ASCII text describing one or more servers.  A server description
   appears like this:

   [@@]>[@]

  Examples would help here.  The above text isn't ABNF, or any other 
machine-readable definition of a text string.

  Perhaps the text could be updated to say that "@" is a delimiter. for text 
strings:

0 @ = host
1 @ = host@key
2 @ = @protocol@host
3 @ = @protocol@host@key

 ...  If a key is supplied, the client MAY use the key in order to
   authenticate to that host.  If more than one host is specified, they
   MUST be separated by an ASCII Carriage Return (0x0D).

  Some questions here.  What happens if the client doesn't want to use that 
key?  What 

Re: [OPSAWG] TACACS+ informational document.

2016-04-21 Thread Alan DeKok
On Apr 20, 2016, at 3:49 PM, Warren Kumari  wrote:
> I'd really appreciate it if people could review the current document (Link 
> for easy clicking: 
> https://datatracker.ietf.org/doc/draft-ietf-opsawg-tacacs/) and provide 
> feedback to the list by Friday April 29th.

  Overall, it's a lot more focussed than the original draft.  This is good.

  More detailed comments:

Section 2

   ... Authentication generally takes place when the user first logs in to a
   machine or requests a service of it.

  What is meant by "requests a service of it" ?  The subsequent paragraph 
explains in a bit more detail, but the phrasing here sounds awkward.

  Also "log into a machine" is a bit vague.  Perhaps also use "privilege" as 
per the subsequent text:

   ... Authentication generally  takes place when the user first connects to the
   machine for device administration, and may also happen later when
   additional privileges are requested.

 Authentication is not mandatory; it is a site-configured option.

  I would suggest adding "... not mandatory in the protocol".  From a security 
point of view, unauthenticated device administration is bad.

  ... The server receives TACACS+ protocol requests, and replies according
   to its business model

  What's a "business model" in relation to a protocol?  Perhaps it could 
instead say

 ... The server receives TACACS+ protocol requests, and replies according
   to its configured policy

Section 3.1.1

   ... It is NOT recommended to deploy TACACS+ without Body encryption,
   other than for test environments.

  Perhaps "NOT RECOMMENDED"

3.2.  Session

   The concept of a session is used throughout this document.  A TACACS+
   session is a single authentication sequence, a single authorization
   exchange, or a single accounting exchange.

  I find that a little confusing.  When an admin logs into a device, the common 
terminology is to call one particular connection as a session.  i.e. user logs 
in at 10am, runs 3 commands, and logs out at 10:10am.

  Using "session" to refer to request/reply exchanges is a bit confusing. 

3.3.  Single Connect Mode

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

   If a client sets this flag, this indicates that it supports
   multiplexing TACACS+ sessions over a single TCP connection. 

  Can exchanges for multiple *users* be sent over a single TCP connection?  The 
document doesn't seem to say anything about this.

  From Section 3.4:

   ... The sequence number must never wrap i.e. if the sequence number 2^8-1
   is ever reached, that session must terminate and be restarted with a
   sequence number of 1.

  I have a few questions about this.  The use of "session" here is confusing.  
It was earlier defined to be a request/reply exchange.  But here the 
implication is that it's something else.  Perhaps what's meant is a 
"connection"?

  There is also the implication that the authentication / authorization process 
starts again.  It would be good to clarify this text to use a term other than 
"session".  And to explain what happens when a session/connection is restarted.

  If the intention is to open a new TCP connection, we presume that the user 
must re-authenticate.  Do they use the same credentials as last time?  How are 
these cached on the client?

... session_id

   The Id for this TACACS+ session.  The session id should be randomly
   chosen.  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

  Again, what is meant by "session" here?  If we use the definition from 
Section 3.2, a session is a request/reply exchange.  So can the "session_id" be 
different for every exchange?  I presume not, otherwise the authentication step 
couldn't be associated with subsequent authorization an accounting steps.

  I suggest changing Section 3.2 to use a term other than "session".  Perhaps 
"exchange" would be better.  The term "session" should be reserved for a series 
of authentication / authorization / accounting exchanges, which are all tied to 
the same user login.  I see this change as critical to getting the document 
published.  As it stands today, I have no real idea what a "session" is, as 
it's used in multiple contradictory ways.

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

  I'm not sure what is meant by "packets are never padded".  This is TCP, not 
UDP.  UDP packets can be larger than the application data packets they 
transport.  TCP cannot, as it is a stream protocol.

  Perhaps what is meant here is that the data following "length" octets MUST be 
another valid TACACS+ header.

  And the corollary (from a security perspective) is that any time the data is 
*not* a 

[OPSAWG] TACACS+ informational document.

2016-04-20 Thread Warren Kumari
Hi there all,

The authors of draft-ietf-opsawg-tacacs have updated the document and would
like some review.

As a reminder, this document is supposed to describe how TACACS currently
works.
We will then publish a new document which improves the security of TACACS.

I'd really appreciate it if people could review the current document (Link
for easy clicking:
https://datatracker.ietf.org/doc/draft-ietf-opsawg-tacacs/) and provide
feedback to the list by Friday April 29th.

W
___
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg