Subject: [Netconf] WG LC for zerotouch

From: Mahesh Jethanandani <[email-masked]>
Date: Mon, 25 Sep 2017 18:45:17 -0700

The last round of WG LC on Zerotouch draft <https://tools.ietf.org/html/draft-ietf-netconf-zerotouch-17> resulted in lot comments being posted about the draft. Kent has since then updated the document to address all those comments and posted -17 version of the draft.

We believe the document is now ready for a re-run of that call. This starts a two-week WG LC on the draft. Please indicate your support if you believe the document is now ready for LC. We need affirmation of support for the LC to succeed. Unfortunately, silence is not affirmation. If you believe the document is still not ready, please indicate why. The LC will conclude on Oct. 9.

Authors, please indicate if you are aware of any IPRs related to the document. Also, please indicate if there are any known implementations of the draft.

Thanks.

Mahesh Jethanandani
[email-masked]

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 26 Sep 2017 19:27:01 +0200 (CEST)

Hi,

> We believe the document is now ready for a re-run of that call. This starts a two-week WG LC on the draft. Please indicate your support if you believe the document is now ready for LC. We need affi…

I have reviewed this document, and I believe the document is ready to
be published after a couple of issues have been addressed (see below).
I don't have the expertise to tell if the solution is secure enough or
not.

Here are my comments.

Major issues
------------

o  Section 7.2

  Adding custom query parameters like this breaks the normal YANG
  contract.  If I understand this correctly, the instance data tree
  presented to the client is supposed to change based on these query
  parameters.

  If you really need to have that functionality, it would better to
  model the device-specific data as an action; input would be the
  os-name, os-version etc, and output would be the
  zerotouch-information and voucher etc.

  Also, the draft doesn't discuss things like "one-time use voucher";
  this term is just used here.  It seems to me that this requires more
  descriptive text.

o  Section 4.4

     When a device is
     not able to trust a bootstrap server, it MUST NOT send its IDevID
     certificate in the form of a TLS client certificate

  How will the server authenticate the client in this case?

  I see that in section 7.3 you have:

   Note that the bootstrap server MUST NOT process a progress update
   from a device without first authenticating the device.  This is in
   contrast to when a device is fetching data from the server, a read-
   only operation, in which case device authentication is not strictly
   required (e.g., when sending signed information).

  But the server is a normal RESTCONF server, so it will follow
  section 2.5 of RFC 8040; it will require clients to be
  authenticated.

Normal issues
-------------

o  Section 1.2
       The term "manufacturer" is used herein to refer to the
       manufacturer of a device or a delegate of the manufacturer.

  In the text you sometimes use "manufacturer" and sometimes
  "manufacturer ot delegate".  Perhaps the term "manufacturer" should
  be reserved to mean just the manufacturer, and then use
  "manufacturer or delegate" in the places where that's appropriate.

o  Section 3.2

  In the first paragraph, maybe include a reference to RFC 5280.

o  Section 5.1

  The numbers in the picture don't match the numbers in the list
  (specifically 4-5).

o  Section 5.6

     Upon rebooting, the device MUST still be
     in its initial state, causing the bootstrapping process to run again,
     which will eventually come to this very point,

  Why this MUST?  What if the new boot image contains other factory
  defaults that does not enable ZTP, would that be illegal?

  I would think that implementations are free to do whatever they want
  in case of errors.

    In the case of errors, the device MUST reset
    itself in such a way that forces a reinstallation of the boot image,
    thereby wiping out any bad state the script may have left behind.

  Is this also required?  Why can't stop and wait for manual
  intervention be allowed?

o  Section 7.3

  The examples have "device=123456" in the URLs; it should be
  "device=123456789".

  I suggest you shorten the lines of the examples even more, so that
  the examples are properly indented in the draft (they are currently
  outdented)

o  Section 9

  Should you also mention that /device list should be protected by
  nacm rules?

  If the RESTCONF user name is the device's unique-id, then a single
  NACM rule can be used:

       <rule>
         <name>allow-device</name>
         <path xmlns:ztbs="urn:ietf:params:xml:ns:yang:\
                           ietf-zerotouch-bootstrap-server">
           /ztbs:device[ztbs:unique-id=$USER]
         </path>
         <access-operations>read</access-operations>
         <action>permit</action>
       </rule>

o  ietf-zerotouch-information.yang

  o sha256 is defined like this:

             leaf sha256 {
                type string;
                  "The hex-encoded SHA-256 hash over the boot
                   image file.

    Should this be type yang:hex-string?  If not, I think you need to
    specify how the hex-chars are encoded in the string.

   o uri is defined like this:

          leaf-list uri {
            type inet:uri;
            min-elements 1;
            description
              "An ordered list of URIs to where the boot-image file MAY
               be obtained.

   This leaf-list should be ordered-by user to indicate that the order is
   significant.

o  ietf-zerotouch-bootstrap-server.yang

  o the action "update-progress" should have a description

  o     If a script is erroneously provided to a device that does not
        support the execution of scripts, the device SHOULD send a
        'script-warning' notification message

    Rephrase to avoid the term "notification message"
    Also, the types are "pre-script-warning" and "post-script-warning".

   o The description of the "script" typedef says:

       No attempt is made to standardize the contents, running context,
       or programming language of the script.
       [...]
       The script returns exit status code '0' on success and non-zero
       on error, with accompanying stderr/stdout for logging purposes.

     I think the last quoted sentence contradicts the first - it does in
     fact make assumptions about the running context of the script.

     I suggest the latter sentence is removed, and the rest of the test
     adjusted.  Don't talk about exit codes, but instead talk about
     "success" or "error" etc.

o  Other comment

  Is it assumed that there will be a vendor-specific YANG module to
  enable the zero-touch process?  Did you consider an
  ietf-zerotouch-device module for this purpose?  Such a configuration
  must be carefully designed to allow for a "merge" configuration file
  to actually disable the zero touch process.

Editorial nits:
---------------

o  use "" instead of '' consistently  (except in YANG strings)

o  you probably should include a reference to ITU-T X.690.

o  s/bootstrap data/bootstrapping data/

o  I suggest you run the modules through 'pyang -f yang
   --keep-comments' in order to fix some inconsistent indentations

/martin

---

From: Kent Watsen <[email-masked]>
Date: Thu, 28 Sep 2017 03:29:34 +0000

Hi Martin,

Thanks for your review!  Comments below.

K.

> be published after a couple of issues have been addressed (see below).

We're getting there.

> I don't have the expertise to tell if the solution is secure enough or

This comment has been made by others as well.  The assumption is that
SecDir will do that evaluation.  Is that fair?

> Major issues

Correct.  The idea is that the response could vary based on the input
query params.  Of course, in case there was any doubt, the response 
would always conform to same YANG schema.

>  zerotouch-information and voucher etc.

I thought of this as well, but didn't do anything about it as my 
last message to the list said that we'd add "query parameters"
and I wanted to follow through on that statement.

But what is the issue?  Is it primarily that using query params 
on GET should be limited to returning different representations
of a resource, and this use seems to be returning different 
resources altogether?  I think that this is a grey area, but
appreciate the perspective.

Alternatively, is the issue more pragmatic, in that the list of
query params seems unbounded, and we might run out of room on
the URL string (1024 chars?) someday?  Or maybe perhaps that
input can be hierarchal whereas params can't?

I imagine the answer is "all of the above"  ;)

FWIW, if we were to do this, we might also consider moving the 
'serial-number' field from the URL to an input field, and make
a similar change to the 'update-progress' action.  But that then
would mess up your NACM rule below.  Thoughts?

>  this term is just used here.  It seems to me that this requires more

Good catch.  draft-ietf-anima-voucher informally calls it an "audit
voucher", which is an unfortunate misnomer.   Nonetheless, point taken,
the draft should tie into what's in the voucher draft.  Max Pritikin
told me a couple weeks ago that calling it a "nonced voucher" or a 
"voucher with a nonce" was probably best.  BTW, this seems like an
editorial fix to me, but I can see how it could look like a major
issue if you didn't know what the text was alluding to... 

>  How will the server authenticate the client in this case?

The idea is that the server doesn't auth the client in this case.
The goal behind not sending the IDevID certificate was so a rouge
bootstrap server wouldn't be able to discover the device's serial
number, which is why Appendix B suggests also masking the serial
number the device sends in the URL.  That said, we might need to
rethink this, especially in conjunction with your next comment.  
(continued below)  

>  section 2.5 of RFC 8040; it will require clients to be

I was hoping that the draft could suppress this auth requirement
for this specific "untrusted" interaction.  In essence, reducing
the bootstrap server to a simple file server providing anonymous
read-access.

But in the security analysis of things, it comes down to what's 
more important to prevent:
 a) a rogue server discovering a valid device's serial number,
    while not being able to provide the device a response the
    device can trust, or
 b) a valid server giving signed redirect information to a
    rogue device, where the information given doesn't provide
    the device any more information than the device must have
    already had to make the request in the first place, other
    than a confirmation that indeed that serial number is one
    that the server is expecting.  

Looking at it this way, (b) is more important to prevent and, 
besides:
1) there is already a Security Consideration related to the use
   of serial numbers in the solution, so (a) seems to be just
   more of the same, 
2) rogue connections could play havoc with the server's logs, and
3) (to your point) it would be tricky business to define an
   exception for the standard RESTCONF server to follow.

All this is to say that I think we should go back to the device
always sending its IDevID cert and, of course, the server always
authenticating the device.  But I also think that a device should
continue NOT sending any other information (i.e. query params,
progress updates, etc.), beyond its serial number & IDevID cert,
until it can fully authenticate the server.

> Normal issues

I mean for it to always be "manufacturer or delegate".  The better
fix is to remove "or delegate" throughout the document.  Agreed?

>  In the first paragraph, maybe include a reference to RFC 5280.

Yes.

>  The numbers in the picture don't match the numbers in the list

Ooops, I meant 2 & 3 to be 2a and 2b respectively.  Will fix, most
likely by flattening the (2) text into distinct 2 & 3 sections
again.

>  I would think that implementations are free to do whatever they want

Maybe not *illegal*, but it would certainly brick the device at that
point.  Would just reducing the "MUST" to a "must" suffice, or should
we add a qualifier along the lines of "in order for the solution to
work..." ?

>  Is this also required?  Why can't stop and wait for manual

It could, but again, it would brick the device.  Same question as above,
would reducing the "MUST" to a "must" suffice, or should we add a 
qualifier along the lines of "in order for the solution to work..." ?

>  The examples have "device=123456" in the URLs; it should be

Yes.

>  the examples are properly indented in the draft (they are currently

It's hard to make the lines much shorter with the URN being so long.
Options are 1) don't indent just the xmlns line or 2) move from two-
space indent to single-space indent.  Do you have a preference?

>  Should you also mention that /device list should be protected by

Yes, indeed, I left out the standard security template for the bootstrap
server YANG module.  Will add.

>         <path xmlns:ztbs="urn:ietf:params:xml:ns:yang:\ietf-zerotouch-bootstrap-server">

Yes, but would you expect this to appear in the draft?  Typically, the specific
NACM rule syntax is left as an exercise to the reader...

>    specify how the hex-chars are encoded in the string.

Yes, it should be yang:hex-string.

>   This leaf-list should be ordered-by user to indicate that the order is

Yes, it should be ordered-by user.

>  o the action "update-progress" should have a description

Strange, this wasn't caught by any of:
  pyang --ietf --strict ...
  pyang --canonical ...
  yanglint ...

But will add anyways.

>    Also, the types are "pre-script-warning" and "post-script-warning".

Will fix.

>     "success" or "error" etc.

Will do.

>  to actually disable the zero touch process.

It is assumed to be vendor-specific currently.  I have not thought to
define a ietf-zerotouch-device module as of yet that, presumably, would
contain a boolean or an enum (but not a p-container) for enabling the
service.  Do you think we should add such a module to the draft, or just
put a note somewhere about it?

> Editorial nits:

Thanks. Will double-check all.

> o  you probably should include a reference to ITU-T X.690.

Will do.

> o  s/bootstrap data/bootstrapping data/

Good catch, thought I got them all before.

>   --keep-comments' in order to fix some inconsistent indentations

Will do.

Thanks again!
Kent

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 28 Sep 2017 09:45:14 +0200 (CEST)

> SecDir will do that evaluation.  Is that fair?

Fine with me!

> would always conform to same YANG schema.

See below.

> resources altogether?

Yes, this is my concern.  A normal RESTCONF / YANG server has *one*
single instance of the operational state, and it just provides an API
to this data.  It can be filtered and pruned based on access rights,
but not modified from the outside based on query parameters.

> I think that this is a grey area, but
> 'serial-number' field from the URL to an input field

Hmm, did you mean 'unique-id'?

> , and make

We can have:

  list device {
    key unique-id;

    leaf unique-id { ... }

    action get-bootstrapping-data {
      input {
        leaf os-name { ... }
        leaf os-version { ... }
        ...
      }
      output {
        leaf zerotouch-information { ... }
        leaf ownership-voucher { ... }
        leaf owner-certificate { ... }
      }
    }

    action report-progress {  // I like this name better :)
      ...
    }
  }

And then NACM can still be used to provide access control to this
list.

> issue if you didn't know what the text was alluding to... 

Ok.

> until it can fully authenticate the server.

Ok.

> fix is to remove "or delegate" throughout the document.  Agreed?

Ok.

> we add a qualifier along the lines of "in order for the solution to

I would just say something like 

     Upon rebooting, if the new image also enables zerotouch, the
     device will still be
     in its initial state, causing the bootstrapping process to run again,
     which will eventually come to this very point,

> qualifier along the lines of "in order for the solution to work..." ?

Well, this depends if the error is transient or not.  If it is not
transient, it will just happen again and again.

I don't have a strong opinion, but I would probably be silent about
what to do in case of these kinds of errors.  Leave that to the
implementation to figure out.

> space indent to single-space indent.  Do you have a preference?

In the second example you use (1).  You can also use "\", which you
are also already using.

I just found another outdented figure in section 5.3, which probably
is easier to fix.

> NACM rule syntax is left as an exercise to the reader...

In order to do this, the "username" must be the same as the
"unique-id".  This requires the client cert to be constructed
properly.

Since the NACM rule is not obvious, and due to the cert implications,
I think it would be very helpful with an appendix that describes how
this can be set up.

>   pyang --ietf --strict ...

Yes, this is b/c --ietf implements 6087, and actions were not listed
there.  Will be fixed!

> put a note somewhere about it?

I don't have a strong opinion, but if there's not a standard module
for this, the text should mention that it is assumed that vendors
define this themselves.   Hmm, I think I would prefer a standard
module; it would make the solution more complete.

/martin

---

From: Kent Watsen <[email-masked]>
Date: Fri, 29 Sep 2017 01:44:16 +0000

Hi Martin,

I trimmed the discussion down to just the open threads.

> but not modified from the outside based on query parameters.

I agree, it seems more proper.  Okay, let's make this change.  No
objections from the WG, right?  As a heads-up for those following
along, this change doesn't affect the essence of the solution, but
the diff will appear somewhat big...

> Hmm, did you mean 'unique-id'?

Yes. But what about the idea, moving 'unique-id' from the URL path
to an input param?  Or, even more dramatically, we could remove 
'unique-id' altogether, relying instead on the RESTCONF username
extracted from the DevID certificate?

If relying on the extracted username, note that none of the 
'cert-to-name' identities defined in RFC 7407, which is also
used in draft-ietf-netconf-restconf-client-server, would work.
The 'common-name' identity is closest, but what is really needed
is a combination of a 'serialNumber' attribute and an optional
'hardwareModuleName' attribute (not all DevID certs define it).
I suppose that this is an issue for the afore-mentioned restconf
client/server draft more so than this one though.  Still, since
we're talking about it, assuming we did this, would that draft
add said identities, or would a 7407bis be better?

>    action report-progress {  // I like this name better :)

Yes, this is roughly what switching to an action statement would look
like though, pending on the above discussion, we may use top-level RPCs
instead of actions, if relying on the RESTCONF username.

I agree, 'report-progress' does seem better.  It's a minor thing, but
happy to oblige.  (no objections from the WG, right?)

>> until it can fully authenticate the server.

Okay, it seems that we agree.  Any objections from the WG?  In case it's
unclear, this change is fairly significant from a solution-perspective.
It will also have a non-trivial impact from a draft-diff perspective.
Please chime in now with any opinions now.

>     which will eventually come to this very point,

Okay.

> transient, it will just happen again and again.

Maybe, maybe not.  The device could potentially land onto another/different
source of bootstrapping data that works the next go around.  The hope is
that the device keeps trying (and posting progress-updates regarding its
errors) ad infinitum until things eventually clear up.  For instance, an
admin eventually takes notice and administers corrective action.

> implementation to figure out.

I feel that it's critical that manual intervention is never required.
Yes, a manual step could be employed, but it should be a rare step.  I
think it's best to say something along these lines.  I'll try to state
this more clearly.

> In the second example you use (1).  You can also use "\", which you

So I do, albeit only in the HTTP-header parts, not the HTTP-body parts.

FWIW, my build-script inserts source files in situ as directed by
processing instructions. I can modify the build-script to do the 
auto-'\' insertion logic on some column-boundary, but it may need
a complimentary update on the extraction tools.  Do we need some 
sort of standard around this?

I'm aware that here you're only talking about examples, which 
currently are not extracted via tools, but the same auto-'\' 
insertion logic could also help with the YANG modules, as they
could become unwieldy too...though I suppose liberal use of 
groupings could always be employed to keep line-lengths in
check.  Okay, but still, a YANG Doctors discussion at IETF 99
suggested introducing the ability to auto-extract and validate
examples in drafts too, to automate as much of the YD review
process as possible, so still I think there may be a need for
a '\' standard of some sort here.  Thoguhts?

> I just found another outdented figure in section 5.3, which probably

Huh?  I don't see it.  The figure in 5.3 looks okay to me...

> "unique-id".  This requires the client cert to be constructed

Agreed.  In this case, the client cert is a DevID cert, and  802.1AR 
governs its construction.  Not too many options there.  Of course, 
the mapping of said cert to username is being discussed above wrt the
cert-to-name mapping logic... 

>> I think it would be very helpful with an appendix that describes how

I think that this will depend on if we go with an action or a top-level 
RPC. If an action, then NACM may have a place and an example may help.
But, if using a top-level RPC, then it seems that the server has no 
option but to auth the DevID cert and provide a RESTCONF username-specific
response - NACM can't constrain it any further, right?  (note: this seems
more secure to me - I like it)

> module; it would make the solution more complete.

We can add a module, but maybe it should be called something like
ietf-zerotouch-bootstrap-client, to mirror the ietf-zerotouch-
bootstrap-server module? - though this may be a mismatch as the
former regards a southbound protocol and the latter regards a
configuration model...

Maybe we can discuss what would be in this model.  I'm not sure if
I'm oversimplifying it, but I think the module might be just a single
config true leaf called something like "enabled".   There might also
be some config false leafs for the stuff mentioned in section 5.1, 
though it doing so is not so important since the bootstrapping only
occurs once (not ongoing management).  So a module would primarily
be for the "enabled" leaf.  Anything else?  Still think it's worth it?

Thanks again,
Kent