On Dec 22, 2023, at 11:53 AM, Douglas Gash (dcmgash) 
<dcmgash=40cisco....@dmarc.ietf.org> wrote:
> Some brief notes regarding the broader topics raised in v3, all items of 
> course, are open for re-aligning as the group sees fit.
>  
>       • Regarding the allocation of a specific port, a key motivation 
> concerns the pervasive use of default ports in current configurations. 
> Ensuring the client implementations work correctly with default ports now TLS 
> is introduced, to minimise burden for operators whilst avoiding wrinkles with 
> downgrade attacks does require said new default port to be allocated, and we 
> will explicitly mention this in a new section in the new doc. We hope this 
> should help account for our request for an allocated port.

  I think it's reasonable to allow use of the old port.  I would add a section 
explaining why this choice was made.  Perhaps also adding some text on how 
packet inspection systems / firewalls can distinguish the two protocols over 
the same port.

  For example, it may be useful for firewalls to allow TACACS+ traffic, but 
only if it's encrypted.

  The non-TLS TACAC+ connections have the first octet as 0xc0 or 0xc1, and the 
second octet is 0x01, 0x02, or 0x03.  Whereas TLS begins with a TLS handshake 
0x16, followed by the TLS major version 0x03.

   I think it would be sufficient to just check the first octet.  If it's 0xc0 
... 0xcf, it's unlikely to be TLS and could be blocked.  Those values are 
unassigned in the TLS ContentType registry, and are likely to remain unassigned 
for the foreseeable future.

 I suspect that by the time those values are assigned for TLS ContentType, all 
uses of non-TLS TACACS+ would be long deprecated.

>       • RFC9325 does look a timely reference, and we have tried to delegate 
> what we can from the new TACACS+ doc to it.
>       • Tracking the discussion on the deprecation of obfuscation option 
> inside TLS, our current reading is that:
>               • All TLS traffic must NOT use obfuscation.

  I would phrase that as "TACACS+ application data sent inside of a TLS tunnel 
MUST NOT use obfuscation".

>               • Setting the non-obfuscation option (TACACS has a flag for 
> unencrypted)  is mandatory for all TLS TACACS+ traffic.

  Yes.

>               • The aim is to avoid any ambiguity and to remove MD5 
> operations from this level of the protocol.

  It would be good to give some explanation as to why MD5 is being removed.


>       • We hope we have addressed the raised issues nits and ambiguities.

  It's looking good.

  Some additional nits on the new version:

* Section 2

  I would suggest some modifications to the terms.  Perhaps:

     Historic TACACS+: as defined in RFC 8907

  The intention here is to say also that it's not just "TACACS+ without TLS", 
but it's *historic* and should not be used.

    TACACS+TLS: TLS transport with TACACS+ as the application data
  
* Section 3

  Perhaps "TACACS+ over TLS" instead of "TLS for TACACS+" ?

  I don't think it's necessary for the first paragraph to re-explain how TACAC+ 
connections work.

  Perhaps also explain that TACACS+TLS is little more than a TLS wrapper around 
un-obfuscated TLS.  i.e. could it be implemented simply with stunnel + a 
historic TLS client/server?

* Section 3.1

  Perhaps explain why the same port is being (or could be) used.

  "Therefore, TLS Hello MUST be initiated ..."

  What's a "Hello" ?  Perhaps just say instead that the connection begins with 
TLS, and leave it at that.

 "This document favors the predictable use of TLS security for a deployment"

  I'm not sure what that means.  Maybe "prefers" or "mandates" the use of TLS?  
I find the phrasing to be confusing.

* Section 3.2.2

  It may be useful to move the TLS-PSK discussion from 5.1.3 to here.

  Though the RADEXT WG recently went through a long process of defining TLS-PSK 
for RADIUS.  It wasn't trivial.  See 
https://datatracker.ietf.org/doc/draft-ietf-radext-tls-psk/

  If the document is going to say "TLS-PSK is possible" but nothing more, then 
I would suggest just forbidding it.  There are a lot of details to get right 
with TLS-PSK.

* Section 3.4

  ALPN needs to have ALPN strings defined.  There's a similar (and long) 
discussion in https://datatracker.ietf.org/doc/draft-ietf-radext-radiusv11/

  TACACS+TLS doesn't need that level of complexity.  The RADEXT document is 
trying to be compatible with historic RADIUS, which is hard.  So it's a good 
idea to require the use of ALPN now.

  Perhaps for TACACS+TLS, just say that the client and server MUST use a 
particular ALPN string.  Maybe "tacacs+/12.1", which could mean both 12.0 and 
12.1 versions of TACACS+,

 Over all, I'd like to see more content in the document.  Right now it outlines 
how TACACS+TLS should work, but gives very little guidance to implementors or 
operators.  I'd suggest reading the RADEXT TLS-PSK and ALPN documents linked 
above.  They go into exhaustive detail about how every little bit is supposed 
to work.  I've found that documenting the protocol in such detail greatly 
improves the quality of the implementations, and is more likely to result in 
interoperation between the implementations.

  Alan DeKok.



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

Reply via email to