Subject: Re: [netmod] I-D Action: draft-ietf-netmod-factory-default-05.txt

From: Kent Watsen <[email-masked]>
Date: Fri, 1 Nov 2019 15:21:56 +0000

This begins a two-week Working Group Last Call (WGLC) on draft-ietf-netmod-factory-default-05.  The WGLC ends on Nov 15 (two days before the NETMOD 106 session).  Please send your comments to the working group mailing list.

Positive comments, e.g., "I've reviewed this document and believe it is ready for publication", are welcome!  This is useful and important, even from authors.  Objections, concerns, and suggestions are also welcomed at this time.

Thank you,
NETMOD Chairs

> On Nov 1, 2019, at 1:59 AM, [email-masked] wrote:
> 
> 
> A New Internet-Draft is available from the on-line Internet-Drafts directories.
> This draft is a work item of the Network Modeling WG of the IETF.
> 
>        Title           : Factory Default Setting
>        Authors         : Qin Wu
>                          Balazs Lengyel
>                          Ye Niu
> 	Filename        : draft-ietf-netmod-factory-default-05.txt
> 	Pages           : 11
> 	Date            : 2019-10-31
> 
> Abstract:
>   This document defines a method to reset a server to its factory-
>   default content.  The reset operation may be used e.g. during initial
>   zero-touch configuration or when the existing configuration has major
>   errors, so re-starting the configuration process from scratch is the
>   best option.
> 
>   A new factory-reset RPC is defined.  Several methods of documenting
>   the factory-default content are specified.
> 
>   Optionally a new "factory-default" read-only datastore is defined,
>   that contains the data that will be copied over to the running
>   datastore at reset.
> 
> 
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-netmod-factory-default/
> 
> There are also htmlized versions available at:
> https://tools.ietf.org/html/draft-ietf-netmod-factory-default-05
> https://datatracker.ietf.org/doc/html/draft-ietf-netmod-factory-default-05
> 
> A diff from the previous version is available at:
> https://www.ietf.org/rfcdiff?url2=draft-ietf-netmod-factory-default-05
> 
> 
> Please note that it may take a couple of minutes from the time of submission
> until the htmlized version and diff are available at tools.ietf.org.
> 
> Internet-Drafts are also available by anonymous FTP at:
> ftp://ftp.ietf.org/internet-drafts/
> 
> _______________________________________________
> netmod mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netmod

---

From: Kent Watsen <[email-masked]>
Date: Fri, 1 Nov 2019 15:23:54 +0000

I have reviewed -05 and support it so long as the following comments are considered:

Kent // contributor

==== review ====

Section 1 is missing a NMDA-compliance statement, per
 https://tools.ietf.org/html/rfc8407#section-3.5.

Section 2 says:

   Factory-default content SHALL be specified by one of the following
   means in descending order of precedence

   1.  For the <running>,<candidate> and <startup> datastores as the
       content of the <factory-default> datastore, if it exists;

The (1) sentence doesn't flow from the sentence before.   Maybe you
mean something like:

   1.  Network management protocol (e.g., NETCONF, RESTCONF)
        operations may be used to access the contents  of <factory-default>.

Section 2 says:

   For the server supporting zero touch bootstrapping mechanisms, the
   factory default configuration causes the bootstrapping process to
   execute,e.g.,the server might reset configuration to device's factory
   default configuration,for the version of operating system software it
   is running.

s/the server might reset /the server resets /

Section 2 says:
   In addition,the "factory-reset" RPC might also be used
   to trigger some other restoring and resetting tasks such as files
   cleanup, restarting the node or some of the software processes,
   setting some security data/passwords to the default value, removing
   logs, or removing any temporary data (from datastore or elsewhere),
   etc.

s/the "factory-reset" RPC might /the "factory-reset" RPC MAY / ???

Section 3 says:

   this document introduces a new datastore resource named
   'Factory-Default' ...

'Factory-Default' should not be capitalized.

Section 3 says:

    The contents of the datastore can be read using NETCONF, 
    RESTCONF <get-data> and <get-config> operations.

Which doesn't make sense.  Perhaps:

    The contents of the datastore can be read using NETCONF 
     <get-data> and <get-config> operations, and the RESTCONF
    protocol equivalents.

Section 3 says:

      The operation <factory-
      reset> can be used to copy the factory default content to a set of
      read-write configuration datastores and then the content of these
      datastores is propagated automatically to any other read only
      datastores, e.g., <intended> and <operational>.

This is confusing.  I think what you want to say is

      The operation <factory-
      reset> copies the factory default content to <running> and,
      if present, <startup>.

Section 4 says:

  import ietf-netconf { prefix nc ; }
  import ietf-datastores { prefix ds; }

These statements are missing "reference" statements.

Section 4 says:

    description "The read-only datastore contains the configuration that
      will be copied into e.g., the running datastore by the
      factory-reset operation if the target is the running
      datastore.";

which excludes <startup> and confusingly mentions a "target" when
the RPC itself has no parameters.  Perhaps:

    description "The read-only datastore contains the configuration
    that  will be copied into <running> and, if present, <startup>.";

Section 5.

Please make the registrations have single-spaced lines.

Section 6.

The last paragraph doesn't make a point.  Perhaps conclude with
something like:

  "This module does not itself set "nacm:default-deny-write" on the 
   'factory-reset' RPC, leaving it to applications to configure the
    access control settings."

Appendix B should have a note to the RFC Stream Editor to 
remove it when the draft is published.

Kent 

> On Nov 1, 2019, at 11:21 AM, Kent Watsen <[email-masked]> wrote:
> 
>

---

From: "Joe Clarke (jclarke)" <[email-masked]>
Date: Fri, 1 Nov 2019 15:30:31 +0000

On Nov 1, 2019, at 11:21, Kent Watsen <[email-masked]<mailto:[email-masked]>> wrote:

In general, I think it’s ready to go.  Two nits:

* My name is misspelled in the acknowledgements

* In Section 2:

OLD:

2. by vendors using YANG Instance Data […] file format in vendor’s website or other places where off-line document is kept;

NEW:

2. by vendors using a file in YANG Instance Data […] format on the vendor’s website or in other places where similar off-line documents are kept;

Joe

---

From: Andy Bierman <[email-masked]>
Date: Fri, 1 Nov 2019 15:42:02 -0700

Hi,

I have read draft-ietf-netmod-factory-default-05 and have the following
comments:

* sec 2. Specifying factory-reset content

This section uses SHALL (equivalent to MUST) to declare the implementation
details for
the server to load the factory-default content.  This is not appropriate
for a server implementation detail.
What hard to the Internet is caused if the server has some other way to
load the factory config?
This section should be removed.

point 1 is unclear what it means to derive the factory-config from the
current config.

point 2 specifies a file format but there is no way to specify the file.
What is the value added here?
Some servers can use an XML file (and will continue to do so, per point 3).

Why would this document specify that a dynamic datastore SHALL be empty
upon reset?
This is an implementation detail or a standard detail for some future work.

* Sec. 4: rpc factory-reset

This RPC has no NACM protections.
There should be a nacm:default-deny-all extension added to restrict access.
The client invoking the RPC MUST have permission to write all the existing
config
that is being replaced with factory-reset contents.

There is no mention of any operational disruption caused by setting the
config to factory-reset contents.
This will vary greatly depending on the implementation and current config.

What if the config includes session and client config?
This RPC can prevent any further management of the device.
That seems worth mentioning in the security considerations.

Overall the draft provides useful functionality so I support its
publication.

(BTW, my name is also misspelled in the draft)

Andy

On Fri, Nov 1, 2019 at 8:22 AM Kent Watsen <[email-masked]> wrote:

>
> This begins a two-week Working Group Last Call (WGLC) on
> draft-ietf-netmod-factory-default-05.  The WGLC ends on Nov 15 (two days
> before the NETMOD 106 session).  Please send your comments to the working
> group mailing list.
>
> Positive comments, e.g., "I've reviewed this document and believe it is
> ready for publication", are welcome!  This is useful and important, even
> from authors.  Objections, concerns, and suggestions are also welcomed at
> this time.
>
> Thank you,
> NETMOD Chairs
>
>
>
>

---

From: "Rob Wilton (rwilton)" <[email-masked]>
Date: Mon, 11 Nov 2019 11:58:25 +0000

Hi,

I've reviewed draft -06 and have the following LC comments.

Section 1, in Introduction (probably the abstract could be similarly tweaked):

  1.  Suggest changing "... major errors, so re-starting ..." to "... major errors and so restarting ...".  This could also be fixed in the abstract.

  1.  Suggest deleting the sentence "When resetting a datastore all previous configuration settings will be lost and replaced by the
   factory-default content".  I think that this can covered further in the document, and doesn't need to be stated in the introduction.

  1.  Suggest changing "A new factory-reset RPC" to "A factory-reset RPC"

  1.  Regarding "Optionally a new "factory-default" read-only datastore is defined, that contains the data that will be copied over to all read-write configuration datastores at reset".

I suggest changing "read-write configuration datastores" to "read-write conventional configuration datastores", and import "conventional configuration datastore" from RFC 8342.  Also, I think that section 3 should be updated to indicate that the datastore is OPTIONAL to implement.  Hence proposed replacement text:

"A "factory-default" read-only datastore is defined, that contains the data to replace the contents of implemented read-write conventional configuration datastores when reset."

  1.  I'm not sure that the paragraph about the <delete> operation (presumably this should be <delete-config>) is required at all.  If you do want to retain it, then I would suggest condensing it from:

"   NETCONF defines the <delete> operation that allows resetting the
  <startup> datastore and the <discard-changes> operation that copies
   the content of the <running> datastore into the <candidate>
   datastore.  However it is not possible to reset the running
   datastore, to reset the candidate datastore without changing the
   running datastore or to reset any dynamic datastore.

   A RESTCONF server MAY implement the above NETCONF operations, but
   that would still not allow it to reset the running configuration."

to

"NETCONF defines the <delete-config> RPC operation,  but that only acts on the <startup-datastore>, whereas the <factory-reset> RPC operation can perform additional changes to the device to fully reset the device back to a factory-default state."

Section 1.1, Terminology,

  1.    "candiate => candidate".

  1.  "factory-default datastore", I think that this should be defined as a "A read-only configuration datastore ..."

Section 2. Factory-Reset RPC:

  1.  I think that this could be more prescriptive about how the datastores are reset:
     *   All supported conventional read-write configuration datastores (i.e. <running>, <startup>, and <candidate>) are all reset to the contents of <factory-default>,
     *   Read-only datastores receive their content from other datastores, e.g., <intended> is updated due to the config changes to <running>,
     *   All data in any ephemeral datastores MUST be discarded,
     *   The contents of the <operational> datastore MUST be reset back to an appropriate factory-default state,.

  1.  I agree with Andy about deleting the text on how the factory-default contents is defined.

Section 3, Factory-Default Datastore
  "Management operations":

  1.  Should indicate that the datastore is OPTIONAL to implement.  A device MAY only implement the <factory-reset> RPC without implementing the datastore, thus loosing the ability to see what configuration the device would be reset back to.

  1.  It is unclear what a "specialized, dedicated operation" is.  Does this mean an NETCONF/RESTCONF RPC, or something outside of YANG management protocols altogether?  Perhaps change this to "dedicated RPC operation"

  1.  Suggest changing "The contents of the datastore can be read using NETCONF <get-data> and <get-config> operations, and the RESTCONF protocol equivalents. to "The datastore can be read using the standard NETCONF/RESTCONF protocol operations."

  1.  Suggest changing "The operation <factory- reset>" to "The <factory-reset> RPC operation".  Perhaps also import the "RPC operation" definition from RFC 7950?

  1.  " On devices that support non-volatile storage, the contents of <factory > MUST persist across restarts."

The factory-default datastore is only useful if persists across restarts, so I would change this statement to: "The contents of <factory-default> MUST persist across device restarts."

Section 4: YANG Module

  1.  Please add references to the import statements.
  2.  Remove reference to <get-config> in the description, and realign the description text.
  3.  Change feature name from "factory-default-as-datastore" to "factory-default-datastore"
  4.  Please expand on the description associated with the "factory-reset" RPC since it can be impact other datastores and file contents, or reference back to the appropriate section of the RPC.
  5.  Change description of "factory-default" identity to "This read-only datastore contains the configuration data used to replace the contents ofthe read-write conventional configuration datastores during a factory-reset RPC operation."

Appendix A:

  1.  I wasn't sure that this section is required at all.

Thanks,
Rob

From: netmod <[email-masked]> On Behalf Of Kent Watsen
Sent: 01 November 2019 15:22
To: [email-masked]
Cc: [email-masked]
Subject: Re: [netmod] I-D Action: draft-ietf-netmod-factory-default-05.txt

Thank you,
NETMOD Chairs

On Nov 1, 2019, at 1:59 AM, [email-masked]<mailto:[email-masked]> wrote:

Abstract:

_______________________________________________
netmod mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/netmod

---

From: Joel Jaeggli <[email-masked]>
Date: Mon, 11 Nov 2019 04:52:36 -0800

> On Nov 1, 2019, at 08:21, Kent Watsen <[email-masked]> wrote:
> 
> 

No chair hat. 

I support moving this document along at this point. kent had a great set of edits which should be applied. but it otherwise is ready to go.

joel

> Thank you,
> NETMOD Chairs
> 
> 
> 
> 
>> On Nov 1, 2019, at 1:59 AM, [email-masked] <mailto:[email-masked]> wrote:
>> 
>> 
>> https://datatracker.ietf.org/doc/draft-ietf-netmod-factory-default/ <https://datatracker.ietf.org/doc/draft-ietf-netmod-factory-default/>
>>