Path Computation Element Communication Protocol (PCEP) Extension for Associating Policies and Label Switched Paths (LSPs)
draft-ietf-pce-association-policy-16

The use of BCP 14 language in Sections 9.1 and 9.2 seems awkward, since it appears to be discussing operator-facing features of an implementation rather than interoperability concerns.  See, in particular, Section 6 of RFC 2119.

Thanks to Scott Kelly for the SECDIR review.

I mostly just have editorial/nit-level remarks, though there are a few
substantive notes, mostly relating to the security and operational
considerations.

Section 3

   PCEP speaker can use the generic mechanism as per [RFC8697] to
   associate a set of LSPs with a policy, without the need to know the
   details of such a policy, which simplifies network operations, avoids
   frequent software upgrades, as well as provides an ability to
   introduce new policies faster.

A few nits here; maybe
NEW:

   PCEP speakers can use the generic mechanism of [RFC8697] to
   associate a set of LSPs with a policy, without the need to know the
   details of such a policy.  This simplifies network operations and avoids
   frequent software upgrades, as well as provides the ability to
   introduce new policies more quickly.

Section 3.1

   or a PCE or both.  Consider a Label Switch Router (LSR) with a policy

nit: https://www.rfc-editor.org/materials/abbrev.expansion.txt lists it
as "Label Switching Router".

   enabled PCC, it receives a service request via signaling, including
   over a Network-Network Interface (NNI) or User-Network Interface
   (UNI) reference point, or receives a configuration request over a
   management interface to establish a service.  [...]

I'm not really sure what this sentence is trying to say.  (The grammar
is also a little bit weird.)  Is there supposed to be some policy
associated with the received requests?  There's not much connection to
the following sentence, which says that the PCC might additionally apply
other policies, but doesn't tie into anything received by the requests
enumerated here.  Unless the following sentence is supposed to say that
the policy that gets applied depend on how it got the request, or
something like that (right now the "also" implies a distinct and
independent step)?

   PCEP speaker can use the generic mechanism as per [RFC8697] to
   associate a set of LSPs with policy and its resulting path
   computation constraints.  [...]

Nit: we probably do want to be specific about "a policy" vs "policies"
plural -- whether or not we have to consider the intersection/union of
policies is important.

Section 4

   This Association type is operator-configured [RFC8697] association in
   nature and created by the operator manually on the PCEP peers.  An
   LSP belonging to this association is conveyed via PCEP messages to
   the PCEP peer.  Operator-configured Association Range MUST NOT be set
   for this association-type, and MUST be ignored, so that the full
   range of association identifier can be utilized.

(editorial) I think the last sentence would be improved by reframing it
to say that, by definition, all associations of type 3 are
operator-configured, so there is no need to convey an explicit
operator-configured association range, which could only serve to
artificially limit the available association IDs.

   A PAG can have one or more LSPs.  The association parameters
   including association identifier, Association type (PAT), as well as
   the association source IP address is manually configured by the
   operator and is used to identify the PAG as described in [RFC8697].

nit: singular/plural mismatch parameters/is (twice).

   and Error-Value 1 "Association type is not supported".  Since the PAG
   is opaque in nature, the PAG and the policy MUST be configured on the
   PCEP peers as per the operator-configured association procedures.

(editorial) I see that the association ID (as operator-configured) and
the policy details are opaque, but it seems that the PAG structure
itself is well-specified and not opaque.

   Associating a particular LSP to multiple policy groups is authorized
   from a protocol perspective, however, there is no assurance that the

(nit) "authorized" doesn't seem like the right word, here -- "allowed"
seems like it would work well.

   related parameters.  The encoding format and the order MUST be known
   to the PCEP peers, this could be done during the configuration of the
   policy (and its association parameters) for the PAG.  [...]

Do we expect the flexibility to specify the format at this level of fine
granularity to be used often, as opposed to defining a single "well
known" format for use within some well-defined domain of operation?
Is the format allowed to depend on the contents of (e.g.) the
VENDOR-INFORMATION TLV?
The security considerations might note that ensuring agreement among all
relevant parties (within whatever domain of operation that might be) as
to the format and layout of the policy parameters information is key for
correct operation.

   unacceptable in the context of the associated policy (e.g. out of

nit: comma after "e.g.".

Section 7

Thank you for referencing RFC 7525 as BCP 195!  (We are probably due for
an update to the BCP...)

   problems in handling of the policy for the legitimate LSPs.  It
   should be noted that, Policy association could provide an adversary

nit: the comma is unneeded, and the 'P' should be a minuscule 'p'.

   Further, extra care needs to be taken by the implementation with
   respect to POLICY-PARAMETERS-TLV while decoding, verifying, and
   applying these policy variables.  This TLV parsing could be exploited
   by an attacker and thus extra care must be taken while configuring
   policy association that uses POLICY-PARAMETERS-TLV and making sure
   that the data is easy to parse and verify before use.

I think it's worth expounding on how the parser code is particularly
sensitive since the protocol element is just opaque and can be used to
convey data with many different internal structure/formats.  That is,
since what decoder to use is dependent on the additional metadata
associated with the policy, not just the protocol element, there is
additional risk of trying to use the wrong decoder and getting
"nonsense" results.  Magic numbers in policy formats might help
alleviate those risks.

Section 9.1

   PCEP peers and associate it with the LSPs.  They MAY also allow
   configuration to related policy parameters, in which case the
   operator MUST also be allowed to set the encoding format and order to
   parse the associated policy parameters TLV.

I'm a little confused at how the operator would directly "set the
encoding format" (order is perhaps more plausible".  In general this
could be an arbitrary complex binary protocol, not amenable for
description in pure configuration.  What is the actual intent of the
MUST-level requirement?

Section 9.2

   [RFC7420] describes the PCEP MIB, there are no new MIB Objects for
   this document.

nit: comma splice.

   The PCEP YANG module is defined in [I-D.ietf-pce-pcep-yang].  This
   module supports associations as defined in [RFC8697] and thus
   supports the Policy Association groups.

nit: I think s/This/That/, since "this" implies some level of locality
to the current document.

Section 9.4

Would it not be possible to verify correct operation for the operation
of applying a given indicated policy (and parameters)?

Section 11.2

If use of RFC 8253 is RECOMMENDED, that would typically promote it to
being a normative reference, per
https://www.ietf.org/about/groups/iesg/statements/normative-informative-references/
.