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

[Ladislav Lhotka <[e-mail]>]

Resending to the mailing lists - I changed my company mail address to [e-mail] and it causes problems once in a while.

Lada

-------------------- Start of forwarded message --------------------
From: Ladislav Lhotka <[e-mail]>
Date: Tue, 05 May 2020 16:16:27 +0200

"Joe Clarke (jclarke)" <[e-mail]> writes:

> 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.,

In fact, Tom pointed out other things that I also had in my review (I didn't know about that thread before, I swear:-).

> 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.

I was also thinking about the "bits" type, it is IMO designed for cases like this. Note that in XML or JSON representation (not in CBOR, though), the on-the-wire instance encoding is also self-describing, e.g.

<server-type>authentication authorization</server-type>

Lada

>
> 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 <[e-mail]<mailto:[e-mail]>> wrote:
>
> 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.
>
>
>

-- 
Ladislav Lhotka 
Head, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67
-------------------- End of forwarded message --------------------

-- 
Ladislav Lhotka 
Head, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67