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

Reply via email to