Hi Dale, Thank you for your review! Please see below for responses to your comments.
Kent > On Jan 22, 2024, at 10:46 AM, Dale Worley via Datatracker <nore...@ietf.org> > wrote: > > 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 > '\'. Egads, how embarrassing ;) Fixed! > 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. Added the following paragraph (to each of the nine drafts in this suite of drafts): Please note that the arrows in the diagram point from referencer to referenced. For example, the "crypto-types" RFC does not have any dependencies, whilst the "keystore" RFC depends on the "crypto-types" RFC. > 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. s/Label in Diagram to RFC Mapping/Label to RFC Mapping/ Good enough? > 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. Agreed, but since it’s always only used for binary data, I elided the “usually” bit, so the sentence now reads: Various examples in this document use "BASE64VALUE=" as a placeholder value for 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? Fixed (great catch!) > 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. I think I’ll not make this change. > 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. Ack. > 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. Fixed. > 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? For the most part, yes, I see your point. Maybe s/The/For the/ or s/The/Regarding the/? In any case, be aware that there exists an IETF-defined template for the Security Considerations section that is to be used for each YANG module defined in a draft. So, if a draft defines the three modules: ietf-foo-common, ietf-foo-client, and ietf-foo-server, the Security Considerations section contains the three subsections: The “ietf-foo-common" YANG Module The “ietf-foo-client" YANG Module The “ietf-foo-server" YANG Module Each containing an instance of the template for that YANG module. > 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: This text comes from the aforementioned template. That said, I agree that it’s not great. Perhaps, even better, “*The following* subtrees and data nodes have particular sensitivities/vulnerabilities”? > [END] Thanks again, Kent
_______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
