Subject: [yang-doctors] Yangdoctors last call review of draft-ietf-opsawg-tacacs-yang-03

From: Ladislav Lhotka via Datatracker <[email-masked]>
Date: Mon, 04 May 2020 06:16:32 -0700

Reviewer: Ladislav Lhotka
Review result: Ready with Nits

The YANG module specified in this I-D defines a relatively simple augmentation
of the "ietf-system" module that enables configuration of TACACS+
authentication. The ietf-system-tacacsplus module is in a good shape, I found
no substantial problems.

**** Comments

- In sec. 3, the text says: 'The ietf-system-tacacsplus module is intended to
augment the "/sys:system" path defined in the ietf-system module with
"tacacsplus" grouping.' It would be more precise to say '... with the contents
of the "tacacsplus" grouping.'

- Description of the leaf
/ietf-system-tacacsplus:tacacsplus/statistics/sessions is cryptic and unclear.

- Typo in error-message of
/ietf-system:system/ietf-system-tacacsplus:tacacsplus: s/sysytem/system/

- Is it correct that the server type may be either one of "authentication",
"authorization" or "accounting", or all of them? Is it impossible for a server
to be authentication & authorization but not accounting? Such a variant cannot
be configured.

- The "case" statements in ietf-system-tacacsplus:tacacsplus/source-type are
unnecessary because each contains only one leaf of the same name; I suggest to
remove them.

- Security Considerations should specifically address the "shared-secret" leaf.

- The purpose of Appendix A is unclear, the information it provides is (or
should be) in the previous text, the YANG module, and RFC 7317. Instead, it
would be useful to provide an example of TACACS+ configuration, e.g. in JSON
representation.

---

From: "Joe Clarke (jclarke)" <[email-masked]>
Date: Mon, 4 May 2020 13:28:57 +0000

Thank you for your review, Lada.  One item came up in WGLC about the server-type leaf (and you have called that out below).

Right now, this is an enumeration.  As you say (and also Tom Petch pointed out) that typically a server will have multiple types (i.e., a server will be used for authn, authz, and acct).  So Bo proposed a leaf-list solution whereby one could specify multiple values for server-type.  Tom counter-proposed a bit string (akin to chmod and NACM CRUDX handling).  As a contributor, I liked the leaf-list idea, but Tom is still leaning toward the bit string.  We wanted to get YAND Doc’s opinion on this.

The full thread can be found at https://mailarchive.ietf.org/arch/msg/opsawg/Q_ov6M-PZF4rlsCae0qZh1aE6tg/ .  If you could provide some guidance on this that would be appreciated.

Joe

On May 4, 2020, at 09:16, Ladislav Lhotka via Datatracker <[email-masked]<mailto:[email-masked]>> wrote:

**** Comments