Thank you Mohamed for addressing my comments. Best Regards, Ines.
On Tue, Jun 24, 2025 at 11:31 AM <mohamed.boucad...@orange.com> wrote: > 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 <nore...@ietf.org> > > Envoyé : mardi 24 juin 2025 01:35 > > À : gen-...@ietf.org > > Cc : draft-ietf-opsawg-secure-tacacs-yang....@ietf.org; last- > > c...@ietf.org; opsawg@ietf.org > > 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. > >
_______________________________________________ OPSAWG mailing list -- opsawg@ietf.org To unsubscribe send an email to opsawg-le...@ietf.org
