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) <lana.w...@huawei.com> > Sent: 19 September 2020 08:47 > To: Rob Wilton (rwilton) <rwil...@cisco.com>; opsawg <opsawg@ietf.org>; > draft-ietf-opsawg-tacacs-yang....@ietf.org > 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:rwil...@cisco.com] > 发送时间: 2020年9月15日 18:53 > 收件人: Wubo (lana) <lana.w...@huawei.com>; opsawg <opsawg@ietf.org>; > draft-ietf-opsawg-tacacs-yang....@ietf.org > 主题: 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) <lana.w...@huawei.com> > > Sent: 29 August 2020 09:40 > > To: Rob Wilton (rwilton) <rwil...@cisco.com>; opsawg > > <opsawg@ietf.org>; draft-ietf-opsawg-tacacs-yang....@ietf.org > > 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:rwil...@cisco.com] > > 发送时间: 2020年8月20日 18:38 > > 收件人: opsawg <opsawg@ietf.org>; draft-ietf-opsawg-tacacs- > > yang....@ietf.org > > 主题: 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) <rwil...@cisco.com> > > > Sent: 20 August 2020 11:23 > > > To: opsawg <opsawg@ietf.org>; > > > draft-ietf-opsawg-tacacs-yang....@ietf.org > > > 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 OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
