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

From: Matthew Miller <[email-masked]>
Date: Fri, 07 Apr 2017 10:58:52 -0700

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?

* 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".  Also, assuming I'm reading the model
correctly,
it appears this feature applies to the whole chain, which I think is
worth calling out.

* 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?

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

Non-nits:

* I note that idnits is calling out some odd spacing issues, but I
think
they are safe to ignore.

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Sat, 8 Apr 2017 22:55:35 +0000

Hi Matt,

Thanks for the review.

>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

>"hexidecimal-string".

Yes - I¹ll add that.

>  Also, assuming I'm reading the model
>correctly,

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.
>algorithms, such as md5 and (arguably) sha1?

I can add something for MD5.
>between "configuration" and "an" in the phrase "support configuration

Good catch. I will fix.
>* I note that idnits is calling out some odd spacing issues, but I

Though the line numbers don¹t match the draft, I was able to fix these.

Thanks,
Acee

---

From: "Matthew A. Miller" <[email-masked]>
Date: Mon, 10 Apr 2017 10:05:09 -0600

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

Responses inline ...

- m&m

Matthew A. Miller

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

>> For instance, AES key-wrapped key-strings probably require using

Thanks.

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