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

From: "David Smith (djsmith)" <[email-masked]>
Date: Tue, 9 Mar 2021 14:39:31 +0000

Hi,

We believe we’ve addressed all of Alvaro’s comments. If any other feedback, please let us know. Otherwise, we’ll post v13 on Monday 4/15.

Thanks,

/dave

From: Idr <[email-masked]> on behalf of Alvaro Retana <[email-masked]>
Date: Wednesday, February 24, 2021 at 10:35 AM
To: "[email-masked]" <[email-masked]>, "[email-masked]" <[email-masked]>, "Juan Alcaide (jalcaide)" <[email-masked]>
Cc: "[email-masked]" <[email-masked]>, "idr@ietf. List" <[email-masked]>
Subject: Re: [Idr] AD Review of draft-ietf-idr-bgp-flowspec-oid-12

Juan:

Hi!

That part looks ok to me.

Thanks!

Alvaro.

On February 24, 2021 at 7:52:47 AM, Juan Alcaide (jalcaide) ([email-masked]<mailto:[email-masked]>) wrote:

We assumed it was understood, but it's always better to have a new set of eyes 😉

We did some rewording below. Tried to simplify the abstract and expand some explanation in introduction (simpler than in Motivation section)

Let us know if there is need for more rewording.

---

From: "Susan Hares" <[email-masked]>
Date: Tue, 9 Mar 2021 11:25:44 -0500

David: 

I would like to review the draft after your revision for the AD’s Comments.   I can do this after you submit -13.txt or before.   

I suspect that Alvaro will have a few minor comments on -13.txt so it can work in parallel. 

Cheers, Sue 

From: David Smith (djsmith) [mailto:[email-masked]] 
Sent: Tuesday, March 9, 2021 9:40 AM
To: Alvaro Retana; Susan Hares; [email-masked]; Juan Alcaide (jalcaide)
Cc: [email-masked]; IDR List
Subject: Re: [Idr] AD Review of draft-ietf-idr-bgp-flowspec-oid-12

Hi,

Thanks,

/dave

Juan:

Hi!

Thanks!

Alvaro.

On February 24, 2021 at 7:52:47 AM, Juan Alcaide (jalcaide) ([email-masked]) wrote:

---

From: Alvaro Retana <[email-masked]>
Date: Tue, 9 Mar 2021 09:03:49 -0800

Dave:

Hi!

I would appreciate a reply to my comments — especially to the points that
you might not agree with or require more discussion.  It would make my
review of the changes easier/faster.  I look forward to -13.

Thanks!

Alvaro.

On March 9, 2021 at 9:39:39 AM, David Smith (djsmith) ([email-masked])
wrote:

We believe we’ve addressed all of Alvaro’s comments. If any other feedback,
please let us know. Otherwise, we’ll post v13 on Monday 4/15
<http://airmail.calendar/2021-04-15 12:00:00 EDT>.

---

From: "Juan Alcaide (jalcaide)" <[email-masked]>
Date: Wed, 10 Mar 2021 17:03:49 +0000

Alvaro,

Apologies. I sent out doc with changes but I forgot a few ones. I commented in the original review you sent (look at [JUAN] after your comment).
Let me know if it's clear and enough.

I can send you proposed complete document with extra changes if it helps.

-J

-----Original Message-----
From: Alvaro Retana <[email-masked]> 
Sent: Thursday, January 28, 2021 12:04 AM
To: [email-masked]
Cc: Susan Hares <[email-masked]>; [email-masked]; IDR List <[email-masked]>
Subject: AD Review of draft-ietf-idr-bgp-flowspec-oid-12

Dear authors:

Thank you for your work on this document.

In general, I think that this document is simple and straight forward.
However, I have some major concerns (see details inline) that should be cleared up before proceeding.

Thanks!

Alvaro.

[Lines from idnits.]

...
4	Updates: 5575bis (if approved)                                J. Alcaide

[major] s/5575bis/rfc8955/g

...
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.  This allows only BGP speakers within the data
23	   forwarding path (such as autonomous system border routers) to
24	   originate BGP Flow Specifications.  Though it is possible to
25	   disseminate such Flow Specifications directly from border routers, it
26	   may be operationally cumbersome in an autonomous system with a large
27	   number of border routers having complex BGP policies.  The
28	   modification proposed herein enables Flow Specifications to be
29	   originated from a centralized BGP route controller.

[] rfc8955 and origination from ASBRs

Specifically on that point, the "originator" as defined in rfc8955 is "reset" at the AS boundary.  Regardless of whether the Flow Specification was originally generated at the ASBR or a centralized route controller, the receiving eBGP speaker will consider its eBGP peer to be the originator.

You would see the problem internal to the originating AS, unless the controller also originates the routes internally.

The motivation as laid out in the Abstract is making assumptiona that are not explained, resulting in lack of accuracy.  Please reword.
Note that many times "less is more" -- perhaps focus the Abstract on what the draft is proposing: updating the validation rules for iBGP and route server scenarios.

[JUAN]: Already shown in doc changes. New abstract + added introduction section

31	   This document updates RFC5575bis.

[major] s/This document updates RFC5575bis./This document updates the validation procedure in rfc8955.

[JUAN]: Will update

...
81	1.  Requirements Language

83	   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
84	   "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
85	   document are to be interpreted as described in RFC 2119 [RFC2119].

[major] Use the rfc8174 template!

[JUAN]: Already shown in doc changes

87	2.  Introduction

89	   [I-D.ietf-idr-rfc5575bis] defined a new BGP [RFC4271] capability that
90	   can be used to distribute traffic Flow Specifications amongst BGP
91	   speakers in support of traffic filtering.  The primary intention of
92	   [I-D.ietf-idr-rfc5575bis] is to enable downstream autonomous systems
93	   to signal traffic filtering policies to upstream autonomous systems.
94	   In this way, traffic is filtered closer to the source and the
95	   upstream autonomous system(s) avoid carrying the traffic to the
96	   downstream autonomous system only to be discarded.  [I-D.ietf-idr-
97	   rfc5575bis] also enables more granular traffic filtering based upon
98	   upper layer protocol information (e.g., protocol port numbers) as
99	   opposed to coarse IP destination prefix-based filtering.  Flow
100	   specification NLRIs received from a BGP peer are subject to validity
101	   checks before being considered feasible and subsequently installed
102	   within the respective Adj-RIB-In.

[]
OLD>
   [I-D.ietf-idr-rfc5575bis] defined a new BGP [RFC4271] capability that
   can be used to distribute traffic Flow Specifications amongst BGP
   speakers in support of traffic filtering.

NEW>
   [RFC8955] defines a BGP NLRI [RFC4271] that can be used to distribute
   traffic Flow Specifications amongst BGP speakers in support of traffic
   filtering.

[JUAN]: Will update

104	   The validation procedure defined within [I-D.ietf-idr-rfc5575bis]
105	   requires that the originator of the Flow Specification NLRI matches
106	   the originator of the best-match unicast route for the destination
107	   prefix embedded in the Flow Specification.  This allows only BGP
108	   speakers within the data forwarding path (such as autonomous system
109	   border routers) to originate BGP Flow Specification NLRIs.  Though it
110	   is possible to disseminate such Flow Specification NLRIs directly
111	   from border routers, it may be operationally cumbersome in an
112	   autonomous system with a large number of border routers having
113	   complex BGP policies.

[] See the related comment in the Abstract.

[JUAN]: I added an extra section in the introduction at this point. Is that enough, or is there something in the paragraph above that you would like to see changed?

115	   This document describes a modification to the [I-D.ietf-idr-
116	   rfc5575bis] validation procedure allowing Flow Specification NLRIs to
117	   be originated from a centralized BGP route controller within the
118	   local autonomous system that is not in the data forwarding path

---

From: Alvaro Retana <[email-masked]>
Date: Wed, 10 Mar 2021 15:05:27 -0500

Hi!

Not necessary, I can wait for -13 to be posted.

Thanks!!

Alvaro.

On March 10, 2021 at 12:04:06 PM, Juan Alcaide (jalcaide) (
[email-masked]) wrote:

---

From: "Juan Alcaide (jalcaide)" <[email-masked]>
Date: Tue, 16 Mar 2021 00:31:59 +0000

Hi Alvaro,

For my dismay, I realized I only had gotten part of your email (till review line 118). I think a weird problem with my email server. It was more in-deep review that I thought.
My changes will reflect more or less your suggestions.

Before, I have some contentious points I’d like to raise:

  *   Any alternative naming to ‘ingress border routers’ ? They are just the routers receiving traffic and far away from FS peering

  *   Around line 311, you mention a new security hole because of the new AS_PATH validation rule. You are referring to the case of a route-server, right? Let’s say AS1—(route-server)—AS2.   AS1, instead of AS_PATH = 1, could send us AS_PATH = 100 for the -rogue - FS route. The best-match unicast route could be actually coming from another route-server client om AS 100 (that is sending -correctly -the unicast route with AS_PATH=100)

  *   Around line 322, you mention we could use the new AS_PATH check for eBGP-confed routes as well. If we did, then we should also extend it for iBGP peers. But I don’t see much of a point to it, as long as we do the right check of the eBGP received routes any further check is redundant. Same concept that checking on AS_PATH loops only for eBGP received routes.

  *   Around line 324, you mention we should suppress ‘Other RFC5575bis Considerations.’. This section had more points that were trimmed when rfc8995 included those points. Only references left are about 2byte AS old speakers. Not sure if those references were ever considered for rfc8995. I guess the meaning of AS_PATH could be inferred w/o these references. rfc8955 already seems to infer them (although it is basically a copy of the old rfc4271 rule). Is it assumed that the meaning of AS_PATH is also inferred for any rfc after rfc6793?

  *   In the security section, you mention ‘ibgp are not trusted’. That’s strictly true, but in the sense of rfc8955 it would seem they are (rfc8955 is only concerned about trust for eBGP peerings). I can add a note, but is anything else is required?

-J

From: Alvaro Retana <[email-masked]>
Sent: Wednesday, March 10, 2021 9:05 PM
To: [email-masked]; Juan Alcaide (jalcaide) <[email-masked]>
Cc: Susan Hares <[email-masked]>; [email-masked]; IDR List <[email-masked]>
Subject: RE: AD Review of draft-ietf-idr-bgp-flowspec-oid-12

Hi!

Thanks!!

Alvaro.

On March 10, 2021 at 12:04:06 PM, Juan Alcaide (jalcaide) ([email-masked]<mailto:[email-masked]>) wrote:
I can send you proposed complete document with extra changes if it helps.

---

From: Alvaro Retana <[email-masked]>
Date: Tue, 16 Mar 2021 11:21:16 -0700

On March 15, 2021 at 8:32:06 PM, Juan Alcaide wrote:

Juan:

> For my dismay, I realized I only had gotten part of your email (till review
> line 118). I think a weird problem with my email server. It was more in-deep
> review that I thought.

Yeah...there seems to be an issue with long messages and Outlook (as
far as I can see): the messages are truncated.  The IETF archive
always gets the whole message.  I have also been including an "[End of
Review]" signal. :-)

> My changes will reflect more or less your suggestions.

Ok -- I'll take a look at the details when you post -13.

> Any alternative naming to ‘ingress border routers’ ? They are just the
> routers receiving traffic and far away from FS peering

Right -- that is what is confusing: these routers are not at the
border or are ingress routers.

I don't have a better suggestion.  The name is used only 3 times, so
perhaps the simply describe where they are...

> Around line 311...

[It is better if you reply in-line...so we make sure we're both
talking about the same thing. ;-)]

> ...you mention a new security hole because of the new AS_PATH validation rule.
> You are referring to the case of a route-server, right? Let’s say
> AS1—(route-server)—AS2. AS1, instead of AS_PATH = 1, could send us AS_PATH =
> 100 for the -rogue - FS route. The best-match unicast route could be actually
> coming from another route-server client om AS 100 (that is sending -correctly
> -the unicast route with AS_PATH=100)

This is what I wrote:

====
306	      Redefinition of this AS_PATH validation rule for a Flow
307	      Specification does not mean that the original rule in [I-D.ietf-
308	      idr-rfc5575bis] cannot be enforced as well.  Its enforcement
309	      remains optional per [RFC4271] section 6.3.  That is, we can
310	      enforce the first AS in the AS_PATH to be the same as the neighbor
311	      AS for any address-family route (including a Flow Specification).

[major] Both rfc5575 and rfc8955 mentioned that the enforcement is
required "for security reasons".  I'm sure the question will come up
about the potential security holes that are now opened if the
enforcement is not performed.  Please add (preferably in the Security
Considerations section) text including any recommendations on when the
enforcement can/should be performed.

For the general eBGP case, for example, there is the possibility that
the left-most ASN matches for both the FS/unicast routes, but that it
doesn't correspond to the peer AS.  I believe this is a case where an
additional vulnerability is introduced --- but also one that can be
mitigated by recommending/requiring the enforcement in this type of
cases (with appropriate caveats).
====

Yes. The route-server case is a subcase of the "general eBGP case".

> Around line 322, you mention we could use the new AS_PATH check for
> eBGP-confed routes as well. If we did, then we should also extend it for iBGP
> peers. But I don’t see much of a point to it, as long as we do the right
> check of the eBGP received routes any further check is redundant. Same
> concept that checking on AS_PATH loops only for eBGP received routes.

===
318	      Using the new rule to validate a Flow Specification route received
319	      from an External Border Gateway Protocol (eBGP) peer belonging to
320	      the same local domain (in the case that the local AS belongs to a
321	      confederation of ASes) is out of the scope of this document.  Note
322	      that although it's possible, its utility is dubious.

...
[minor] Why can't the same check be applied?  The text above doesn't
talk about segments so it should be able to apply.

[minor] As for the value, you seem to be assuming that just because
the confederation is in the same administrative domain than there
won't be configuration errors or rogue routers.  I believe that is a
dangerous assumption.
===

The "normal" iBGP case is different than the confederation case where
we do find an eBGP relationship.

> Around line 324, you mention we should suppress ‘Other RFC5575bis
> Considerations.’. This section had more points that were trimmed when
> rfc8995 included those points. Only references left are about 2byte AS old
> speakers. Not sure if those references were ever considered for rfc8995. I
> guess the meaning of AS_PATH could be inferred w/o these references. rfc8955
> already seems to infer them (although it is basically a copy of the old
> rfc4271 rule). Is it assumed that the meaning of AS_PATH is also inferred for
> any rfc after rfc6793?

In short, yes.  rfc6793 formally Updated rfc4271, so any
interpretation of the AS_PATH has to consider what rfc6793 says.

> In the security section, you mention ‘ibgp are not trusted’. That’s strictly
> true, but in the sense of rfc8955 it would seem they are (rfc8955 is only
> concerned about trust for eBGP peerings). I can add a note, but is anything
> else is required?

====
412	   BGP updates learned from iBGP peers are trusted so the Traffic Flow
413	   Specifications contained in BGP updates are trusted.  Therefore it is
414	   not required to validate that the originator of an intra-domain
415	   Traffic Flow Specification matches the originator of the best-match
416	   unicast route for the flow destination prefix.  This proposal
417	   continues to enforce the validation Procedure for eBGP learned
418	   Traffic Flow Specifications, as per [I-D.ietf-idr-rfc5575bis] rules.
419	   In this way, the security properties of [I-D.ietf-idr-rfc5575bis] are
420	   maintained such that an EBGP peer cannot cause a denial-of-service
421	   attack by advertising an inter-domain Flow Specification for a
422	   destination prefix that it does not provide reachability information
423	   for.

[major] "iBGP peers are trusted"   This is a flawed assumption because
it doesn't take into account the possibility of a rogue iBGP speaker.
Consider the case where an attacker takes control of a router and
creates (or limits) Flow Specifications -- not checking would open the
network up to any router becoming an attack point.

Yes, I understand that this is not a new vulnerability introduced in
this draft.  However, the type of risk (creating a Flow Specification)
is new to this work and a direct result.

Because the origin check won't happen for iBGP, you should mention the
risk clearly.
====

As you mention, rfc8955 considers eBGP.  The fact that it doesn't say
anything about iBGP shouldn't be interpreted as trusting those peers.
Even if that was the intent in rfc8955, this document deals directly
with iBGP so it makes it the best place to call the risk out.

Thanks!

Alvaro.

---

From: "Juan Alcaide (jalcaide)" <[email-masked]>
Date: Tue, 16 Mar 2021 19:36:05 +0000

Inline

The "normal" iBGP case is different than the confederation case where we do find an eBGP relationship.

[JUAN]:

Consider 
R1(AS-65001)--R2(AS-65001)--- R3(AS-65002)--R4(AS65002) ==== R5 (AS 701)
R1, R2, R3, R4 belong to the same confederation (AS 100)
R2-R3 have an ebgp-confed peering session

R5 advertises the FS route and the unicast route
R4 does the AS_PATH check (i.e. making sure the AS_PATH of the unicast route and the FS route received from R5 are both AS_PATH ='701 XXX')
R3 does not do the AS_PATH check (from the route received from R4)
Why R2 should do the AS_PATH check from a router received from R3? (i.e. making sure the AS_PATH of the unicast route and the FS route are both AS_PATH = '(XXX) 701 XXX')

In short, yes.  rfc6793 formally Updated rfc4271, so any interpretation of the AS_PATH has to consider what rfc6793 says.
[JUAN]: I'll be happy to remove it. It adds too much detail. But it seems to me that all new rfcs are assuming rfc6793 is already implemented. Trying to implement rfc8955 without rfc6793 would be undefined.

As you mention, rfc8955 considers eBGP.  The fact that it doesn't say anything about iBGP shouldn't be interpreted as trusting those peers.
Even if that was the intent in rfc8955, this document deals directly with iBGP so it makes it the best place to call the risk out.

[JUAN]: I think that's a bit open for interpretation. Quoting rfc8955:

<snip>
The Flow Specification received from an internal BGP
   peer within the same autonomous system [RFC4271] is assumed to have
   been validated prior to transmission within the internal BGP (iBGP)
   mesh of an autonomous system.
</snip>

You can read that iBGP validation is not needed because eBGP validation was successful, what implies iBGP is trusted.
Rfc8955 should have mentioned if that assumption was not true 100% of the time. There are different 'levels' of what it means to be 'trusted'.

Regardless, it doesn't hurt to mention it here. 

-J