Re: [netconf] [Gen-art] Genart last call review of draft-ietf-netconf-crypto-types-28

[Lars Eggert <[e-mail]>]

Dale, thank you for your review. I have entered a No Objection ballot for this document.

Lars


> On Jan 22, 2024, at 17:46, Dale Worley via Datatracker <[e-mail]> 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
> '\'.
> 
> 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]
> 
> 
> 
> _______________________________________________
> Gen-art mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art