[Gen-art] Gen-art LC review: draft-ietf-netconf-restconf-15
Robert Sparks <rjsparks@nostrum.com> Fri, 29 July 2016 20:11 UTC
Return-Path: <rjsparks@nostrum.com>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 6A2E912D86C; Fri, 29 Jul 2016 13:11:48 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -3.187
X-Spam-Level:
X-Spam-Status: No, score=-3.187 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, RP_MATCHES_RCVD=-1.287] 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 hEN7jPme0mH4; Fri, 29 Jul 2016 13:11:46 -0700 (PDT)
Received: from nostrum.com (raven-v6.nostrum.com [IPv6:2001:470:d:1130::1]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id CD65512D86A; Fri, 29 Jul 2016 13:11:46 -0700 (PDT)
Received: from unnumerable.local ([173.57.161.14]) (authenticated bits=0) by nostrum.com (8.15.2/8.15.2) with ESMTPSA id u6TKBjTx082092 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK); Fri, 29 Jul 2016 15:11:46 -0500 (CDT) (envelope-from rjsparks@nostrum.com)
X-Authentication-Warning: raven.nostrum.com: Host [173.57.161.14] claimed to be unnumerable.local
To: General Area Review Team <gen-art@ietf.org>, "ietf@ietf.org" <ietf@ietf.org>, netconf@ietf.org, draft-ietf-netconf-restconf.all@ietf.org
From: Robert Sparks <rjsparks@nostrum.com>
Message-ID: <5c5a0c28-7cb3-da17-53dc-ec6401be9d1c@nostrum.com>
Date: Fri, 29 Jul 2016 15:11:45 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.2.0
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/gen-art/FQ6zdq_NPOWDhBf9OAA9QZpyiuU>
Subject: [Gen-art] Gen-art LC review: draft-ietf-netconf-restconf-15
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 29 Jul 2016 20:11:48 -0000
I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. Document: draft-ietf-netconf-restconf-15 Reviewer: Robert Sparks Review Date: 28Jul2016 IETF LC End Date: 3Aug2016 IESG Telechat date: not yet scheduled Summary: Major issues: * I am not finding any discussion in the Security Considerations or in the text around what a server's options are if a client is asking it to keep more state than it is willing or capable of holding. The possible values of the "depth" query parameter (particularly "unbounded") points out that a misconfigured or compromised client might start creating arbitrarily deep trees. Should a server have the ability to say no? * The third paragraph of 3.7 paraphrases to "SHOULD NOT delete more than one instance unless a proprietary query parameter says it's ok". This isn't really helpful in a specification. Proprietary things are proprietary. The SHOULD NOT already allows proprietary things to do something different without trainwrecking the protocol. Please just delete the 2nd and 3rd sentence from the paragraph. * Section 2.3 says "If X.509 certificate path validation fails and the presented X.509 certificate does not match a locally configured certificate fingerprint, the connection MUST be terminated as defined in [RFC5246]." RFC5246 doesn't really talk about certificate validation, and it certainly doesn't say "the connection MUST be terminated" when certificates fail to validate. What are you trying to point to in RFC5246 here? Should you be pointing somewhere else? (It's perfectly reasonable for the document to reference RFC5246, and it does so elsewhere without problem). Minor issues: * "A server MUST support XML or JSON encoding." is ambiguous. (2nd paragraph of 5.2). Did you mean the server MUST support at least one of XML or JSON but not necessarily both? I think you really intended that the server support BOTH types of encoding. * I _think_ I can infer that PUT can't be used with datastore resources. Section 3.4 only speaks of POST and PATCH. Section 4.5 speaks about "target data resource" and is silent about datastore resources. If I've understood the intent, please be explicit about datastore resources in 4.5. If I've misunderstood then more clarity is needed in both 3.4 and 4.5. * In 3.5.3.1 you restrict identifiers with "MUST NOT start with 'xml' (or any case variant of that string). Please call out why (or point to an existing document that explains why). * The text in 5.3 about access control interacting with caching (added based on my early review I think) doesn't mesh well with paragraph 3 of section 5.5. There you tell the client to use Etag and Last-Modified, but in 5.3 you say it won't work reliably when access permissions change. At the very least 5.5 should point back into the paragraph in 5.3. Nits/editorial comments: * Introduction, 4th paragraph - please change "MAY provide" to "provides". Section 3.6 explains the cases where there is choice in what to provide. * Section 2.3 paragraphs 1 and 2. There is edit-itis here left (I suspect) from working in matching fingerprints. Consider combining and simplifying these two paragraphs after improving the reference issue called out above. * Section 4 says "Access control mechanisms MUST be used to limit..." This is not a good use of a 2119 MUST. I suggest replacing "MUST be" with "are". The subsequent text already captures the actual normative requirements on the server. * Section 12 says "this protocol SHOULD be implemented carefully". That is not a good use of a 2119 SHOULD. It is not a protocol requirement. I suggest reformulating this into something like "There are many patterns of attack that have been observed through operational practice with existing management interfaces. It would be wise for implementers to research them, and take them into account when implementing this protocol." It would be far better to provide a pointer to where the implementer should start this research. * (micronit) Lots of examples are internally inconsistent wrt dates. For instance, look at the 200 OK in section 3.3.3 - it says that back in 2012, a server returned something talking about a library versioned in 2016.
- Re: [Gen-art] Gen-art LC review: draft-ietf-netco… Robert Sparks
- Re: [Gen-art] [Netconf] Gen-art LC review: draft-… Robert Sparks
- Re: [Gen-art] [Netconf] Gen-art LC review: draft-… t.petch
- Re: [Gen-art] [Netconf] Gen-art LC review: draft-… Kent Watsen
- Re: [Gen-art] [Netconf] Gen-art LC review: draft-… t.petch
- Re: [Gen-art] Gen-art LC review: draft-ietf-netco… Andy Bierman
- Re: [Gen-art] Gen-art LC review: draft-ietf-netco… Andy Bierman
- [Gen-art] Gen-art LC review: draft-ietf-netconf-r… Robert Sparks