Subject: Re: [Netconf] Mandatory local configuration in Keystore groupings

From: Balázs Kovács <[email-masked]>
Date: Tue, 9 Oct 2018 06:50:22 +0000

Hi Kent,

First of all, thank you for the keystore update.

I was checking it at the moment and noticed something that may be a miss.

  typedef asymmetric-key-ref {
    type leafref {
      path "/ks:keystore/ks:asymmetric-keys/ks:asymmetric-key"
           + "/ks:name";
      require-instance false;
    }
    description
      "This typedef enables modules to easily define a reference
       to an asymmetric key stored in the keystore. The require
       instance attribute is false to enable the referencing of
       asymmetric keys that exist only in <operational>.";
    reference
      "RFC 8342: Network Management Datastore Architecture (NMDA)";
  }

I think we were discussing changing 'require-instance' to 'true' in relation to the change toward alternative (C) as discussed below. Should it be 'true' instead of 'false'?

Br,
Balazs

-----Original Message-----
From: Martin Bjorklund <[email-masked]> 
Sent: Tuesday, September 18, 2018 10:12 PM
To: [email-masked]
Cc: Balázs Kovács <[email-masked]>; Balázs Lengyel <[email-masked]>; [email-masked]
Subject: Re: [Netconf] Mandatory local configuration in Keystore groupings

Kent Watsen <[email-masked]> wrote:
> 
> I like Martin's "twist" - I don't view it as an extreme hardship (the 
> 1% case), and I prefer (C) for supporting "require-instance true" as 
> well as having one less action.  So, let's go with (C).
> 
> Martin, you mentioned some details might be wrong, are they wrong in 
> (C)?  - I'd like to have it right when posting the next update...

Yes; in (C), "generate-hidden-key" and "load-hidden-key" doesn't take "name" as input.

/martin

> Balazs, the desire to reference an IDevID could be from any 
> SSH/TLS-based app, it would be hard to know in advance where.  No 
> matter, with (C)+twist, we can have "require-instance true" always.
> 
> Kent // contributor
> 
> 
> -----Original Message-----
> 
> Hi,
> 
> I'd just emphasize that in creation step of hidden key, the name must 
> be coordinated between step A/1&2 by external application logic. Same 
> holds for deletion of hidden key for A/1&2. "Same name" is important.
> 
> Enabling the possibility for 'require instance true' would be also 
> good remark on the side of (C).
> 
> Regarding manufactured keys, I would not expect manufactured keys to 
> be used in other common models, such as in TLS/SSH use cases. If they 
> have some specific use case, maybe those models could either have 
> 'require instance false' or it could be said to create an empty key 
> for them as Martin described.
> 
> Br,
> Balazs
> 
> -----Original Message-----
> From: Martin Bjorklund <[email-masked]>
> Sent: Tuesday, September 18, 2018 9:02 AM
> To: [email-masked]
> Cc: Balázs Kovács <[email-masked]>; Balázs Lengyel 
> <[email-masked]>; [email-masked]
> Subject: Re: [Netconf] Mandatory local configuration in Keystore 
> groupings
> 
> Hi,
> 
> Ok, so we have these two alternatives illustrated below.  There are 
> some details that probably are wrong, but it doesn't really matter at 
> this stage.  With (A), there is one more action to control the 
> lifespan of keys (delete-hidden-key).
> 
> In the case that the key is given as config, A and C works in the same 
> way.
> 
> In the case that the key is permanent, A and C also works in the same 
> way.
> 
> A and C differ in how hidden keys are handled.
> 
> 
> In order to create a hidden key + certificate, the steps are:
> 
> A:  1. call generate-hidden-key with a given name
>     2. create an asymmetric-key with the same name in the config
>     3. call generate-certificate-signing-request
>     4. <get hold of a cert>
>     5. create the certificate in the config
> 
> (step 1 and 2 can be done in reverse order)
> 
> C   1. create an asymmetric-key in the config
>     2. call generate-hidden-key on this key
>     3. call generate-certificate-signing-request
>     4. <get hold of a cert>
>     5. create the certificate in the config
> 
> In order to delete a hidden key completely:
> 
> A:  1. call delete-hidden-key on the asymmetric-key to be deleted
>     2. delete the asymmetric-key from the config
> 
> C:  1. delete the asymmetric-key from the config
> 
> 
> 
> In summary, they are very similar.
> 
> One possible twist that probably works best for C could be to use 
> leafrefs with require instance true, and say that in order to use a 
> permanent key (created during manufacturing), you first need to create 
> config for the key (more or less empty config).  The benefit is that 
> we get proper leafref validation (no dangling pointers to handle in 
> all other models).  FWIW, this is the approach taken in 
> ietf-interfaces.
> 
> If we don't do this twist, I think that we can use either of A and C.
> 
> 
> 
> /martin
> 
> 
> 
> 
> Kent Watsen <[email-masked]> wrote:
> > 
> > > With (A), once you have generated the key, you first have to create
> > > the config for it, then invoke the action.   With (C) (99% of) the 
> > > keys already exist in the config, so you can just invoke the action.
> > 
> > I don't follow.  What I see is that, with (A), a hidden key can be 
> > created with a single action invocation while, with (C), first a 
> > configuration operation is needed to create the "asymmetric-key"
> > (without the three leafs) followed by an action invocation (to 
> > populate the three leafs).
> > 
> > 
> > > To summarize, A and C both get the job done.  Which one is better 
> > > is probably subjective.  I think (C) is might be easier to understand.
> > 
> > Okay, let's see how it looks.  Bubbling all the ideas to the top, I 
> > created the below tree-diagrams.  Note, in both cases, the three 
> > "mandatory true" leafs are replaced with an all-or-nothing "must"
> > expression:
> > 
> > 
> > (A) Uses RFC 8342 Section C.3.2 style config overlays for all hidden 
> > keys;
> >     and an action statement is used to delete all hidden keys.
> > 
> >      +--rw keystore
> >         +--rw asymmetric-keys
> >            +--rw asymmetric-key* [name]
> >            |  +--rw name                            string
> >            |  +--rw algorithm?                      ct:key-algorithm-ref
> >            |  +--rw public-key?                     binary
> >            |  +--rw private-key?                    union
> >            |  +---m "(algorithm and public-key and private-key)
> >            |  |       or not (algorithm or public-key or private-key)";
> >            |  +--rw certificates
> >            |  |  +--rw certificate* [name]
> >            |  |     +--rw name                      string
> >            |  |     +--rw cert                      ct:end-entity-cert-cms
> >            |  |     +---n certificate-expiration
> >            |  |        +-- expiration-date?         yang:date-and-time
> >            |  +---x generate-certificate-signing-request
> >            |  |  +---w input // KEY MAY BE IN <OPERATONAL>
> >            |  |  |  +---w subject                   binary
> >            |  |  |  +---w attributes?               binary
> >            |  |  +--ro output
> >            |  |     +--ro certificate-signing-request    binary
> >            |  +---x delete-hidden-key
> >            |        // no params
> >            +---x generate-hidden-key
> >            |  +---w input                           // RESULT IN <OPERATONAL>
> >            |     +---w name                         string
> >            |     +---w algorithm                    ct:key-algorithm-ref
> >            +---x load-hidden-key
> >               +---w input                           // RESULT IN <OPERATONAL>
> >                  +---w name                         string
> >                  +---w algorithm                    ct:key-algorithm-ref
> >                  +---w public-key                   binary
> >                  +---w private-key                  union
> > 
> > 
> > (C) Uses RFC 8342 Section C.3.2 style config overlays only for 
> > hidden keys
> >     created in <operational> (i.e., during manufacturing).  But if there
> >     is
> >     even just one key created during Manufacturing, and it doesn't appear
> >     in <intended>, then leafrefs still need to be "require-instance
> >     false",
> >     right? Also, it seems that such keys could never be deleted, unlike 
> >     other hidden keys, but that is probably okay (correct behavior).
> > 
> >      +--rw keystore
> >         +--rw asymmetric-keys
> >            +--rw asymmetric-key* [name]
> >               +--rw name                            string
> >               +--rw algorithm?                      ct:key-algorithm-ref
> >               +--rw public-key?                     binary
> >               +--rw private-key?                    union
> >               +---m "(algorithm and public-key and private-key)
> >               |       or not (algorithm or public-key or private-key)";
> >               +--rw certificates
> >               |  +--rw certificate* [name]
> >               |     +--rw name                      string
> >               |     +--rw cert                      ct:end-entity-cert-cms 
> >               |     +---n certificate-expiration
> >               |        +-- expiration-date?         yang:date-and-time
> >               +---x generate-hidden-key
> >               |  +---w input                        // RESULT IN <OPERATONAL>
> >               |     +---w name                      string
> >               |     +---w algorithm                 ct:key-algorithm-ref
> >               +---x load-hidden-key
> >               |  +---w input                        // RESULT IN <OPERATONAL>
> >               |     +---w name                      string
> >               |     +---w algorithm                 ct:key-algorithm-ref
> >               |     +---w public-key                binary
> >               |     +---w private-key               union
> >               +---x generate-certificate-signing-request
> >                  +---w input // KEY MAY BE IN <OPERATONAL>
> >                  |  +---w subject                   binary
> >                  |  +---w attributes?               binary
> >                  +--ro output
> >                     +--ro certificate-signing-request    binary
> > 
> > 
> > 
> > 
> > Kent // contributor
> > 
> > 
> > 
> 
>

---

From: Balázs Kovács <[email-masked]>
Date: Tue, 9 Oct 2018 10:25:47 +0000

Hi,

One more question. I found this in ct:

  grouping asymmetric-key-pair-with-certs-grouping {
    description
      "A private/public key pair and associated certificates.";
    uses asymmetric-key-pair-grouping;
    container certificates {
      description
        "Certificates associated with this asymmetric key.
         More than one certificate supports, for instance,
         a TPM-protected asymmetric key that has both IDevID
         and LDevID certificates associated.";
      list certificate {
        must "../../algorithm
               and ../../public-key
                 and ../../private-key";
        key name;
        description
...

Is it ok to have the must statement in the certificate list? I think with (C) the idea was to have hidden keys represented in the configuration for which certificates can still be configured, but hidden keys will have no algorithm/public-key/private-key in configuration. What's your view?

Br,
Balazs

-----Original Message-----
From: Netconf <[email-masked]> On Behalf Of Balázs Kovács
Sent: Tuesday, October 9, 2018 8:50 AM
To: Martin Bjorklund <[email-masked]>; [email-masked]
Cc: [email-masked]
Subject: Re: [Netconf] Mandatory local configuration in Keystore groupings

Hi Kent,

Br,
Balazs

-----Original Message-----
From: Martin Bjorklund <[email-masked]>
Sent: Tuesday, September 18, 2018 10:12 PM
To: [email-masked]
Cc: Balázs Kovács <[email-masked]>; Balázs Lengyel <[email-masked]>; [email-masked]
Subject: Re: [Netconf] Mandatory local configuration in Keystore groupings

/martin

> >                     +--ro certificate-signing-request    binary

_______________________________________________
Netconf mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/netconf

---

From: Kent Watsen <[email-masked]>
Date: Wed, 10 Oct 2018 02:48:05 +0000

Hi Balazs,

Answering both of your messages here.

> I think we were discussing changing 'require-instance' to 'true' 
> in relation to the change toward alternative (C) as discussed 
> below. Should it be 'true' instead of 'false'?

Yes.  I removed the "require-instance" statement, and fixed 
the "description" statement in my local copy.  New description
statement:

      "This typedef enables modules to easily define a reference 
       to an asymmetric key stored in the keystore.";

I did the same for the asymmetric-key-certificate-ref typedef, as
we should apply the same (C) trick there as well.  But doing this
required removing the "mandatory true" from end-entity-cert-grouping.
For full measure, I also removed the "mandatory true" from the
trust-anchor-cert-grouping, as that seemed to be the same issue
as the others.

> Is it ok to have the must statement in the certificate list? 
> I think with (C) the idea was to have hidden keys represented
> in the configuration for which certificates can still be
> configured, but hidden keys will have no algorithm/public-key\
> /private-key in configuration. What's your view?

Yes.  I removed the "must" statement in my local copy and changed the
description statement for /keystore/asymmetric-keys/asymmetric-key/name
as follows:

OLD:
            "An arbitrary name for the asymmetric key.";
NEW:
            "An arbitrary name for the asymmetric key.  If the name
             matches the name of a key that exists independently in
             <operational> (i.e., a 'permanently-hidden' key), then
             the 'algorithm', 'public-key', and 'private-key' nodes
             MUST NOT be configured.";

As well the description statement for asymmetric-key-pair-with-
certs-grouping/certificates/certificate/name:

OLD:
            "An arbitrary name for the certificate.";
NEW:
            "An arbitrary name for the certificate.  If the name
             matches the name of a certificate that exists
             independently in <operational> (i.e., an IDevID), 
             then the 'cert' node MUST NOT be configured.";

BTW, you probably noticed this comment in keystore:

  // MOVED TO CRYPTO TYPES DRAFT? - OKAY TO REMOVE HERE NOW?
  // <snip/>

Any thoughts about that move? - better?

Thanks,
Kent

---

From: Balázs Kovács <[email-masked]>
Date: Thu, 11 Oct 2018 14:47:04 +0000

Hi Kent,

One more issue. There are 3 local-or-keystore definitions in keystore. For example:

  grouping local-or-keystore-asymmetric-key-with-certs-grouping {
    description
      "A grouping that expands to allow the key to be either stored
       locally within the using data model, or be a reference to an
       asymmetric key stored in the keystore.";
    choice local-or-keystore {
      mandatory true;
      case local {
        if-feature "local-keys-supported";
        uses ct:asymmetric-key-pair-with-certs-grouping;
      }
      case keystore {
        if-feature "keystore-supported";
        leaf reference {
          type ks:asymmetric-key-ref;
          description
            "A reference to a value that exists in the keystore.";
        }
      }
      description
        "A choice between an inlined definition and a definition
         that exists in the keystore.";
    }
  }

I was thinking if the 'leaf reference' in 'case keystore' should rather be 'mandatory true'. I know it does not make a difference now when there is only a single leaf in that case (and we don't plan more), but maybe it is more secure to have the mandatory flag on.

Regarding the groupings:

I think the 'local' configurations in local-or-keystore groupings got too much capability in the end, and the locals became kind of full-fledged keystores. Shouldn't the local be simplified to binary privkey and certificate? I think local should be much simpler, but this is just my opinion. This of course impacts what should be moved to ct. Some detailed thoughts below:

1. Is the 'grouping local-or-keystore-asymmetric-key-with-certs-grouping' grouping used by any module? Client/server models seem to use only the ks:local-or-keystore-end-entity-certificate-grouping. Latter is lacking the action and cert is not in list. What was the objective with the local configuration? Shouldn't it be much more simple than the keystore capability? Shouldn't it be just for configuring a non-hidden key and corresponding cert? Even the current local configuration with hidden key generate/install seem to be stepping on the toes of keystore. Hidden key generation I guess would also need CSR creation to bind the hidden key with a cert (which is not part of the local branches).

2. I guess crypto-types should contain the groupings that are foreseen to be reused. But those that we see only for keystore, could possibly remain in keystore. 'asymmetric-key-pair-with-certs' grouping contain way too much keystore specific implementation in my opinion, so it should be in keystore and not reused (unless you really want to boost the capabilities of the 'local' configuration). I would also keep 'end-entity-cert-grouping' in keystore with the certificate expiration information. 'trust-anchor-cert-grouping' could stay in trust anchors, it just adds a leaf. ' asymmetric-key-pair-grouping' should be in keystore IF it is only for keystore. Maybe if this latter was simpler that the local configurations should also use, then that could be in ct.

Br,
Balazs

-----Original Message-----
From: Kent Watsen <[email-masked]> 
Sent: Wednesday, October 10, 2018 4:48 AM
To: Balázs Kovács <[email-masked]>; Martin Bjorklund <[email-masked]>
Cc: [email-masked]
Subject: Re: [Netconf] Mandatory local configuration in Keystore groupings

Hi Balazs,

>      "RFC 8342: Network Management Datastore Architecture (NMDA)";  }
>
> in relation to the change toward alternative (C) as discussed below. 
> Should it be 'true' instead of 'false'?

Yes.  I removed the "require-instance" statement, and fixed the "description" statement in my local copy.  New description
statement:

I did the same for the asymmetric-key-certificate-ref typedef, as we should apply the same (C) trick there as well.  But doing this required removing the "mandatory true" from end-entity-cert-grouping.
For full measure, I also removed the "mandatory true" from the trust-anchor-cert-grouping, as that seemed to be the same issue as the others.

> I think with (C) the idea was to have hidden keys represented in the 
> configuration for which certificates can still be configured, but 
> hidden keys will have no algorithm/public-key\ /private-key in 
> configuration. What's your view?

Yes.  I removed the "must" statement in my local copy and changed the description statement for /keystore/asymmetric-keys/asymmetric-key/name
as follows:

OLD:

OLD:

Thanks,
Kent

---

From: Kent Watsen <[email-masked]>
Date: Fri, 12 Oct 2018 01:41:47 +0000

// All, I plan to update this set of drafts before the cutoff, to address 
// the few items that have come up here, as well to add (finally!) a fix
// for "Should algorithm identities be moved from ietf-[ssh/tls]-common
// to crypto-types?" issue from IETF 102 (I've been working with a few
// cryptologists, and we have a proposal that we'd like to put in for
// review).  That would leave just the "Add support for TCP Keepalives?"
// issue remaining, for which I still don't have a good answer.

Hi Balazs,

> I was thinking if the 'leaf reference' in 'case keystore' should
> rather be 'mandatory true'. I know it does not make a difference
> now when there is only a single leaf in that case (and we don't
> plan more), but maybe it is more secure to have the mandatory flag
> on.

I guess it doesn't hurt but, assuming that there is no intention to
ever add more descendants to the "keystore" case, it's unnecessary
(as you say).  What is your concern?

> Regarding the groupings:
>
> I think the 'local' configurations in local-or-keystore groupings
> got too much capability in the end, and the locals became kind of
> full-fledged keystores. Shouldn't the local be simplified to binary
> privkey and certificate? I think local should be much simpler, but
> this is just my opinion. 

Below is the tree diagram for the "local" case from the
local-or-keystore-asymmetric-key-with-certs-grouping.  I picked
this grouping because it's the most complicated of them all.

   +--:(local) {local-keys-supported}?
      +-- algorithm?                       ct:key-algorithm-ref
      +-- public-key?                      binary
      +-- private-key?                     union
      +---x generate-hidden-key
      |  +---w input
      |     +---w algorithm                ct:key-algorithm-ref
      +---x install-hidden-key
      |  +---w input
      |     +---w algorithm                ct:key-algorithm-ref
      |     +---w public-key?              binary
      |     +---w private-key?             binary
      +-- certificates
      |  +-- certificate* [name]
      |     +-- name?                      string
      |     +-- cert?                      ct:end-entity-cert-cms
      |     +---n certificate-expiration
      |        +-- expiration-date?        yang:date-and-time
      +---x generate-certificate-signing-request
         +---w input
         |  +---w subject                  binary
         |  +---w attributes?              binary
         +--ro output
            +--ro certificate-signing-request    binary

I assume that you're having issue with the actions and perhaps 
also the notification.   Of course, these are due to the 
crypto-type groupings being used here.  As you've likely
noticed, the YANG modules aggressively use groupings.

Do you feel that "local" keys should never be "hidden"?  Why 
should keys used in keystore be the only ones that can be hidden?
What about devices that do not have the "keystore-supported"
feature, can they not have hidden keys? Assuming local keys
can be hidden, than the "generate-certificate-signing-request"
action is needed, right?  As for the notification, how is it 
not a good thing, even for "local" keys?

> This of course impacts what should be moved to ct. Some detailed
> thoughts below:
>
> 1. Is the 'local-or-keystore-asymmetric-key-with-certs-grouping'
> grouping used by any module? 

Not currently.  It was being used by the [ssh/tls]-client-server
modules, but I switched it out to the "local-or-keystore-end-entity-
certificate-grouping" because I think only a single certificate can
be specified in those cases.  Perhaps we should go back to using 
that grouping and have a "refine" statement to set "max-elements 1"?
This didn't occur to me before, but looking at it now, I think we 
should do it, and then remove local-or-keystore-asymmetric-key-with-
certs-grouping.

That said, I generally don't view unused groupings as a bad thing.
E.g., there are some unused groupings in crypto-types, though they
clearly seem like they could be useful in other contexts.   FWIW,
if we choose to keep the 2nd grouping, we should also add a "local-
or-keystore-trust-anchor-certificate-grouping", for symmetry.

> Client/server models seem to use only the ks:local-or-keystore-
> end-entity-certificate-grouping. Latter is lacking the action and
> cert is not in list. What was the objective with the local 
> configuration? Shouldn't it be much more simple than the keystore
> capability? Shouldn't it be just for configuring a non-hidden key
> and corresponding cert? Even the current local configuration with
> hidden key generate/install seem to be stepping on the toes of 
> keystore. Hidden key generation I guess would also need CSR 
> creation to bind the hidden key with a cert (which is not part
> of the local branches).

A lot of this I already touched on.  I do think local hidden keys
are a good thing.  As for the rest, I think you're making a case
for doing the alternative mentioned above (i.e., using a "refine"
statement to set "max-elements 1").

> 2. I guess crypto-types should contain the groupings that are
> foreseen to be reused. But those that we see only for keystore,
> could possibly remain in keystore. 'asymmetric-key-pair-with-certs'
> grouping contain way too much keystore specific implementation in
> my opinion, so it should be in keystore and not reused (unless 
> you really want to boost the capabilities of the 'local' 
> configuration). I would also keep 'end-entity-cert-grouping' in
> keystore with the certificate expiration information. 'trust-
> anchor-cert-grouping' could stay in trust anchors, it just adds
> a leaf. ' asymmetric-key-pair-grouping' should be in keystore IF
> it is only for keystore. Maybe if this latter was simpler that 
> the local configurations should also use, then that could be in ct.

The above covers all this but, to provide insight, know that I moved
all groupings that were NOT keystore specific.  That they're somehow
overly robust was not a factor in that decision making process.  Yes,
I do want to "boost" local keys, per my comments above.
 
I was about to write that, if we remove "ks:local-or-keystore-end-
entity-certificate-grouping", we could remove "ct:end-entity-cert-
grouping", but then notice that "ct:asymmetric-key-pair-with-certs-
grouping" is using it too.  Hmmm....

I presume that you're not picking on ct:trust-anchor-cert-grouping
only because it doesn't define a notification, but know that I went
back and forth on that, and I think I made a mistake.  I now think
that it should define a notification and that the trust-anchors 
draft should move to using it, with a refine statement setting 
"max-elements 1".

Regards,
Kent