Terminal Access Controller Access-Control System Plus over TLS 1.3 (TACACS+ over TLS)
draft-ietf-opsawg-tacacs-tls13-24

# Andy Newton, ART AD, comments for draft-ietf-opsawg-tacacs-tls13-21
CC @anewton1998

* line numbers:
  - https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-opsawg-tacacs-tls13-21.txt&submitcheck=True

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

Many thanks to Rich Salz for the ARTART review.

I support Ketan's discuss.  This protocol is in wide use, it would be useful to have the whole protocol be PS.

Thanks to Russ Housley for their secdir review.

Section 3, last sentence:  This draft is almost in the RFC Editor's queue.  It would be better if it could be a normative reference, since it is exactly what you are trying to specify here.

Section 3.4.1:  Sometimes these (both cert path validation and revocation checking) can be quite hard.  Many implementations of TLS allow a bypass in the case of network latency issues.  And while it pains me to say this as 'a PKI person', you may need to consider whether there needs to be an allowance for a bypass. What happens if there are network issues blocking chains or OCSP?  

Section 5.1.2:  There is an 'early data extension' that the client could include to enable the ability to send early data.  Perhaps the statement 'A TLS client or server MUST NOT include the "early_data" extension. See Section 2.3 and 4.2.10 of [RFC8446] for security concerns.' or something similar could be added.

Section 5.1.5:  'subject to eavesdropping' because SNI is sent in the clear in the Client Hello?  Would it be more direct to say something like, 'the TLS SNI extension is part of the TLS client hello, which is sent in cleartext and is, therefore...'  That might make for an awkward sentence, which could be split into two sentences.  

Section 5.2:  Nit:  just be sure the (TBD) is properly flagged for IANA.

Section 5.3:  I'm not sure it is necessary to throw STARTTLS 'under the bus' as it were.  The point can be made w/out it (especially since no reference to STARTTLS is supplied).  I'd be happy to help with the re-write.  [https://en.wikipedia.org/wiki/Throw_under_the_bus]

# Gunter Van de Velde, RTG AD, comments for draft-ietf-opsawg-tacacs-tls13-23

# The line numbers used are rendered from IETF idnits tool: https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-opsawg-tacacs-tls13-23.txt

# This is a great write-up and needed extension to TACACS+

# I noted some non-blocking observations in the text that follows. Mostly idnits and clarifications.

# Detailed Review
# ===============

16	Abstract
17
18	   The Terminal Access Controller Access-Control System Plus (TACACS+)
19	   protocol provides device administration for routers, network access
20	   servers, and other networked computing devices via one or more
21	   centralized TACACS+ servers.  This document adds Transport Layer
22	   Security (TLS 1.3) support to TACACS+ and obsoletes former inferior
23	   security mechanisms.
24
25	   This document updates RFC 8907.

GV> What about the following textblob alternative:

"
This document specifies the use of Transport Layer Security (TLS) version 1.3 to secure the communication channel between a TACACS+ client and server. TACACS+ is a protocol used for Authentication, Authorization, and Accounting (AAA) in networked environments. The original TACACS+ protocol, defined in RFC 8907, does not mandate the use of encryption or secure transport. This specification defines a profile for using TLS 1.3 with TACACS+, including guidance on authentication, connection establishment, and operational considerations. The goal is to enhance the confidentiality, integrity, and authenticity of TACACS+ traffic, aligning the protocol with modern security best practices.

This document updates RFC 8907.
"

183	3.1.  Separating TLS Connections
184
185	   All data exchanged by TACACS+ peers MUST be encrypted, including the
186	   mutual authentication of the peers.  Therefore, when a TCP connection
187	   is established for the service, a TLS handshake begins immediately.

GV> This reads like a storm of energy of change and leaves no room for introduction. The text states "peers MUST be encrypted". While true when supporting this RFC-to-be, it was not the situation before this RFC-to-be. Maybe mention in the text that it MUST be supported when this RFC-to-be is supported.

189	   To ensure separation of TACACS+ traffic that uses TLS from that which
190	   does not (Section 5.3), TLS TACACS+ servers MUST be deployed on a
191	   separate TCP/IP port number from Non-TLS TACACS+ servers (preferably
192	   on a separate host, as recommended in Section 5.1.1).  Because of the
193	   widespread use of default port number settings in numerous existing
194	   TACACS+ client configurations, a well-known system TCP/IP port number
195	   is assigned: the designated port number is [TBD] (Section 7) with the
196	   service name tacacss (Section 7).  This ensures that the client can
197	   separate TLS and Non-TLS traffic even where default port numbers are
198	   omitted from its TACACS+ server connection configuration.
199
200	   Under exceptional circumstances, this document permits any other TCP
201	   port number to be configured when required by deployment specifics,
202	   but the implications in Section 5.3 have to be considered by

GV> What about the following alternative textblob:

"
To ensure clear separation between TACACS+ traffic using TLS and that which does not (see Section 5.3), servers supporting TACACS+ over TLS MUST listen on a TCP/IP port distinct from that used by non-TLS TACACS+ servers. Where feasible, it is further RECOMMENDED to deploy the TLS and non-TLS services on separate hosts, as discussed in Section 5.1.1.

Given the prevalence of default port usage in existing TACACS+ client implementations, this specification assigns a well-known TCP port number for TACACS+ over TLS: [TBD], with the associated service name "tacacss" (see Section 7). This allows clients to unambiguously distinguish between TLS and non-TLS connections, even in the absence of an explicitly configured port number.

While the use of the designated port is strongly encouraged, deployments with specific requirements MAY use alternative TCP port numbers. In such cases, operators must carefully consider the operational implications described in Section 5.3.
"

213	   TLS 1.3 [RFC8446] must be used for transport, though it is expected
214	   that TACACS+ as described in this document will work with future
215	   versions of TLS.  Earlier versions of TLS MUST NOT be used.

GV> Would it be beneficial to state that "minimum TLS 1.3 MUST be used for transport ..."? 

217	   Once the TLS connection is established, the exchange of TACACS+ data
218	   proceeds as defined in [RFC8907], except that it is transmitted over
219	   TLS as TLS application data and without TACACS+ obfuscation
220	   (Section 4).

GV> What about:

"
Once the TLS connection has been successfully established, the exchange of TACACS+ data SHALL proceed in accordance with the procedures defined in [RFC8907]. However, all TACACS+ messages SHALL be transmitted as TLS application data. The TACACS+ obfuscation mechanism defined in [RFC8907] MUST NOT be applied when operating over TLS (see Section 4)
"

225	   negotiated, when an inactivity timeout occurs.  Why it closed has no
226	   bearing on TLS resumption, unless closed by a TLS error, in which
227	   case the ticket might be invalidated.

GV> What is meant with "ticket"?

229	   TACACS+ connections are not long-lived.  Non 'Single Connection Mode'
230	   (Section 4.3 of [RFC8907]) connections are closed as soon as the
231	   TACACS+ session completes.  Single Connection Mode connections are
232	   longer lived, but even these are timed out and closed after a short
233	   period of inactivity.  For this reason, keepalives are not required
234	   to be supported.

GV> What about:

"
TACACS+ connections are generally not long-lived. For connections not operating in Single Connection Mode (as defined in Section 4.3 of [RFC8907]), the TCP session SHALL be closed upon completion of the associated TACACS+ session. Connections operating in Single Connection Mode MAY persist for a longer duration but are typically subject to timeout and closure after a brief period of inactivity. Consequently, support for transport-layer keepalive mechanisms is NOT REQUIRED.
"

421	   The introduction of TLS PSK, certificate peer authentication, and TLS
422	   encryption to TACACS+ replaces these former mechanisms and so
423	   obfuscation is hereby obsoleted. 

GV>  Is TLS PSK in this document not documented after the TLS Encryption? Maybe follow order in the above phrase to be conmsistent with the documentation in this RFC-to-be for readability?

502	   completes.  Replay of this data is a risk.  Given the sensitivity of
503	   TACACS+ data, clients MUST NOT send data until the full TLS handshake
504	   completes; that is, clients MUST NOT send 0-RTT data and TLS TACACS+
505	   servers MUST abruptly disconnect clients that do.

GV> Beyond the abruptly disconnect, is an operational logging recommended?


Many thanks for this document,
Gunter Van de Velde,
Routing AD

Thanks for the work on this document and updating TACACS+ for TLS. This is a much needed security upgrade for the widely used protocol.

Thanks for clarification on the history of why TACACS+ was developed as informational document to help clear my DISCUSS.

I am still wondering whether informational status is not more appropriate for this document as well.

Thanks to Russ Housley for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/cNyvkP_-A4zahKP-Ln_-DgozZzg).

In general, found the document easy to read. I have some minor COMMENT and NIT.

-------------------------------------------------------------------------------
COMMENT
-------------------------------------------------------------------------------

No reference entries found for these items, which were mentioned in the text:
[draft-ietf-radext-tls-psk] and [I-D.ietf-uta-require-tls13].

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Term "man"; alternatives might be "individual", "people", "person"

-------------------------------------------------------------------------------
NIT
-------------------------------------------------------------------------------

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 3.4, paragraph 1
>    Deploying TLS certificate-based uthentication correctly will
>    considerably improve the security of TACACS+ deployments.  It is
>    essential for implementers and operators to understand the
>    implications of a TLS certificate-based authentication solution,
>    including the correct handling of certificates, Certificate
>    Authorities (CAs), and all elements of TLS configuration.  For
>    guidance, start with [BCP195].

s/uthentication/authentication/

Section 5.1.6, paragraph 0
>    The use of wildcards in TLS server identities creates a single point
>    of failure: a compromised private key of a wildcard certificate
>    impacts all servers using it.  Their use MUST follow recommendations
>    of Section 7.1 of [RFC9525].  Operators MUST ensure that the wildcard
>    is limited to a subdomain dedicated solely to TLS TACACS+ servers.
>    Further, operators MUST ensure that the TLS TACACS+ servers covered
>    by a wildcard certificate MUST be impervious to redirection of
>    traffic to a different server (for example, due to MiTM attacks or
>    DNS cache poisoning).

Though expanded later, would prefer that MiTM is expanded here, as it is the first use of the acronym.

Uncited references: [TLSCSREC].

Section 3.6, paragraph 2
> lient and servers MUST operate with regards to the obfuscation mechanism. Pe
>                                ^^^^^^^^^^^^^^^
Use "in regard to", "with regard to", or more simply "regarding".

I second the comments around referencing ECH if the visibility of the Client Hello is a concern.

Thank you to Rich Salz for the ARTART review.

Thank you to Russ Housley for the GENART review.