Subject: Genart last call review of draft-ietf-pce-pceps-14

From: Dale Worley <[email-masked]>
Date: Tue, 11 Jul 2017 20:08:17 -0700

Reviewer: Dale Worley
Review result: Ready with Nits

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-pce-pceps-14
Reviewer:  Dale R. Worley
Review Date:  2017-07-11
IETF LC End Date:  2017-07-12
IESG Telechat date:  2017-08-03

Summary:

       This draft is basically ready for publication, but has nits
       that should be fixed before publication.

Nits/editorial comments:

A few of these may rise to the level of minor technical issues,
especially:
section 3.2: whether error TBA2/2 is distinct from error 1/1
section 3.2: the receipt of an PCErr during the failure of TLS
             establishment
section 3.3: the distinction between StartTLSWait and OpenWait
section 5:   a full discussion of backward compatibility considerations
section 8.2: MIB extension

1.  Introduction

   This document describes a security container for the transport of
   PCEP messages, and therefore they do not affect the flexibility and
   extensibility of PCEP.

There is no plural antecedent for "they" to reference.  I think you
could use "it", but the Editor may have better suggestions.

3.2.  Initiating the TLS Procedures

   with a PCErr message with Error-Type set to [TBA2 by IANA] (PCEP
   StartTLS failure)

It seems that we shouldn't use this Error-Type to object to the use of
a message other than Open and StartTLS as an initial message, as that
error isn't a misuse of StartTLS per se and is only incidentally
related to StartTLS.  Indeed, isn't there already an Error-Type for
this error (unexpected initial message), since two RFC 5440
implementations can commit/detect it?  Looking at RFC 5440 section
7.15, I see:

   Error-Type    Meaning
      1          PCEP session establishment failure
                 Error-value=1: reception of an invalid Open message or
                                a non Open message.

It seems to me that this document is adjusting the meaning of error
1/1, rather than defining a new error situation.

   If the PCEP speaker that does not support PCEPS, receives a StartTLS
   message, it MUST behave according to the existing error mechanism
   described in section 6.2 of [RFC5440] (in case message is received
   prior to an Open message) or section 6.9 of [RFC5440] (for the case
   of reception of unknown message).

I think you want s/MUST/will/, since the described behavior isn't
specified by this document, but rather by RFC 5440.

In any case, this paragraph might want to reference the
backward-compatibility consideration that I would expect to appear in
section 5 -- How does the PCEPS-supporting element deal with the
non-PCEPS-supporting element after the connection attempt with
StartTLS fails?

   After the exchange of startTLS messages, if a PCEP speaker cannot
   establish a TLS connection for some reason (e.g. the required
   mechanisms for certificate revocation checking are not available), it
   MUST return a PCErr message (in clear) with Error-Type set to [TBA2
   by IANA] (PCEP StartTLS failure) and Error-value set to:

s/startTLS/StartTLS/

Is there a well-defined way for a participant in a TLS connection
start to receive *either* a PCErr message in the clear *or* whatever
comes next in TLS setup -- and know which case has happened?  Is there
a way to use popular modular TLS libraries and have the application
above the library receive such a PCErr message?  I don't understand
TLS nearly well enough to know the answer to this, but it would
probably help implementors if answers were given to these questions.

      The peer MAY choose to re-establish the PCEP session
      without TLS next.

I think you mean "The peer that initiated the connection MAY choose to
re-establish ...".  As written, "the peer" seems to refer to the peer
that generated the PCErr, but if it was the receiving peer that
generated the PCErr, you probably don't want it to attempt to
re-establish the session but rather wait for the initiating peer to do
so.

   Given the asymmetric nature of TLS for connection establishment it is
   relevant to identify the roles of each of the PCEP peers in it.  The
   PCC SHALL act as TLS client, and the PCE SHALL act as TLS server,
   according to [RFC5246].

See comments re section 4 about terminology.  I think you need to have
terms for the element that is initiating the connection (either a PCC
or a PCE) and the element that is receiving the connection (always a
PCE).

3.3.  The StartTLS Message

   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).  A RECOMMENDED value for StartTLSWait timer
   is 60 seconds.

Really, the timer is the time to wait for *any* message to received.
Open messages will cause start of upward-compatibility mechanisms (if
any); any other message will be immediately rejected as an error.

Indeed, isn't there already a timer which a peer uses to wait for the
other peer to send a message?  Isn't StartTLSWait functionally the
same as OpenWait (RFC 5440 section 6.2)?

   ... it MUST send a PCErr message and releases ...

This sentence is grammatically interesting.  One part is logically "it
MUST send a PCErr message".  The second part might be "it MUST release
the TCP connection", or "it releases the TCP connection".  Strictly
speaking, the rules of English grammar cause the release/releases
distinction to disambiguate whether MUST applies to both parts or only
to the first part.  But I don't think you want to rely on that, and
you want to say "... and MUST release the TCP connection ...".

3.4.  TLS Connection Establishment

   1.  Immediately negotiate TLS sessions according to [RFC5246].

In this case, you'd say "Immediately negotiate a TLS session ...".

3.5.  Peer Identity

   At least the following parameters of the X.509 certificate SHOULD
   be exposed:

   o  Peer's IP address

   o  Peer's fully qualified domain name (FQDN)

To the naive (me), these two items seem to refer to the TCP connection
that was established.  But I suspect that they're intended to refer to
the IP address and FQDN that might be encoded in the certificate (as
in, "The following precedence applies: for DNS name validation,
subjectAltName:DNS has precedence over CN; for IP address validation,
subjectAltName:iPAddr has precedence over CN.")

And it seems that the actual remote IP address of the TCP connection
and whatever remote FQDN or IP address was used to initiate the
connection should also be exposed to the administrative system.

So I think there's room to expand on and clarify exactly the data
items that are intended here.

4.  Discovery Mechanisms

   A PCE can advertise its capability to support PCEPS using the IGP
   advertisement and discovery mechanism.

If I understand this correctly, IGP is "Interior Gateway Protocol",
and it's a category of protocols, not a specific protocol.  And what
you're saying is that a PCE can advertise using the relevant IGP's
mechanism.  In that case, I think you'd say "the IGP's advertisement
and discovery mechanism".  (Unless somehow IS-IS and OSPF are seen as
variants of the same protocol, which can generically be called IGP.)

   A new
   capability flag bit for the PCE-CAP-FLAGS sub-TLV that can be
   announced as attribute to distribute PCEP security support
   information is proposed in [I-D.wu-pce-discovery-pceps-support]

s/announced as attribute/announced as an attribute/.

But you don't want to say "is proposed in ..." because that suggests
that the proposal might not be approved, and you've already positively
stated "A PCE can advertise ...".  So you want to say "A new
capability flag ... is defined in ..." -- and raise
draft-wu-pce-discovery-pceps-support to a normative reference.

Similar considerations apply to discovery via DNS
(draft-wu-pce-dns-pce-discovery) -- you want to mention it here and
consider it a normative reference.

-- Unless you choose to make this part of the section clearly
hypothetical by starting "This document does not specify any method a
PCE can advertise that it supports (or requires) PCEPS, but two
mechanisms have been proposed:"

   When DNS is used by a PCC (or a PCE acting as a client, for the rest
   of the section, PCC refers to both) willing to use PCEPS to locate an
   appropriate PCE [I-D.wu-pce-dns-pce-discovery], the PCC as an
   initiating entity, chooses at least one of the returned FQDNs to
   resolve, which it does by performing DNS "A" or "AAAA" lookups on the
   FDQN.

s/FDQN/FQDN/

This one sentence uses the terms "acting as a client" and "as an
initiating entity" to mean the same thing.  You should fix on one
term.  And given that the same concept arises in the discussion of TLS
initiation (section 3.2), you probably want to define the term for "a
PCC (or a PCE acting as a client)" near the beginning as document-wide
terminology.  Once you've got names for the concepts of the initiating
entity and the receiving entity, a number of things in the document
can then be stated more clearly.

   If the PCC receives a response to its SRV query but it is not able to
   establish a PCEPS connection using the data received in the response,
   as initiating entity it MAY fall back to lookup a PCE that uses TCP
   as transport.

This is unclear -- if the process of resolving the original FQDN into
addresses fails to produce an address that can be contacted, how can
the PCC "fall back to lookup a PCE that uses TCP as transport" -- it has
already done the looking up, and that failed.

I suspect you mean that the PCC is required to first attempt to
contact each address using TLS, and then only if all of those attempts
fail is it then permitted to fall back to using TCP.  But you don't
actually say that.

5.  Backward Compatibility

It would help if this section gave a more comprehensive discussion of
how PCEPS-supporting PCEs/PCCs and non-PCEPS-supporting PCEs/PCCs can
interwork, i.e., how to incrementally deploy PCEPS into an AS.  The
two major points seem to be (1) arranging that any two elements will
communicate with the highest level of security that they both
implement, and (2) any system of PCEPS-supporting and
non-PCEPS-supporting elements can interwork successfully.  There
probably are some "interesting" management and security issues
involved in this.

6.2.  New Error-Values

I've noted above that the function of Error-Type=TBA2/Error-Value=2
seems to duplicate that of Error-Type=1/Error-Value=1.

The meaning of Error-Type=TBA2 is named "StartTLS Failure" here, but
the rest of the text uses "StartTLS failure".  The table given in RFC
5440 section 7.15 is not consistent about capitalization in the names
of Error-Types, but most entries do not capitalize non-initial words.
That suggests "StartTLS failure" should be used here.

7.  Security Considerations

There needs to be some consideration of interoperation of mixed
PCEPS-capable and non-PCEPS-capable elements (unless such mixtures are
NOT RECOMMENDED).

8.1.  Control of Function and Policy

   A PCEP implementation SHOULD allow configuring the following PCEP
   security parameters:

   o  StartTLSWait timer value

   PCEPS implementations MAY provide ...

If I read this correctly, there is only one security parameter in this
list.  But since the text says "the following PCEP security
parameters:", on my first reading, I assumed that "PCEPS
implementation MAY provide ..."  was the beginning of a second item
(with the initial "o" omitted).

   provide ways for the operator to complete the following tasks:

It looks like all of the following tasks are with regard to a selected
PCEP session, though only the first task is labeled as such, whereas
the 2nd through 4th are not.  It seems like you want to say "to
complete the following tasks in regard to any PCEP session:" and
change the 1st to "Determine if the session is protected via PCEPS."

8.2.  Information and Data Models

   The PCEP MIB module SHOULD be extended to include PCEPS capabilities,
   information, and status.

This isn't a "SHOULD" because it isn't a constraint on
implementations, it's a statement of what would be a desirable action
by the IETF. -- Unless the idea is that the implementor SHOULD add a
(necessarily proprietary) extension to the MIB.  In any case, RFC 7420
should be referenced.)

8.4.  Verify Correct Operations

This section title is a verb phrase while the rest of the titles are
noun phrases.  Perhaps "Verification of Correct Operations".

8.5.  Requirements on Other Protocols

   Mechanisms defined in this document do not imply any new requirements
   on other protocols.

There is a correlated discovery mechanism:
draft-wu-pce-discovery-pceps-support defines a correlated change to
OSPF and IS-IS.  I suppose that isn't *required* by this document, as
draft-wu-pce-dns-pce-discovery or configuration might be used.  But
conceptually, the two discovery mechanisms are implied by this
document.

10.1.  Normative References

It seems to me that draft-wu-pce-discovery-pceps-support and
draft-wu-pce-dns-pce-discovery should be considered normative
references.

---

From: Dhruv Dhody <[email-masked]>
Date: Fri, 28 Jul 2017 10:36:34 +0000

Hi Dale, 

Thanks for your detailed review and providing suggested text. 
Sorry for being a little late with this reply. 
Please see inline...

> -----Original Message-----
> From: Pce [mailto:[email-masked]] On Behalf Of Dale Worley
> Sent: 12 July 2017 08:38
> To: [email-masked]
> Cc: [email-masked]; [email-masked]; [email-masked]
> Subject: [Pce] Genart last call review of draft-ietf-pce-pceps-14
> 
> Reviewer: Dale Worley
> 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.
> 
[[Dhruv Dhody]] The context of these error are different, one is before TLS establishment (startTLS) and another is during PCEP session establishment (which is after TLS has been established). IMHO it is better to keep them distinct. 

section 3.2:
> the receipt of an PCErr during the failure of TLS
>              establishment
[[Dhruv Dhody]] The idea behind Error TBA2/3 and TBA2/4, is to let the peer know if it is unwilling (or willing) to establish PCEP session without TLS from this peer. This is useful information to figure out how the peer should react without human intervention.  
 
> section 3.3: the distinction between StartTLSWait and OpenWait
[[Dhruv Dhody]] Both timers are wait timers but they wait for different messages and used in different context. Also the values can be set differently based on the deployment. 

> section 5:   a full discussion of backward compatibility considerations
[[Dhruv Dhody]] I have added some more details. 

> section 8.2: MIB extension
[[Dhruv Dhody]] I have added some more details.

More details on above inline. 

> There is no plural antecedent for "they" to reference.  I think you could
> use "it", but the Editor may have better suggestions.
> 

[[Dhruv Dhody]] Ack

> It seems that we shouldn't use this Error-Type to object to the use of a
> message other than Open and StartTLS as an initial message, as that error
> isn't a misuse of StartTLS per se and is only incidentally related to
> StartTLS.  Indeed, isn't there already an Error-Type for this error
> (unexpected initial message), since two RFC 5440 implementations can
> commit/detect it?  Looking at RFC 5440 section 7.15, I see:
> 
>    Error-Type    Meaning
> It seems to me that this document is adjusting the meaning of error 1/1,
> rather than defining a new error situation.
> 
[[Dhruv Dhody]] We wanted to differentiate between the message exchanges during PCEP session establishment (which starts with open) and with StartTLS (which starts with StartTLS). Note that in case of TLS, we would move to the PCEP session establishment phase after the TLS is established, and we could still receive error 1/1. 
Regarding TBA2/2, in case of PCEPS, a peer could respond with StartTls, Open or PCErr (but all in the context of this document), so we needed a new error. The peer was expecting StartTLS related message but something else happened and thus an error! IMHO putting this under "PCEP StartTLS failure" error-type is better! 

Do you feel strongly about changing this? 

> specified by this document, but rather by RFC 5440.

[[Dhruv Dhody]] Ack, updated.
> 
> In any case, this paragraph might want to reference the backward-
> compatibility consideration that I would expect to appear in section 5 --
> How does the PCEPS-supporting element deal with the non-PCEPS-supporting
> element after the connection attempt with StartTLS fails?

 [[Dhruv Dhody]] I have added that in section 5. 
>    by IANA] (PCEP StartTLS failure) and Error-value set to:

[[Dhruv Dhody]] Ack.
> 
> Is there a well-defined way for a participant in a TLS connection start to
> receive *either* a PCErr message in the clear *or* whatever comes next in
> TLS setup -- and know which case has happened?  Is there a way to use
> popular modular TLS libraries and have the application above the library
> receive such a PCErr message?  I don't understand TLS nearly well enough
> to know the answer to this, but it would probably help implementors if
> answers were given to these questions.

[[Dhruv Dhody]] If the TLS handshake is successful, both local and remote parties are aware of this. Same is the case for TLS failure. So receiving an error message in clear will not be a surprise. 
Also, I checked some of the other documents that describe the use of TLS, but did not find any such handling. 

> I think you mean "The peer that initiated the connection MAY choose to re-
> establish ...".  As written, "the peer" seems to refer to the peer that
> generated the PCErr, but if it was the receiving peer that generated the
> PCErr, you probably don't want it to attempt to re-establish the session
> but rather wait for the initiating peer to do so.
> 

[[Dhruv Dhody]] Ack. Changed peer to receiver. 

> terms for the element that is initiating the connection (either a PCC or a
> PCE) and the element that is receiving the connection (always a PCE).
> 

[[Dhruv Dhody]] I have updated this. 

> Open messages will cause start of upward-compatibility mechanisms (if any);
> any other message will be immediately rejected as an error.
> 
> other peer to send a message?  Isn't StartTLSWait functionally the same as
> OpenWait (RFC 5440 section 6.2)?
> 

[[Dhruv Dhody]] I have updated the description, but I would still like to keep a different timer similar to OpenWait and KeepWait (in the spirit of RFC 5440). 

> MUST send a PCErr message".  The second part might be "it MUST release the
> TCP connection", or "it releases the TCP connection".  Strictly speaking,
> the rules of English grammar cause the release/releases distinction to
> disambiguate whether MUST applies to both parts or only to the first part.
> But I don't think you want to rely on that, and you want to say "... and
> MUST release the TCP connection ...".

[[Dhruv Dhody]] Updated.

> In this case, you'd say "Immediately negotiate a TLS session ...".

[[Dhruv Dhody]] Ack

> that was established.  But I suspect that they're intended to refer to the
> IP address and FQDN that might be encoded in the certificate (as in, "The
> following precedence applies: for DNS name validation, subjectAltName:DNS
> has precedence over CN; for IP address validation, subjectAltName:iPAddr
> has precedence over CN.")
> 
> And it seems that the actual remote IP address of the TCP connection and
> whatever remote FQDN or IP address was used to initiate the connection
> should also be exposed to the administrative system.
> 
> So I think there's room to expand on and clarify exactly the data items
> that are intended here.

[[Dhruv Dhody]] The statement above the list clearly state that this is the information from X.509 certificate.
The TCP socket IP address is definitely known and exposed as that this way to identify a session. I have added this text - 

"Note that the remote IP address used for the TCP session establishment is also exposed."

> If I understand this correctly, IGP is "Interior Gateway Protocol", and
> it's a category of protocols, not a specific protocol.  And what you're
> saying is that a PCE can advertise using the relevant IGP's mechanism.  In
> that case, I think you'd say "the IGP's advertisement and discovery
> mechanism".  (Unless somehow IS-IS and OSPF are seen as variants of the
> same protocol, which can generically be called IGP.)
> 

[[Dhruv Dhody]] Updated.

> s/announced as attribute/announced as an attribute/.

[[Dhruv Dhody]] Updated.

> But you don't want to say "is proposed in ..." because that suggests that
> the proposal might not be approved, and you've already positively stated
> "A PCE can advertise ...".  So you want to say "A new capability flag ...
> is defined in ..." -- and raise draft-wu-pce-discovery-pceps-support to a
> normative reference.
> 
> -- Unless you choose to make this part of the section clearly hypothetical
> by starting "This document does not specify any method a PCE can advertise
> that it supports (or requires) PCEPS, but two mechanisms have been
> proposed:"

[[Dhruv Dhody]] I would like to avoid normative reference as these are 
not mandatory and we would not want to block publication of this draft. 
I have updated the text accordingly and removed the text from this document 
Regarding DNS. 

> initiating entity" to mean the same thing.  You should fix on one term.
> And given that the same concept arises in the discussion of TLS initiation
> (section 3.2), you probably want to define the term for "a PCC (or a PCE
> acting as a client)" near the beginning as document-wide terminology.
> Once you've got names for the concepts of the initiating entity and the
> receiving entity, a number of things in the document can then be stated
> more clearly.
> 
> addresses fails to produce an address that can be contacted, how can the
> PCC "fall back to lookup a PCE that uses TCP as transport" -- it has
> I suspect you mean that the PCC is required to first attempt to contact
> each address using TLS, and then only if all of those attempts fail is it
> then permitted to fall back to using TCP.  But you don't actually say that.
> 

[[Dhruv Dhody]] I have removed the details from this document. 

> It would help if this section gave a more comprehensive discussion of how
> PCEPS-supporting PCEs/PCCs and non-PCEPS-supporting PCEs/PCCs can
> interwork, i.e., how to incrementally deploy PCEPS into an AS.  The two
> major points seem to be (1) arranging that any two elements will
> communicate with the highest level of security that they both implement,
> and (2) any system of PCEPS-supporting and non-PCEPS-supporting elements
> can interwork successfully.  There probably are some "interesting"
> management and security issues involved in this.
> 

[[Dhruv Dhody]] Updated. 

   If a PCEP implementation that does not support PCEPS receives a
   StartTLS message, it would behave according to the existing error
   mechanism of [RFC5440].  On receiving the error, based on the local
   policy, a peer could try to establishing PCEP session without TLS as
   per the procedures defined in [RFC5440].  For successful TLS
   operations with PCEP, both PCEP peers in the network would need to be
   upgraded to support this document.

   An existing PCEP session cannot be upgraded to PCEPS, the session
   needs to be terminated and reestablished as per the procedure
   described in this document.  During the incremental upgrade, the PCEP
   speaker SHOULD allow session establishment with and without TLS.
   Once both PCEP speakers are upgraded to support PCEPS, the PCEP
   session is re-established with TLS, otherwise PCEP session without
   TLS is setup.  A redundant PCE MAY also be used during the
   incremental deployment to take over the PCE undergoing upgrade.  Once
   the upgrade is completed, support for unsecured version SHOULD be
   removed.

> 6.2.  New Error-Values
> 
> I've noted above that the function of Error-Type=TBA2/Error-Value=2 seems
> to duplicate that of Error-Type=1/Error-Value=1.
> 

[[Dhruv Dhody]] See reply above. 

> The meaning of Error-Type=TBA2 is named "StartTLS Failure" here, but the
> rest of the text uses "StartTLS failure".  The table given in RFC
> 5440 section 7.15 is not consistent about capitalization in the names of
> Error-Types, but most entries do not capitalize non-initial words.

[[Dhruv Dhody]] Ack.

> There needs to be some consideration of interoperation of mixed PCEPS-
> capable and non-PCEPS-capable elements (unless such mixtures are NOT
> RECOMMENDED).
> 

[[Dhruv Dhody]] yes strict TLS is recommended. 
.

> list.  But since the text says "the following PCEP security parameters:",
> on my first reading, I assumed that "PCEPS implementation MAY provide ..."
> was the beginning of a second item (with the initial "o" omitted).
> 

[[Dhruv Dhody]] Updated.

> PCEP session, though only the first task is labeled as such, whereas the
> 2nd through 4th are not.  It seems like you want to say "to complete the
> following tasks in regard to any PCEP session:" and change the 1st to
> "Determine if the session is protected via PCEPS."
> 

[[Dhruv Dhody]] Updated.

> This isn't a "SHOULD" because it isn't a constraint on implementations,
> it's a statement of what would be a desirable action by the IETF. --
> Unless the idea is that the implementor SHOULD add a (necessarily
> proprietary) extension to the MIB.  In any case, RFC 7420 should be
> referenced.)
> 

[[Dhruv Dhody]] Ack.

> This section title is a verb phrase while the rest of the titles are noun
> phrases.  Perhaps "Verification of Correct Operations".
> 

[[Dhruv Dhody]] Changed to "Verifying Correct Operation".

> draft-wu-pce-discovery-pceps-support defines a correlated change to OSPF
> and IS-IS.  I suppose that isn't *required* by this document, as draft-wu-
> pce-dns-pce-discovery or configuration might be used.  But conceptually,
> the two discovery mechanisms are implied by this document.
> 

[[Dhruv Dhody]] Ack.

> It seems to me that draft-wu-pce-discovery-pceps-support and draft-wu-pce-
> dns-pce-discovery should be considered normative references.
> 
> 

[[Dhruv Dhody]] Updated the text so that they could remain informative. 

Thanks again for your review. 
I have attached the working copy and the diff. 

Regards,
Dhruv

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

---

From: [email-masked] (Dale R. Worley)
Date: Sun, 30 Jul 2017 20:56:26 -0400

Dhruv Dhody <[email-masked]> writes:

The bulk of these items I either agree with completely or am willing to
defer to your judgment.  The items that seem to merit further
discussion are below.  Most of these items are consolidations of several
remarks I made that seem to be the same fundamental question.

> [[Dhruv Dhody]] The context of these error are different, one is
> before TLS establishment (startTLS) and another is during PCEP session
> establishment (which is after TLS has been established). IMHO it is
> better to keep them distinct. 

It's more complicated than that:  If a PCE does not like the first
message it receives, if it implements PCEPS, it replies TBA2/2.  But if
it does not implement PCEPS, it replies 1/1.  Similarly, a PCC may
reject an initial message with either of these error codes, depending on
the situation.  If the other endpoint does not implement PCEPS, it might
be surprised by receiving TBA2/2, which it has no way of understanding
in detail (although it will probably simply disconnect, which is what it
would do in reaction to a 1/1).

A non-implementing endpoint rejects an initial message only with 1/1.

OTOH, once TLS has been established, then both endpoints can only object
to the first encrypted message with 1/1.

There's nothing intrinsically incorrect about this, but it is rather
complicated, particularly in the upward-compatibility case where the PCC
is willing to accept either PCEPS-implementing or non-PCEPS-implementing
connections.

> [[Dhruv Dhody]] Both timers are wait timers but they wait for
> different messages and used in different context. Also the values can
> be set differently based on the deployment. 

There's a complication if a PCE allows both PCEPS and PCEP connections.
In that case, it does not send its initial message until it receives a
message from the PCC.  If the message is StartTLS, the PCC wants to
connect using PCEPS and the PCE sends StartTLS; if the message is Open,
the PCC wants to connect using PCEP and the PCE sends Open.

The question is, what timeout does the PCE use while waiting for the
initial message?  If the connection *will be* PCEPS, it should use
StartTLSWait (per draft-ietf-pce-pceps), if the connection *will be*
PCEP, it should use OpenWait (per RFC 5440).  But it does not have the
information to choose the timer.  The difficulty is that initially, the
PCE is simulating both a PCEP and a PCEPS device, and using the message
it receives from the PCC to disambiguate its situation.  But the timeout
values prescribed for the two situations are different.

My feeling is that the two situations -- waiting for Open and waiting
for StartTLS -- are not sufficiently distinguishable to allow
prescribing two different timers.  Of course, this could be cured by
proper configuration -- giving the same value to StartTLSWait and
OpenWait.

OTOH, once TLS has been established and the Open messages are awaited,
both endpoints use OpenWait.  However, this situation is not really the
same as when the TCP connection has been just been established.  And
yet, a PCE may be obligated to use the same timer for both situations.

>> 3.2.  Initiating the TLS Procedures
...
> Do you feel strongly about changing this? 

Of course, I'm only a reviewer, you needn't defer to my opinion about
anything.

> [[Dhruv Dhody]] If the TLS handshake is successful, both local and
> remote parties are aware of this. Same is the case for TLS failure. So
> receiving an error message in clear will not be a surprise. 
>

OK, I think I understand now; my previous comment was based on a
misunderstanding.  The correct sequence of operations for PCEPS
setup is:  Both endpoints send a StartTLS to each other.  After that, the
initiator starts the TLS setup.  Alternatively, the PCC sends StartTLS
and the non-PCEPS-implementing PCE sends a PCErr.  In either case, the
PCC sends a StartTLS and then listens for a PCEP message in response.
In any case, neither endpoint is ever in a situation where it could
receive either a PCEP message or a TLS setup message.

>> 5.  Backward Compatibility
...
>    StartTLS message, ...

I think this is a significant improvement, as you can never be too
careful about upward-compatibility issues, especially when security is
involved.

--

Concerning the figures:

                  +-+-+                 +-+-+
                  |PCC|                 |PCE|
                  +-+-+                 +-+-+
                    |                     | Does not send
                    |      StartTLS       | StartTLS as
                    |-------------------->| cannot establish
                    |                     | TLS
                    |                     |
                    |<--------------------| Send Error
                    |      PCErr          | Error-Value 3/4
                    |                     |

   Figure 2: Both PCEP Speaker supports PCEPS, But cannot establish TLS

                  +-+-+                 +-+-+
                  |PCC|                 |PCE|
                  +-+-+                 +-+-+
                    |                     |  Does not support
                    | StartTLS            |  PCEPS and thus
                    | msg                 |  sends Open
                    |-------              |
                    |       \   Open      |
                    |        \  msg       |
                    |         \  ---------|
                    |          \/         |
                    |          /\         |
                    |         /  -------->|
                    |        /            |
                    |<------              |
                    |                     |
                    |<--------------------| Send Error
                    |       PCErr         | (non-Open message
                    |                     |  received)

             Figure 3: One PCEP Speaker does not support PCEPS

IIUC, in Figure 2 the error has error-value 3/4 and is "Not supported
object" ... is that right?  I'd think it would be TBA2/4, "Failure,
connection without TLS possible" (or TBA2/3, depending on
circumstances).  And in Figure 3, the error is 1/1.  It seems like it
would be clearer if both the number and the text were shown for the
error messages in both the figures.

Is there also this flow?

                  +-+-+                 +-+-+
                  |PCC|                 |PCE|
                  +-+-+                 +-+-+
                    |                     |  Does not support
                    | StartTLS            |  PCEPS and thus
                    | msg                 |  sends Open
                    |-------              |
                    |       \   Open      |
                    |        \  msg       |
                    |         \  ---------|
                    |          \/         |
                    |          /\         |
                    |         /  -------->|
                    |        /            |
                    |<------              |
                    |                     |
                    |-------------------->|
                    |       PCErr         |
                    |                     |

Where the error indicates "Expecting a StartTLS but received an Open"?
Or is this situation always signaled by the receiver (as in Figure 3)
rather than the initiator?

Dale

---

From: Dhruv Dhody <[email-masked]>
Date: Mon, 31 Jul 2017 13:35:50 +0000

Hi Dale,

Thanks for your further comments, please see inline..

> From: Dale R. Worley [mailto:[email-masked]]
> Sent: 31 July 2017 06:26
> To: Dhruv Dhody <[email-masked]>
> Cc: [email-masked]; [email-masked]; [email-masked];
> [email-masked]; [email-masked]
> Subject: Re: [Pce] Genart last call review of draft-ietf-pce-pceps-14
> 
> defer to your judgment.  The items that seem to merit further discussion
> are below.  Most of these items are consolidations of several remarks I
> made that seem to be the same fundamental question.
> 
> It's more complicated than that:  If a PCE does not like the first message
> it receives, if it implements PCEPS, it replies TBA2/2.  But if it does
> not implement PCEPS, it replies 1/1.  Similarly, a PCC may reject an
> initial message with either of these error codes, depending on the
> situation.  If the other endpoint does not implement PCEPS, it might be
> surprised by receiving TBA2/2, which it has no way of understanding in
> detail (although it will probably simply disconnect, which is what it
[[Dhruv Dhody]] You are right about this case, which I have clarified now -  

   If the PCEP speaker that only supports PCEPS connection (as a local
   policy), receives an Open message, it MUST treat it as an unexpected
   message and reply with a PCErr message with Error-Type set to 1 (PCEP
   session establishment failure) and Error-value set to 1 (reception of
   an invalid Open message or a non Open message).

In your description you mentioned the error TBA2/2, but the description of TBA2/2 is  - 

   A PCEP
   speaker receiving any other message apart from StartTLS, Open, or
   PCErr as the first message, MUST treat it as an unexpected message
   and reply with a PCErr message with Error-Type set to [TBA2 by IANA]
   (PCEP StartTLS failure) and Error-value set to 2 (reception of any
   other message apart from StartTLS, Open, or PCErr message), and MUST
   close the TCP connection.

So receiving of open message would not trigger this error. The new text above would take care of that. 

> is willing to accept either PCEPS-implementing or non-PCEPS-implementing
[[Dhruv Dhody]] I have created a table that describe all the cases as part the updated text.  
See https://github.com/dhruvdhody-huawei/ietf/raw/master/Compare.pdf 

> StartTLSWait (per draft-ietf-pce-pceps), if the connection *will be* PCEP,
> it should use OpenWait (per RFC 5440).  But it does not have the
> PCE is simulating both a PCEP and a PCEPS device, and using the message it
> receives from the PCC to disambiguate its situation.  But the timeout
> My feeling is that the two situations -- waiting for Open and waiting for
> StartTLS -- are not sufficiently distinguishable to allow prescribing two
> different timers.  Of course, this could be cured by proper configuration
> -- giving the same value to StartTLSWait and OpenWait.
> 
> same as when the TCP connection has been just been established.  And yet,
> a PCE may be obligated to use the same timer for both situations.
> 
[[Dhruv Dhody]] You have noted something important, thanks for that. 

I would like to fix this by making sure that a PCEP speaker that supports PCEPS does not start OpenWait timer immediately after TCP session, it uses StartTLSWait timer instead. 
I would clarify that the OpenWait timer should be started after TLS establishment only. 

So a PCE that supports connection with TLS only or with/without TLS, it would use StartTLSWait timer initially, the OpenWait timer is started once the TLS is established. 
I also state the StartTLSWait should never be set less than OpenWait timer. 

Please see the diff, I hope these changes should solve the issue.  

> >> comes next in TLS setup -- and know which case has happened?  Is
> >> there a way to use popular modular TLS libraries and have the
> >> application above the library receive such a PCErr message?  I don't
> >> understand TLS nearly well enough to know the answer to this, but it
> >> would probably help implementors if answers were given to these
> questions.
> >
> misunderstanding.  The correct sequence of operations for PCEPS setup is:
> Both endpoints send a StartTLS to each other.  After that, the initiator
> starts the TLS setup.  Alternatively, the PCC sends StartTLS and the non-
> PCEPS-implementing PCE sends a PCErr.  In either case, the PCC sends a
> StartTLS and then listens for a PCEP message in response.
> I think this is a significant improvement, as you can never be too careful
> about upward-compatibility issues, especially when security is involved.
> 
> --
> 
> Concerning the figures:
> 
>                   +-+-+                 +-+-+
>                   |PCC|                 |PCE|
>                   +-+-+                 +-+-+
>                     |                     | Does not send
>                     |      StartTLS       | StartTLS as
> connection without TLS possible" (or TBA2/3, depending on circumstances).
> And in Figure 3, the error is 1/1.  It seems like it would be clearer if
> both the number and the text were shown for the error messages in both the
> figures.
> 
[[Dhruv Dhody]] Error-Value 3/4 meant Error TBA2/3 and Error TBA2/4 as I was focusing only on the error-values and not on error-type, but I agree this could be confusing and I have updated the figures. Also I have added some more figures. 

> Is there also this flow?

 [[Dhruv Dhody]] The text has been added which is generic and works for both PCC/PCE.  

I don't think we need a figure for that :)

Thanks again for your reviews. 

The diff between the last update and this is attached. 

Working copy, Diff: https://github.com/dhruvdhody-huawei/ietf

Regards,
Dhruv