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
