Hi Tom, Thanks for your detailed review!
On Thu, Oct 6, 2022 at 5:13 PM tom petch <ie...@btconnect.com> wrote: > From: Pce <pce-boun...@ietf.org> on behalf of julien.meu...@orange.com < > julien.meu...@orange.com> > Sent: 26 September 2022 14:01 > > This message starts a 2-week WG Last Call for > draft-ietf-pce-pcep-yang-19. Please review and share any feedback using > the PCE mailing list. > This WGLC will end on Tuesday October 11. > > <tp> > I commented before that this has inadequate security since it mandates > TLS1.3 where early data opens the door to all sorts of nasties. Here are > my other comments. > > I have added this text -> [RFC8253] describes the use of TLSv1.2 [RFC5246] or later in PCEP. Further, [I-D.dhody-pce-pceps-tls13] specify how to protect PCEP messages with TLS 1.3 [RFC8446] by disallowing the use of early data (0-RTT) and listing the cipher suites that need to be supported with TLS 1.3. ... The YANG module uses the TLS grouping in [I-D.ietf-netconf-tls-client-server]. Note that any TLS version can be configured but [I-D.ietf-netconf-tls-client-server] recommends the use of TLS 1.3 only. At the time of publication of this document, TLS 1.2 is still in common use for PCEP and can still be enabled with feature "tls12" even though it is marked with status as "deprecated". I hope with this we can make progress. > pce-pcep-stateful-pce-gmpls I think is now RFC8779 > > Ack > s.4.1.1.1 just one List I see > > Updated > s.6.1 the list is also keyed on lsp-id > > Added > The YANG module has lower case must/should; is this intended? > > I checked, the usage seems to be okay as they are describing the base protocol handling and not related to YANG elements > The timer names are not those of RFC5440 - perhaps worth a note giving the > mapping even if it is only hyphen-minus > > container SR > set to true if SR is enabled > Where is that enabled, for what scope? > > likewise msd; other I-D decompose MSD three ways on a per signalling > basis, I am not clear which MSD applies here. A bit like MTU, it might > need a context to be clear. > > Updated. > The reference for path-key looks like it is a line too long > > Ack > RFC8231 says srp-id ffffffff is reserved in which case the range should > not be ..max; this was correct in -18 > > Oops! Fixed! > I do not understand the use of must + error-message for config false. I > am used to it for validating an update and cannot see when this message > will be generated. This occurs in a number of places. > > https://www.rfc-editor.org/rfc/rfc7950.html#section-7.5.3 says "The constraint is enforced according to the rules in Section 8 <https://www.rfc-editor.org/rfc/rfc7950.html#section-8>." where https://www.rfc-editor.org/rfc/rfc7950.html#section-8.1 says "If the constraint is defined on state data, it MUST be true in a valid state data tree." I did not see any guidance on what happens with error-message in this case. I have kept this comment pending for now. I will check with YANG experts! > RPC often have a nacm default-deny-all > > Added > s.9 > The YANG modules .../is/are/ > > Ack I have posted an updated version - https://www.ietf.org/archive/id/draft-ietf-pce-pcep-yang-20.html Thanks! Dhruv > Tom Petch > > > Thanks, > > Julien > > > > _______________________________________________ > Pce mailing list > Pce@ietf.org > https://www.ietf.org/mailman/listinfo/pce >
_______________________________________________ Pce mailing list Pce@ietf.org https://www.ietf.org/mailman/listinfo/pce