Hi Bo, Thanks for addressing my comments, and your work on this document. I've now requested IETF LC on this document.
Regards, Rob > -----Original Message----- > From: Wubo (lana) <[email protected]> > Sent: 12 March 2021 10:31 > To: Rob Wilton (rwilton) <[email protected]>; opsawg <[email protected]>; > [email protected] > Subject: Re: AD review of draft-ietf-opsawg-tacacs-yang-07 > > Hi Rob, > > We have posted a -09 version that fixes the issues from you and Tom Petch. > Please see below: > Diff: https://www.ietf.org/rfcdiff?url2=draft-ietf-opsawg- > tacacs-yang-09 > > Thanks, > Bo > > -----邮件原件----- > 发件人: Rob Wilton (rwilton) [mailto:[email protected]] > 发送时间: 2021年3月11日 19:23 > 收件人: Wubo (lana) <[email protected]>; opsawg <[email protected]>; > [email protected] > 主题: RE: AD review of draft-ietf-opsawg-tacacs-yang-07 > > 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
