Subject: [Pce] Shepherd review of draft-ietf-pce-pceps

From: Cyril Margaria <[email-masked]>
Date: Mon, 27 Mar 2017 13:10:11 -0400

Dear authors, PCE WG

I am the shepherd for this document, Please find below my shepherd's review
of the aforementioned I-D.

* General

The document does not need much work to to progress, they are mainly
clarifications.

* Detailed comments.

Section 3.2
--
"Thus the PCEP session is secured via TLS from the
   start before exchange of any other PCEP message including the Open
   message."

This sentence comes after the reference to discovery procedures while it
refers to when the startTLS message is to be used.
This could be rephrased.
--
"Securing via TLS of an existing PCEP session is not
   permitted, the session must be closed and re-established with TLS as
   per the procedure described in this document"

 Should the must be replaced by MUST?

--
"If the PCEP speaker supports PCEPS but cannot establish a TLS
   connection for some reason"

I believe that in that case both speakers must support PCEPS in order for
them to send the startTLS and start TLS negotiation. Its unclear when those
errors can be sent, shouldn't they be sent after the TLS negotiations have
taken place?
This comment is repeated in the next sections/comments.

Section 3.3.

--
"On receiving a StartTLS message from the PCEP
   peer (i.e.  when the PCEP speaker has sent and received StartTLS
   message) it is ready to start TLS negotiation and establishment and
   move to steps described in Section 3.4."

>From section 3.2  a peer MAY also send Error-Type set to [TBA2 by IANA]
(PCEP StartTLS failure) and Error-value 3 (not without TLS) or 4 (ok
without TLS).
At which point of the PCEP session are the PCErr sent? is it in step 3 of
 section 3.4?

--
"Once the TCP connection has been successfully established and the
   StartTLS message sent, the sender MUST start a timer called
   StartTLSWait timer, after the expiration of which, if no StartTLS
   message has been received, it sends a PCErr message and releases the
   TCP connection with Error-Type set to [TBA2 by IANA] and Error-value
   set to 5 (no StartTLS message received before the expiration of the
   StartTLSWait timer)."

the following text would be more clear in my opinion

"Once the TCP connection has been successfully established and the
   StartTLS message sent, the sender MUST start a timer called
   StartTLSWait timer, after the expiration of which, if no StartTLS
   message has been received, it MUST send a PCErr message and releases the
   TCP connection with Error-Type set to [TBA2 by IANA] and Error-value
   set to 5 (no StartTLS message received before the expiration of the
   StartTLSWait timer)."

Section 3.5

--
"PCEPS implementations SHOULD provide mechanisms for
   associating peer identities with different levels of access and/or
   authoritativeness, and they MUST provide a mechanism for establish a
   default level for properly identified peers. "

typo "for establish" -> "for establishing"

"PCEPS implementations SHOULD provide mechanisms for
   associating peer identities with different levels of access and/or
   authoritativeness, and they MUST provide a mechanism for establishing a
   default level for properly identified peers. "

--

" Implementations
   that want to support a wide variety of trust models should expose as
   many details of the presented certificate to the administrator as
   possible so that the trust model can be implemented by the
   administrator. "

should -> SHOULD or it could be rephrased if its not the intent.

--
"As a suggestion, at least the following parameters of
   the X.509 certificate should be exposed:"

it can be rephrased as
"At least the following parameters of
   the X.509 certificate SHOULD be exposed:"

or

"its RECOMMENDED that at least the following parameters of
   the X.509 certificate are exposed:"

Section 3.6
--

Section 3.2 defines error-values for TLS negotiation failures, should they
be used in that section? i.e the peer SHOULD send a PCErr and MUST
terminate the session?

Section 4.

--
I am not sure if the second and third paragraph are relevant for PCEPS
session establishement. Could you indicate what is the relation between
PCEPS and dns resolution?

Section 6.1

--

Following rfc5226, please  clarify the request for new allocation and
indicate which registry need to be updated, for instance:

 IANA is requested to allocate new message types within the "PCEP
 Messages" sub-registry of the PCEP Numbers registry, as follows:

                 Value     Meaning
         Reference
                   TBA1      The Start TLS Message (StartTLS)         This
document

Section 6.2

--
same as section 6.1, a possible text can be:

IANA is requested to allocate new Error Types and Error Values within
   the " PCEP-ERROR Object Error Types and Values" sub-registry of the
   PCEP Numbers registry, as follows:

Section 8.2

--
should the YANG modules also be updated?

Thanks,
Cyril

---

From: "Diego R. Lopez" <[email-masked]>
Date: Mon, 27 Mar 2017 23:37:06 +0000

Hi Cyril,

Are you here in Chicago? It would be good just to say hello and discuss F2F any urgent issue you have in mind regarding this document…

Be goode,

On 27 Mar 2017, at 12:10 , Cyril Margaria <[email-masked]<mailto:[email-masked]>> wrote:

Dear authors, PCE WG

I am the shepherd for this document, Please find below my shepherd's review of the aforementioned I-D.

* General

The document does not need much work to to progress, they are mainly clarifications.

* Detailed comments.

Section 3.2

This sentence comes after the reference to discovery procedures while it refers to when the startTLS message is to be used.
This could be rephrased.
--
"Securing via TLS of an existing PCEP session is not
   permitted, the session must be closed and re-established with TLS as
   per the procedure described in this document"

I believe that in that case both speakers must support PCEPS in order for them to send the startTLS and start TLS negotiation. Its unclear when those errors can be sent, shouldn't they be sent after the TLS negotiations have taken place?
This comment is repeated in the next sections/comments.

Section 3.3.

From section 3.2  a peer MAY also send Error-Type set to [TBA2 by IANA] (PCEP StartTLS failure) and Error-value 3 (not without TLS) or 4 (ok without TLS).
At which point of the PCEP session are the PCErr sent? is it in step 3 of  section 3.4?

Section 3.5

--

" Implementations

it can be rephrased as

or

Section 3.6
--

Section 3.2 defines error-values for TLS negotiation failures, should they be used in that section? i.e the peer SHOULD send a PCErr and MUST terminate the session?

Section 4.

--
I am not sure if the second and third paragraph are relevant for PCEPS session establishement. Could you indicate what is the relation between PCEPS and dns resolution?

Section 6.1

--

Following rfc5226, please  clarify the request for new allocation and indicate which registry need to be updated, for instance:

 IANA is requested to allocate new message types within the "PCEP  Messages" sub-registry of the PCEP Numbers registry, as follows:

                 Value     Meaning                                                  Reference
                   TBA1      The Start TLS Message (StartTLS)         This document

Section 6.2

Section 8.2

Thanks,
Cyril
_______________________________________________
Pce mailing list
[email-masked]<mailto:[email-masked]>
https://www.ietf.org/mailman/listinfo/pce

--
"Esta vez no fallaremos, Doctor Infierno"

Dr Diego R. Lopez
Telefonica I+D
http://people.tid.es/diego.lopez/

e-mail: [email-masked]
Tel:    +34 913 129 041
Mobile: +34 682 051 091
----------------------------------

---

From: Dhruv Dhody <[email-masked]>
Date: Mon, 27 Mar 2017 23:05:02 -0500

Hi Cyril,

Thanks for being our shepherd and providing review comments. Please see
inline...

> The document does not need much work to to progress, they are mainly

​[Dhruv]: How about -

This message MUST be the first PCEP message
issued by the PCEP speaker, willing to use TLS. Note that, this includes the
open message

​[Dhruv] Ack​

​[Dhruv]: Yes, this is after startTLS messages have been exchanged, and once
the PCEP speaker finds out that the TLS connection cannot be established.

We could reword this to -

After the exchange of startTLS messages, if the PCEP speaker cannot
establish a TLS connection
for some reason...

> Section 3.3.
​[Dhruv]: Yes. It is the error handling part of step 3 IMHO. ​

> ​​

​[Dhruv]: Ack. ​

> typo "
> ​​
> for establish" -> "for
> ​​
> establishing"
​[Dhruv]: Ack. ​

​​[Dhruv]: Ack. ​
​

> "
> ​​
> At least the following parameters of
> "its
> ​​
> RECOMMENDED that at least the following parameters of
 [Dhruv]: Ack. ​​

> Section 3.6
> --
>
> Section 3.2
> ​​
> defines error-values for TLS negotiation failures, should they be used in
> that section? i.e the peer SHOULD send a PCErr and MUST terminate the
> session?
>
>
>
​[Dhruv]: Yes, updated the text to -

   In case the initial TLS negotiation or the peer identity check fails,
   according to the procedures listed in this document, the peer MUST
   send the PCErr message as per the Section 3.2 and then MUST terminate
   the session.

> Section 4.
​[Dhruv​]: If DNS is used to discover the PCE IP address, it can also
specify if the PCE supports TLS via the NAPTR records, you can find an
example of this in
https://tools.ietf.org/html/draft-wu-pce-dns-pce-discovery-09#section-6.2.1

Thus this is similar to learning the PCEPS capability via IGP.

> Section 6.1
​[Dhruv]: Ack​

> Section 8.2
​[Dhruv]: How about - ​

​   An implementation SHOULD allow the operator to configure the PCEPS
   capability as well as various TLS related parameters, as well as view
   the current TLS status for a PCEP session.  To serve this purpose,
   the PCEP YANG module [I-D.ietf-pce-pcep-yang] can be extended to
   include TLS related configuration and state.​

​Thanks for your comments! ​I have attached the diff file.

Working XML:
https://github.com/dhruvdhody-huawei/ietf/blob/master/draft-ietf-pce-pceps-12.xml

​We can update a new version after a confirmation from you! ​

​Regards,
Dhruv​