Re: [netconf] Yangdoctors last call review of draft-ietf-netconf-crypto-types-18

[Juergen Schoenwaelder <[e-mail]>]

On Thu, Feb 11, 2021 at 12:04:36AM +0000, Kent Watsen wrote:
> Thank you for your review, Juergen!
> 
> Below are responses to your comments.
> 
> PS: this response took longer because I had to triage with Security Experts on a couple points.
> 

It always takes longer. ;-) I cut things resolved.
 
  > - Does it make sense to have a new notation like
> > 
> >      |  The diagram above uses syntax that is similar to but not
> >      |  defined in [RFC8340].
> > 
> >  or shall this simply become regular text?
> 
> I find using tree-diagram like notation for features, identities, and typedefs helpful, especially in showing the “if-feature” constraints and hierarchical relationships between items.  I recommend adding these enhancements to RFC 8340.  It would be helpful if someone could whip up an I-D so these documents could reference the I-D instead of having the "The diagram above uses syntax that is similar to but not defined in [RFC8340]” language.
> 
> That said, I also used a "rfc8340-like" diagram to list all the groupings in a module.  It seems like the consistent thing to do, but I can’t imagine the RFC 8340 being updated to support that output, so I replaced that diagram, with a normal "xml2rfc" list, in the entire suite of drafts.

I am sorry, I should have been clearer. My comment did not concern the
diagrams but the mere fact that you have

      |  The diagram above uses syntax that is similar to but not
      |  defined in [RFC8340].

instead of simply

   The diagram above uses syntax that is similar to but not
   defined in [RFC8340].

and this is really an editorial nit if at all. The diagram itself I
like, it would be nice to have them in RFC 8340bis.

> >  What is 'encoded in the format specified by the "format" identity' I
> >  assume this refers to the key value that is encrypted and stored in
> >  encrypted-value, no?
> 
> Not only was the solution unclear, but it was rather upside-down in terms of how to encode encrypted values.  Please see the “diffs” to get a sense for how the solution was changed.

I happily trust Russ here. ;-) 

> > - I wonder why this is a SHOULD and not a MUST:
> > 
> >           "Identifies the symmetric key's format.  Implementations
> >            SHOULD ensure that the incoming symmetric key value is
> >            encoded in the specified format.";
> > 
> >  This statement shows up several times when the key-format identities
> >  are used. I wonder what this means to an implementer. If I receive a
> >  key format (say one-symmetric-key-format) and a corresponding blob
> >  of data, do I have to decode this to see whether the format works
> >  out? If later the key-format is changed to something else (lets say
> >  octet-string-key-format), do I reject such a change or is it OK as
> >  long as the binary data I have would be a valid value given the new
> >  format? Perhaps this is implementation specific? Well, since we deal
> >  with keys, ...
> 
> It currently a "SHOULD” because some implementations may assume that clients always configure perfect values, and so it becomes squishy as to if they MUST.   If a server fails to validate the base64 contents (which are outside YANG validation) as they’re being applied, then the server may run into nasty exceptions when needing to use the value at runtime.  A nice system will decode the base64 values and verify that they contain valid values during the commit process.
> 
> Make sense?  - leave as SHOULD okay?

Thanks for the explanation. I am not religious about SHOULD vs MUST
here.

I have browsed through the diffs and they look good. I did not do
another full review again.

/js

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