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

[Kent Watsen <[e-mail]>]

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