Re: [netconf] [Last-Call] Yangdoctors last call review of draft-ietf-netconf-trust-anchors-13

[Juergen Schoenwaelder <[e-mail]>]

See inline, I cut what I feel has been resolved or does not deserve a
further comment.

On Thu, Feb 11, 2021 at 12:05:01AM +0000, Kent Watsen wrote:
> 
> > On Jan 12, 2021, at 1:41 PM, Jürgen Schönwälder via Datatracker <[e-mail]> wrote:
> > 
> > Reviewer: Jürgen Schönwälder
> > Review result: Ready with Issues
> > 
> > The crypto modules aim at providing a flexible reusable infrastructure
> > of groupings for modeling cryptographic keys and related concepts. The
> > flexibility of the definitions provided of course comes with a certain
> > amount of complexity.
> > 
> >> From a YANG perspective, draft-ietf-netconf-trust-anchors-13.txt is in
> > a good and close to publish state (a couple of minor issues left).  I
> > also tried to understand what is being modeled here and hence I also
> > have some questions concerning the concepts modeled and I hope these
> > are easy to answer/resolve as well.
> > 
> > - I have compiled the YANG modules using yanglint 0.16.105.
> > 
> > - The prefix 'ts' is rather short, the set of two letter prefixes is
> >  rather small limited. This comment also applies to the other
> >  documents, crypto-types uses 'ct. Perhaps this is not a problem
> >  since collisions can be handled but if we go for rather short
> >  prefixes, we will have to exercise the collision resolution. (I see
> >  that 'ct' has already been used by ietf-complex-types, RFC 6095.) A
> >  possible alternative could be to use sec-ct, sec-ts, sec-ks, ...).
> 
> Ugh (me, whenever a naming-discussion comes up ;)
> 
> <rant>I don’t know why prefixes are registered, as the prefix is defined at the top of each module.  The only value I see is not syntactic but rather for consistency, to aid human readability across disconnected sets of modules.</rant>
> 
> Prefixing the prefix with “sec-“ may work for trio of drafts you just reviewed, but it seems undesirable when getting to the client-server drafts, as they're more *protocol* oriented.  If “ks” and “ts” aren’t taken, then maybe we claim “first mover” advantage?  If not, then maybe “kstore” and “tstore” might be meaningful fallbacks?  Assuming we claim “first mover” advantage for “ks” and “ts”, then we just need to handle “ct”, which maybe could be “ctypes” or “cryptypes” or “ct2”?  So, 1) prefix the trio or 2) claim first-mover advantage on two and figure out something for “ct”?  …or maybe 3) keep “ct” also noting that a) “SHOULD NOT” is not a “MUST NOT” (in the text below), b) my “rant” might have some merit, and c) short and terse is nice (per your next comment).
>

I suggest you think over it and pick whatever you think seems to be a
good solution. ;-)

> > - Is the feature truststore-supported really needed? Does the YANG
> >  library not already provide the information whether a module has
> >  been implemented or just imported to access type and grouping
> >  definitions?
> 
> This was discussed before.  The bottom of Section 2.1.4 says: "The reason for why the "truststore-grouping" exists separate from the protocol-accessible nodes definition is to enable instances of the truststore to be instantiated in other locations, as may be needed or desired by some modules.”.  Case in point, the MacOS “Keychain Access” utility maintains both system-level and user-level trust anchors.  [UPDATE: you had a similar comment in the keystore draft, which you self-resolved there and then said to ignore this comment.  Just the same, let’s ensure the drafts are clear.]

This explanation gives a reason why there are groupings. My question
concerned the feature truststore-supported. Is the YANG library not
already providing the information whether a module has been
implemented or only used for getting access to definitions? In RFC
8525, you have a list of modules implemented and a list of modules
used for import only. My question is does feature truststore-supported
mean the same as the module shows up in the implemented module set in
the YANG library?

> > - Does it make sense to be explicit about the fact that the cert/key
> >  references are all weak references in the sense that they can exist
> >  without the corresponding cert/key being present? In other words, in
> >  order to safely delete a cert/key, one would have to check that
> >  there are no dangling references left (which is difficult in case
> >  references are scattered over multiple (proprietary) data models).
> >  For YANG savvy people this may be obvious since there is no
> >  require-instance statement in the leafref type definition but not
> >  every implementer may recall this - and it would be good to document
> >  that using weak references was an explicit design decision and not
> >  an oversight.
> 
> Correct, there are no "require-instance” statements, but the "require-instance” statement defaults to “true”.  Do you mean the text should be explicit that strong references are enforced?

Oops, my understanding of YANG is aging. Since referential integrity
is required to be valid, nothing needs to be said I think. Ignore my
comment.

I have looked through the diffs, looked all good to me.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: [phone]         Campus Ring 1 | 28759 Bremen | Germany
Fax:   [phone]         <https://www.jacobs-university.de/>