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

["Wubo (lana)" <[e-mail]>]

Hi Lada,

Thanks for the review. Please see the response inline.

Regards,
Bo

-----邮件原件-----
发件人: Ladislav Lhotka via Datatracker [mailto:[e-mail]] 
发送时间: 2020年5月4日 21:17
收件人: [e-mail]
抄送: [e-mail]; [e-mail]; [e-mail]
主题: Yangdoctors last call review of draft-ietf-opsawg-tacacs-yang-03

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.'
[Bo] OK, I will change as suggested.

- Description of the leaf
/ietf-system-tacacsplus:tacacsplus/statistics/sessions is cryptic and unclear.
[Bo] OK, I will change as follows:
"Number of sessions completed with the server. If the Single Connection Mode was not enabled, the number of sessions is the same as the number of connection opens. 
If the Mode was enabled, a single TCP connection may contain multiple TACACS+ sessions."

- Typo in error-message of
/ietf-system:system/ietf-system-tacacsplus:tacacsplus: s/sysytem/system/
[Bo] OK, will correct.

- 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.
[Bo] OK, will correct when the final guidance on this issue is received.

- 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.
[Bo] I need to wait for the further guidance from WG. The "choice case" is added based on the email discussion of the WG, which provides some flexibility in specifying the IP address for server communication. Some vendors prefer IP addresses, and some vendors derive IP addresses through interfaces.

- Security Considerations should specifically address the "shared-secret" leaf.
[Bo] OK, will add this and also some other nodes as Tom Petch commented.

- 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.
[Bo] OK, will change Appendix A into an example of TACACS+ configuration.