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

Kent Watsen <kent+ietf@watsen.net> Sun, 09 February 2020 19:16 UTC

Return-Path: <010001702b618021-ade19230-3c05-4d0f-be88-e735dad32390-000000@amazonses.watsen.net>
X-Original-To: netmod@ietfa.amsl.com
Delivered-To: netmod@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 77493120071; Sun, 9 Feb 2020 11:16:19 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.897
X-Spam-Level:
X-Spam-Status: No, score=-1.897 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_NONE=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=amazonses.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Drq1YysFmR6S; Sun, 9 Feb 2020 11:16:16 -0800 (PST)
Received: from a48-95.smtp-out.amazonses.com (a48-95.smtp-out.amazonses.com [54.240.48.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 7B053120047; Sun, 9 Feb 2020 11:16:16 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=6gbrjpgwjskckoa6a5zn6fwqkn67xbtw; d=amazonses.com; t=1581275775; h=From:Message-Id:Content-Type:Mime-Version:Subject:Date:In-Reply-To:Cc:To:References:Feedback-ID; bh=ION/rCYavkkFisl7BTdCEQK1CW05wsyn/cG727jCIG4=; b=hAKb114ddsSVc/hzwk2gGGuPhqkcG2H1K9HbI4hDhYRR4jhGW9mJiDThhuUcsLYq CKSdpWXkkQmsOxAaJgsM3L7cNOaoXvI3nd14PmOrlcmOyBjaXbH1cY2FyZxRCinma5I zgcgNre9iipG/6zsk1FTLm5HSWEIE1frmn40JKmk=
From: Kent Watsen <kent+ietf@watsen.net>
Message-ID: <010001702b618021-ade19230-3c05-4d0f-be88-e735dad32390-000000@email.amazonses.com>
Content-Type: multipart/alternative; boundary="Apple-Mail=_42844381-9007-4FBA-8B0D-5AE8385ACD51"
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\))
Date: Sun, 9 Feb 2020 19:16:15 +0000
In-Reply-To: <B8F9A780D330094D99AF023C5877DABAA962BD87@dggeml511-mbx.china.huawei.com>
Cc: "netmod@ietf.org" <netmod@ietf.org>
To: draft-ietf-netmod-factory-default <draft-ietf-netmod-factory-default@ietf.org>
References: <B8F9A780D330094D99AF023C5877DABAA9629F8D@dggeml511-mbx.china.huawei.com> <010001700b8532d3-24b47a66-44ab-425b-a8a3-53b326a33cee-000000@email.amazonses.com> <B8F9A780D330094D99AF023C5877DABAA962BD87@dggeml511-mbx.china.huawei.com>
X-Mailer: Apple Mail (2.3445.104.11)
X-SES-Outgoing: 2020.02.09-54.240.48.95
Feedback-ID: 1.us-east-1.DKmIRZFhhsBhtmFMNikgwZUWVrODEw9qVcPhqJEI2DA=:AmazonSES
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/-Hu9EV6qzgFOOYDq3ul-RocFyTg>
Subject: Re: [netmod] I-D Action: draft-ietf-netmod-factory-default-09.txt
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETMOD WG list <netmod.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netmod>, <mailto:netmod-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netmod/>
List-Post: <mailto:netmod@ietf.org>
List-Help: <mailto:netmod-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netmod>, <mailto:netmod-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sun, 09 Feb 2020 19:16:20 -0000

[catching up on an old shepherd action item]


Authors,

While processing the shepherd writeup, I discovered some issues that should be fixed before this draft is forwarded to the IESG.  Please submit an update so that I can continue the review.

WG,

How is it that so many issues got through the Last-Call process?!


Comments by Section:

1) The abstract and Introduction both say “When resetting a datastore, …”, but this isn’t the case anymore, I mean, we only reset the device, right? 

2) The Abstract says “…datastore…that contains data that will be copied over to the running datastore at reset.”  This is inconsistent with what is stated elsewhere about the contents going into all conventional datastores.  Recommendation is to instead say that it contains the factory default configuration for the device.

3) The Introduction says "This datastore can also be used in <get-data> operation” - missing the word “the” before <get-data>.

4) The Introduction’s 4th paragraph serves no purpose and asymmetrically calls out NETCONF.  Please delete.

6) Section 1.1 says "A read-only configuration datastore holding a preconfigured minimal initial configuration that can be used to initialize the configuration of a server.”  Remove the word “minimal” and replace “can be” with “is”.

7) Section 1.1 says "The content of the datastore is usually static, but MAY depend on external factors like available HW.”  This is not true.  The contents are static, full stop.  However, the application of the contents may vary based on factors such as HW present.   But since this is always true, I recommend the entire sentence be removed.  

8) Section 2 says "The contents of the <operational> datastore MUST be reset back to an appropriate factory-default state.”  I think words “reset” and "factory-default state” are misleading.  Better would be to say “…MUST reflect the operational state of the device after applying the factory default configuration."

9) Section 2 says “...MUST restore storage to factory condition, including remove log files, remove temporary files (from datastore or elsewhere).”  This is both grammatically incorrect and confusing.  Recommend replacing with “...MUST restore non-volatile storage to factory condition.  Depending on the system, this may entail deleting dynamically generated files, such as those containing keys (e.g., /etc/ssl/private), certificates (e.g., /etc/ssl), and logs (e.g., /var/log), temporary files (/tmp/*).

10) After applying the fix from (9) above, the following two sentences could be replaced with “All security sensitive data (i.e., private keys, passwords, etc.) SHOULD be overwritten with zeros or a pattern before deletion."

11) Section 3: "Following guidelines for defining...” should be "Following the guidelines for defining…"

12) Section 3: replace 'factory-default’ with "factory-default”.  That is, use double-quotes. The only time single-quotes are used is when needing to quote something inside a YANG string.

13) Section 3:apply the same change described in (6) above wrt “minimal” and “can be”.

14) Section 3: remove the word “only” from "A device MAY only implement the <factory-reset> RPC without implementing the 'factory-default’ datastore”, or replace “without” with “, i.e., without”.

15) Section 3: replace "which make it lose the ability to see what configuration the device would be reset back to” with “which would only eliminate the ability to programmatically determine the factory default configuration”.

16) Section 3; replace "RESTCONF,the CLI etcunless” with “RESTCONF, the CLI, etc. unless” 

17) Section 3 needs to also mention that <candidate>, if present, is initialized to the contents of <factory-default>

18) Section 3: the word “usually” in the sentence "The datastore content is usually defined by the device vendor.” Should either be removed or explained how it could ever not be the case.  (I recommend the former)

19) Section 3: apply the same change described in (7) above.

20) both of the ‘import’ statements are missing ‘reference’ statements.”

21) Section 4: the module’s “description” statement is both incomplete and hard to read. How about replacing it with “This YANG module defines an RPC called ‘factory-reset’, a datastore identity called ‘factory-default’, and and identity called 'factory-default-datastore’."

22) Section 4, in the "factory-reset” RPC: replace "(i.e.,<running> , <startup>,and <candidate>)” with  "(i.e., <running> , <startup>, and <candidate>)”.  

23) Section 4, in the description statement for the "factory-default” identity, apply the same change from (2) above.  Also, “of the” should be “of the”.

24) Section 5, why are the registrations triple-spaced?

25) Section 6: replace the 2nd paragraph with “Access to the <factory-reset> RPC operation is considered sensitive in and therefore has been restricted using the 'default-deny-all’ access control defined in [RFC8341].”



Comments Not Bound to Specific Sections:

A) The ‘<factory-default>’ notation should be introduced before first use, which occurs inside a parentheses in Section 2.

B) “factory-default” and “factory default” (without the hyphen) are used inconsistently.  Recommend using the hyphenated form when referring to YANG node, i.e., "the ‘factory-default’ datastore” and "<factory-default>”.

C) “factory-reset” and “factory reset" (without the hyphen) are used inconsistently.  Recommend using the hyphenated form when referring to YANG node, i.e., "the ‘factory-reset’ RPC” and "<factory-reset>”.

D) Replace “HW” with “hardware” (occurs three places in the draft).


Thanks,
Kent // as shepherd