Subject: Re: [Netconf] Comments on draft-ietf-netconf-server-model-03.txt

From: Kent Watsen <[email-masked]>
Date: Fri, 3 Oct 2014 19:02:51 +0000

Hi Alan,

Thank you for a fantastic review.  I have already incorporated all
your Nit comments.  Please see below for other responses.

Thanks again,
Kent

>General comments
>----------------
>
>Not sure if all of the groupings would be useful outside of this document.
>Some of the groupings have only a single "uses" within the document; e.g.,
>call-home-transport-config, call-home-config, etc.  It seems as if this
>was done to break up one large config into smaller chunks.  I find this
>use 
>of grouping/uses makes the YANG module more difficult to read and
>understand 
>than if these single-use grouping/users were simply inserted in-line.

Correct, all but 2 of the 11 groupings are only used once.  Interestingly,
I did this to help make the module more readable.  Of course I understand
that chasing the definition can be cumbersome, but the tradeoff is that
now each parent container can easily fit on the screen.  I don't know, is
there best practice to follow on this?

FWIW, I added https://github.com/netconf-wg/server-model/issues/12 to
track this issue.

>Comments on the listen-per-transport-config grouping, Page 12:
>--------------------------------------------------------------
>
>Under the description of the address leaf:
>
>    s/IP address\/name/IP address or host name/

Fixed!

>Under the port leaf:
>
>    Should this leaf be mandatory, or have a default of 0?

In both places where this grouping is used, a transport-specific default
is provided (830 for SSH, 6513 for TLS).  Is it good enough?

>The  listen-per-transport-config  grouping seems roughly like a
>"sockaddr_storage" structure.  An "endpoint", or perhaps a "sockaddr"
>or "socket" grouping seems like it would be useful in many YANG
>modules beyond this one.

<SNIP/>
True, it would've been a nice addition to ietf-inet-types.  What do
others think?

>Comments on the call-home-per-transport-config grouping, Page 14:
>-----------------------------------------------------------------
>
>This grouping seems generally useful beyond the configuration of
>call-home configuration.  It looks like basically like a list of
>endpoints, or "sockaddrs".  Perhaps generalizing this and putting it
>into the same YANG module with the revised listen-per-transport-config
>grouping might simplify its wider use.

Another good point.  What do others think?

>I _suspect_ I know the answer to this, but what is the purpose of the
>"endpoints" container around the "endpoint" list?

I thought that it was best practice to always have a parent container for
a list.  In RESTCONF terms, the container would be the "collection" and
would render URLs like .../widgets/widget=<key>/, which is a pretty common
idiom.  That said, the module is itself inconsistent in that the top-level
"listen" and "call-home" do NOT have parent containers.  The module is
trying to follow guildlines set by
http://tools.ietf.org/html/draft-schoenw-netmod-yang-pattern-00.  One
option might be use this:

   +--rw netconf-server
      +--rw listen
      |  +--rw endpoint* [name]
      |     +--rw name    string
      |     ...
      +--rw call-home
         +--rw application* [name]
            +--rw name    string
            | ...

What do you think, is it better?

>The name leaf seems to serve basically as a key for the list.  Would
>it be possible to eliminate this leaf, and use the address and port
>leaves as keys?  This would slightly simplify the grouping.   It might
>also use the "endpoint" grouping suggested above, and further re-use
>definitions.

As Juergen mentioned already, this would prevent augmentations from
supporting multiple routing instances (VRFs) if needed.

>Comments on the listen-config grouping, Page 11:
>------------------------------------------------
>
>It seems like the  listen-per-transport-config  could be promoted from
>from below each case branch to above the "choice transport".  Does the
>name leaf does have a function other than providing a list key?  If the
>listen-per-transport-config  were promoted, then the address and port
>could be used as keys, no?  This would simplify the grouping.  Perhaps
>something like:
>
>     grouping listen-config {
>       description
>         "Grouping for listen configuration.";
>
>       uses endpoint;   /* The new grouping, described above */
>       
>       choice transport {
>         mandatory true;
>         description
>           "Selects between SSH and TLS transports.";
>
>         case ssh {
>           if-feature ssh-listen;
>           container ssh {
>             description
>               "SSH-specific listening configuration for inbound
>                connections.";
>              refine port {
>                 default 830;
>              }
>           }
>         }
>         case tls {
>           if-feature tls-listen;
>           container tls {
>             description
>               "TLS-specific listening configuration for inbound
>                connections.";
>             refine port {
>               default 6513;
>             }
>           }
>         }
>       }
>     }
>
>Then, where this grouping is used:
>
>     container netconf-server {
>       description
>         "Top-level container for NETCONF server configuration.";
>
>       list listen {
>         key "address port";
>
>         description
>           "List of endpoints to listen for connections on.";
>         //if-feature "(ssh-listen or tls-listen)";
>         uses listen-config;

The solution used follows the guidelines set by
draft-schoenw-netmod-yang-pattern-00.  That said, I agree with you that
flattening the model some would be nice.  One issue in doing that, though,
is how to set the transport-specific default port values, as the port node
would longer be refine-able...

>Comments on the call-home-connection-type-config grouping, Page 15:
>-------------------------------------------------------------------
>
>Under the persistent-connection case, there is a "container persistent"
>and, under that, a "container keep-alives".  What is the function of the
>extra containment layer?  Would it make sense to remove one of these
>containment layers?

This is me just grouping things together for readability and
extensibility.  We could flatten the model some by having
"keep-alive-interval-secs" and "keep-alive-count-max" - does that seem
bester to you?  Also, while the model doesn't currently have any other
nodes under the "persistent" node, it may someday (perhaps through an
augmentation), such that having this structure helps keeps things nice and
organized.  Thoughts?

>Comments on the call-home-reconnection-strategy-config grouping, Page 17:
>-------------------------------------------------------------------------
>
>What determines the order in which management applications are contacted
>by call-home connections?   Is it the  name key  of the  endpoint  list
>in the  call-home-per-transport-config  grouping?
>
>How is the  last-connected  reconnection strategy handled across reboots
>of devices without stable storage?  Or is the "no stable storage" situ-
>ation out-of-scope?

No order is expected, a device could even connect to all it's "call-home"
nodes in parallel.  The draft defines the "call-home" list as unordered -
is it good enough? - if not, what text would you suggest adding?

The draft says in the description statement: "If no previous connection
has ever been established, last-connected defaults to the first endpoint
listed".  What the draft fails to say is if the "previous connection"
information needs to survive reboots.  Do you think this choice could be
left to implementations?

>Comments on the trusted-ca-certs-grouping grouping, Page 17:
>------------------------------------------------------------
>
>Is it expected that this grouping may be used elsewhere in other YANG
>modules?
>
>Is it intended that the  trusted-ca-cert  leaf-list is used for root
>(self-signed) CA certs only, or are subordinate CA certs allowed also?
>
>If this leaf-list is intended for self-signed CA certs only, then how
>are CA cert chains handled?

AFAIK, there is no known intension to use the grouping elsewhere.  That
said, you never know and, besides, I like to use groupings because I think
it improves readability (YMMV).

Yes, absolutely, subordinate certs are allowed.

That said, there may still be an issue with supporting chains, such as
when a client cert is signed by an intermediate CA that is signed by trust
anchor CA.  That is, the end user would like to configure the device to
auth any client cert having a chain of trust leading to the trust anchor.
Currently this is not possible because the wording "A client's certificate
is authenticated if its Issuer matches one of the configured trusted CA
certificates." does allow for indirections.    This seems like an
oversight to me, but I inherited this text and don't know if there is a
reason behind it.

Added https://github.com/netconf-wg/server-model/issues/13 to track this.

>Comments on the trusted-client-certs-grouping grouping, Page 17:
>----------------------------------------------------------------
>
>What is the purpose of this grouping?  As it is documented, this
>grouping may be unnecessary.
>
>When a management application connects to the NETCONF Server, either
>by forward or reverse (call-home) TLS, the management application
>will present an application cert (or cert chain) that identifies it.
>Two things have to happen for the NETCONF Server to authenticate the
>management application:  1)  The application cert presented by the
>management application must pass certificate verification up through
>one of the CA certs configured (in the NETCONF server) in the
>trusted-ca-certs-grouping, and  2) the NETCONF server must be able
>to convert this application cert to the NETCONF NACM username.
>
>Based upon the way the TLS authentication works (described in the
>previous paragraph), and the description of the trusted-client-certs
>container, I don't see what the function of this grouping is.  What
>am I missing here?  Should the trusted-client-certs-grouping be
>deleted?

This grouping is provides an alternate mechanism to authenticate client
certs.  It is to auth a specific user without authenticating every other
user's cert having the same Issuer.  This came from  issue #3 in this
thread: 
http://www.ietf.org/mail-archive/web/netconf/current/msg08825.html. Makes
sense?

>Other comments on the  NETCONF-over-TLS  configuration:
>-------------------------------------------------------
>
>When a NETCONF over TLS session is initiated, the NETCONF server must
>present an X.509 certificate to the NETCONF client so the client can
>verify the identity of the NETCONF server.  I do not see any mechanism
>for configuring an X.509 certificate that identifies the NETCONF server.
>
>Should there be such an object, or is this something that can only be
>configured out-of-band, or perhaps only by the manufacturer?

Yes, and this issue is tracked here:
https://github.com/netconf-wg/server-model/issues/4

Question, do you think configuring the TLS-server-cert/SSH-host-key to use
should be a system-wide configuration, or  something specific to the
NETCONF server?

Thanks again,
Kent

---

From: Alan Luchuk <[email-masked]>
Date: Mon, 13 Oct 2014 16:39:37 -0400 (EDT)

Hello,

Thanks for considering, and replying to,  my comments on  draft-ietf-
netconf-server-model-03.txt.  For most of the issues raised, I have no
strong opinion about what the "correct" answers are, and can align with 
WG consensus.  

I attempted to give closer attention to the schema nodes related to NETCONF 
over TLS.  Just after pressing "send" for my comments, I realized I failed 
to suggest the schema nodes related to NETCONF over TLS using pre-shared keys
should be removed to match the text of draft-ietf-netconf-rfc5539bis-06.txt.
This includes the removal of the  "feature tls-map-pre-shared-keys", the
"grouping psk-maps-grouping", and the single "uses psk-maps-grouping".

That said, here are replies to Juergen's and Kent's replies.

On Wed, Oct 1, 2014 at 08:34:56 +0200, Juergen Schoenwaelder wrote:

>> could be used as keys, no?  This would simplify the grouping. 
>
>I would be very careful about making addres and port the key since
>this makes extensions impossible, e.g., if you have to support VRFs.
>See also draft-schoenw-netmod-yang-pattern-00.txt.
>
>/js

I looked this document over, and did a bit more reading about VRFs.  I'm 
not well-versed on the related issues, so I'll defer to Juergen on this.

>is provided (830 for SSH, 6513 for TLS).  Is it good enough?

This answer is fine.  The only problem that _might_ arise is if the  
listen-per-transport-config  grouping is used again in the future, 
and the necessary port refinement is forgotten.  In this case, the port 
would be underspecified, and presumably be implementation-dependent, no?

>option might be use this:

OK, thanks for the explanation.

>What do you think, is it better?

This arrangement of the schema nodes seems clearer, but that doesn't
mean it is clearer :-)   Deferring to WG consensus is a good plan.

>supporting multiple routing instances (VRFs) if needed.

OK.  Others understand the issues better than I do, so I can align with
the WG consensus.

>augmentation), such that having this structure helps keeps things nice and

Leave the schema as it is.  This explanation is reasonable, and the
suggested simplification(s) are not sufficient to justify a change.

>information needs to survive reboots.  Do you think this choice could be

That sounds fine, but perhaps something along following line should be 
added to the description clause for the last-connected enum:

     "If a NETCONF server does not have non-volatile memory to store
      the last-connected endpoint across reboots, then upon reboot, the
      NETCONF server chooses an reconnect transport endpoint in an
      implementation-dependent manner."

>Added https://github.com/netconf-wg/server-model/issues/13 to track this.

OK, I think this should be fixed.  I'll look at it a bit more.

>http://www.ietf.org/mail-archive/web/netconf/current/msg08825.html. Makes

OK.  So basically, if a specific X.509 cert is stored in the 
trusted-client-cert list, then when a connecting NETCONF client presents 
that X.509 cert to the NETCONF server, the NETCONF server authorizes it.
The mere presence of an X.509 cert in the trusted-client-cert list
authorizes it when that cert is presented by a NETCONF client.

>should be a system-wide configuration, or  something specific to the

System-wide configuration seems simpler.  Something specific to the 
NETCONF server would allow multiple instances of a NETCONF server on 
a single system, and thus seems more flexible.

Regards,
--Alan

 ------------------------------------------------------------------------------
 Alan Luchuk               SNMP Research, Inc.          Voice:  +1 865 573 1434
 Senior Software Engineer  3001 Kimberlin Heights Road  FAX:    +1 865 573 9197
 luchuk at snmp.com        Knoxville, TN  37920-9716    http://www.snmp.com/
 ------------------------------------------------------------------------------

---

From: Kent Watsen <[email-masked]>
Date: Wed, 15 Oct 2014 23:23:32 +0000

Hi Alan,

>"grouping psk-maps-grouping", and the single "uses psk-maps-grouping".

Yes, the server-model will be updated to reflect what 5539bis settles on.

>would be underspecified, and presumably be implementation-dependent, no?

When a grouping is used, it is the responsibility of the modeler to ensure
it's augmented/refined as needed for their use.   I think it's OK the way
it is now.

>mean it is clearer :-)   Deferring to WG consensus is a good plan.

Cool, I just added https://github.com/netconf-wg/server-model/issues/15 to
track this issue.

>>better to you?  Also, while the model doesn't currently have any other

Will do.

>      implementation-dependent manner."

Just added https://github.com/netconf-wg/server-model/issues/16 to track
this

>authorizes it when that cert is presented by a NETCONF client.

Correct.

>a single system, and thus seems more flexible.

True, but I think the WG consensus is for this model to be specifically
for the NETCONF server.  I'll add the ability to configure them in this
model and them folks can discuss if it should be moved to another model.

Thanks again,
Kent

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 16 Oct 2014 22:34:57 +0200 (CEST)

Hi,

Below is a comment on Alan's comment, and some additional comments
from my review of this document.

> FWIW, I added https://github.com/netconf-wg/server-model/issues/12 to

I also think that excessive usage of groupings make models more
difficult to follow.  So I suggest the groupings are inlined.

--------------------

In the grouping listen-per-transport-config, address leaf has the type
inet:host.  Do we really suggest that the listen address is a DNS
name?  If we do, how should an implementation handle multiple
addresses (and/or address families)?

--------------------

In the grouping call-home-transport-config, there is a list host-key,
indexed by 'name'.  The 'name' is specified as "the name of a host
key".  What does this name refer to?  (I think we have discussed this
one before...?)

--------------------

In the grouping call-home-connection-type-config, there is a container
keep-alives.  When I read this I didn't know what this keep alive
thing refered to.  I suggest at minimum add a reference statement,
refering to rfc XXXX, section 4.  Maybe also additional text.

/martin

---

From: Kent Watsen <[email-masked]>
Date: Fri, 17 Oct 2014 01:28:43 +0000

Hi Martin,

>difficult to follow.  So I suggest the groupings are inlined.

OK, that makes 4, so I'll collapse some of the groupings

>addresses (and/or address families)?

Good catch.  I had already fixed this in my local copy too.

>key".  What does this name refer to?  (I think we have discussed this

Yes, this came up in Toronto and is tracked here:
https://github.com/netconf-wg/server-model/issues/4

I'm going to add new top-level containers netconf-server/{ssh,tls} to the
configured certificates and host-keys, and then this can become an
instance-identifier to an entry in said lists.   These lists will be
config=false.  In order to add new entries, I'm thinking to have RPCs like
<generate-ssh-host-key> and <generate-certificate-signing-request>.  What
do you think?

>refering to rfc XXXX, section 4.  Maybe also additional text.

Here's the new text, what do you think?

            container keep-alives {
              description
                "Configures the keep-alive policy, to proactively
                 test the aliveness of the NETCONF client, in order
                 to know when a new call home connection should be
                 established.  How keep-alives are implemented is
                 described in RFC XXXX, section 4.";

Thanks,
Kent

---

From: Martin Bjorklund <[email-masked]>
Date: Fri, 17 Oct 2014 09:16:18 +0200 (CEST)

Hi,

> <generate-ssh-host-key> and <generate-certificate-signing-request>.  What

Why do we have specified host-keys for callhome, but not for the
normal netconf server case?  And should we really have separate
host-keys for callhome and normal server?

>                  described in RFC XXXX, section 4.";

Ok, but also add:

     reference
       "RFC XXX: NETCONF Server Configuration Model
                 Section 4";

/martin

---

From: Kent Watsen <[email-masked]>
Date: Fri, 17 Oct 2014 15:20:39 +0000

>host-keys for callhome and normal server?

For zero-touch, which uses call home, it is critical that an X.509-based
host-key is the *only* host-key advertised, as the client needs it to
automatically authenticate the device.  Normal SSH typically uses RSA or
DSA host-keys; the server could also advertise an X.509-based key, but
it's not required. 

Keep in mind that there are multiple instances of `sshd` running - one for
port 22, another (or maybe the same) for port 830, and one for each
call-home.   In OpenSSH terms, there can be a distinct sshd_config file
for each.

>       "RFC XXX: NETCONF Server Configuration Model

Added in my local copy.

Thanks,
Kent

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Mon, 20 Oct 2014 08:30:18 +0200

> DSA host-keys; the server could also advertise an X.509-based key, but

Call home requires an X.509-based host-key? If so, why?

/js

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

---

From: Kent Watsen <[email-masked]>
Date: Tue, 21 Oct 2014 15:51:17 +0000

> Call home requires an X.509-based host-key? If so, why?

No, draft-ieft-netconf-call-home does not require X.509 based host-keys.
Further, the next version of draft-ietf-netconf-server-model will show
that the host-keys can be any standard SSH host-key.

The requirement for X.509 host-keys is only in
draft-ietf-netconf-zero-touch.   This use of call-home (e.g. zero-touch)
places a requirement on the NETCONF server's host-key being something that
encodes both an identity as well as a chain of trust (e.g., PKI), hence
X.509.   

FWIW, we could try to also support PGP-based host-keys (defined in RFC
4253), but I don't think PGP is used much anymore (if ever) as a SSH
host-key, and so I rather not invest the effort to update the draft to
support PGP keys too.

Thanks,
Kent