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. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
