Re: Gen-art LC review: draft-ietf-netconf-restconf-15
Andy Bierman <andy@yumaworks.com> Fri, 29 July 2016 20:37 UTC
Return-Path: <andy@yumaworks.com>
X-Original-To: ietf@ietfa.amsl.com
Delivered-To: ietf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 210C412D88F for <ietf@ietfa.amsl.com>; Fri, 29 Jul 2016 13:37:00 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.9
X-Spam-Level:
X-Spam-Status: No, score=-1.9 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, SPF_PASS=-0.001] autolearn=unavailable autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=yumaworks-com.20150623.gappssmtp.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 om2N_JuG88Bp for <ietf@ietfa.amsl.com>; Fri, 29 Jul 2016 13:36:58 -0700 (PDT)
Received: from mail-ua0-x22f.google.com (mail-ua0-x22f.google.com [IPv6:2607:f8b0:400c:c08::22f]) (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 D333B12D878 for <ietf@ietf.org>; Fri, 29 Jul 2016 13:36:55 -0700 (PDT)
Received: by mail-ua0-x22f.google.com with SMTP id j59so69711228uaj.3 for <ietf@ietf.org>; Fri, 29 Jul 2016 13:36:55 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yumaworks-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Y2E/7l4KhSVXrx4majPbGM/Rq+bq866fk/6YHW4tR1A=; b=W6A/xIdMxoUI5jAz9Z/iK1P+yftHIcrUKQyEyRT+F6mpPF8HoFiIku7bcVugEI6S50 yGkjG7iMLORCJp5PdSYlCjUvmunn1rvAcSHr8n6wMpKkrLBCgWBampPN4jfthyA9duAq bDCVOYKOwVCJf+cmc+0OhGHRY7ygRoMbnCvktYBF3XqGs1MKJbORhguq+6mr+th3he4u KqBP3MerbmdMqvUbRNy71NKnuCJ7fi79hsQ8sPZsHakZI9rbCoMYvLKA142OqLcsiqLW AFWNn/m5k1cLD0Hj9ERkaUz+IW1Ixa0gZLH4XEEkTWBYkRow+f5H6BOwlmqSuUv4tm0v y/Zg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Y2E/7l4KhSVXrx4majPbGM/Rq+bq866fk/6YHW4tR1A=; b=LJ8JXDI5vi/8afnF6otC85/fj9R8w4mTvRVlB3b6WpDhPYXM//UhlzYfUjSbd9HnyA Y0Hhod7kBneo6IQbCaVIyaylK6bKECxaoGN7Me5rGfQ6lMJHx2Y+DQsztDMnjioWT1IL zZh0NrkRoJRFyKpyRO5BYtBYgN6gu3d75Q7DFEzz24qQ22GLi2B8ehYu8iAWNd5O4OOD OMmvDRt9RW1hbEioKTq7GMmGQzisW4HMnaQNAWZEKIjH2SPKSM9mpodEZ4Ngd14KhOW0 EpO/VH5H4y7LVYiaCyoxYdEWDUzI4jgXagbn5YhxSMPqiTcT6j13nBzGx2+QrX1PAfTx 6sJw==
X-Gm-Message-State: AEkooutRYNcZa0eZtnz1mhXpwBUkR/4r3vuZ3zDvKS2Y94Oq5BbAji7TDLR9OVIgnCuLEXZ3QFdPSaA0BPtUpQ==
X-Received: by 10.176.67.4 with SMTP id k4mr19777127uak.47.1469824614969; Fri, 29 Jul 2016 13:36:54 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.103.4.198 with HTTP; Fri, 29 Jul 2016 13:36:54 -0700 (PDT)
In-Reply-To: <5c5a0c28-7cb3-da17-53dc-ec6401be9d1c@nostrum.com>
References: <5c5a0c28-7cb3-da17-53dc-ec6401be9d1c@nostrum.com>
From: Andy Bierman <andy@yumaworks.com>
Date: Fri, 29 Jul 2016 13:36:54 -0700
Message-ID: <CABCOCHSEG0VyWvP=oSFDD8kkWR=+-5ACV=1Es=2WfbjYhaV1UQ@mail.gmail.com>
Subject: Re: Gen-art LC review: draft-ietf-netconf-restconf-15
To: Robert Sparks <rjsparks@nostrum.com>
Content-Type: multipart/alternative; boundary="94eb2c09507a0ed21f0538cc363a"
Archived-At: <https://mailarchive.ietf.org/arch/msg/ietf/9LqO6FXclMNpCK3b5i8ZYusg14k>
Cc: General Area Review Team <gen-art@ietf.org>, draft-ietf-netconf-restconf.all@ietf.org, "ietf@ietf.org" <ietf@ietf.org>, Netconf <netconf@ietf.org>
X-BeenThere: ietf@ietf.org
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: IETF-Discussion <ietf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ietf>, <mailto:ietf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ietf/>
List-Post: <mailto:ietf@ietf.org>
List-Help: <mailto:ietf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ietf>, <mailto:ietf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 29 Jul 2016 20:37:00 -0000
Hi, I will add this review to the list. A new version in in progress. Some comments inline On Fri, Jul 29, 2016 at 1:11 PM, Robert Sparks <rjsparks@nostrum.com> wrote: > 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? > I guess we need more text somewhere explaining the "depth" parameter is a retrieval filter. It is not used to create anything in the server. The server does not maintain any state except during the processing of the retrieval request > > * 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. > OK > > * 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). > Please suggest replacement text if we are citing the wrong RFC. I will ask Kent to look into this issue > > 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. > No -- it will be clarified that the server must support at least 1 of the 2 > > * 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. > > The next draft will be clarified to allow PUT on a datastore resource > * 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). > OK > > * 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. > > Andy
- Re: [Netconf] Gen-art LC review: draft-ietf-netco… Kent Watsen
- Re: [Netconf] Gen-art LC review: draft-ietf-netco… Robert Sparks
- Fw: [Netconf] Gen-art LC review: draft-ietf-netco… tom p.
- Re: [Netconf] Gen-art LC review: draft-ietf-netco… t.petch
- Re: Gen-art LC review: draft-ietf-netconf-restcon… Andy Bierman
- Re: Gen-art LC review: draft-ietf-netconf-restcon… Robert Sparks
- Re: Gen-art LC review: draft-ietf-netconf-restcon… Andy Bierman
- Gen-art LC review: draft-ietf-netconf-restconf-15 Robert Sparks