> On 20 Feb 2017, at 13:53, Acee Lindem (acee) <acee@xxxxxxxxx> wrote: > > Hi Lada, > I believe we¹ve addressed all of these other than adding an explicit leaf > for key direction (which was discussed on the RTGWG list). The current > version is https://www.ietf.org/id/draft-ietf-rtgwg-yang-key-chain-15.txt Yes, I saw it, looks good. Thanks, Lada > > Thanks, > Acee > > On 2/20/17, 6:49 AM, "Ladislav Lhotka" <lhotka@xxxxxx> wrote: > >> Reviewer: Ladislav Lhotka >> Review result: Almost Ready >> >> # General Comments >> >> ## Cryptographic algorithm types >> >> What is the reason for representing these as a YANG choice with empty >> leaves? I think it would be more natural to use a single leaf, either >> an enumeration or (if extensibility is important) identityref. >> >> ## Reusability >> >> The module defines key-chain as a grouping with the aim of making it >> reusable in other modules. However, this approach has known problems >> that are discussed in draft-ietf-netmod-schema-mount. I am not sure >> how relevant they are in this case but, for one, the "key-chain-ref" >> type is not applicable if the "key-chain" grouping is used in another >> module. An alternative is not to use the grouping and rely on schema >> mount. >> >> ## Key string style >> >> The difference between ASCII and hexadecimal formats of key strings >> should be explained. I understand that the latter is a hash of the key >> and, if so, I'd suggest to include "hexadecimal-string" also in state >> data. >> >> Also, I believe that storing clear-text key in configuration is >> insecure and Security Considerations should warn against it. >> >> ## Example >> >> It might be useful to include an appendix with example instance data. >> >> # Specific comments >> >> ## Sec. 2 >> >> - paragraph 2: s/where ever/wherever/ >> >> ## Sec. 3 >> >> - paragraph 1: replace both Key-Id a Key-ID with Key ID (the latter >> is used in other places of the >> text). >> - paragraph 2: the suggested way of supporting asymmetric keys looks >> like a hack, I would suggest >> a more explicit representation, e.g. using a choice. >> >> ## Sec. 4 >> >> - The module has inconsistent indentation: up to "grouping >> crypto-algorithm-types", top-level >> statements are indented with four spaces, the subsequent ones with >> five spaces. >> >> ## Sec. 6 >> >> - The statement "Given that the key chains themselves are sensitive >> data, it is RECOMMENDED >> that the NETCONF communication channel be encrypted." is >> misleading because RFC 6241 >> requires that transport protocols for NETCONF guarantee >> confidentiality (and RFC 8040 does the >> same for RESTCONF). >> >> > -- Ladislav Lhotka, CZ.NIC Labs PGP Key ID: 0xB8F92B08A9F76C67