A YANG Data Model for Terminal Access Controller Access-Control System Plus (TACACS+)
draft-ietf-opsawg-tacacs-yang-12

As anticipated, I am clearing my DISCUSS (below) following the discussion the IESG has had regarding the intended track of YANG documents, and having the document followed all correct process.

Francesca

========== Original Ballot - 2021-04-21 ==============

Thank you for the work on this document.
This is a discuss DISCUSS - while reading this document and considering the normative downref to RFC 8907 TACACS+, which is informational, I agree with Elliot [1] that to me this document would make more sense as informational. I have followed the mail thread and saw the authors' responses, which quoted RFC 3967 :

   o  A standards document may need to refer to a proprietary protocol,
      and the IETF normally documents proprietary protocols using
      informational RFCs.

I am not convinced that this is one of the cases that this bullet was supposed to cover.
Additionally, I could not find in meeting minutes that this was ever discussed in the wg, as was suggested in the mail thread [2]. I'd like to know if the resp AD is aware of any related discussion after this point was raised.

Another point the authors made in favor of keeping this std track was that they haven't seen any YANG data model published as informational. Again, I am not convinced that this is reason enough to progress this as std track.

I note that this was reported in the shepherd write up [3] and in the last call [4], so won't block progress after a discussion, but I do think it is worth talking about. Please let me know if I missed anything.

Thanks,
Francesca

[1] https://mailarchive.ietf.org/arch/msg/opsawg/2mRkaXy5M9XCPp4_wXNpQd9GLdk/ 
[2] https://mailarchive.ietf.org/arch/msg/opsawg/MOnCfYBS3j4wBnZWDjl_YQHfvzg/
[3] https://datatracker.ietf.org/doc/draft-ietf-opsawg-tacacs-yang/shepherdwriteup/
[4] https://mailarchive.ietf.org/arch/msg/opsawg/FJmtUtB0x8tV0MUdO9Uhvc2e1p0/

Below I've provided some small editorial suggestions in the form of a diff against the plain text version of the draft. You may choose to incorporate them in some way, or ignore them as you see fit. There is no need to let me know what you did with these suggestions.

--- draft-ietf-opsawg-tacacs-yang-10.txt	2021-04-20 21:02:05.000000000 -0400
+++ draft-ietf-opsawg-tacacs-yang-10-jgs.txt	2021-04-20 21:04:01.000000000 -0400
@@ -151,7 +151,7 @@
 
 3.  Design of the TACACS+ Data Model
 
-   This module is used to configure TACACS+ client on a device to
+   This module is used to configure a TACACS+ client on a device to
    support deployment scenarios with centralized authentication,
    authorization, and accounting servers.  Authentication is used to
    validate a user's username and password, authorization allows the
@@ -160,7 +160,7 @@
    user who has accessed the device.
 
    The ietf-system-tacacs-plus module augments the "/sys:system" path
-   defined in the ietf-system module with the contents of the"tacacs-
+   defined in the ietf-system module with the contents of the "tacacs-
    plus" grouping.  Therefore, a device can use local, RADIUS, or
 
 
@@ -416,7 +416,7 @@
         type yang:counter64;
         description
           "Number of aborted connections to the server. These do
-           not include connections that are close gracefully.";
+           not include connections that are closed gracefully.";
       }
       leaf connection-failures {
         type yang:counter64;
@@ -528,7 +528,7 @@
               description
                 "The shared secret, which is known to both the
                  TACACS+ client and server. TACACS+ server
-                 administrators should configure shared secret of
+                 administrators should configure a shared secret of
                  minimum 16 characters length.
                  It is highly recommended that this shared secret is
                  at least 32 characters long and sufficiently complex
@@ -644,13 +644,13 @@
 
    /system/tacacsplus/server:  This list contains the data nodes used to
       control the TACACS+ servers used by the device.  Unauthorized
-      access to this list could cause a complete control over the device
+      access to this list could enable an attacker to assume complete control over the device
       by pointing to a compromised TACACS+ server.
 
    /system/tacacsplus/server/shared-secret:  This leaf controls the key
       known to both the TACACS+ client and server.  Unauthorized access
       to this leaf could make the device vulnerable to attacks,
-      therefore has been restricted using the "default-deny-all" access
+      therefore it has been restricted using the "default-deny-all" access
       control defined in [RFC8341].  When setting, it is highly
       recommended that the leaf is at least 32 characters long and
       sufficiently complex with a mix of different character types i.e.

Thank you to Yaron Sheffer for the SECDIR review, and for the changes in response.

Thanks for addressing my DISCUSS around compatibility with RFC8907 and the COMMENT feedback.

Per my DISCUSS on publishing this document as a proposed standard and characterizing it as new functionality that standardizes an API to an insecure protocol (that is itself has informational status), I will clear per the discussion had in the IESG and subsequent document updates:

-- With the updates in -11 and -12, the YANG module is now feature equivalent to RFC8907 (so there isn't any new functionality). 

-- As to the API mechanism being new functionality, I believe that it is.  Unlike RFC8907, publication of this document doesn't seem like it would be the inform the design of a successor protocol. Nevertheless, there are operational realities of how widely TACACS+ is already deployed that necessitates an improved ability (this document) to manage the as-is infrastructure while an improved protocol is defined.

-- As to the issue of the underlying protocol having a “lower” status (information for RFC8907) than the associated YANG module (PS for this document), I leave that to the convention of OPS area.

Thank you for the work put into this document. 

Please find below some non-blocking COMMENT points (but replies would be appreciated), and one nit.

I hope that this helps to improve the document,

Regards,

-éric

== COMMENTS ==

I like to be able to specify the VRF via vrf-instance as well as the source-interface, which is not the case of RFC 7317 (if not mistaken).

-- Section 4 --
Should the leaf "server-type" also be mandatory ?

== NITS ==

-- Section 2.1 --
s/Tree diagrams used/The tree diagram used/ as there is a single tree ;-)

I have very little to add that was not already mentioned by my
colleagues (but I agree with Roman and Francesca that a compelling case
for standards-track has not been visibly made).  These are basically
nit-level comments.

Section 3

   authorization, and accounting servers.  Authentication is used to
   validate a user's username and password, authorization allows the
   user to access and execute commands at various command levels

I think that RFC 8907 uses the term "privilege level" for what is being
referred to as "command level" here.

Section 4

        leaf timeout {
          type uint16 {
            range "1..300";
          }

How was the maximum of 300 seconds chosen?