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

Qin Wu <bill.wu@huawei.com> Mon, 10 February 2020 03:44 UTC

Return-Path: <bill.wu@huawei.com>
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 97275120091; Sun, 9 Feb 2020 19:44:26 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -4.199
X-Spam-Level:
X-Spam-Status: No, score=-4.199 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
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 VwfCK6llPqHE; Sun, 9 Feb 2020 19:44:23 -0800 (PST)
Received: from huawei.com (lhrrgout.huawei.com [185.176.76.210]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id D535212008C; Sun, 9 Feb 2020 19:44:22 -0800 (PST)
Received: from lhreml705-cah.china.huawei.com (unknown [172.18.7.107]) by Forcepoint Email with ESMTP id 3C9B6178C3004D70D7F2; Mon, 10 Feb 2020 03:44:20 +0000 (GMT)
Received: from DGGEML402-HUB.china.huawei.com (10.3.17.38) by lhreml705-cah.china.huawei.com (10.201.108.46) with Microsoft SMTP Server (TLS) id 14.3.408.0; Mon, 10 Feb 2020 03:44:19 +0000
Received: from DGGEML511-MBX.china.huawei.com ([169.254.1.52]) by DGGEML402-HUB.china.huawei.com ([fe80::fca6:7568:4ee3:c776%31]) with mapi id 14.03.0439.000; Mon, 10 Feb 2020 11:44:15 +0800
From: Qin Wu <bill.wu@huawei.com>
To: Kent Watsen <kent+ietf@watsen.net>, draft-ietf-netmod-factory-default <draft-ietf-netmod-factory-default@ietf.org>
CC: "netmod@ietf.org" <netmod@ietf.org>
Thread-Topic: I-D Action: draft-ietf-netmod-factory-default-09.txt
Thread-Index: AdXfwyK0JLSi2UxiRF6STzrC5sXmTw==
Date: Mon, 10 Feb 2020 03:44:15 +0000
Message-ID: <B8F9A780D330094D99AF023C5877DABAA96514D0@dggeml511-mbx.china.huawei.com>
Accept-Language: zh-CN, en-US
Content-Language: zh-CN
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [10.138.33.123]
Content-Type: multipart/alternative; boundary="_000_B8F9A780D330094D99AF023C5877DABAA96514D0dggeml511mbxchi_"
MIME-Version: 1.0
X-CFilter-Loop: Reflected
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/yIy-V0E3q2PpBVA-WObXBtWNuFQ>
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: Mon, 10 Feb 2020 03:44:27 -0000

Thanks Kent for shepherd review comments and will issue new version soon to address these comments.

发件人: Kent Watsen [mailto:kent+ietf@watsen.net]
发送时间: 2020年2月10日 3:16
收件人: draft-ietf-netmod-factory-default <draft-ietf-netmod-factory-default@ietf.org>
抄送: netmod@ietf.org
主题: Re: I-D Action: draft-ietf-netmod-factory-default-09.txt

[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?!

[Qin]: I believe many of them are wordsmith input, I am happy to accept them, thanks Kent.
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?

[Qin]:Yes, will update the draft to reflect this.
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.

[Qin]:Good suggestion to address inconsistency issue. Thanks.
3) The Introduction says "This datastore can also be used in <get-data> operation” - missing the word “the” before <get-data>.
[Qin]: Okay.
4) The Introduction’s 4th paragraph serves no purpose and asymmetrically calls out NETCONF.  Please delete.
[Qin]: Works for me.
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”.
[Qin]:Okay.
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.
[Qin]: Works for me.
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."
[Qin]:Good suggestion.
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/*).
[Qin]: Make sense, thanks.
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."
[Qin]: Okay.
11) Section 3: "Following guidelines for defining...” should be "Following the guidelines for defining…"
[Qin]:Okay.
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.
[Qin]:Okay.
13) Section 3:apply the same change described in (6) above wrt “minimal” and “can be”.
[Qin]:Okay.
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”.
[Qin]:Okay.
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”.
[Qin]:Okay.
16) Section 3; replace "RESTCONF,the CLI etcunless” with “RESTCONF, the CLI, etc. unless”
[Qin]:Good catch for the typo, will fix.
17) Section 3 needs to also mention that <candidate>, if present, is initialized to the contents of <factory-default>
[Qin]:Okay.
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)
[Qin]:Okay.
19) Section 3: apply the same change described in (7) above.
[Qin]:Okay.
20) both of the ‘import’ statements are missing ‘reference’ statements.”
[Qin]:Okay.
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’."
[Qin]:Okay.
22) Section 4, in the "factory-reset” RPC: replace "(i.e.,<running> , <startup>,and <candidate>)” with  "(i.e., <running> , <startup>, and <candidate>)”.
[Qin]:Okay.
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”.
[Qin]:Okay.
24) Section 5, why are the registrations triple-spaced?
[Qin]:Okay.
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].”
[Qin]:Okay.


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.
[Qin]: Will add a term in the terminology section.
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>”.
[Qin]:Okay.
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>”.
[Qin]:Okay.
D) Replace “HW” with “hardware” (occurs three places in the draft).
[Qin]:Okay.

Thanks,
Kent // as shepherd