Subject: [Pce] Benjamin Kaduk's No Objection on draft-ietf-pce-pcep-extension-for-pce-controller-12: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Wed, 24 Feb 2021 22:59:00 -0800

Benjamin Kaduk has entered the following ballot position for
draft-ietf-pce-pcep-extension-for-pce-controller-12: No Objection

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-pce-pcep-extension-for-pce-controller/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

I agree with Roman about the prospects for ensuring a solid security
baseline with what is essentially greenfield deployment.

(throughout) Is the TLV LSP-IDENTIFIER or LSP-IDENTIFIERS (with final
'S')?

Thanks to Yaron Sheffer for the secdir reviews, and to the authors for
updating in light of that review.

I note in a few places in the section-by-section comments that the
figures seem to indicate a 'D' flag in PCInitiate and PCUpd messages,
and the only D flag I see mentioned in the prose is the Delegate flag in
a PCRpt.  This seems particularly important to check and get right
(though I admit that I could just be missing something).

Section 1

   A PCE-based Central Controller (PCECC) can simplify the processing of
   a distributed control plane by blending it with elements of SDN and
   without necessarily completely replacing it.  Thus, the LSP can be
   calculated/setup/initiated and the label forwarding entries can also
   be downloaded through a centralized PCE server to each network
   devices along the path while leveraging the existing PCE technologies
   as much as possible.

nit: "each network device" singular.

Section 2

   The terminology used in this document is the same as that described
   in the draft [RFC8283].

"That RFC doesn't look like a draft to me"

Section 3

   This document also allows a case where the label space is maintained
   by the PCC itself, and the labels are allocated by the PCC, in this
   case, the PCE should request the allocation from PCC as described in
   Section 5.5.8.

nit: comma splice.

Section 4

   4.  PCEP procedures need to provide a mean to update (or clean up)
       the label-download entry to the PCC.

   5.  PCEP procedures need to provide a mean to synchronize the labels
       between the PCE and the PCC via PCEP messages.

nits: "provide a means" plural (twice); s/the label-download entry/label
entries downloaded/ (I think)

Section 5.4

   A legacy PCEP speaker (that does not recognize the PCECC Capability
   sub-TLV) will ignore the sub-TLV in accordance with [RFC8408] and
   [RFC5440].  As per [RFC8408], the legacy PCEP speaker (that does not
   support PST), it will:

Sending a PCErr for an unrecognized PST in the
PATH-SETUP-TYPE-CAPABILITY would break extensibility; the 21/1 error
type/value pair is only used in RFC 8408 when a PST is attempted to be
used in a PCRpt, PCInitiate, or PCUpd.  I think we should just say that
it ignores the PST in the PATH-SETUP-TYPE-CAPABILITY TLV.

Section 5.5.1

   An LSP-IDENTIFIER TLV MUST be included for PCECC LSPs, the tuple
   uniquely identifies the LSP in the network.  [...]

Which tuple?

Also, RFC 8231 says that the LSP-IDENTIFIERS TLV (note final 'S') must
be used for RSVP-signaled LSPs, but PCECC is not (to my knowledge)
requiring the use of RSVP.  Do we need to say anything to generalize
LSP-IDENTIFIERS for other use?

   The ingress PCC MUST also set D (Delegate) flag (see [RFC8231]) and C
   (Create) flag (see [RFC8281]) in the LSP object of the PCRpt message
   to the PCE in the initial exchange.  The PCC responds with a PCRpt
   message with the status set to "GOING-UP" and carrying the assigned
   PLSP-ID (see Figure 1).  [...]

nit: "responds" feels a bit out of place here, since the first sentence
has already talked about setting flags in the PCRpt.  Switching the
order of the sentences might help, but there still wouldn't be a very
clear connection in the prose between the PCRpt and triggering
PCInitiate.

                                                              Each PCC
   further responds with the PCRpt messages including the central
   controller instruction (CCI) and the LSP objects.  The PCC responds
   with a PCRpt message to acknowledge the central controller
   instruction.

Likewise, the second "responds" here feels out of place.

                |PCC    |                              |  PCE  |
                 |ingress|                              +-------+
          +------|       |                                  |
          | PCC  +-------+                                  |
          | transit| |                                      |
   +------|        | |<--PCInitiate,PLSP-ID=0,PST=TBD1,D=1--| PCECC LSP
   |PCC   +--------+ |                                      | Initiate

[sorry for truncation] In the PCInitiate line, what does D=1 refer to?
The only mention of a D flag I can find is that the PCC must set D=1 in
the initial PCRpt to delegate the LSP.

       |       |     |<---PCUpd,PLSP-ID=2,PST=TBD1,D=1------| PCECC LSP
       |       |     |      (UP)                            | Update

Likewise, what is the 'D=1' in PCUpd?

(Both remarks seem to apply to Figure 2 as well.)

       - The O bit is set as before (and thus not included)

            Figure 2: PCE-Initiated PCECC LSP (PCC allocation)

(It doesn't look like we currently indicate the O bit in Figure 1, so
this remark feels a little out of place.  We do indicate the O bit in
Figure 3, though.)

Section 5.5.3

   As per [RFC8281], following the removal of the Label forwarding
   instruction, the PCC MUST send a PCRpt message.  The SRP object in
   the PCRpt MUST include the SRP-ID-number from the PCInitiate message
   that triggered the removal.  The R flag in the SRP object MUST be
   set.

This text seems to indicate that the R flag is set in the SRP object in
the PCRpt, but this does not seem to be reflected in Figure 5.

Section 5.5.4

                                New CC-IDs are used to identify the
   updated instructions, the identifiers in the LSP object identify the
   existing LSP.  [...]

nit: comma splice.

       |       |     |<---PCUpd,PLSP-ID=1,PST=TBD1,D=1-----| PCECC
       |       |     |    SRP=S                            | LSP Update

(I remain unsure about the D flag in the PCUpd.)

Section 5.5.8

   the Label.  If the allocation is successful, the PCC MUST report via
   the PCRpt message with the CCI object.  Else, it MUST send a PCErr
   message with Error-Type = TBD5 ("PCECC failure") and Error Value =
   TBD9 ("Invalid CCI").  If the value of the Label in the CCI object is
   valid, but the PCC is unable to allocate it, it MUST send a PCErr
   message with Error-Type = TBD5 ("PCECC failure") and Error Value =
   TBD10 ("Unable to allocate the specified CCI").

I might be misreading, but this seems to say that if allocation failed
but the value of the label in the CCI object is valid, you have to send
*two* PCErr messages, one with TBD9 and one with TBD10 (there are two
MUST-level requirements that seem to both apply).  I mostly assume that
just one would suffice, so a bit of rewording might be in order.

   If the PCC wishes to withdraw or modify the previously assigned
   label, it MUST send a PCRpt message without any Label or with the
   Label containing the new value respectively in the CCI object.  The
   PCE would further trigger the removal of the central controller
   instruction as per this document.

This seems quite vague about which CCI is to be removed from where.

Section 6.1

   At most two instances of the CCI object can be included, in the case
   of transit LSR to encode both in-coming and out-going label
   forwarding instructions.  Other instances MUST be ignored for P2P
   LSP.  [...]

It's a little hard to square up "at most two instances" and "other
instances MUST be ignored", even if the former doesn't use normative
language.

Section 7.1.1

   o  L bit (Label): if set to 1 by a PCEP speaker, the L flag indicates
      that the PCEP speaker support and willing to handle the PCECC

nits: "supports and is willing"

Section 7.3

   CC-ID:  A PCEP-specific identifier for the CCI information.  A PCE
      creates a CC-ID for each instruction, the value is unique within
      the scope of the PCE and is constant for the lifetime of a PCEP
      session.  The values 0 and 0xFFFFFFFF are reserved and MUST NOT be
      used.

In the vein of draft-gont-numeric-ids-sec-considerations, please include
some discussion on whether the uniqueness property is the only one
needed (e.g., no ordering or gap detection), as well as some analysis of
what information is leaked (including side channels) if the CC-ID is
revealed to outside parties.

My preliminary instinct is that if the value is only in scope for a
single live PCEP session (i.e., two fixed peers) and the session is
protected by TLS, there is no harm in doing sequential allocation and
that makes ensuring uniqueness easier, but there are any number of ways
in which such a trivial analysis could be flawed.

Section 7.3.1

IANA already shows IPV4-ADDRESS and IPV6-ADDRESS TLVs allocated by RFC
8779, with what appears to be identical structure to these.  Why are new
TLV types necessary?

Section 9

I agree with the secdir reviewer that it would be worthwhile to discuss
authorization as well as authentication.  In a system with the
privileged central controller, confirming that a given authenticated
entity is authorized to act as the central controller is important to
the overall security of the system (so that a compromised PCC node
cannot claim to be a PCE to other, uncompromised, PCC nodes).  This
might be done, for example, via an attribute in the PCE's X.509
certificate used for PCEPS or a local policy with a specific accept-list
of X.509 certificate.

Section 9.1

I think we should reiterate the guidance that PCCs need to check that
labels provided by the PCE are in the proper range.

   general precaution, it is RECOMMENDED that this PCEP extension be
   activated on mutually-authenticated and encrypted sessions across
   PCEs and PCCs belonging to the same administrative authority, using
   TLS [RFC8253], as per the recommendations and best current practices
   in [RFC7525].

It's probably best to cite this as BCP 195, as there is likely to be an
updated version in the next couple years.

                                 [RFC8281] provides a mechanism to
   protect PCC by imposing a limit.  The same can be used for the PCECC
   operations as well.

nit: either "PCCs" or "the PCC".

Section 11.6

   IANA is requested to create a new sub-registry to manage the Flag
   field of the CCI object called "CCI Object 16-bits Flag Field".  New

Are these flags expected to be specific to the Object-Type 1 (MPLS
Label) CCI Object?  If so, perhaps the registry name should indicate
that.

Section 13.2

We generally see RFC 8126 listed as normative when used for the
registration procedures when defining a new registry.

Per
https://www.ietf.org/about/groups/iesg/statements/normative-informative-references/
it seems that the recommended behavior with RFCs 8253 and 7525 should
make them normative references.

---

From: "Pengshuping (Peng Shuping)" <[email-masked]>
Date: Fri, 26 Feb 2021 03:10:46 +0000

Hi Benjamin, 

Thank you for your comments! Please find the diff and the responses in line below. Thank you!

Diff: https://tools.ietf.org/tools/rfcdiff/rfcdiff.pyht?url1=draft-ietf-pce-pcep-extension-for-pce-controller-12&url2=https://raw.githubusercontent.com/dhruvdhody/ietf/master/draft-ietf-pce-pcep-extension-for-pce-controller-13.txt

-----Original Message-----
From: Benjamin Kaduk via Datatracker [mailto:[email-masked]] 
Sent: Thursday, February 25, 2021 2:59 PM
To: The IESG <[email-masked]>
Cc: [email-masked]; [email-masked]; [email-masked]; Julien Meuric <[email-masked]>; [email-masked]
Subject: Benjamin Kaduk's No Objection on draft-ietf-pce-pcep-extension-for-pce-controller-12: (with COMMENT)

When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.)

I agree with Roman about the prospects for ensuring a solid security baseline with what is essentially greenfield deployment.

Shuping> As mentioned to Roman, this is a case of blending elements of SDN into the existing distributed control plane and devices without necessarily completely replacing it. 

(throughout) Is the TLV LSP-IDENTIFIER or LSP-IDENTIFIERS (with final 'S')?

Shuping> Updated to LSP-IDENTIFIERS (as per RFC 8231)

Thanks to Yaron Sheffer for the secdir reviews, and to the authors for updating in light of that review.

I note in a few places in the section-by-section comments that the figures seem to indicate a 'D' flag in PCInitiate and PCUpd messages, and the only D flag I see mentioned in the prose is the Delegate flag in a PCRpt.  This seems particularly important to check and get right (though I admit that I could just be missing something).

Shuping> Yes, D flag is the Delegate flag. Changed the text so that it is applicable for other PCEP messages as well - "The ingress PCC and PCE MUST also set D (Delegate) flag (see [RFC8231]) and C (Create) flag (see [RFC8281]) in the LSP object in the initial exchange."

Section 1

Shuping> Ack

Section 2

Shuping> removed

Section 3

nit: comma splice.

Shuping> Ack

Section 4

nits: "provide a means" plural (twice); s/the label-download entry/label entries downloaded/ (I think)

Shuping> Ack

Section 5.4

Sending a PCErr for an unrecognized PST in the PATH-SETUP-TYPE-CAPABILITY would break extensibility; the 21/1 error type/value pair is only used in RFC 8408 when a PST is attempted to be used in a PCRpt, PCInitiate, or PCUpd.  I think we should just say that it ignores the PST in the PATH-SETUP-TYPE-CAPABILITY TLV.

Shuping> Changed to "As per [RFC8408], the legacy PCEP speaker on receipt of an unsupported PST in RP/SRP Object will:" to make it clear.

Section 5.5.1

Which tuple?

Shuping> Removed.

Also, RFC 8231 says that the LSP-IDENTIFIERS TLV (note final 'S') must be used for RSVP-signaled LSPs, but PCECC is not (to my knowledge) requiring the use of RSVP.  Do we need to say anything to generalize LSP-IDENTIFIERS for other use?

Shuping> Added a sentence to clearly state that!

nit: "responds" feels a bit out of place here, since the first sentence has already talked about setting flags in the PCRpt.  Switching the order of the sentences might help, but there still wouldn't be a very clear connection in the prose between the PCRpt and triggering PCInitiate.

Shuping> Updated.

                                                              Each PCC

Shuping> Updated.

                |PCC    |                              |  PCE  |
                 |ingress|                              +-------+
          +------|       |                                  |
          | PCC  +-------+                                  |
          | transit| |                                      |

[sorry for truncation] In the PCInitiate line, what does D=1 refer to?
The only mention of a D flag I can find is that the PCC must set D=1 in the initial PCRpt to delegate the LSP.

Shuping> Text is generalized to be applicable for PCInitiate as well!

(It doesn't look like we currently indicate the O bit in Figure 1, so this remark feels a little out of place.  We do indicate the O bit in Figure 3, though.)

Shuping> Updated figure!

Section 5.5.3

This text seems to indicate that the R flag is set in the SRP object in the PCRpt, but this does not seem to be reflected in Figure 5.

Shuping> Updated figure!

Section 5.5.4

nit: comma splice.

Shuping> Ack

Shuping> See above

Section 5.5.8

I might be misreading, but this seems to say that if allocation failed but the value of the label in the CCI object is valid, you have to send
*two* PCErr messages, one with TBD9 and one with TBD10 (there are two MUST-level requirements that seem to both apply).  I mostly assume that just one would suffice, so a bit of rewording might be in order.

Shuping> Updated

Shuping> Updated to "The PCE would further trigger the Label cleanup of older label as per Section 5.5.3.2."

Section 6.1

It's a little hard to square up "at most two instances" and "other instances MUST be ignored", even if the former doesn't use normative language.

Shuping> Updated to "For the P2P LSP setup via PCECC technique, at the transit LSR two CCI objects are expected for in-coming and outgoing label associated with the LSP object. If any other CCI object is included in the PCInitiate message, it MUST be ignored"

Section 7.1.1

Shuping> Ack

Section 7.3

In the vein of draft-gont-numeric-ids-sec-considerations, please include some discussion on whether the uniqueness property is the only one needed (e.g., no ordering or gap detection), as well as some analysis of what information is leaked (including side channels) if the CC-ID is revealed to outside parties.

My preliminary instinct is that if the value is only in scope for a single live PCEP session (i.e., two fixed peers) and the session is protected by TLS, there is no harm in doing sequential allocation and that makes ensuring uniqueness easier, but there are any number of ways in which such a trivial analysis could be flawed.

Shuping> I added the advice: 

   Note
   that [I-D.gont-numeric-ids-sec-considerations] gives advice on
   assigning transient numeric identifiers such as the CC-ID so as to
   minimize security risks.

Section 7.3.1

IANA already shows IPV4-ADDRESS and IPV6-ADDRESS TLVs allocated by RFC 8779, with what appears to be identical structure to these.  Why are new TLV types necessary?

Shuping> They can be reused, updated accordingly!

Section 9

I agree with the secdir reviewer that it would be worthwhile to discuss authorization as well as authentication.  In a system with the privileged central controller, confirming that a given authenticated entity is authorized to act as the central controller is important to the overall security of the system (so that a compromised PCC node cannot claim to be a PCE to other, uncompromised, PCC nodes).  This might be done, for example, via an attribute in the PCE's X.509 certificate used for PCEPS or a local policy with a specific accept-list of X.509 certificate.

Shuping> Updated text - "It further provide mechanisms for associating peer identities with different levels of access and/or authoritativeness via an attribute in X.509 certificates or a local policy with a specific accept-list of X.509 certificate. This can be used to check the authority for the PCECC operations.

Section 9.1

I think we should reiterate the guidance that PCCs need to check that labels provided by the PCE are in the proper range.

Shuping> Added

It's probably best to cite this as BCP 195, as there is likely to be an updated version in the next couple years.

Shuping> Ack

Shuping> Ack

Section 11.6

Are these flags expected to be specific to the Object-Type 1 (MPLS
Label) CCI Object?  If so, perhaps the registry name should indicate that.

Shuping> Updated.

Section 13.2

We generally see RFC 8126 listed as normative when used for the registration procedures when defining a new registry.

Per
https://www.ietf.org/about/groups/iesg/statements/normative-informative-references/
it seems that the recommended behavior with RFCs 8253 and 7525 should make them normative references.

Shuping> Ok

Best regards, 
Shuping

---

From: Benjamin Kaduk <[email-masked]>
Date: Sat, 27 Feb 2021 12:11:38 -0800

Hi Shuping,

Thanks for the updates/diff.
Some comments inline...

> https://datatracker.ietf.org/doc/draft-ietf-pce-pcep-extension-for-pce-controller/

It's good to have the prose and figure consistent, but I'm still a little
confused as to whether it's needed to set the D flag for PCInitiate and
PCUpd -- RFC 8281 does not appear to say anything at all about the flag in
those messages (whether to set it or clear it), so I guess the only
guidance we have is what implementations of 8281 do in practice.  If they
do set the D flag, then continuing to do so (and documenting that) makes
sense, but otherwise it's not clear to me that there is a reason to change
things in this document.

> Shuping> Changed to "As per [RFC8408], the legacy PCEP speaker on receipt of an unsupported PST in RP/SRP Object will:" to make it clear.

That should work too, thanks!

> Shuping> Added a sentence to clearly state that!

Thanks!

> Likewise, the second "responds" here feels out of place.

I had expected that we would add the 'O' bit to Figure 1 and leave Figure 2
(and this note) alone, but maybe it is okay this way.  (I don't remember if
the 'O' bit is important for understanding Figure 1.)

> Shuping> Updated to "For the P2P LSP setup via PCECC technique, at the transit LSR two CCI objects are expected for in-coming and outgoing label associated with the LSP object. If any other CCI obj…

Thanks.  I guess in principle we could have a separate paragraph to cover
that the ingress and egress nodes have at most one CCI object, with the
proper value of the 'O' flag (or maybe we already do and it's just not
visible in the diff?), but I expect that what we currently have is enough
to convey the proper intent.

> Shuping> They can be reused, updated accordingly!

Thanks!

Everything else looks good, so thanks again :)

-Ben

---

From: Dhruv Dhody <[email-masked]>
Date: Sun, 28 Feb 2021 09:50:53 +0530

Hi Ben, Authors,

> sense, but otherwise it's not clear to me that there is a reason to change

My suggestion to the authors would be to remove D=1 from the figures
for the PCInitiate message and match that to the prose as well. The
behavior here is not new and it should simply refer to RFC8281.

D=1 for PCUpd is fine and should stay as per RFC8231.

Thanks!
Dhruv