Re: [Pce] Benjamin Kaduk's No Objection on draft-ietf-pce-association-policy-15: (with COMMENT)

["Chengli (Cheng Li)" <[e-mail]>]

Hi Ben,

Many thanks for your comments!  :)

We have updated the document to address your comments, please check https://www.ietf.org/rfcdiff?url2=draft-ietf-pce-association-policy-16.

Looking forward to your reply.
Cheng




Name:		draft-ietf-pce-association-policy
Revision:	16
Title:		Path Computation Element (PCE) Communication Protocol (PCEP) extension for associating Policies and Label Switched Paths (LSPs)
Document date:	2021-01-21
Group:		pce
Pages:		17
URL:            https://www.ietf.org/archive/id/draft-ietf-pce-association-policy-16.txt
Status:         https://datatracker.ietf.org/doc/draft-ietf-pce-association-policy/
Htmlized:       https://datatracker.ietf.org/doc/html/draft-ietf-pce-association-policy
Htmlized:       https://tools.ietf.org/html/draft-ietf-pce-association-policy-16
Diff:           https://www.ietf.org/rfcdiff?url2=draft-ietf-pce-association-policy-16



-----Original Message-----
From: Benjamin Kaduk via Datatracker [mailto:[e-mail]] 
Sent: Thursday, January 21, 2021 9:49 AM
To: The IESG <[e-mail]>
Cc: [e-mail]; [e-mail]; [e-mail]; Hariharan Ananthakrishnan <[e-mail]>; [e-mail]
Subject: Benjamin Kaduk's No Objection on draft-ietf-pce-association-policy-15: (with COMMENT)

Benjamin Kaduk has entered the following ballot position for
draft-ietf-pce-association-policy-15: 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-association-policy/



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

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/