A YANG Data Model for Factory Default Settings
draft-ietf-netmod-factory-default-15

Section 2:
* "All security sensitive data (i.e., private keys, passwords, etc.)  SHOULD be overwritten ..." presents a choice.  Why would an implementer not do this?
* "Implementors SHOULD reboot the device or otherwise restart processes needed to bootstrap it." leads me to the same question.

Nits:
* "Upon receiving the RPC" is followed by a list, so please add a colon
* "datastores(e.g.," -- add a space after "datastores"

Section 3:
Nits:
* "The contents of <factory-default> is defined  ..." -- s/is/are/

Section 5:
* "This document registers one URI in the IETF XML Registry [RFC3688]. ..." should say explicitly that it's the "ns" sub-registry receiving a new entry.

Thank you for addressing my DISCUSS items.

Thank you for the work put into this document. The document is clear, easy to read and quite useful.

Please find below some non-blocking COMMENTs. An answer will be appreciated.

I also support Barry's comment.

I hope that this helps to improve the document,

Regards,

-éric

== COMMENTS ==

If the "factory-default" is optional (per section 3), then it may be worth to specify this quality in the abstract and in the introduction.

-- Section 2 --
What happens with the different counters in the <operational> data store ?

Why is this a SHOULD for overwritting sensitive data and not a MUST? At least section 6 writes that "owner of the device MUST NOT rely on any sensitive data (e.g., private keys) being forensically unrecoverable"

[Thank you for addressing the rtg-dir review.]

The Abstract mentions the YANG data model and the datastore, but not the Factory-Reset RPC.  I think it should mention that as well.

While many of the secdir reviewer's complaints stem from the YANG security
considerations boilerplate, it still seems like it would be worth some form
of response to the review.

Section 1

   This document defines a method to reset a server to its factory
   default content.  The reset operation may be used, e.g., when the
   existing configuration has major errors so re-starting the
   configuration process from scratch is the best option.

   A "factory-reset" RPC is defined.  When resetting a device, all
   previous configuration settings will be lost and replaced by the
   factory default content.

nit: these two paragraphs talk about the same thing, but the next paragraph
is a different thing.  It may be better to combine these two in to a single
paragraph.

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

Can I suggest instead:

% A "factory-default" read-only datastore is defined, that reflects what the
% conventional read-write datastores would be overwritten with in the case of
% a factory-reset operation.

Section 2

                                                          All security
   sensitive data (i.e., private keys, passwords, etc.)  SHOULD be
   overwritten with zeros or a pattern before deletion.  [...]

I might suggest instead:

% When this process includes security-sensitive data such as cryptographic
% keys or passwords, it is RECOMMENDED to perform the deletion in as
% thorough a manner as possible (e.g., overwriting the physical storage
% medium with zeros and/or random bits) to reduce the risk of the sensitive
% material being recoverable.

It's probably worth noting that since this is only dymanically generated
files, any cryptographic keys that are part of the factory-installed image
will be retained (such as an IDevID certificate).

Section 3

   Following the guidelines for defining Datastores in the appendix A of
   [RFC8342], this document introduces a new optional datastore resource
   named "factory-default" that represents a preconfigured initial
   configuration that can be used to initialize the configuration of a

nit/soapbox: "preconfigured initial configuration" feels like an awkward
wording to me; perhaps "pre-set initial configuration" or "fixed initial
configuration"?

Section 4

        description
          "This read-only datastore contains the factory default
          configuration for the device used to replace the contents
          of the read-write conventional configuration datastores
          during a 'factory-reset' RPC operation.";

nit: the grammar here is off; maybe "for the device that will be used"?
(Or some adaptation of my proposed text from earlier.)

Section 6

If the factory-default configuration is an "open" one, then performing the
reset could leave the device (and thus the network!) vulnerable to attack
until it is properly configured.  The rtgdir reviewer's comments seem
related to this.

An attacker that could somehow cause the factory-reset to be performed would
cause the loss of running state/crypto keys that would potentially require a
lot of operator effort to recover (in addition to the more immediate DoS
issues).

There is some discussion in draft-ietf-anima-bootstrapping-keyinfra about
attacks that are possible when a device is restored to its factory default
state; it might be worth trying to incorporate some of that discussion in
some manner (whether inline or by reference).

   The "factory-reset" RPC can prevent any further management of the
   device if the session and client config are included in the factory
   default contents.

I'm not sure this is 100% correct.  If the factory default config overwrites
this items, then yes, it will prevent further management.  But we also say
to delete dynamic files from nonvoliatile storage, which at least to me
seems like it could include this class of items and cause the same symptoms
even if the configuration items in question are not included in the factory
default contents.