[netconf] Genart last call review of draft-ietf-netconf-keystore-29

[Reese Enghardt via Datatracker <[e-mail]>]

Reviewer: Reese Enghardt
Review result: Ready with Issues

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://wiki.ietf.org/en/group/gen/GenArtFAQ>.

Document: draft-ietf-netconf-keystore-29
Reviewer: Reese Enghardt
Review Date: 2024-01-22
IETF LC End Date: 2024-01-24
IESG Telechat date: Not scheduled for a telechat

Summary: The document is clear and fairly concise, and seems useful. It is
ready to be published after confirming that all important security
considerations have been documented.

Major issues: None, assuming the use of clear text keys has been sufficiently
discussed and its impacts carefully considered.

Minor issues:

I was surprised to see that the model allows to store keys in the clear. I
assume that the WG has carefully considered the security implications here, and
at least Section 4.2 says a key SHOULD be encrypted. Still, I wonder, if it's
worth giving a bit of context in the Introduction, maybe saying why clear text
keys are supported but that they SHOULD NOT be used unless there's no other way?

Introduction:
"This document intends to support existing practices; it does not intend to
define new behavior for servers to implement." This document is a Proposed
Standard, so I find the above sentence a bit confusing. If the point here is
that many or most server implementations already support the features and
conform to the behavior defined in this document, that's great. But otherwise I
assume we do want the implementations to change so they support the features or
conform to the behavior? If I got the intention of this sentence right, I
suggest changing it to "This document is intended to reflect existing practices
that many server implementations already support at the time of writing" or
some such.

Section 5 (Security Considerations):
Given the sensitive nature of the data stored using this model, I suggest
another pass at this section following RFC 3552 (BCP 72): Are there any other
possible attacks on confidentiality, integrity, authenticity, or availability,
which are in scope for this section? Is there any specific threat model that
needs to be considered? I also suggest naming some of the possible impacts of
these attacks. Some possible impacts seem obvious, but I wonder if I'm missing
any significant impacts because I'm not very familiar with the model.

Section 5.3:
"access control mechanisms like NACM SHOULD be used to limit access to the
key's secret data to only the most trusted authorized clients (e.g., an
organization’s crypto officer)." Does "client" refer to a person here, rather
than the RFC 6241 definition of client, which the document says it uses? If so,
I suggest finding a different term here.

Nits/editorial comments:

Section 5.1:
"The YANG module defined in this document defines a mechanism called a
"keystore" that, by its name, suggests that it will protect its contents from
unauthorized disclosure and modification." This might be semantics, but: In my
mind, the name suggests that there's sensitive data, which implies that the
data needs to be protected, with obvious consequences otherwise. Please
consider rephrasing this sentence.