Re: [netconf] draft-ietf-netconf-keystore-09.txt

[Nick Hancock <[e-mail]>]

Hi Kent, 

> -----Original Message-----
> From: Kent Watsen <[e-mail]>
> Sent: 08 May 2019 00:24
> To: Nick Hancock <[e-mail]>
> Cc: [e-mail]
> Subject: Re: [netconf] draft-ietf-netconf-keystore-09.txt
> 
> Hi Nick,
> 
> 
> > Given that a client has the required permissions, the current model
> > allows a client to create an entry in the list 'asymmetric-key'
> > without the 3-tuple leafs 'algorithm', 'public-key' and 'private-key'
> > as allowed via the must statement on the list. At this point there
> > would be no 3-tuple in <operational> either. Defining the 3-tuple in
> > <intended> or <operational> would be a second step, such as invoking
> > the action 'generate-hidden-key'. Since there is, as you indicated,
> > no 'must' statement relevant to the 'certificate' list, the model
> > also allows a client to configure a certificate (chain) on an entry
> > in the list 'asymmetric-key' without the 3-tuple in either <intended>
> > or <operational>. The asymmetric key entry would not be able to be
> > used to establish a TLS session due to the missing 3-tuple, of course.
> > 
> > Is this a valid use case? Possibly. I could argue that a client may
> > wish to 'park' certificate chains in the keystore without necessary
> > immediately using them on the server or even just pre-provision them
> > and then provide the actual public key later maybe through some other
> > process based on an operator's security policies? It makes no sense
> > to reference a certificate in the keystore that is missing the
> > 3-tuple as the server identity of a NETCONF server or client identity
> > of a NETCONF client as it cannot be used, so the question is whether
> > it would make more sense to constrain the actual reference to
> > reference certificates that have 3-tuple, if this is at all possible
> > considering that the 3-tuple may only be in <operational>?
> 
> If you look at my "wide-screen needed" email from last Friday, you'll see that I'm
> trying to make a case that these 3-tuple nodes should be "mandatory true".   If
> that discussion falls apart, then we can revisit your proposal here.
> 
> BTW, please jump into that discussion, as comments from implementations are
> highly valued.
> 
> 
> 
> > During analysis of deploying the keystore and truststore in our
> > implementation, I have made some further observations that may be of
> > interest:
> 
> I see you're calling it "truststore", which I take as a confirmation for the rename
> proposal sent out last Thursday.
> 
> 
> > 
> > 1. There is nothing in the model to prevent a client configuring a
> >  certificate to an asymmetric key that is not applicable to that
> >   asymmetric key or even a certificate chain in the keystore or
> >   truststore that is incompatible to the required type
> >   ('end-entity-cert-cms' or 'trust-anchor-cert-cms' respectively)
> >   or is this to be rejected by the implementation, in which case
> >  this behavior is not described in the YANG?
> 
> Such incompatibilities must not be allowed.  As neither issue can be enforced via
> YANG validation constraints, the best we can do is add comments to the
> description statements.
> 
> For the latter case, I believe that both the 'end-entity-cert-cms' or 'trust-anchor-
> cert-cms' description statements have a sufficient number of MUST statements.
> - agreed?
> 

Agreed.

> For the former, how about this:
> 
> @@ -1614,7 +1624,9 @@ module ietf-crypto-types {
> 
>    grouping asymmetric-key-pair-with-cert-grouping {
>      description
> -      "A private/public key pair and an associated certificate.";
> +      "A private/public key pair and an associated certificate.
> +       Implementations SHOULD assert that certificates contain
> +       the matching public key.";
> 
>      uses asymmetric-key-pair-grouping;
>      uses end-entity-cert-grouping;
> @@ -1692,7 +1704,9 @@ module ietf-crypto-types {
> 
>    grouping asymmetric-key-pair-with-certs-grouping {
>      description
> -      "A private/public key pair and associated certificates.";
> +      "A private/public key pair and associated certificates.
> +       Implementations SHOULD assert that certificates contain
> +       the matching public key.";
>      uses asymmetric-key-pair-grouping;
>      container certificates {
> 

Yes, but I believe that these SHOULD statements should also be 
included on the description statement of 'cert' leafs as well as they 
may get overlooked, if they are only to be found in the description 
statement on the groupings.

> 
> 
> 
> > 2. I am not convinced whether the use of the prefix 'pinned-' on the
> > containers and lists in ietf-trust-anchors is correct. The
> >  certificates are available in the truststore for pinning, yes, but
> >  does not the 'pinning' actually take place when the certificates
> >  are configured for client authentication on a server or server
> >  authentication on a client? So I would argue that a certificate
> >  may be in the trust store, but not necessarily pinned, so the name
> >  may be misleading or am I missing something? Thus, I would have
> >  expected the containers and lists to be without the 'pinned-'
> >  prefix within the truststore itself. The 'pinned-' prefix being
> >  only on the references, which is the case in the NETCONF client
> >  and server models.
> 
> 
> Agreed, especially with renaming the module and top-level container node to
> "truststore" (what else would a truststore contain, right?).  I just all the "pinned"
> prefixes in my local copy (s/pinned.//g)
> 
> 
> 
> 
> > 3. As an initial step we are analyzing the standalone use of the
> >   keystore in truststore modules and were considering some
> >  proprietary notifications that would include the certificate
> >  (chain) received during TLS session establishment, but could not
> >  'use' the groupings in ietf-crypt-types because they include
> >  actions and notifications, which are not allowed in notifications.
> >  Maybe it would be advantageous to provide basic groupings without
> >  actions and notifications for use in proprietary RPCs and
> >  notifications and provide refined versions of these groupings that
> >  include the actions and notifications? Obviously this would mean
> >  even more groupings, but wouldn't this increase their reusability?
> 
> I don't understand "standalone use of the keystore in truststore modules" -
> maybe you meant "and"?

Yes, I meant 'and'.

> 
> I know what you mean.   I recently wanted to use some groupings from another
> module but couldn't for a similar reason, and hence had a similar copy/paste
> yuck experience.
> 
> If I understand you correctly, you're saying that tooling such as `pyang` or
> `yanglint` is complaining because your notification "uses" the crypto-type
> grouping, which itself defines a notification - yes?

Yes, that is correct.

> 
> RFC 7950 Section 7.16 says"
> 
>    A notification MUST NOT be defined within an rpc, action, or another
>    notification, i.e., a notification node MUST NOT have an rpc, action,
>    or a notification node as one of its ancestors in the schema tree.
>    For example, this means that it is an error if a grouping that
>    contains a notification somewhere in its node hierarchy is used in an
>    rpc definition.
> 
> Ouch, that's rather specific.  I was hoping there might be some wiggle room
> allowing for "inner" notifications to just be ignored.  That sounds like a better
> solution, don't you think, perhaps a YANG-Next candidate?
> 

It would be a possible solution, but that is something for the future though.

> You're right in guessing we're reluctant to create more groupings unnecessarily
> and, in this case, these groupings are very small - just a single "leaf" or "leaf-
> list", so the yuck isn't all that much.  Copy/paste might be reasonable in this
> case, no?
> 

Yes, copy/paste is always an option, but this also has its drawbacks, 
including having multiple duplicate definitions within the YANG model; 
the opportunity to modify the copy, intentionally or unintentionally, 
such as to change the names of leafs; but most importantly copying and 
pasting would mean that copy will not follow any changes made to 
the original grouping, such as corrections and the addition of new 
nodes in newer revisions, meaning additional maintenance and possible 
discrepancies across YANG models going forward.  
And this is why I am a keen user of groupings to ensure consistency 
across models, even though it makes the YANG modules even harder to 
read. If something is the same then it should always be the same 
wherever is appears in the model and using a grouping does just that.

Regards
Nick


> 
> Kent // contributor
> 
> 
>