Hi Ines, Thank you for the review.
As a general note, some of your comments are related to parts that were already in RFC 9105, while this revision is scoped to add TLS support. Please see inline for more context. Cheers, Med (as author) > -----Message d'origine----- > De : Ines Robles via Datatracker <[email protected]> > Envoyé : mardi 24 juin 2025 01:35 > À : [email protected] > Cc : [email protected]; last- > [email protected]; [email protected] > Objet : draft-ietf-opsawg-secure-tacacs-yang-11 ietf last call > Genart review > > > Document: draft-ietf-opsawg-secure-tacacs-yang > Title: A YANG Data Model for Terminal Access Controller Access- > Control System Plus (TACACS+) Reviewer: Ines Robles Review result: > Almost Ready > > I am the assigned Gen-ART reviewer for this draft. The General Area > Review Team (Gen-ART) reviews all IETF documents being processed by > the IESG for the IETF Chair. Please treat these comments just like > any other last call comments. > > For more information, please see the FAQ at > > > Document: draft-ietf-opsawg-secure-tacacs-yang-11 > Reviewer: Ines Robles > Review Date: 2025-06-23 > IETF LC End Date: 2025-06-23 > IESG Telechat date: Not scheduled for a telechat > > Summary: > This document defines a Terminal Access Controller Access-Control > System Plus > (TACACS+) client YANG module that augments the System Management > data model, defined in RFC 7317, to allow devices to make use of > TACACS+ servers for centralized Authentication, Authorization, and > Accounting (AAA). Specifically, this document defines a YANG module > for TACACS+ over TLS 1.3. This document obsoletes RFC 9105. > > I have the following comments/questions: > > Comments: > > 1- In Section 3, the draft states:"The 'server' list, which is > directly under the 'tacacs-plus' container, holds a list of TACACS+ > servers and uses 'server-type' to distinguish between > Authentication, Authorization, and Accounting (AAA) services. The > list of servers is for redundancy." > [Med] This text is inherited from RFC9105. > It would be nice to clarify: > > 1.1-Whether the order in the list has semantic meaning (e.g., first > server is tried first), > > 1.2-How fallback should occur (e.g., failover after timeout, round- > robin), > > 1.3-Whether parallel attempts or load balancing are allowed. [Med] The work on this data model triggered similar questions in the past (e.g., https://mailarchive.ietf.org/arch/msg/opsawg/CXMtDH_GWRlZfCRhKhggA4zapuA/). These are not covered here because the exact behavior to follow belongs to the protocol itself not the data model. > > 2- Section 4: Related to the "leaf port". > This draft removed the YANG default statement for the port leaf > (previously default 49) to accommodate the future IANA-assigned port > for TACACS+ over TLS, which is currently referenced as TBD in the > description. Thus, my understanding is the following: > > * In the absence of a "default" or "mandatory true" statements, the > port leaf becomes optional; > > * YANG does not interpret prose in the description field for default > behavior; > > * Therefore, if the port is not configured explicitly by the user, > the system behavior is undefined by the schema, and implementations > may diverge. > > * Is this interpretation correct? > > Thus, to avoid ambiguity at runtime and to enforce consistent > behavior across implementations, what about? > > leaf port { > > type inet:port-number; > > mandatory true; > > "The port number of TACACS+ server. Default port number for legacy > TACACS+ is > 49, while it is TBD for TACACS+TLS."; > > } > > OR > > leaf port { > > type inet:port-number; > > default "TBD"; // Replace with the actual IANA-assigned port > > "The port number of TACACS+ server. Default port number for legacy > TACACS+ is > 49, while it is TBD for TACACS+TLS."; > > } [Med] You have a valid point here. Went with a default with TBD. > > 3- Section 4: Related to the "must" constraints for TLS Version in > hello-params. > The draft refines tls-versions/min and tls-versions/max (from ietf- > tls-common) to disallow TLS 1.2 using a must sentence. Based on my > understanding, this correctly prevents TLS 1.2 from being configured > at validation time, but it does not stop tools from offering TLS 1.2 > as a selectable value in UIs or APIs. > This is because The identityref type still allows TLS 1.2; and the > "must" only applies during validation, not during input. So, a user > might configure TLS 1.2 without knowing it's invalid, and only see > an error later when the config is applied. Thus, it would be nice to > update the description for min and max to explain that: > > * TLS 1.2 is rejected by a must statement during configuration > validation. > > * Tools should avoid presenting TLS 1.2 to the user. > > * The server must enforce the rule. > > what do you think? [Med] I don't think we need a change here as the meaning of "must" is assumed to be known: https://datatracker.ietf.org/doc/html/rfc7950#section-7.5.3. > > 4- Section 4: case obfuscation > > The model includes the case obfuscation and its related > configuration nodes (such as shared-secret) as valid and usable > parts of the schema. However, the draft states that obfuscation is > deprecated and that TLS should be used instead. This creates > ambiguity for implementers regarding how this configuration path > should be treated. Thus, it would be nice to clarify: > > 4.1- Is this configuration path intended only for legacy > compatibility? [Med] Added: NEW: This choice is provided in the model to accommodate installed base."; > > 4.2- Should Obfuscation be disabled when TLS is enabled? > [Med] We don't need normative/narrative text for this as this is mandated by YANG language itself: case tls { description "TLS is used to secure TACACS+ exchanges."; reference "RFC SSSS: Terminal Access Controller Access-Control System Plus (TACACS+) over TLS 1.3"; uses tls-client; } case obfuscation { leaf shared-secret { type string { length "1..max"; } Please refer to rfc7950#section-7.9, which says: The "choice" statement defines a set of alternatives, only one of which may be present in any one data tree. > 4.3- Are implementations expected to reject, warn about, or continue > supporting obfuscation in new deployments? > > 4.4- Should systems still support obfuscation in configurations? If > so, under what conditions? > > 4.5- Is there a migration path for legacy deployments using > obfuscation? [Med] All these are good gestions. These considerations are discussed in the base spec: https://datatracker.ietf.org/doc/html/draft-ietf-opsawg-tacacs-tls13-23#name-migration. Will add a pointer to remind these. > > 5- Section 4: VRF and source-interface > > The draft includes a must constraint on the vrf-instance leaf to > ensure consistency with the source-interface, if both are > configured. However, based on my understanding, the model does not > prevent both vrf-instance and source-interface from being set at the > same time, [Med] When both are provided, the model ensures that the instance is bound to that same source interface. nor does it specify which one takes precedence in > determining the routing or source binding behavior. This could lead > to ambiguous or inconsistent behavior across implementations. It > would be helpful to explicitly document the intended logic and > precedence in the description. > > 6- Should the model include configuration for retry count, retry > interval, or timeout per server? [Med] We do already have timeout inherited from RFC9105. That's the parameter actually supported by implementations I'm aware of. > > 7- Should the model include elements for managing the credential > lifecycle, such as support for expiration of shared secrets or keys? [Med] These are out of scope. We are adhering to the same scope as RFC9645. > > 8-Nits: > "Obfuscation is obsoleted in favor of TLS support" --> what about? > "The use of obfuscation is deprecated in favor of TLS-based > security." [Med] ACK. > > Thanks for this document, > > Ines. > ____________________________________________________________________________________________________________ Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration, Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci. This message and its attachments may contain confidential or privileged information that may be protected by law; they should not be distributed, used or copied without authorisation. If you have received this email in error, please notify the sender and delete this message and its attachments. As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified. Thank you. _______________________________________________ Gen-art mailing list -- [email protected] To unsubscribe send an email to [email protected]
