Reviewer: Dale Worley Review result: Ready with Nits 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: review-draft-ietf-netconf-crypto-types-28 Reviewer: Dale R. Worley Review Date: 2024-01-22 IETF LC End Date: 2024-01-24 IESG Telechat date: [not known] Summary: This draft is basically ready for publication, but has nits that should be fixed before publication. Nits/editorial comments: Editorial Note (To be removed by RFC Editor) Tree-diagrams in this draft may use the '/' line-folding mode defined in RFC 8792. However, nicer-to-the-eye is when the '//' line-folding mode is used. The AD suggested suggested putting a request here for the RFC Editor to help convert "ugly" '/' folded examples to use the '//' folding mode. "Help convert" may be interpreted as, identify what looks ugly and ask the authors to make the adjustment. Throughout this paragraph, slash '/' should be replaced by backslash '\'. 1.1. Relation to other RFCs The dependency relationship between the primary YANG groupings defined in the various RFCs is presented in the below diagram. Perhaps there is a convention that I am not aware of, but when I see in the figure e.g. crypto-types ^ ^ / \ / \ truststore keystore does that mean that crypto-types contains a reference to truststore, or does it mean that truststore contains a reference to crypto-types? The usual convention is that arrows point from referencer to referenced, but also the usual convention is that the referenced thing is written physically below the referencer. Perhaps add an explanatory sentence. Table 1: Label to RFC Mapping In -28, this caption appears visually to be the caption of both the dependency diagram at the top of page 5 and the label-to-RFC mapping table at the bottom of page 5, and so probably should be amended to describe both of them together. 1.4. Conventions Various examples used in this document use a placeholder value for binary data that has been base64 encoded (e.g., "BASE64VALUE="). This would be clearer if it stated directly that the (only) placeholder value used is "BASE64VALUE=". Perhaps Various examples in this document use "BASE64VALUE=" as a placeholder value for (usually binary) data has has been base64 encoded. 2.1.2. Identities +-- csr-format +-- p10-csr-format {p10-csr-format?} This construct ends with "?}", whereas a number of other constructs end with "}?". Are all of these correct? 2.1.3. Typedefs * Additionally, all the typedefs define a type for encoding an ASN.1 [ITU.X680.2021] structure using DER [ITU.X690.2021]. It seems like it would be useful to have a typedef "asn-1-der" that extends "binary", to be used specifically for DER-encoded ASN.1 data, and which in turn is extended here. E.g. binary +-- asn-1-der +-- csr-info +-- csr +-- x509 | +-- trust-anchor-cert-x509 ... Unfortunately, what would make such an extended type valuable is that DER-encoded ASH.1 strings are used in a number of RFCs, which means that this document might not be the best place to introduce such an extended type. 2.3. YANG Module I am no expert on Yang, so my examination of the module itself was superficial. The Datatracker says that Yang doctors looked at -18 on 2021-01-12, which presumably means that -19 reflected their report. The differences between the module in -19 and -28 appear to me to to be minor. 3.5. Strength of Keys Conveyed ... it is desireable ... Wiktionary describes "desireable" as "an archaic form of desirable". The RFC Editor's opinion on this should probably be sought. 3.10. The "ietf-crypto-types" YANG Module The title of this section seems to be uninformative given that 'The "ietf-crypto-types" YANG Module' is the subject of the entire document. Is this title what was intended? Some of the readable data nodes defined in this YANG module may be considered sensitive or vulnerable in some network environments. It is thus important to control read access (e.g., via get, get-config, or notification) to these data nodes. These are the subtrees and data nodes and their sensitivity/vulnerability: The use of "These" in the last sentence does not have an unambiguous referent as I read it. Perhaps "These subtrees/data nodes have these particular sensitivities/vulnerabilities:" Similar considerations apply to the last sentence of: Some of the operations in this YANG module may be considered sensitive or vulnerable in some network environments. It is thus important to control access to these operations. These are the operations and their sensitivity/vulnerability: [END] -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
