Re: [Idr] AD Review of draft-ietf-idr-bgp-flowspec-oid-12

["Juan Alcaide (jalcaide)" <[e-mail]>]

Thanks for your comments Alvaro. I'll add them and also a few more nits/clarifications inspirited by them.


In particular the following. Let me know if you agree or you have other suggestions




--> Match conditional statement with condition in past tense

<snip>
A possible case would be if AS_PATH contained
       only ASNs that *were* known to belong to a common administrative domain.
</snip>



-->  The text between ** is also added, so we are removing it from RFC8955 (our new rule is no a rule of the BGP specification)
<snip>
   [RFC8955] states:

      BGP implementations MUST also enforce that the AS_PATH attribute
      of a route received via the External Border Gateway Protocol
      (eBGP) contains the neighboring AS in the left-most position of
      the AS_PATH attribute.  **While this rule is optional in the BGP
      specification, it becomes necessary to enforce it here for
      security reasons.**
</snip>



-->  4.1: Remove use cases when redefining rules b.2.2 and b.2.3. Everything is moved to the explanation section (since there was some duplication).

--> Redefinition of the rule removes the word NLRI (between **), so it's consistent with the naming of the rest of the document.

<snip>
      BGP Flow Specification implementations MUST enforce that the AS in
      the left-most position of the AS_PATH attribute of a Flow
      Specification route received via the External Border Gateway
      Protocol (eBGP) matches the AS in the left-most position of the
      AS_PATH attribute of the best-match unicast route for the
      destination prefix embedded in the Flow Specification** NLRI**.
</snip>



--> I originally added AS_CONFED_SET because the   [I-D.ietf-idr-deprecate-as-set-confed-set] is not an RFC yet. But I'll gladly remove AS_CONFED_SET reference for clarity if that's ok. 
And just add a note in case it's not deprecated yet. Only concern is that it was (could have been) mentioned in both sections 4.1 and 5. So I thought perhaps the best place to add the note would
be at the end of the introduction. My proposal:

<snip>
   Throughout this document, some references are made to
   AS_CONFED_SEQUENCE AS_PATH segments.  If AS_CONFED_SET ASPATH
   segments are supported, same considerations MUST apply to them.
   Note, however, that AS_CONFED_SET segments are expected to be
   deprecated per [I-D.ietf-idr-deprecate-as-set-confed-set], and thus
   support for them will soon not be required.
</snip>


-J


-----Original Message-----
From: Alvaro Retana <[e-mail]> 
Sent: Tuesday, April 20, 2021 9:18 PM
To: Juan Alcaide (jalcaide) <[e-mail]>
Cc: David Smith (djsmith) <[e-mail]>; Susan Hares <[e-mail]>; James Uttaro <[e-mail]>; [e-mail]; IDR List <[e-mail]>
Subject: RE: [Idr] AD Review of draft-ietf-idr-bgp-flowspec-oid-12

On April 12, 2021 at 7:32:04 PM, Juan Alcaide wrote:

[+ WG + Chairs]


Juan:

Hi!

> Here it goes: 
> https://tools.ietf.org/html/draft-ietf-idr-bgp-flowspec-oid-13

I looked at the update and have some remaining comments -- mostly minor and nits.  I am starting the IETF Last Call -- you can address my comments as part of other potential LC comments.

Thanks!

Alvaro.


[Line numbers from idnits for -13.]

  == Unused Reference: 'RFC6793' is defined on line 487, but no explicit
     reference was found in the text


...
15	Abstract

17	   This document describes a modification to the validation procedure
18	   defined for the dissemination of BGP Flow Specifications.  The
19	   dissemination of BGP Flow Specifications requires that the originator
20	   of the Flow Specification matches the originator of the best-match
21	   unicast route for the destination prefix embedded in the Flow
22	   Specification.  For an iBGP received route, the originator is
23	   typically a border router within the same autonomous system.  The
24	   objective is to allow only BGP speakers within the data forwarding
25	   path to originate BGP Flow Specifications.  Sometimes it is desirable
26	   to originate the BGP Flow Specification any place within the
27	   autonomous system itself, for example, from a centralized BGP route
28	   controller.  However, the validation procedure will fail in this
29	   scenario.  The modification proposed herein relaxes the validation
30	   rule to enable Flow Specifications to be originated within the same
31	   autonomous system as the BGP speaker performing the validation.
32	   Additionally, this document revises AS_PATH validation rules so Flow
33	   Specifications received from an eBGP peer can be validated when such
34	   peer is a BGP route server.

[nit] s/Specification any place/Specification from any place

[nit] s/revises AS_PATH/revises the AS_PATH


...
93	2.  Introduction
...
127	   Figure 1 illustrates this principle.  R1 (the upstream router) and RR
128	   need to validate the Flow Specification whose embedded destination
129	   prefix has a best-match unicast route (dest-route) originated by
130	   ASBR2.  ASBR2 could originate the Flow Specification, and it would be
131	   validated when received by RR and R1.  Sometimes the Flow
132	   Specification needs to be originated on AS1.  ASBR1 could originate
133	   it, and Flow Specification would still be validated.  In both cases,
134	   the Flow Specification is originated by a router in the same
135	   forwarding path as the dest-route.  For the case where AS1 has
136	   thousands of ASBRs, it becomes impractical to originate different
137	   rules on each ASBR in AS1 based on which ASBR each dest- route is
138	   learned from.  The objective is to advertise all the Flow
139	   Specifications from the same route-controller.

[nit] s/originated on AS1/originated inside AS1

[minor] s/different rules/different Flow Specification rules

[nit] s/dest- route/dest-route


...
159	3.  Motivation
...
234	   In addition, an operator MAY extend the requirements above for a
235	   group of ASes via policy.  This is described in section (b.2.3) of
236	   the validation procedure.

[minor] Let's keep the Normative actions to the mechanism: s/MAY/may


...
251	4.1.  Revision of Route Feasibility
...
264	      2.  The AS_PATH attribute of the Flow Specification is empty or
265	          contains only AS_CONFED_SEQUENCE and/or AS_CONFED_SET segments
266	          [RFC5065].

[] I had suggested only AS_CONFED_SEQUENCE because of the recommendations in rfc6472 and draft-ietf-idr-deprecate-as-set-confed-set.  I'm ok if you want to leave the current text -- an alternative would be to mention (in the "Explanation" below) that only AS_CONFED_SEQUENCE should be present and to add an Informative reference to draft-ietf-idr-deprecate-as-set-confed-set.


268	          1.  This condition SHOULD be enabled by default (it may be
269	              disabled by explicit configuration as described on the
270	              next list item (b.2.1))..  an empty AS_PATH.

[] The update left some orphaned text.  Also, the new text above (b.2) now mentions the empty AS_PATH so there's no need for this bullet.
And it refers to itself (b.2.1).


272	          2.  This condition MAY be disabled by explicit configuration
273	              on a BGP speaker.  A possible case would be if we know for
274	              a fact that only the right egress border routers (i.e.
275	              those that are also egress border routers for the best
276	              routes) are originating a Flow Specification NLRI.

[nit] Who are "we"?   s/if we know for a fact /if it is known


278	          3.  As an extension to this rule, a given non-empty AS_PATHs
279	              (or AS_PATHS containing only AS_CONFED_SEQUENCE and/or
280	              AS_CONFED_SET segments), MAY be validated by policy.  A
281	              possible case would be if the AS_SEQUENCE and AS_SET
282	              contained only ASes that are known to belong to our own
283	              administrative domain.

[minor] The new text is confusing mentioning first AS_CONFED_* and then AS_*.

Suggestion>

   3.  As an extension to this rule, a given non-empty AS_PATH MAY be
       validated by policy.  A possible case would be if AS_PATH contained
       only ASNs that are known to belong to a common administrative domain.


...
306	4.2.  Revision of AS_PATH Validation

308	   [RFC8955] states:

310	   BGP implementations MUST also enforce that the AS_PATH attribute of a
311	   route received via the External Border Gateway Protocol (eBGP)
312	   contains the neighboring AS in the left-most position of the AS_PATH
313	   attribute.

[minor] Indent this paragraph to show that it is a quote from rfc8955.

315	   This rule prevents the exchange of BGP Flow Specification NLRIs at
316	   Internet exchanges with BGP route servers [RFC7947].  Therefore, this
317	   document also redefines the [RFC8955] AS_PATH validation procedure
318	   referenced above as follows:

320	   BGP Flow Specification implementations MUST enforce that the AS in
321	   the left-most position of the AS_PATH attribute of a Flow
322	   Specification route received via the External Border Gateway Protocol
323	   (eBGP) matches the AS in the left-most position of the AS_PATH
324	   attribute of the best-match unicast route for the destination prefix
325	   embedded in the Flow Specification NLRI.

[minor] Indent this paragraph to show that it is the updated text.


...
370	5.  Topology Considerations

372	   [RFC8955] indicates that the originator may refer to the originator
373	   path attribute (ORIGINATOR_ID) or (if the attribute is not present)
374	   the transport address of the peer from which the BGP speaker received
375	   the update.  If the latter applies, a network should be designed so
376	   it has a congruent topology amongst ipv4 unicast routes and Flow
377	   Specification routes.  By congruent topology, it is understood that
378	   for the two equivalent routes (i.e. the Flow Specification route and
379	   its best-match unicast route) are learned from the same peer accross
380	   the AS.  That would likely not be true, for instance, if some peers
381	   only negotiated one type of address-family or if each address-family
382	   had a different set of policies.

[minor] the generic text should apply to any AF.  s/ipv4 unicast routes/unicast routes


...
389	   Explanation:

391	      Consider the validation procedure preceding this document.  The
392	      second condition (b.2) does not exist.  The two following
393	      scenarios have a non-congruent topology:

[] The original text made sense.  Suggestion>

   Consider the following scenarios of a non-congruent topology without the
   second condition (b.2) being added to the validation procedure:


...
430	7.  Security Considerations

432	   This document updates the route feasibility validation procedures for
433	   Flow Specifications learned from iBGP peers and through route
434	   servers.  This change is in line with the procedures in [rfc8955] and
435	   thus maintain security characteristics equivalent to the existing
436	   security properties of BGP unicast routing.

[nit] s/rfc8955/RFC8955

[End -13.]