Subject: Re: [Netconf] mbj review of draft-ietf-netconf-restconf-server-model-09

From: Kent Watsen <[email-masked]>
Date: Thu, 7 Apr 2016 03:15:22 +0000

Hi Martin,

Thank you for your review.  Below are my responses:

>o  Section 3
>
>  The picture should s/<augments>/<uses>/g

Right you are - fixed.

>o  Section 4
>
>  It would be usefule with a reference to the term IDevID in this
>  document.

Added reference to 802.1AR-2009

>o  Section 4.1
>
>  OLD:
>
>   o  A configurable list of lists of trust anchor certificates.  This
>      enables the server to have use-specific trust anchors.  For
>
>  NEW:
>
>   o  A configurable list of trust anchor certificates.  This
>      enables the server to have use-specific trust anchors.  For
>
>  ... but what does "use-specific" mean?

Well, it really is a list of lists:

       +--rw trusted-certificates* [name]
          +--rw name                   string
          +--rw description?           string
          +--rw trusted-certificate* [name]
             +--rw name           string
             +--rw certificate?   Binary

The example in Section 4.1.2 illustrates what is meant by use-specific:

     <!-- trusted netconf/restconf client certificates -->

     <trusted-certificates>
       <name>explicitly-trusted-client-certs</name>

       ...
     </trusted-certificates>

     <!-- trust anchors for netconf/restconf clients -->
     <trusted-certificates>
       <name>deployment-specific-ca-certs</name>

       ...
     </trusted-certificates>

     <!-- trust anchors for random HTTPS servers on Internet -->
     <trusted-certificates>
       <name>common-ca-certs</name>

       ...
     </trusted-certificates>

Does it makes sense now?  - how would you like to see the text improved?

>o  Section 4.1
>
>  The text says:
>
>   o  An RPC to request the server to load a new private key.
>
>  When I first read this, I didn't understand what this meant.  See
>  more below.
>
>
>o  Section 4.1
>
>  I think the "semi-configurable" design has some issues.  You have
>  defined some actions that actually modifies the configuration of the
>  system.  It is not clear which config datastore is modified.
>  Presumably running.  Interactions with locking and access control
>  are not described.  Also, the resulting configuration is not
>  complete - i.e., you cannot do <copy-config> to save/restore a
>  backup.  This is fine, since you really don't want to expose the
>  private keys in the config backup.  But some discussion is needed
>  around this subject.  What happens if I generate a private key with
>  your action, backup that config and then restore it?  What happens
>  with config that has references to such a key?
>
>  One way to avoid that the actions modify the configuration could be
>  to move them into the private-key list.  One drawback is that two
>  operations are needed in order to create a (usable) key - first
>  create the config in running, then call the action.
>
>  Another option might be to model the keys as config false data.
>  This also solves the problem that some keys (in TPM for example)
>  are not deletable.

Yes, the action statements are intended to update the running datastore.  Presumably because the actions are either generating or loading a new key, there would not be a real world locking issue, but I can see how this might lead to undesirable results.  Access control (NACM) was removed when we moved to the keychain based approach, but presumably the private key would need to protected by nacm:default-deny-all.  

I see what you're saying.  You're right about this breaking backup and restore.  The only way to fix it is for the backup (<get-config>) to contain the private keys.  But note that systems using a TPM actually have NO WAY to get the private key out of the TPM - only the TPM itself has access to the private key.  So for these systems, backup/restore (RMA) is impossible, even with root-level access on the command line.

I'm not sure if I understand your first option.  Do you mean the client would create a dummy private key entry (a placeholder) and then call an action to populate the key with data?

I don't think the private keys can be config false, since they are referenced by config true nodes in the tls/ssh-server modules.

I'll add this to the Open Issues to be discussed in tomorrow's meeting.

>o  Section 4.1.3
>
>  OLD:
>
>  import ietf-yang-types {     // RFC 6991
>    prefix yang;
>  }
>
>  NEW:
>
>  import ietf-yang-types {
>    prefix yang;
>    reference "RFC 6991: Common YANG Data Types";
>  }
>
>  (same for the other modules)

Fixed.

>o  Section 4.1.3
>
>  I think the "algorithms" typedef should be called "algorithm".  But
>  actually, it should probably have a more descriptive name
>  "algorithm" is rather generic...
>
>  Also, this is an fixed enumeration.  Section 5 says: 
>
>    In additional
>    algorithms are needed, they MAY be augmented in by another module,
>
>  [note: s/In/If/]
>
>  You can't really augment in additional enums.  You'd have to augment
>  an additional leaf of some other type.
>
>  What happens to the "algorithm" leaf when I load a key with your
>  action?  Will other algorithms be rejected?
>
>  If you keep the enums, a reference statement for each enum would be
>  useful.

Hmmm, would an identity be a better way to achieve do this?

>o  Section 4.2.1
>
>  I was confused by the containers that the RFC editor are supposed to
>  remove.  Then I realized they exist due to a deficiency in pyang...
>  So I fixed pyang; you can now pass --tree-print-groupings (only in
>  1.1 branch).

Thank you!

>  I don't have a solution for the long leafrefs though.  But in any
>  case, I think the current long lines make the tree picture very
>  difficult to grasp.  It would be better to manually tweak the output
>  before posting the next draft - unless we can come up with some
>  clever way to handle this in pyang.

Note to others, this issue is being tracked by https://github.com/mbj4668/pyang/issues/168.  I just updated the issue with an idea...

>o  Section 4.2.3
>
>  You have a leaf:
>
>            leaf certificate {
>              if-feature ssh-x509-certs;
>              type leafref {
>                path "/kc:keychain/kc:private-keys/kc:private-key/"
>                     + "kc:certificate-chains/kc:certificate-chain/"
>                     + "kc:certificate";
>              }
>
>   This is a leafref that will actually contain the entire binary
>   certificate.  This may be a bit awkward...  Unless you really meant
>   kc:name as the last part of the path?  In either case, the
>   description statement probably needs an update.
>
>   Also, the path crosses two lists, which in general means that that
>   the leafref may refer to multiple leafs.  But in this case it might
>   be ok.

Changed to kc:name.  I don't understand, how does crossing two lists result in the leafref referring to multiple leafs?

>
>o  Section 4.2.3
>
>      leaf address {
>        type inet:ip-address;
>        description
>         "The IP address of the interface to listen on. The SSH
>          server will listen on all interfaces if no value is
>          specified.";
>      }
>
>  What about "0.0.0.0" and "::"?

Is your recommendation to add text indicating that there values also result in the server listening on all interfaces?

>So a missing 'address' leaf should
>  mean any ipv4 and ipv6 address on all interfaces?

Yes, this is the intent, isn't it a common idiom?

>      leaf port {
>        type inet:port-number;
>        mandatory true;  // will a default augmented in work?
>        description
>         "The local port number on this interface the SSH server
>          listens on.";
>      }
>
>  I suggest you remove the mandatory statement, and then use refine to
>  set the default port when the grouping is used; OR use refine to set
>  mandatory true if no reasonable default exists.

Fixed.

>o  Section 4.4.1
>
>  You have:
>
>       +--rw listen {(ssh-listen or tls-listen)}?
>
>  Hmm, the design allows for other protocols than ssl and tls (just
>  augment new cases).  But this if-feature statement essentially
>  REQUIRE an implementation to _also_ support ssh or tls.  Is that
>  intentional?

Not intentional.  Is there any way that a future version of YANG could enable a new feature to be OR-ed into an existing if-feature statement?

>o  Section 5
>
>  ietf-system-keychain vs. ietf-routing-key-chain
>
>  Is it "keychain" or "key-chain"?

I've never seen "key-chain" before.  Many OSs (e.g., mac, linux, openbsd, freebsd) have a utility called "keychain".

>o  General remark.
>
>  Unless it is too much of a burden, I think it would make sense to
>  move the generic tls and ssh grouping models (and keychain) into a
>  separate draft.   It might also be useful with corresponding
>  groupings for ssh/tls clients (which you almost already have).

This will be discussed in tomorrow's meeting

Thanks,
Kent

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 07 Apr 2016 12:48:32 +0200 (CEST)

> Thank you for your review.  Below are my responses:

[...]

>              +--rw certificate?   Binary

Right... but the description of trusted-certificates says that it is:

   A list of trusted certificates.

You are also using the terms "trust anchor", "trust anchor
certificate", and "trusted certificate" somewhat interchangeably.
This maybe adds to the confusion.

Should the "trusted-certificates" list be called
"trusted-certificate-list" (or "-set")?

> Does it makes sense now?  - how would you like to see the text improved?

I just didn't understand the term "use-specific".  I would have
expected "use-case-specific" or something...  (but english is not my
native language, obviously)

> Yes, the action statements are intended to update the running
> datastore.  Presumably because the actions are either generating or
> loading a new key, there would not be a real world locking issue,
> but I can see how this might lead to undesirable results.  Access
> control (NACM) was removed when we moved to the keychain based
> approach, but presumably the private key would need to protected by
> nacm:default-deny-all.   
> 
> I see what you're saying.  You're right about this breaking backup
> and restore.  The only way to fix it is for the backup
> (<get-config>) to contain the private keys.  But note that systems
> using a TPM actually have NO WAY to get the private key out of the
> TPM - only the TPM itself has access to the private key.  So for
> these systems, backup/restore (RMA) is impossible, even with
> root-level access on the command line. 
>
> I'm not sure if I understand your first option.  Do you mean the
> client would create a dummy private key entry (a placeholder) and
> then call an action to populate the key with data?

Yes, but since the private key is not part of the config, the action
will not touch the running config.

> I don't think the private keys can be config false, since they are
> referenced by config true nodes in the tls/ssh-server modules. 

You can have a require-instance false leafref to the config false of
keys, and then describe what will happen if some server is configured
to point to a private key that doesn't exist.

There are systems with tamper-proof hw that won't allow you to access
the keys unless a physical token is present (e.g. a USB stick).  In
such systems, you may very well end up with config that refers to a
key that isn't available.

Question 1:  If the private key is not stored in special HW, do we
  want to support backup/restore of such keys?

  If the answer is yes:
      Use a config list and a separate -state list.
      Make all references point to the -state list (in order to handle
      TPM etc)

  If the answer is no:
      Use only a config false list and define actions to manipulate
      the list.

> I'll add this to the Open Issues to be discussed in tomorrow's meeting.

[...]

> Hmmm, would an identity be a better way to achieve do this?

Yes, if you need the flexibility (which I think you do).

[...]

> Changed to kc:name.  I don't understand, how does crossing two lists
> >result in the leafref referring to multiple leafs?

Suppose you have:

  list foo {
    key id;
    ...
    list bar {
      key id;
      ...
    }
  }
  leaf x {
    type leafref {
      path "/foo/bar/id";
    }
  }

and this instance data:

  <foo>
    <id>F1</id>
    <bar>
      <id>B</id>
    </bar>
  </foo>
  <foo>
    <id>F2</id>
    <bar>
      <id>B</id>
    </bar>
  </foo>

and also:

  <x>B</x>

The leaf x now points to two leafs:

  /foo[id='F1']/bar[id='B']/id
  /foo[id='F2']/bar[id='B']/id

> Is your recommendation to add text indicating that there values also
> result in the server listening on all interfaces?

After thinking some more, I'd say "no".  (But we should at least be
aware of the fact that :: means different things on different OSes)

> Yes, this is the intent, isn't it a common idiom?

I am more used to seeing explicit 0.0.0.0 or ::.  In (the very
expired) draft
https://tools.ietf.org/html/draft-schoenw-netmod-yang-pattern-00
we recommend that the ip-address is mandatory, forcing the explicit
wildcard if needed.

> Not intentional.  Is there any way that a future version of YANG
> could enable a new feature to be OR-ed into an existing if-feature
> statement?

Nope.  There are two options; either remove the feature on the listen
container, or define a new feature "listen" which the server will
advertise if it supports listening on *some* protocol.

/martin

---

From: t.petch <[email-masked]>
Date: Fri, 8 Apr 2016 17:12:24 +0100

----- Original Message -----
From: "Kent Watsen" <[email-masked]>
To: "Martin Bjorklund" <[email-masked]>; <[email-masked]>
Sent: Thursday, April 07, 2016 4:15 AM

> Thank you for your review.  Below are my responses:

<snip>

> I've never seen "key-chain" before.  Many OSs (e.g., mac, linux,
openbsd, freebsd) have a utility called "keychain".

Kent

The IETF is riddled with them e.g.

"   The key-chain YANG model groups several keys into a single key
chain."

in draft-chen-rtgwg-key-table-yang along with 16 other YANG I-Ds that I
have seen lately, containing snippets such as

     container key-chains {
       list key-chain-list {
         key "name";
         description
           "List of key-chains.";
         uses key-chain;

I think that there should be a consistent spelling across the IETF.

Tom Petch

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

---

From: Kent Watsen <[email-masked]>
Date: Mon, 18 Apr 2016 16:54:53 +0000

I discussed this naming issue with Acee (CC-ed) in the hallway at BA.  He said that he used "key-chain" because that is what Cisco/Redback CLI uses.  We then searched on "juniper key-chain" and found that JUNOS uses "keychain".  I'm not sure if a more exhaustive search has been made.

I think the netconf draft should stick with "keychain" for now.  I'd like to see some discussion in the routing area if they might be better off using "keychain"...

Kent

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Mon, 18 Apr 2016 17:25:46 +0000

Hi Kent, 
I did follow up on this. The reason for “key-chain” in the model name is
that “keychain” is not a well-known compound word. Additionally, Cisco
OSs, use “key chain” and Ericsson uses “key-chain”. I’d be interested in
other thoughts on this.

I did get some negative feedback with respect to adding “routing-“ to the
model name since key chains are used for other non-routing applications as
well. What are your thoughts on this given that you now see the ubiquitous
usage of key chains across vendors?

Thanks,
Acee

On 4/18/16, 12:54 PM, "Kent Watsen" <[email-masked]> wrote:

>
>I discussed this naming issue with Acee (CC-ed) in the hallway at BA.  He
>said that he used "key-chain" because that is what Cisco/Redback CLI
>uses.  We then searched on "juniper key-chain" and found that JUNOS uses
>"keychain".  I'm not sure if a more exhaustive search has been made.
>
>I think the netconf draft should stick with "keychain" for now.  I'd like
>to see some discussion in the routing area if they might be better off
>using "keychain"...
>
>Kent
>
>
>
>
>
>

---

From: Mahesh Jethanandani <[email-masked]>
Date: Mon, 18 Apr 2016 13:43:35 -0700

> On Apr 18, 2016, at 10:25 AM, Acee Lindem (acee) <[email-masked]> wrote:
> 
> well. 

One of those non-routing protocols is BFD. I am fine if the model is called protocol-key-chain, but I wonder what happens the next entity needing key-chain is not a protocol.

The bigger question in my mind is, are these really different types of key-chains models, or are we talking about one key-chain model?

Mahesh Jethanandani
[email-masked]

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Mon, 18 Apr 2016 22:04:54 +0000

From: Mahesh Jethanandani <[email-masked]<mailto:[email-masked]>>
Date: Monday, April 18, 2016 at 4:43 PM
To: Acee Lindem <[email-masked]<mailto:[email-masked]>>
Cc: Kent Watsen <[email-masked]<mailto:[email-masked]>>, Tom Petch <[email-masked]<mailto:[email-masked]>>, Martin Bjorklund <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, Routing WG <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
Subject: Re: [Netconf] mbj review of draft-ietf-netconf-restconf-server-model-09

On Apr 18, 2016, at 10:25 AM, Acee Lindem (acee) <[email-masked]<mailto:[email-masked]>> wrote:

The rtgwg key chain model is the one we all know and love associated with the graceful rollover of configurable keys. The netconf model is list of certificates for a public key. Please look at the information content of the two models. I hope I don’t have to answer this question again ;^)

Acee

Mahesh Jethanandani
[email-masked]<mailto:[email-masked]>

---

From: Kent Watsen <[email-masked]>
Date: Tue, 19 Apr 2016 00:50:17 +0000

>   A list of trusted certificates.

This text needs to be updated (pending the outcome of this thread)

>This maybe adds to the confusion.

A trust anchor [certificate] (e.g., a self-signed cert, the root of a CA tree) is a trusted certificate, but not all trusted certificates are trust anchors.  For instance, when a NETCONF server pins a client's TLS certificate for netconf-tls client auth, the trusted certificate is not an anchor/root certificate.

>"trusted-certificate-list" (or "-set")?

Sure, or even "-bag", as sometimes in PKI there are references to a bag of certificates...

>native language, obviously)

Gotcha, okay, I changed it to "use-case specific" - thanks.

>><SNIP/>

I suppose, but wouldn't there still be a pyang validation error?

>      Use only a config false list and define actions to manipulate

It seems that it would be expected to support backup/restore of private keys, when possible.  My assumption is that such access would have to be protected by nacm:default-deny-all, so that even read-only access to the private key data would be guarded.  

For cases when a one or more private keys are hidden by specialized hardware, it would be good if get-config (RESTCONF's GET) response could indicate something (a warning?) indicating the private key's value is not available - what do you think?  I know this doesn't exist now, but it shows potential for how this could be resolved.

I'm not following your "use a config list and a separate -state list" suggestion.  It seems to me that in such cases where the private key data is unavailable, a new "key-data" node (under "private-key") would simply be missing (e.g., it's not mandatory true).  

Of course, we would then likely expect the private key to be configurable by editing the key-data field, but that won't work all the time, and hence the action statements (at least the 'generate-private-key' action) would still be needed.

What do you think?  

>Yes, if you need the flexibility (which I think you do).

Okay, I now have:

  identity key-algorithm {
    description
      "Base identity from which all key-algorithms are derived.";
  }
  identity rsa {
    base key-algorithm;
    description
      "The RSA algorithm.";  // FIXME
  }

  ...

And changed the type to an identityref:

        leaf algorithm {
          type identityref {
            base "key-algorithm";
          }
          config false;
          description
            "The algorithm used by the private key.";
        }

So now new algorithms can be augmented in when needed.  All good now?

>> >   the leafref may refer to multiple leafs.  But in this case it might

Hmmm, I don't think that it's okay that a certificate name might be the same for two different private keys.  I wish this could be controlled in the model, like with a 'unique' statement referencing descendant nodes (ex: unique "certificate-chains/certificate-chain/name").

So what do we do?  Document the issue in the YANG module (see below) or change the model somehow to eliminate the problem?  - suggestions?

OLD:
            leaf name {
              type string;
              description
                "An arbitrary name for the certificate chain.";
            }

NEW:
            leaf name {
              type string;
              description
                "An arbitrary name for the certificate chain.  So as
                 to ensure that the certificate chain can be uniquely
                 referenced by other modules, the name must be unique
                 across all private keys.";
            }

>Suppose you have: <SNIP/>
>and this instance data: <SNIP/>
>and also: <SNIP/>
>The leaf x now points to two leafs: <SNIP/

Okay, but then maybe then we should use an instance-identifier, and just document that it needs to point to a keychain certificate chain...

>> Is your recommendation to add text indicating that those values also

Should the description statement above be modified with a warning regarding "::" meaning different things on different OS's?

>we recommend that the ip-address is mandatory, forcing the explicit

I'm happy to document that "0.0.0.0" means 'any ipv4 address' and "::" means 'any ipv6 address', but above you said not to...maybe we should?

>Nope.

I meant as a YANG-next thing.   It seams that OR-ing in an feature statement would backwards compatible.  Admittedly, targeting a feature statement is totally different than targeting a tree node.  You think it's worth adding to the yang-next list?

>There are two options; either remove the feature on the listen

This idiom is also used on the "call-home" node:

       +--rw call-home {(ssh-call-home or tls-call-home)}?

where we definitely want a feature statement, as certainly not all servers will support call home.  So the option to simply remove the if-feature statement altogether is not possible.

The only viable option is your idea to create generic "listen" and "call-home" features.  So a NETCONF server supporting SSH, both listening and call home, would have to advertise (via yang-library of course) the four following features: listen, ssh-listen, call-home, and ssh-call-home.  This seems excessive, but do we care?  

Thanks again,
Kent

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Sat, 30 Apr 2016 18:40:12 +0000

So hopefully we’ve put the issue of combining the module to bed for good… If look at the date nodes for these two models, it is patently clear that these serve two different purposes.

What about the naming issue? I got a comment that I should take “routing-“ back out due to the fact that this is what that these key-chains can be used for many non-routing purposes. For example, BFD - http://www.juniper.net/documentation/en_US/junos14.2/topics/reference/configuration-statement/key-chain-edit-security-authentication-key-chains.html

Thanks,
Acee

From: rtgwg <[email-masked]<mailto:[email-masked]>> on behalf of Acee Lindem <[email-masked]<mailto:[email-masked]>>
Date: Monday, April 18, 2016 at 6:04 PM
To: Mahesh Jethanandani <[email-masked]<mailto:[email-masked]>>
Cc: Martin Bjorklund <[email-masked]<mailto:[email-masked]>>, Tom Petch <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, Routing WG <[email-masked]<mailto:[email-masked]>>
Subject: Re: [Netconf] mbj review of draft-ietf-netconf-restconf-server-model-09

Acee

Mahesh Jethanandani

---

From: Mahesh Jethanandani <[email-masked]>
Date: Sat, 30 Apr 2016 15:10:38 -0700

That or we could also rename it to protocol-key-chain to disambiguate it
from system-key-chain.

On Sat, Apr 30, 2016 at 11:40 AM, Acee Lindem (acee) <[email-masked]> wrote:

> So hopefully we’ve put the issue of combining the module to bed for good…
> If look at the date nodes for these two models, it is patently clear that
> these serve two different purposes.
>
> What about the naming issue? I got a comment that I should take “routing-“
> back out due to the fact that this is what that these key-chains can be
> used for many non-routing purposes. For example, BFD -
> http://www.juniper.net/documentation/en_US/junos14.2/topics/reference/configuration-statement/key-chain-edit-security-authentication-key-chains.html
>
> Thanks,
> Acee
>
> From: rtgwg <[email-masked]> on behalf of Acee Lindem <
> [email-masked]>
> To: Mahesh Jethanandani <[email-masked]>
> Cc: Martin Bjorklund <[email-masked]>, Tom Petch <[email-masked]>, "
> [email-masked]" <[email-masked]>, "[email-masked]"
> <[email-masked]>, Routing WG <[email-masked]>
> Subject: Re: [Netconf] mbj review of
> draft-ietf-netconf-restconf-server-model-09
>
>
>
> From: Mahesh Jethanandani <[email-masked]>
> To: Acee Lindem <[email-masked]>
> Cc: Kent Watsen <[email-masked]>, Tom Petch <[email-masked]>,
> Martin Bjorklund <[email-masked]>, "[email-masked]" <[email-masked]>,
> Routing WG <[email-masked]>, "[email-masked]" <
> [email-masked]>
> Subject: Re: [Netconf] mbj review of
> draft-ietf-netconf-restconf-server-model-09
>
>
> One of those non-routing protocols is BFD. I am fine if the model is
> called protocol-key-chain, but I wonder what happens the next entity
> needing key-chain is not a protocol.
>
> The bigger question in my mind is, are these really different types of
> key-chains models, or are we talking about one key-chain model?
>
>
> The rtgwg key chain model is the one we all know and love associated with
> the graceful rollover of configurable keys. The netconf model is list of
> certificates for a public key. Please look at the information content of
> the two models. I hope I don’t have to answer this question again ;^)
>
> Acee
>
>
>
>
>
> Mahesh Jethanandani
> [email-masked]
>
>
>
>

-- 
Mahesh Jethanandani
[email-masked]