Subject: [Netconf] review of draft-ietf-netconf-rfc5539bis-01

From: Martin Bjorklund <[email-masked]>
Date: Wed, 07 Nov 2012 11:49:40 -0500 (EST)

Hi,

I have reviewed this draft and here are my comments.

o  2.1

     Once the TLS handshake has finished, the client and the server
     MAY begin to exchange NETCONF data.  In particular, the client
     will send complete XML documents to the server containing <rpc>
     elements, and the server will respond with complete XML documents
     containing <rpc-reply> elements.

   I guess the above might be ok, but I think it is unnecessary and
   somewhat confusing since it starts to talk about <rpc> before the
   discussion about <hello>.   I suggest you remove the 3 sentences
   starting with "In particular".

   I think you should also add text here that says that client/server
   identity verification (as described in section 3) is done before
   the <hello> message is sent (for the server this means before the
   NETCONF session has started).

o  2.1

     The previous version [RFC5539] of this document used the same
     framing sequence defined in [RFC6242]

   Did you mean 4742?

o  2.2

     The TLS session is closed as described in [RFC6242] Section 7.2.1.

  s/6242/5246/

o  3.2.1

     The NETCONF server MUST implement the algorithms for deriving
     NETCONF usernames from presented certificates that are documented
     in the ietf-netconf-tls YANG module, defined in Section 3.2.3

  So here is a "MUST implement", but the YANG module defines the
  certificate mapping procedure as a feature.

o  3.2.3

  I suggest you run pyang --ietf on this module, and look into the
  problems reported.  pyang --ietf checks the guidelines defined in
  RFC 6087.

o  container cert-maps

       Once a matching cert-map list entry has been found, the NETCONF
       server uses the map-type list to determine how the NETCONF username
       associated with the session should be determined.  See the map-
       type leaf's description for details on determining the NETCONF
       username value.

    In the text above, 'map-type' is refered to both as a list and a
    leaf.  But it is actually a choice :)

o  list cert-map

    list cert-map {
      key "key";
      ...
      leaf key {
        type string;
        nacm:default-deny-all;
        description
          "The key associated with the cert-map list.";
      }

  Is this an arbitrary name?  If so I suggest you change the leaf's
  name to "name" and change the description to say this.

o  list cert-map

      description
        "A single list entry that specifies a mapping for an incoming
        TLS certificate to a NETCONF username.";

   This text about list entry seems to be copied from the MIB; it
   doesn't really apply in YANG.  You should say "... each list entry
   specifies a mapping..."

   Actually, the list's function is described already in the container
   above.

o  leaf specified

   I think this should be renamed to 'user-name'.  That's more
   descriptive and consistent with list psk-map.

o  leaf key (in list psk-map)

   Shouldn't this leaf be mandatory?

o  leaf user-name

   I have no idea what kind of values people use for PSK identities,
   but is it common to use a value that typically can be used as the
   NETCONF user name?  If so, maybe the 'user-name' leaf can be
   optional.

o  list psk-map

  Is the intention that this list is the configration of psk for the
  tls server, or is it assumed that they are configured in some other
  way?   I.e., if I add a new psk identity / key in this list, will
  the TLS server pick it up?