Thanks Bo. Perhaps make the choice description:
"Encryption mechanism between TACACS+ client and server". Let me know when you are done, and I'll start the IETF LC. Regards, Rob > -----Original Message----- > From: Wubo (lana) <[email protected]> > Sent: 11 March 2021 10:46 > To: Rob Wilton (rwilton) <[email protected]>; opsawg <[email protected]>; > [email protected] > Subject: Re: AD review of draft-ietf-opsawg-tacacs-yang-07 > > Hi Rob, > > Thanks for your comments. I will submit rev-09 to resolve the issues > below. > For the "shared-secret", will change the leaf to: > > choice encryption { > mandatory true; > description > "A choice amongst available encryption types."; > case shared-secret { > leaf shared-secret { > ... > } > > Best regards, > Bo > > -----邮件原件----- > 发件人: Rob Wilton (rwilton) [mailto:[email protected]] > 发送时间: 2021年3月10日 20:06 > 收件人: Wubo (lana) <[email protected]>; opsawg <[email protected]>; > [email protected] > 主题: RE: AD review of draft-ietf-opsawg-tacacs-yang-07 > > Hi Bo, > > Sorry, this doc slipped off my radar. > > I've provided comments inline, but I think that it is only the "shared- > secret" part that needs to be resolved. > > > > > -----Original Message----- > > From: Wubo (lana) <[email protected]> > > Sent: 19 September 2020 08:47 > > To: Rob Wilton (rwilton) <[email protected]>; opsawg > > <[email protected]>; [email protected] > > Subject: Re: AD review of draft-ietf-opsawg-tacacs-yang-07 > > > > Hi Rob, > > > > Thanks for the reply. Please see inline. > > > > Best regards, > > Bo > > > > -----邮件原件----- > > 发件人: Rob Wilton (rwilton) [mailto:[email protected]] > > 发送时间: 2020年9月15日 18:53 > > 收件人: Wubo (lana) <[email protected]>; opsawg <[email protected]>; > > [email protected] > > 主题: RE: AD review of draft-ietf-opsawg-tacacs-yang-07 > > > > Hi Bo, > > > > Thanks for addressing my previous comments. > > > > Please see inline ... > > > > > -----Original Message----- > > > From: Wubo (lana) <[email protected]> > > > Sent: 29 August 2020 09:40 > > > To: Rob Wilton (rwilton) <[email protected]>; opsawg > > > <[email protected]>; [email protected] > > > Subject: Re: AD review of draft-ietf-opsawg-tacacs-yang-07 > > > > > > Hi Rob, > > > > > > v-08 is posted, to address most of the your comments in the two AD > > > reviews. > > > https://www.ietf.org/rfcdiff?url2=draft-ietf-opsawg-tacacs-yang-08 > > > > > > There are still some comments to confirm with you. > > > > > > 3. "shared-secret", should that be put under a choice statement? Is > > > it likely that alternative methods of authenticating the server are > > > likely in future? > > > [Bo] This issue has been discussed in WG before, and it was > > > recommended that the module be updated when the new TACACS+ protocol > > > defined. What's your opinion? > > [RW] > > > > I'm not sure I entirely follow. > > > > By "be updated when the new TACACS+ protocol defined", do you mean: > > https://tools.ietf.org/html/draft-ietf-opsawg-tacacs-11, or something > > else? > > > > If it is this draft, then this is a normative reference and will be > > published shortly. I had presumed that this model covered the client > > functionality from draft-ietf-opsawg-tacacs-11? > > > > [Bo] Yes, this model is defined to cover the client functionality from > > draft-ietf-opsawg-tacacs-11 or the latest version. > > "shared-secret" is used to encrypt the TACACS+ packet body, and is the > > only body encryption method defined in the TACACS+ protocol. > > During WG discussion, a similar issue has been discussed and it was > > suggested that in future an augmented model to be defined to reflect > > alternative methods, such as TLS encryption. > [RW] > > So, the problem that I see is that the shared-secret leaf is mandatory. > I.e., even if an alternative method was specified then a shared-secret > would still always need to be specified. I don't know whether that it is > a reasonable constraint to put on the model. > > If there will always be a shared secret then your augmentation approach > should be fine. > > But, if it is plausible or likely that a shared-secret would not always be > required then I would be better to have a mandatory choice statement with > shared-secret as one of the choices. > > > > > > > > > > > > > > 5. Does the tcsplus-server-type indicate what the server is, or how > > > the server is used? E.g., could a server have the authentication > > > bit set, but then not be used for user authentication? Or should > > > that be prevented with a must statement? > > > [Bo]Yes, tcsplus-server-type indicates what type the server is. But > > > I don't quite understand this comment. > > [RW] > > > > The distinction that I was trying to make is: > > > > Server 'S', might have the capabilities of an authentication and > > accounting server, but Network Device 'D', that is making use of > > Server 'S', might only be making use of its authentication > > capabilities. In this scenario, do the configuration bits on device > > 'D' list Server 'S' as supporting authentication and accounting (since > > that is what the server > > supports) or do they only list Server 'S' capabilities as > "authentication" > > (since that is what is being used by 'D')? > > > > If the intended behaviour is the latter, then I think that we should > > tweak the descriptions to be clear. > > > > [Bo] Yes, the intention is the latter. > > Perhaps you mean the YANG descriptions of 'tcsplus-server-type' are > > not clear enough. Please let us know if this addresses your concerns > > or if there is anything else. > > > > typedef tacacs-plus-server-type { > > type bits { > > bit authentication { > > description > > "When set, the server is an authentication server." > > -> > > "When set, the device use the server for authentication > service."; > [RW] > Yes, I think that is fine, but I propose perhaps "Indicates that the > TACACS server is providing authentication services". The other two > comments should be adjusted similarly. > > > > > > > } > > ... > > } > > description > > "tacacs-plus-server-type can be set to > > authentication/authorization/accounting > > or any combination of the three types. When all three types are > > supported, all the three bits are set."; > > -> > > "When all the three bits are set, the device use all available > > services on the server."; > [RW] > > The last sentence probably isn't needed and could just be removed? > > Regards, > Rob > > > > } > > > > > > > > > > > 6. Should there be a limit on the length of a server name? > > > [Bo]The TACACS+ protocol does not have any restrictions, and I also > > > think this model could follow current ietf-system model, since this > > > module is the augmentation of the system model and there is no > > > restriction on the RADIUS server name in the ietf-system model. > > > How do you think? > > [RW] > > > > I agree that being consistent with ietf-system is a good choice. > > > > I do wonder more generally if this will mean that different vendors will > > impose different limits using deviations, which would effectively make > > interop harder. But that doesn't need to be solved here/now. > > > > [Bo]Thanks for the suggestion. > > > > > > > > > > > > > 7. I dont' know whether this matters, but the must statement doesn't > > > seem to be quite complete, in that it would still allow TACACS+ to be > > > listed as an authentication mechanims, but only include an accounting > > > server in the TACACS+ server list. > > > [Bo] Thanks. Agree that the must statement does not prohibit > > > accounting or authorization TACACS+ server configuration. > > > I updated the must statement with authentication server type > validation. > > [RW] > > > > Okay. > > > > Regards, > > Rob > > > > > > > > > > Thanks, > > > Bo > > > > > > -----邮件原件----- > > > 发件人: Rob Wilton (rwilton) [mailto:[email protected]] > > > 发送时间: 2020年8月20日 18:38 > > > 收件人: opsawg <[email protected]>; draft-ietf-opsawg-tacacs- > > > [email protected] > > > 主题: RE: AD review of draft-ietf-opsawg-tacacs-yang-07 > > > > > > Ok, my bad. It seems that I had already done an AD review of this > > > document :-) > > > > > > Bo, there may be some additional comments that you would like to > > > consider below in your -08 update. > > > > > > Regards, > > > Rob > > > > > > > > > > > > > -----Original Message----- > > > > From: Rob Wilton (rwilton) <[email protected]> > > > > Sent: 20 August 2020 11:23 > > > > To: opsawg <[email protected]>; > > > > [email protected] > > > > Subject: AD review of draft-ietf-opsawg-tacacs-yang-07 > > > > > > > > Hi, > > > > > > > > This is my AD review of draft-ietf-opsawg-tacacs-yang-07. Sorry > > > > that it has been a little while in coming. > > > > > > > > Thank you for this document, I believe that it is in good shape. > > > > I've given my slightly more significant comments first, followed by > > > > some editorial comments. > > > > > > > > > > > > COMMENTS: > > > > > > > > "Section 3": > > > > > > > > The "statistics" container under the "server list" is to record > > > > session statistics and usage information during user access which > > > > include the amount of data a user has sent and/or received during > a > > > > session. > > > > > > > > 1. Looking at the module, the statistics only seem to cover the > > > > number of messages rather than the amount of data. Possibly delete > > > > the part of the sentence from "which include" ... to the end of the > > sentence. > > > > > > > > > > > > "Regarding the YANG module": > > > > > > > > 2. I suggest changing "tacacsplus" to "tacacs-plus" (e.g., in the > > > > module title and top level nodes). > > > > > > > > 3. "shared-secret", should that be put under a choice statement? Is > > > > it likely that alternative methods of authenticating the server are > > > > likely in future? > > > > > > > > 4. I'm not sure that the "tacacsplus" feature is required. > > > > Supporting the ietf-system-tacacsplus module should be sufficient to > > > > indicate that the device supports TACACS+ client configuration. > > > > > > > > 5. Does the tcsplus-server-type indicate what the server is, or how > > > > the server is used? E.g., could a server have the authentication > > > > bit set, but then not be used for user authentication? Or should > > > > that be prevented with a must statement? > > > > > > > > 6. Should there be a limit on the length of a server name? > > > > > > > > 7. I dont' know whether this matters, but the must statement doesn't > > > > seem to be quite complete, in that it would still allow TACACS+ to > > > > be listed as an authentication mechanims, but only include an > > > > accounting server in the > > > > TACACS+ server list. > > > > > > > > > > > > "Security section": > > > > /system/tacacsplus/server: This list contains the objects used > to > > > > control the TACACS+ servers used by the device. Unauthorized > > > > access to this list could cause a user management failure on > the > > > > device. > > > > > > > > 8. I don't know TACACS+, but I would presume that the risk of > > > > accessing this list is much greater than just user management > failure. > > > > If it was possible to modify this configuration to point to a > > > > compromised TACACS+ server then would it not be possible to obtain > > > > complete control over the device? If, so I think then I think that > > > > it would be helpful to make this risk clear. [As a nit, we should > > > > probably also use 'data nodes' rather than 'objects'] > > > > > > > > > > > > "References": > > > > > > > > 9. Please can you ensure that your normative references to all RFCs > > > > that define YANG modules that are imported by the YANG modules > > > > defined in this document. From a quick scan, it looked like some > > > > might be > > > missing. > > > > > > > > > > > > > > > > EDITORIAL COMMENTS: > > > > > > > > > > > > Abstract: > > > > 1. that augment -> that augments > > > > 2. in the RFC 7317 with TACACS+ client model. -> in RFC 7317 with a > > > > TACACS+ client data model. > > > > > > > > 3. The data model of Terminal Access Controller Access Control > > > > System Plus (TACACS+) client allows ...-> The Terminal Access > > > > Controller Access Control System Plus (TACACS+) client data model > > > > allows ... > > > > > > > > Introduction: > > > > > > > > 4. This document defines a YANG module that augment the System > > > > Management data model defined in the [RFC7317] with TACACS+ > client > > > > model. > > > > > > > > -> > > > > > > > > This document defines a YANG module that augments the System > > > > Management data model defined in [RFC7317] with a TACACS+ client > > > > data model. > > > > > > > > 5. TACACS+ provides Device Administration -> > > > > TACACS+ provides device administration > > > > > > > > 6. TACACS+ provides Device Administration for routers, network > access > > > > servers and other networked computing devices via one or more > > > > centralized servers which is defined in the TACACS+ Protocol. > > > > [I-D.ietf-opsawg-tacacs] > > > > > > > > -> > > > > > > > > TACACS+ [I-D.ietf-opsawg-tacacs] provides Device Administration > for > > > > routers, network access servers and other networked computing > > devices > > > > via one or more centralized servers. > > > > > > > > 7. o User Authentication Model: Defines a list of usernames and > > > > passwords and control the order in which local or RADIUS > > > > authentication is used. > > > > > > > > -> > > > > > > > > o User Authentication Model: Defines a list of local usernames > and > > > > passwords. It also controls the order in which local or > RADIUS > > > > authentication is used. > > > > > > > > 8. System Management model -> System Management Model? > > > > > > > > 9. The YANG model can be used -> The YANG module can be used > > > > The YANG data model in this document" => The YANG module in this > > > > document > > > > > > > > > > > > "3. Design of the Data Model" > > > > > > > > 10. Recommend changing heading to "Design of the TACAS+ Data Model" > > > > > > > > 11. client on the device -> client on a device > > > > > > > > 12. user's name and password -> user's username and password? > > > > > > > > 13. user and accounting -> user, and accounting > > > > > > > > 14. is intended to augment -> augments > > > > > > > > 15. A couple of places "e.g." should be replaced with "e.g.," > > > > > > > > Appendix A: > > > > > > > > 16. In the example, possibly delete the "single-connection" leaf > > > > since that is a default value. > > > > > > > > Regards, > > > > Rob _______________________________________________ OPSAWG mailing list [email protected] https://www.ietf.org/mailman/listinfo/opsawg
