Kent Yes, good now,
Tom Petch ----- Original Message ----- From: "Kent Watsen" <[email protected]> Sent: Monday, August 01, 2016 10:17 PM > > To address the Gen-Art comment, below I replaced “as defined in [RFC5246]” with “, as described in Section 7.2.1 of [RFC5246]”. Good now? > > Additionally, while looking at Section 2.3, I realized an opportunity to fix a couple more things. First, when looking at RFC 7589, Section 5, I see the language “certificate obtained by a trusted mechanism”, which I think is better than saying it has to match a locally configured fingerprint. Second, I found a redundant sentence which seemed misleading. Finally, I collapsed the two paragraphs into one since they ’re more connected than not. The net result follows: > > NEW > > 2.3. Certificate Validation > > The RESTCONF client MUST either use X.509 certificate path validation > [RFC5280] to verify the integrity of the RESTCONF server's TLS > certificate, or match the server’s TLS certificate with a certificate > obtained by a trusted mechanism (e.g. a pinned certificate). If X.509 > certificate path validation fails, and the presented X.509 certificate > does not match a certificate obtained by a trusted mechanism, the > connection MUST be terminated, as described in Section 7.2.1 of > [RFC5246]. > > > OLD: > > 2.3. Certificate Validation > > The RESTCONF client MUST either use X.509 certificate path validation > [RFC5280] to verify the integrity of the RESTCONF server's TLS > certificate, or match the presented X.509 certificate with locally > configured certificate fingerprints. > > The presented X.509 certificate MUST also be considered valid if it > matches a locally configured certificate fingerprint. 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]. > > > This change will be made in a few days if no objection is raised by then. > > Thanks, > Kent > > > On 8/1/16, 1:57 PM, "Netconf on behalf of Robert Sparks" <[email protected] on behalf of [email protected]> wrote: > > > > On 7/30/16 6:45 AM, t.petch wrote: > > Robert > > > > Picking up on the point about terminating the connection when a > > certificate validation fails, this is a straight lift from 'Netconf > > over TLS', RFC7589, where the reference is also in Section 4 which makes > > it clear (to me:-) that the reference is to how the connection is > > terminated, as per RFC5246 s.7.2.1, and nothing to do with the > > certificate validation, which is as per RFC5280. > Perhaps having the s.7.2.1 reference in the text in this document (that > part wasn't > included in the lift from 7589) would have let me to interpret the > sentence that way, > after following the reference. I suggest a light touch to the sentence > to make it clear > that you are talking about "how to terminate the connection" > not"terminating the > connection because the cert check failed" with the reference. > > (And this should be moved to a nit since it's an editorial clarification). > > > > > Tom Petch > > > > ----- Original Message ----- > > From: "Robert Sparks" <[email protected]> > > To: "Andy Bierman" <[email protected]> > > Cc: "General Area Review Team" <[email protected]>; > > <[email protected]>; <[email protected]>; "Netconf" > > <[email protected]> > > Sent: Friday, July 29, 2016 9:47 PM > > Subject: Re: [Netconf] Gen-art LC review: draft-ietf-netconf-restconf-15 > > > > > >> > >> On 7/29/16 3:36 PM, Andy Bierman wrote: > >>> 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 <[email protected] > >>> <mailto:[email protected]>> 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. > >> I got that. It's existence, however, caused me to think about the fact > >> that what is stored at the server can be arbitrarily deep. Clients > > using > >> POST can build trees that are arbitrarily deep, with bits at the node > >> that are arbitrarily large (subject to the constraints the YANG models > >> put on the node). There should be some discussion acknowledging that > >> this can happen, and discussion of what the server can do if some > > client > >> starts asking it to store more than it is willing to store. > >>> 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 > >> Hrmm - that makes me less comfortable that you are actually aligned > > with > >> 7231. It may just be that you need to be more precise with your > >> description, but per 7231, PUT never creates resources - it can create > >> or replace the state of a 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 > >>> > >> > > > > ---------------------------------------------------------------------- -- > > -------- > > > > > >> _______________________________________________ > >> Netconf mailing list > >> [email protected] > >> https://www.ietf.org/mailman/listinfo/netconf > >> > > _______________________________________________ > Netconf mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/netconf > > > _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
