Hi Med, all, On the issues of "leaf address" ‘inet:host’ and "leaf vrf-instance", please see inline with [Bo Wu].
Thanks, Bo From: mohamed.boucad...@orange.com <mohamed.boucad...@orange.com> Sent: Thursday, January 23, 2025 6:21 PM To: Reshad Rahman <res...@yahoo.com>; yang-doct...@ietf.org Cc: draft-ietf-opsawg-secure-tacacs-yang....@ietf.org; opsawg@ietf.org Subject: [OPSAWG]Re: [yang-doctors] Yangdoctors early review of draft-ietf-opsawg-secure-tacacs-yang-04 Hi Reshad, Thank you for this review. Please see inline. Cheers, Med De : Reshad Rahman <res...@yahoo.com<mailto:res...@yahoo.com>> Envoyé : mercredi 22 janvier 2025 23:32 À : yang-doct...@ietf.org<mailto:yang-doct...@ietf.org> Cc : draft-ietf-opsawg-secure-tacacs-yang....@ietf.org<mailto:draft-ietf-opsawg-secure-tacacs-yang....@ietf.org>; opsawg@ietf.org<mailto:opsawg@ietf.org> Objet : Re: [yang-doctors] Yangdoctors early review of draft-ietf-opsawg-secure-tacacs-yang-04 One comment I forgot to add. The list server is keyed on name, so in theory we could have multiple entries in that list to the same server (leaf address). Should there be a unique statement for leaf address? list server { key "name"; [Med] Good point. This is actually inherited from the original RFC. I think the list should have unique per address/port. On Wednesday, January 22, 2025 at 05:25:39 PM EST, Reshad Rahman via Datatracker <nore...@ietf.org<mailto:nore...@ietf.org>> wrote: Reviewer: Reshad Rahman Review result: Ready with Issues Disclaimer: I am not a TACACS+ nor a TLS expert. Overall the document looks good. Here are what I perceive are issues which should be addressed. "leaf address": it is of type inet:host, so is not necessarily an IP address as per the name and description. Rename to "server" or "host"? But this would be a non backwards compatible change.... At least change the description to say "IP address or host name of the ACACS+ server." leaf address { type inet:host; mandatory true; description "The address of the TACACS+ server."; } [Med] I let the original authors of 9105 clarify, but I suspect they followed the same approach in RFC7317 (system yang) where that exact identifier and description are used. [Bo Wu] Thanks Med for the clarification. Yes, The comment we received was be consistent with RFC7317. It is not clear to me why "leaf domain-name" was added. Section 3 refers to section 3.3 of [I-D.ietf-opsawg-tacacs-tls13] but that section does not mention domain-name. [Med] That’s to echo this part of [I-D.ietf-opsawg-tacacs-tls13: Implementations MUST support the TLS Server Name Indication extension (SNI) (Section 3 of [RFC6066]), and MUST support the ability to configure the TLS TACACS+ server's domain name, so that it may be included in the SNI "server_name" extension of the client hello **(This is distinct from the IP Address or hostname configuration used for the TCP connection)***. See Section 5.1.5 for security related operator considerations. 'domain-name': Provides a domain name of the server per Section 3.3 of [I-D.ietf-opsawg-tacacs-tls13]. "leaf vrf-instance": not needed if source-type is source-interface (since the VRF of the source interface would be used)? Add "must not()" statement or describe the behaviour if vrf-instance does not have the same value as source-interface's VRF. [Med] This is actually inherited from 9105. I let the authors of 9105 check this one. [Bo Wu] I agree with Reshad on this issue. Thank for identifying this. Indeed, if a source-interface is specified, configuring a VRF instance explicitly is unnecessary. However, when it comes to the operation status, it could apply to both source-type. To correct this, how about the YANG model be fix as follows? leaf vrf-instance { type leafref { path "/ni:network-instances/ni:network-instance/ni:name"; } description "Specifies the VPN Routing and Forwarding (VRF) instance to use to communicate with the TACACS+ server. If 'source-interface' is configured, this value MUST match the network instance bound to the source interface (via bind-ni-name)."; must "(not(../source-interface)) or " + "(current() = /if:interfaces/if:interface" + "[if:name = current()/../source-interface]/bind-ni-name)" { error-message "VRF instance must match the network instance of the source interface."; } reference "RFC 8529: YANG Data Model for Network Instances"; } "leaf port": remove the commented out “default 49”? [Med] ACK. "choice source-type": do we need “mandatary true”? Same question for the 2 instances of “choice ref-or-explicit” [Med] I don’t think so. These are optional. "leaf single-connection": please add a reference. I think that should be to Section 4.3 of [RFC8907]. [Med] OK [Med] The changes made so far can be tracked here: https://author-tools.ietf.org/api/iddiff?url_1=https://boucadair.github.io/secure-tacacs-yang/draft-ietf-opsawg-secure-tacacs-yang.txt&url_2=https://boucadair.github.io/secure-tacacs-yang/Reshad-yangdoctors/draft-ietf-opsawg-secure-tacacs-yang.txt ____________________________________________________________________________________________________________ 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
