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