Looking at -08 Title suggest /yang/YANG/
leaf vrf-instance might benefit from a reference to RFC8529 (ie I did not know where to look:-) security /cause the device vulnerable to attacks/make the device vulnerable to attacks/ IANA Namespace /yang: ietf/yang:ietf/ Tom Petch ________________________________________ From: OPSAWG <[email protected]> on behalf of Rob Wilton (rwilton) <[email protected]> Sent: 20 August 2020 11:38 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 _______________________________________________ OPSAWG mailing list [email protected] https://www.ietf.org/mailman/listinfo/opsawg
