A YANG Data Model for a Keystore and Keystore Operations
draft-ietf-netconf-keystore-35

Thank you for the work on this document.

Section 1.5:
> Various examples in this document use "BASE64VALUE=" as a placeholder value for binary data has has been base64 encoded. 

Please add a reference to RFC 4648 (Section 4) when mentioning that this document uses base64 encoded value in its examples. (Note that this is different from the companion doc draft-ietf-netconf-crypto-types since this one does not define encoding for binary types the same ways that one does, as far as I can tell. However a ref to the base64 spec should be added when mentioned.)

I support Roman's DISCUSS with respect to the text of Section 5.1.

====

Additional comments from incoming AD, Orie Steele:

Same comment, as I made on draft-ietf-netconf-crypto-types, regarding base64 encoding, consider a direct reference to https://datatracker.ietf.org/doc/html/rfc4648/#section-4

> These examples assume the existence of an example module called "ex-keystore-usage" having the namespace "http://example.com/ns/example-keystore-usage".

Since changing the URL changes the namespace, lets make this https before publishing?

You might also consider using the `.example`, https://en.wikipedia.org/wiki/.example


Under: Protocol accessible nodes

"encrypted-private-key/encrypted-private-key" is this duplication intentional?

> If the KEK is an asymmetric key, then the server MAY provide an API enabling the encryption of other keys or, alternatively, assume the administrators can do so themselves using the asymmetric key's public half.

Why not SHOULD?

> A server MUST possess (or be able to possess, in case the KEK has been encrypted by yet another KEK) a KEK's cleartext value so that it can decrypt the other keys in the configuration at runtime.

Is a cleartext value really needed here? or can the server just maintain an API for the key by reference.

Thank you to Sandra Murphy for the IETF LC SECDIR review and Magnus Nyström for the early SECDIR review.

Thanks for the explanation on the Section 4.* crytpo API.  I recommend adding clarifying language to the effect of the details and authorization model associated with this API is out of scope for this document. 

Thanks for addressing my other DISCUSS and COMMENT feedback.

I support Roman's DISCUSS.

Also, when reading this document I initially got excited, thinking that I'd find more text on the 'hidden keys', but my excitement was short lived...:-)

Assuming you add text to the other YANG crypto types document, perhaps you can include it here too?

Thanks for working on this specification.

I support Roman's discuss point on section 5.1, it needs to be clear on what could allow the exception.

# Éric Vyncke, INT AD, comments for draft-ietf-netconf-keystore-30

Thank you for the work put into this document.

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education).

Special thanks to Qin Wu for the shepherd's write-up (even if 15 months old) including the WG consensus and the justification of the intended status.

I hope that this review helps to improve the document,

Regards,

-éric

# COMMENTS (non-blocking)

## Not repeating comments

I won't repeat here the comments of my ballots on the companion I-Ds about key rollover and missing 'not valid before'. Discussion is under way in separate email threads.

## Section 4.1

`the server MUST provide an API for administrators`, while a valid statement, is not about a YANG data model while both the title and the abstract of this document are specific for the data model. It would have been better to have a less restrictive title, e.g., "Keystore operations and YANG data model".

Same applies for the whole section 4.3: a useful section but unrelated to the I-D title/abstract.

## Section 4.3

Please expand NACM at first use and not later ;)