Re: [Gen-art] Gen-art LC review: draft-ietf-netconf-restconf-15

Andy Bierman <> Fri, 29 July 2016 20:36 UTC

Return-Path: <>
Received: from localhost (localhost []) by (Postfix) with ESMTP id 4F02E12D884 for <>; Fri, 29 Jul 2016 13:36:58 -0700 (PDT)
X-Virus-Scanned: amavisd-new at
X-Spam-Flag: NO
X-Spam-Score: -1.9
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=ham autolearn_force=no
Authentication-Results: (amavisd-new); dkim=pass (2048-bit key)
Received: from ([]) by localhost ( []) (amavisd-new, port 10024) with ESMTP id 5JkkS1WjmFxX for <>; Fri, 29 Jul 2016 13:36:56 -0700 (PDT)
Received: from ( [IPv6:2607:f8b0:400c:c08::22e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by (Postfix) with ESMTPS id D473712D87E for <>; Fri, 29 Jul 2016 13:36:55 -0700 (PDT)
Received: by with SMTP id j59so69711225uaj.3 for <>; Fri, 29 Jul 2016 13:36:55 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;; 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;; 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=ea85eNg36OW3r7npvAoKTIJNDkWA4CHf+6YZwIl+yyA5mfKg/6xASpw0Y6uIeYiGad 4FQcdrT7V9qtSb5cQxEYAlHa70evZhIoOTwj0pvK9mptM5J6H4fTJkbKbQGVi6xaa7gv q0ES9mHZXHbii765BKyaAj9E6N5Uy45l8XAaiDdghjaBgnGSm8uezS4tXyho9Z/lXExk qXqo34GWcebXNYNOv2V5dAdc3lsWA/9S0BYZ+VEsgI6RPCrlEg9WtQhcBQVaD58rZvUY GGdLcI4nNjojZ6oamHyX9qpdkGlIIBgnGWYuUqPZnNBgYcvX2utILlyUypQ3XXHyn+2I 1/UA==
X-Gm-Message-State: AEkoous5lQlpP77LQRbqFmHlXIpBsf+p46yMwr2WygB/mptyJ1vky9Eat9GkmkQkwaFyiO7tFE0WUHwawXR2QQ==
X-Received: by with SMTP id k4mr19777127uak.47.1469824614969; Fri, 29 Jul 2016 13:36:54 -0700 (PDT)
MIME-Version: 1.0
Received: by with HTTP; Fri, 29 Jul 2016 13:36:54 -0700 (PDT)
In-Reply-To: <>
References: <>
From: Andy Bierman <>
Date: Fri, 29 Jul 2016 13:36:54 -0700
Message-ID: <>
To: Robert Sparks <>
Content-Type: multipart/alternative; boundary="94eb2c09507a0ed21f0538cc363a"
Archived-At: <>
Cc: General Area Review Team <>,, "" <>, Netconf <>
Subject: Re: [Gen-art] Gen-art LC review: draft-ietf-netconf-restconf-15
X-Mailman-Version: 2.1.17
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <>
List-Unsubscribe: <>, <>
List-Archive: <>
List-Post: <>
List-Help: <>
List-Subscribe: <>, <>
X-List-Received-Date: Fri, 29 Jul 2016 20:36:58 -0000


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 <> 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
> <>.
> 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.


> * 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 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.