Hi Bo, Thank you for your quick response. I am OK with the clarifications and the additional text. Best, Yaron On 5/11/20, 06:01, "Wubo (lana)" <lana.wubo@xxxxxxxxxx> wrote: Hi Yaron, Thanks for the review. Please see inline. Regards, Bo -----邮件原件----- 发件人: Yaron Sheffer via Datatracker [mailto:noreply@xxxxxxxx] 发送时间: 2020年5月9日 1:08 收件人: secdir@xxxxxxxx 抄送: opsawg@xxxxxxxx; draft-ietf-opsawg-tacacs-yang.all@xxxxxxxx; last-call@xxxxxxxx 主题: Secdir last call review of draft-ietf-opsawg-tacacs-yang-03 Reviewer: Yaron Sheffer Review result: Has Nits This document defines a YANG module for the configuration of TACACS+ clients. The document is short and straightforward, and I only have one significant comment. * I am not familiar with common security practices for the devices covered by this protocol. But I am wondering, should the "shared-secret" field be made optional, so that it can be entered "out of band" in applications that prefer not to keep it stored in the YANG configuration store and available to network management tools? [Bo] The "shared-secret" node is indeed sensitive. But there are two main reasons for defining as mandatory. 1) The TACACS+ protocol requires that the secret must be configured. 2) YANG model can use NACM (RFC8341) to ensure node security. The "shared-secret" adds security tagging "nacm:default-deny-all" to restrict only initial device access and some recovery session. Additionally, the definition follows the current System model (RFC7317) , as TACACS+ model is an augmentation of the System model. The definition of the "shared-secret" in the RADIUS authentication of the System model is mandatory and YANG extension "nacm:default-deny-all" is used to protect. Perhaps some addition text could help: /system/tacacsplus/server/shared-secret: Access to this node is considered sensitive and therefore has been restricted using the "default-deny-all" access control defined in [RFC8341]. * Not a security comment: the YANG module includes a reference to draft-ietf-opsawg-tacacs-18, but I assume that you'll want to replace it with the RFC number for that draft once it is published. Yet I don't see an RFC Editor note mentioning that. [Bo] OK, will add in the next revision. * It is confusing that "messages-received" is for messages received by the server, and "errors-received" is for errors received *from* the server. [Bo] Thanks, will correct to "from" the server to keep consistency. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call