> On Jan 23, 2025, at 2:20 AM, mohamed.boucad...@orange.com wrote:
>
> 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.
I agree with Reshad that the name does not match the type definition, which
does not match the description. If the idea is to accept either an ip-address
or a host-name, then calling it “server” or something similar would be better.
Per rfc6991bis a host is:
typedef host {
type union {
type ip-address;
type host-name;
}
description
"The host type represents either an IP address or a (fully
qualified) host name.";
}
If, instead, the desire is to accept an FQHN, then the type should be changed
to “host-name,” and the description should be updated to reflect the type.
Cheers.
>
> 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.
>
> "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
>
> <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.
Mahesh Jethanandani
mjethanand...@gmail.com
_______________________________________________
OPSAWG mailing list -- opsawg@ietf.org
To unsubscribe send an email to opsawg-le...@ietf.org
