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" <linuxwolf+ietf@xxxxxxxxxxxxxxxx> > 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 >> >
Attachment:
signature.asc
Description: OpenPGP digital signature