| 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 <noreply@xxxxxxxx> 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
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”?
Thanks again, Kent
|
