Re: [netconf] More comments was Shepherd's comments on draft-ietf-netconf-crypto-types

[Kent Watsen <[e-mail]>]

Hi Tom,

Thank you for your review of the crypto-types draft.
Please see below for my responses to your comments.

Kent



> On Jan 12, 2023, at 7:21 AM, tom petch <[e-mail]> wrote:
> 
> Some additional comments unrelated to those of the Shepherd
> 
> RFC6960 is in the module so needs adding to I-D references

Added as a normative reference.


> X.690 in the module lacks a date which makes the reference ambiguous

Added.


> X.690 in the I-D Refs is -2015.  This is not the latest version - I do not know what has changed.

Updated both X.680 and X.690 to 02/2021 (the latest)


> RFC6125 has a bis in WGLC

But we're okay with the current version - yes?  Do we need to MISREF?

The bis doc:
https://datatracker.ietf.org/doc/draft-ietf-uta-rfc6125bis/
Service Identity in TLS
datatracker.ietf.org



> The IETF has abolished the page number which makes s.2.2.1 10 pages of stream of consciousness.  I suggest introducing subsections.  I see some obvious places for them

Subsections added.


> In the same vein I suggest including the subsections of  s.2.1 in the ToC. 

I like the ToC as is.


> I find the use of -grouping in the identifier of a grouping prolix.

Could be, but you're the first to mention it.  I'll note that I started doing this as otherwise things get confusing when scaling up to large models...


> 2.1.3.  Typedefs
>   The following diagram illustrates the relationship amongst the
>   "typedef" statements defined in the "ietf-crypto-types" module:
> Well no it does not illustrate for me without a crib to tell me what is going on

Added word "hierarchal", now "hierarchal relationship".


> s.2.2.1 
> OLD
>   The diagram above uses syntax that is similar to but not defined in
> 
> suggest here and elsewhere
>   The diagram above uses syntax that is similar to but not the same as that in 

Replaced with suggested text.


>    feature p10-based-csrs {
>       description
>         "Indicates that the erver implements support
> /erver/server/

Fixed.


> OLD
>        How the associated algorithm is known is outside the
>          scope of this module.  This statement also applies when
>          the octet string has been encrypted.";
> Seems clumsy; I suggest
> NEW
>         The identity of the associated algorithm is outside the
>          scope of this specification.  This is also true when 
>   the octet string has been encrypted.";

Replaced.


>     grouping asymmetric-key-pair-with-cert-grouping {
>     grouping asymmetric-key-pair-with-certs-grouping {
> These are both described as
> '         Implementations SHOULD assert that certificates contain '
> ie plural in both cases.  
Fixed.

> I think a more comprehensive description is needed of what the difference is between the two. 
Made description more comprehensive.

> Also, here is a case where that terminal -grouping is not just redundant but is likely to cause errors.
>     grouping asymmetric-key-pair-with-cert {
>     grouping asymmetric-key-pair-with-certs {
> would make the difference slightly more obvious but depending on what the difference is meant to be, which I do not see in the description clause, I think one or both identifiers need changing
Updated the description clauses.

Kent


> 
> Tom Petch
> 
> ________________________________________
> From: netconf <[e-mail]> on behalf of Kent Watsen <[e-mail]>
> Sent: 12 December 2022 16:07
> 
> Thank you Mahesh for your comments.
> 
> See below for responses.
> 
> K.
> 
> 
> 
> On Dec 4, 2022, at 10:40 PM, Mahesh Jethanandani <[e-mail]<mailto:[e-mail]>> wrote:
> 
> Hi Kent,
> 
> The YANG Data Types and Grouping for Crypto Types draft is short, well written, and easy to understand. Thanks for including plenty of examples on how to use the model.
> 
> There are however a few minor comments that would nice to address before forwarding the document for AD review.
> 
> Thanks again.
> 
> 
> Section 1.1 - Relation to other RFCs
> 
> Now that the set of modules and drafts in the “client-server” suite of drafts is known, can the language in this section be updated to reflect it. Specifically, can the second paragraph be removed or replaced because you list the drafts (if not the modules themselves) in the dependency graph.
> 
> Removed, and cleaned-up the language in the next paragraph.   Now reads:
> 
>    This document presents one or more YANG modules [RFC7950]
>    that are part of a collection of RFCs that work together
>    to, ultimately, enable the configuration of both the clients
>    and servers of both the NETCONF [RFC6241] and
>    RESTCONF [RFC8040] protocols.
> 
>    The normative dependency relationship between the various RFCs
>    in this collection is presented in the below diagram. The labels
>    in the diagram represent the primary purpose provided by each
>    RFC.  Hyperlinks to each RFC are provided below the diagram.
> 
> This change will effect all in the suite of drafts.
> 
> 
> Section 2.1.1 Features
> 
> This is more of a nit, and would not mind if the comment is ignored. Can we remove the leading | (pipe symbol) in front of the sentence - “The diagram above uses syntax …”. The same is true for other “tree diagrams” that follow RFC 8340 syntax. I do understand that other documents in the cluster follow that format, but it is not clear what the pipe symbols mean or why the text needs to be formatted differently from the rest of the text in the document.
> 
> This is how the <aside> element, in the xml2rfc file, renders.  I wish that it rendered as a box but, alas, such is not the case.  In any case, Juergen made the same comment before, so it seems pretty annoying, so I removed all of them, in the entire suite of drafts.
> 
> 
> 
> Section 2.1.1 Identities
> 
> s/format that key data/format for key data/
> 
> Fixed.
> 
> 
> Section 2.1.4.* Groupings
> 
> Most of the grouping display an abridged and a full tree diagram. But the difference between them are a few extra lines. If the diagrams were truly different in size, i.e., the extra number of lines in the full tree diagram was more the size of the abridged tree diagram, I could have understood displaying both of them. Are the two diagrams really necessary in every section?
> 
> Not necessary.  I just removed the "expanded" diagrams.  Now this draft is more like others in the suite of drafts
> 
> 
> Thanks again,
> Kent // author
> 
>