Re: [yang-doctors] Review of draft-ietf-rtgwg-yang-key-chain-13

[Ladislav Lhotka <[e-mail]>]

> On 20 Feb 2017, at 13:53, Acee Lindem (acee) <[e-mail]> 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" <[e-mail]> 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