Subject: Re: [yang-doctors] [netconf] Yangdoctors last call review of draft-ietf-netconf-keystore-20

From: Juergen Schoenwaelder <[email-masked]>
Date: Fri, 16 Apr 2021 14:37:49 +0200

Trimming to the essential parts...

On Thu, Feb 11, 2021 at 12:05:31AM +0000, Kent Watsen wrote:

> > - Is the feature keystore-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? OK, I know see that this is used to make definitions
> >  conditional, hence it makes sense. This means that my comment on
> >  truststore-supported in the other review can be ignored, I found
> >  the answer.
> 
> Please see my response to your comment in that draft’s review.  The net-net appears to us just needing to ensure things are clear enough.
>

See also my comment on the other document. My question was whether
this bit can already be picked up from the YANG library.

> BTW, “keystore-supported” may not be the best name.  “keystore-implemented” is closer, but maybe “keystore-module-implemented” or “central-keystore-implemented would be better still?   One question is if the “central” (or “global” or default?) prefix should be a term used throughout the draft.   The latest update has text like “… the keystore, when this module is implemented”, but this might be better if, e.g., “… the central keystore, when this module is implemented”.  Thoughts? 

I think this is better.
 
> >  YANG's reference mechanism via leafrefs is not really supporting
> >  well what you try to do. I understand the flexibility you want to
> >  achieve but it seems YANG 1.1 does not really support this well
> >  enough. What you would need is a leafref type that can be "anchored"
> >  at different places but we don't really have this...
> 
> I don’t understand this comment.  But looking to my previous comment, a way has been devised that works.  It may be kludgy relative to what you have in mind, but it seems to work.
>

Yes, kludgy is perhaps a good term. And you are hitting YANG's
limitations, there is not much you can do different.

> > - Section 4 points to keys being compromised 'when in transit' but I
> >  think we also want to protect keys at rest, i.e., sitting in a
> >  backup.
> 
> Actually, Section 4 is all about protecting keys at rest.  What text are you looking at?

I do not recall, lets ignore this comment.
 
> > - Section 4.3 talks about _highly_ restricted access mechanisms and
> >  _highly_ authorized clients and I am usually a bit confused what
> >  _highly_ means. But I am YANG doctor, not a security reviewer. ;-)
> 
> Trying to say that, e.g., the NACM should only allow a "crypto officer" access to the MK’s cleartext value.

So is NACM the 'highly restricted access mechanisms'? Should it be

OLD
  highly restricted access mechanisms SHOULD

NEW
  access control mechanisms like NACM SHOULD

OLD
  highly authorized clients

NEW
  authorized clients

My point is that "highly" simply does not mean anything (to me).

I browsed the diff, the changes look good to me.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1 | 28759 Bremen | Germany
Fax:   +49 421 200 3103         <https://www.jacobs-university.de/>

---

From: Kent Watsen <[email-masked]>
Date: Tue, 20 Apr 2021 01:28:44 +0000

Hi Juergen,

Likewise trimming to the essential parts.

K.

>> Please see my response to your comment in that draftb
>> 
> 

Right, and I did just reply that whilst the two point to the same same, only one can be used in “if-feature” statement expressions, which is what is needed.

>> BTW, b

It took me while to determine what text got clipped above which you this is better.  Let me clarify, you think that it would be better to both:

OLD; keystore-supported
NEW: central-keystore-supported

*and*

OLD:  … the keystore, when this module is implemented
NEW: … the central keystore, when this module is implemented

Correct?

>> Trying to say that, e.g., the NACM should only allow a "crypto officer" access to the MKb
> 

Replaced.  Better, thanks!

>  highly authorized clients

NEWEST:  ...only to the most trusted authorized clients

Ok?

> My point is that "highly" simply does not mean anything (to me).

Point taken, “highly” doesn’t mean much.

> I browsed the diff, the changes look good to me.

Thanks,

K.

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 20 Apr 2021 14:44:15 +0200

> NEW: … the central keystore, when this module is implemented

Yes.

/js

---

From: Kent Watsen <[email-masked]>
Date: Thu, 22 Apr 2021 01:26:29 +0000

Hi Juergen,

>> It took me while to determine what text got clipped above which you think is better.  Let me clarify, you think that it would be better to both:
>> 
>> OLD; keystore-supported

Done!

Thanks again.

K.