Re: Genart last call review of draft-ietf-rtgwg-yang-key-chain-17

["Matthew A. Miller" <[e-mail]>]

Thanks for responding Acee, I look forward to the next revision.

Responses inline ...


- m&m

Matthew A. Miller

On 17/04/08 16:55, Acee Lindem (acee) wrote:
> Hi Matt,
> 
> Thanks for the review.
> 
> On 4/7/17, 1:58 PM, "Matthew Miller" <[e-mail]>
> wrote:
> 
>> Reviewer: Matthew Miller
>> Review result: Almost Ready
>>
>> I am the assigned Gen-ART reviewer for this draft. The General Area
>> Review Team (Gen-ART) reviews all IETF documents being processed
>> by the IESG for the IETF Chair.  Please treat these comments just
>> like any other last call comments.
>>
>> For more information, please see the FAQ at
>>
>> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>>
>> Document: draft-ietf-rtgwg-yang-key-chain-17
>> Reviewer: Matthew A. Miller
>> Review Date: 2017-04-07
>> IETF LC End Date: 2017-04-07
>> IESG Telechat date: 2017-04-13
>>
>> Summary:
>>
>> This document is almost ready to be published as a Proposed Standard,
>> once the issues noted herein are resolved.
>>
>> Major issues:
>>
>> NONE
>>
>> Minor issues:
>>
>> * Forgive me for my limited knowledge of YANG, but is there a reason
>> key-strings are only representable as either a YANG string or
>> hex-string type, and not the YANG binary type?
> 
> Let me ask why I¹d want to use this type? I can get all the entropy I want
> with a hex string and implementations are familiar with this
> representation. I¹m not really fond of the obscure base64 representation
> used by the YANG type and if one consults Benoit¹s search tool, the type
> is not widely used. http://www.yangcatalog.org/yang-search/yang-search.php
> 

Thanks for explaining.  I asked because (more) efficient transfer of
binary data has come up for other textual formats (read: JSON), and the
rough consensus was for base64.  That said, if the community is already
fine with hex, then I consider this a non-issue.

>>
>> * This document does not provide much guidance around AES key wrap
>> other than it can be used and the KEK is provided
>> out-of-band/-context.
>> For instance, AES key-wrapped key-strings probably require using
>> "hexidecimal-string".
> 
> Yes - I¹ll add that.
> 

Thanks.

>>  Also, assuming I'm reading the model
>> correctly,
>> it appears this feature applies to the whole chain, which I think is
>> worth calling out.
> 
> In YANG, if you support a feature, it is for the entire model.
> The per-chain boolean indicates whether or not it is applies to all the
> keys in that particular key-chain. I will clarify this.
>>
>> * This document warns against using the "clear-text" algorithm, which
>> the
>> reader is lead to understand is for legacy implementation reasons.
>> However, is there not a similar concern with cryptographically weak
>> algorithms, such as md5 and (arguably) sha1?
> 
> I can add something for MD5.

Thanks.  I know I said arguably sha1, but at the apps layer it is
considered weak with in-the-wild tools able to produce collisions, which
leads to the minimum being sha256.

I would consider including a note about sha1 and md5, but I understand
that maybe operational realities are such that sha256 is not widely
deployed for these devices, or its not otherwise seen as a problem yet.

>>
>> Nits/editorial comments:
>>
>> * In Section 3.2. "Key Chain Model Features", the word "of" is
>> missing
>> between "configuration" and "an" in the phrase "support configuration
>> an
>> acceptance tolerance".
> 
> Good catch. I will fix.
>>
>> Non-nits:
>>
>> * I note that idnits is calling out some odd spacing issues, but I
>> think
>> they are safe to ignore.
> 
> Though the line numbers don¹t match the draft, I was able to fix these.
> 
> Thanks,
> Acee 
>>
>