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(_at_)outer-planes(_dot_)net>
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
signature.asc
Description: OpenPGP digital signature