Hi Joe, Review comments also attached as txt file.
Thanks, Rob > -----Original Message----- > From: Joe Clarke (jclarke) <jcla...@cisco.com> > Sent: 10 July 2020 23:15 > To: Rob Wilton (rwilton) <rwil...@cisco.com> > Cc: draft-ietf-opsawg-tacacs-yang....@ietf.org; opsawg <opsawg@ietf.org> > Subject: Re: AD review of draft-ietf-opsawg-tacacs-yang-07 > > Thanks, Rob. Maybe it’s just me, but your email is truncated as you can > see below. I also didn’t see any attachment. > > Joe > > > On Jul 10, 2020, at 12:52, Rob Wilton (rwilton) <rwil...@cisco.com> > wrote: > > > > Apologies for the delay, but please find my AD review of the TACACS+ > YANG module draft. > > > > I would like to thank the authors for their work on this document, and > the WG for providing reviews and input in this document. > > > > I believe that the document is in good shape but propose some minor > changes to some of the wording in places. > > > > One particular question that I would like to pull to the top is the > naming of the module and identifiers: > > These generally use "tacacsplus", but I think that "tacacs-plus" might > be better and more readable. > > > > > > Full comments are inline in the document below (marked as #) > > > > > > The YANG model can be used with network management protocols such as > > NETCONF[RFC6241] to install, manipulate, and delete the configuration > > of network devices. > > > > Abstract > > > > This document defines a YANG module that augment the System > > Management data model defined in the RFC 7317 with TACACS+ client > > model. The data model of Terminal Access Controller Access > Control > > System Plus (TACACS+) client allows the configuration of TACACS+ > > servers for centralized Authentication, Authorization and > Accounting
TACACS+ AD review The YANG model can be used with network management protocols such as NETCONF[RFC6241] to install, manipulate, and delete the configuration of network devices. Abstract This document defines a YANG module that augment the System Management data model defined in the RFC 7317 with TACACS+ client model. The data model of Terminal Access Controller Access Control System Plus (TACACS+) client allows the configuration of TACACS+ servers for centralized Authentication, Authorization and Accounting. # Perhaps tweak the first paragraph of the abstract slightly to: This document defines a TACACS+ client YANG module, that augments the System Management data model, defined in RFC 7317, to allow devices to make use of TACACS+ servers for centralized Authentication, Authorization and Accounting. This document defines a YANG module that augment the System Management data model defined in the [RFC7317] with TACACS+ client model. # augment -> augments # with TACACS+ client -> to support the configuration and management of TACACS+ clients. 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+ provides -> "TACACS+ [I-D.ietf-opsawg-tacacs] provides" [and remove the reference at the end of the paragraph]. # networked computing devices -> networked devices # centralized servers which ... -> delete from which ... to the end of the sentence. The System Management Model [RFC7317] defines two YANG features to support local or RADIUS authentication: #two YANG features -> separate functionality #or -> and o User Authentication Model: Defines a list of usernames and passwords and control the order in which local or RADIUS authentication is used. # I suggest modifying this to -> o User Authentication Model: Defines a list of usernames with associated passwords and a configuration leaf to decide the order in which local or RADIUS authentication is used. o RADIUS Client Model: Defines a list of RADIUS servers that a device uses. # device uses. -> devices uses to manage users. Since TACACS+ is also used for device management and the feature is not contained in the System Management model, this document defines a YANG data model that allows users to configure TACACS+ client functions on a device for centralized Authentication, Authorization and Accounting provided by TACACS+ servers. # I suggest rewording this paragraph to something like: The System Management Model is augmented with the TACACS+ YANG module defined in this document to allow the use of TACACS+ servers as an alternative to RADIUS servers or local user configuration. Zheng, et al. Expires December 22, 2020 [Page 2] Internet-Draft TACACS+ YANG model June 2020 The YANG model can be used with network management protocols such as NETCONF[RFC6241] to install, manipulate, and delete the configuration of network devices. # I would suggest deleting "to install ..." to the end. The ietf-system-tacacsplus module is intended to augment the "/sys:system" path defined in the ietf-system module with the contents of the"tacacsplus" grouping. Therefore, a device can use local, Remote Authentication Dial In User Service (RADIUS), or Terminal Access Controller Access Control System Plus (TACACS+) to validate users who attempt to access the router by several mechanisms, e.g. a command line interface or a web-based user interface. #intended to augment -> augments #I think that you should just use RADIUS and TACACS+ here rather then spelling our the full names. The "server" list is directly under the "tacacsplus" container, which holds a list of TACACS+ servers and uses server-type to distinguish between the three protocols. The list of servers is for redundancy. # I was confused by "the three protocols" (thought you meant RADIUS, TACACS+ and local), hence suggest explicitly listing the AAA elements here. Most of the parameters in the "server" list are taken directly from the TACACS+ protocol [I-D.ietf-opsawg-tacacs], and some are derived from the various implementations by network equipment manufacturers. For example, when there are multiple interfaces connected to the TACACS+ client or server, the source address of outgoing TACACS+ packets could be specified, or the source address could be specified through the interface setting, or derived from the out-bound interface from the local FIB. For the TACACS+ server located in a Virtual Private Network(VPN), a VRF instance needs to be specified. # Unclear what is meant by "or the source address could be specified through the interface setting"? # out-bound -> outbound 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. # Does it measure the amount of data sent or recieved, or the number of messages? # Also the statistics don't appear to be per user at all, but instead per server? 4. TACACS+ Client Module This YANG module imports typedefs from [RFC6991]. # Do you want to list the RFCs of the other modules that are referenced in the YANG module? <CODE BEGINS> file "ietf-system-tacacsp...@2020-05-22.yang" module ietf-system-tacacsplus { # This might be worth discussing, but I'm not sure whether "tacacs-plus" wouldn't be better than "tacacsplus" in all the identifiers below. typedef tcsplus-server-type { # I think that this should be "tacacsplus-server-type", shortening the name here is probably not helpful. feature tacacsplus { description "Indicates that the device can be configured as a TACACS+ client."; reference "RFC XXXX : The TACACS+ Protocol "; } # This feature isn't required and can be deleted. Support for TACACS+ is implicit by whether or not this YANG module is supported. list server { key "name"; ordered-by user; description "List of TACACS+ servers used by the device."; leaf name { type string; description "An arbitrary name for the TACACS+ server."; # Any restrictions? Are spaces allowed in the TACACS+ server name? Does the TACACS+ protocol limit this at all? }
_______________________________________________ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg