Subject: [secdir] Secdir review of draft-ietf-idr-bgp-flowspec-oid-13

From: Magnus Nyström <[email-masked]>
Date: Sun, 2 May 2021 21:43:38 -0700

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the IESG.
These comments were written primarily for the benefit of the security area
directors.  Document editors and WG chairs should treat these comments just
like any other last call comments.

This document describes "a modification to the validation procedure defined
for the dissemination
of BGP Flow Specifications." More specifically. the memo describes a
mechanism which relaxes  the existing validation rule that requires Flow
Specifications to be originating from the originator of the best-match
unicast route, and now allows such specifications to be originated within
the same AS as the validator. As a result. the Security Considerations
section does call out: "The original AS_PATH validation rule ([RFC4271]
section 6.3 <https://tools.ietf.org/html/rfc4271#section-6.3>) becomes
hereby optional (section 4.2
<https://tools.ietf.org/html/draft-ietf-idr-bgp-flowspec-oid-13#section-4.2>).
If that original rule is actually not enforced it may introduce some
security risks. A peer (or a client of a route server peer) in AS X could
advertise a rogue Flow Specification route...." and "If t[the originator of
a rule] is known for a fact not to be a route server, that optional rule
SHOULD be enforced for Flow Specification routes."

It is not clear to me how a validator would now "for a fact" that a peer
isn't a route server, and thus that it would have to enforce the
now-optional path validation rule. It seems some clarity on this would be
good such that implementations have less of a risk of accepting flow
specifications from unauthorized parties, even if they are on the same AS.

Editorial:

   - "Let's consider the particular case where the Flow Specification is
   originated in any location within the same autonomous system than the
   speaker performing the validation (for example by a centralized BGP route
   controller), and the best-match unicast route is originated in another
   autonomous system." - should the word "than" be replaced with "that" here?
   - In the security considerations section, "becomes hereby optional"
   could probably be simplified to "becomes optional" or similar, and
   "actually" could be removed.

Thanks,
-- Magnus

---

From: "Juan Alcaide (jalcaide)" <[email-masked]>
Date: Mon, 3 May 2021 10:36:11 +0000

Thanks for your comments, Magnus

Inline..

From: Magnus Nyström <[email-masked]>
Sent: Monday, May 3, 2021 6:44 AM
To: [email-masked]; [email-masked]
Subject: Secdir review of draft-ietf-idr-bgp-flowspec-oid-13

I have reviewed this document as part of the security directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written primarily for the benefit of the security area directors.  Document editors and WG chairs should treat these comments just like any other last call comments.

This document describes "a modification to the validation procedure defined for the dissemination
of BGP Flow Specifications." More specifically. the memo describes a mechanism which relaxes  the existing validation rule that requires Flow Specifications to be originating from the originator of the best-match unicast route, and now allows such specifications to be originated within the same AS as the validator. As a result. the Security Considerations section does call out: "The original AS_PATH validation rule ([RFC4271] section 6.3<https://tools.ietf.org/html/rfc4271#section-6.3>) becomes  hereby optional (section 4.2<https://tools.ietf.org/html/draft-ietf-idr-bgp-flowspec-oid-13#section-4.2>). If that original rule is actually not enforced it may introduce some security risks. A peer (or a client of a route server peer) in AS X could advertise a rogue Flow Specification route...." and "If t[the originator of a rule] is known for a fact not to be a route server, that optional rule SHOULD be enforced for Flow Specification routes."

It is not clear to me how a validator would now "for a fact" that a peer isn't a route server, and thus that it would have to enforce the now-optional path validation rule. It seems some clarity on this would be good such that implementations have less of a risk of accepting flow specifications from unauthorized parties, even if they are on the same AS.

[JUAN]: This paragraph was not intended to pressure the operator to know if the peer was a route server, it was just a ‘if’. Note it’s the same case for RFC4271 :

If the UPDATE message is received from an external peer, the local
   system MAY check whether the leftmost (with respect to the position
   of octets in the protocol message) AS in the AS_PATH attribute is
   equal to the autonomous system number of the peer that sent the
   message.

Note that the same challenge of identifying route servers applies for other address-families.
Note also that the route-server itself may enforce the rule.

What about for clarity:

   The original AS_PATH validation rule ([RFC4271] section 6.3) remains hereby still optional
   (section 4.2) for Flow Specification Address Family (changes introduced in [RFC5575] are cancelled).
   If that original rule is not enforced for Flow Specification it may introduce some new security risks.
   A peer (or a client of a route server peer) in AS X could advertise a rogue Flow
   Specification route whose first AS in AS_PATH was Y (assume Y is the
   first AS in the AS_PATH of the best-match unicast route).  This risk
   is impossible to prevent if the Flow Specification route is received
   from a route server peer.  If that peer is known for a fact not to be
   a route server, that optional rule SHOULD be enforced for Flow
   Specification routes. Note that identifying those peers that are route servers may suppose an
   operational challenge. If the condition of the peer is unknown, the rule SHOULD not be
   enforced.

   A route server itself may be in a good position to enforce the AS_PATH validation rule described
  in the previous paragraph. If a route server knowns it’s not peering with any other route server,
   it can enforce the AS_PATH validation rule across all its peers. If, in addition to that,
   the route server is trusted, the security threat described above disappears.

Anybody feel free to reword the two paragraphs above if it helps them for clarity.

Editorial:

  *   "Let's consider the particular case where the Flow Specification is originated in any location within the same autonomous system than the speaker performing the validation (for example by a centralized BGP route controller), and the best-match unicast route is originated in another autonomous system." - should the word "than" be replaced with "that" here?
[JUAN]: Thanks for pointing that out. A few googling tells me the even better grammatical choice would be ‘same as’ in this case. I’ll be using ‘same as’ unless  you disagree.

  *   In the security considerations section, "becomes hereby optional" could probably be simplified to "becomes optional" or similar, and "actually" could be removed.
[JUAN]:  Hmmm, I thought that it was important to emphasize it becomes optional because of *this* draft redefinition of rules. Don’t you think it’s important? (whatever wording you want to use). Regardless, refer to my new reworded 2 paragraphs above for that section .

Thanks,
-- Magnus

---

From: Magnus Nyström <[email-masked]>
Date: Mon, 3 May 2021 12:38:42 -0700

Thanks Juan.
For the editorial ones, agree on the first one and I think the second you
could change to "with this memo, becomes optional." or similar.
For the new text, perhaps I am misunderstanding something, but this
sentence:

Note that identifying those peers that are route servers may suppose an
operational challenge. If the condition of the peer is unknown, the rule
SHOULD not be enforced.

Is it correctly understood, that you're saying that if a validator is
unable to determine if the peer is a route server, they should not enforce
the rule? If so, doesn't that mean "failing open, i.e., opening themselves
to risk? Sorry if I misunderstand.

On Mon, May 3, 2021 at 3:36 AM Juan Alcaide (jalcaide) <[email-masked]>
wrote:

> *From:* Magnus Nyström <[email-masked]>
> *Sent:* Monday, May 3, 2021 6:44 AM
> *To:* [email-masked]; [email-masked]
> *Subject:* Secdir review of draft-ietf-idr-bgp-flowspec-oid-13
>
>
>
> [JUAN]: This paragraph was not intended to pressure the operator to know
> if the peer was a route server, it was just a ‘if’. Note it’s the same case
> for RFC4271 :
>
>
>
> [JUAN]: Thanks for pointing that out. A few googling tells me the even
> better grammatical choice would be ‘same as’ in this case. I’ll be using
> ‘same as’ unless  you disagree.
>
>
>
> [JUAN]:  Hmmm, I thought that it was important to emphasize it becomes
> optional because of **this** draft redefinition of rules. Don’t you think
> it’s important? (whatever wording you want to use). Regardless, refer to my
> new reworded 2 paragraphs above for that section .
>
>
>
>
>
>
>
> Thanks,
>
> -- Magnus
>
>
>

-- 
-- Magnus

---

From: "Juan Alcaide (jalcaide)" <[email-masked]>
Date: Tue, 4 May 2021 00:17:47 +0000

Magnus,

Yes, they’ll be opening to risk. It’s the risk described in the draft. But this risk is necessary for route-servers to work with flowspec. And as mentioned, perhaps route servers can be configured to avoid that risk.

I wanted to use the term ‘remain’ instead of ‘become’.. We went from RFC4271 (rule optional) -> RFC8955 (rule mandatory) -> this draft (rule again optional)
Would the below make sense (I changed the first sentence):

   This document makes the original AS_PATH validation rule ([RFC4271] section 6.3) again optional
   (section 4.2) for Flow Specification Address Family (the rule is no longer mandatory as it was specified by [RFC8955]).

From: Magnus Nyström <[email-masked]>
Sent: Monday, May 3, 2021 9:39 PM
To: Juan Alcaide (jalcaide) <[email-masked]>
Cc: [email-masked]; [email-masked]
Subject: Re: Secdir review of draft-ietf-idr-bgp-flowspec-oid-13

Thanks Juan.
For the editorial ones, agree on the first one and I think the second you could change to "with this memo, becomes optional." or similar.
For the new text, perhaps I am misunderstanding something, but this sentence:
Note that identifying those peers that are route servers may suppose an operational challenge. If the condition of the peer is unknown, the rule SHOULD not be enforced.
Is it correctly understood, that you're saying that if a validator is unable to determine if the peer is a route server, they should not enforce the rule? If so, doesn't that mean "failing open, i.e., opening themselves to risk? Sorry if I misunderstand.

On Mon, May 3, 2021 at 3:36 AM Juan Alcaide (jalcaide) <[email-masked]<mailto:[email-masked]>> wrote:
Thanks for your comments, Magnus

Inline..

From: Magnus Nyström <[email-masked]<mailto:[email-masked]>>
Sent: Monday, May 3, 2021 6:44 AM
To: [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>
Subject: Secdir review of draft-ietf-idr-bgp-flowspec-oid-13

What about for clarity:

Editorial:

Thanks,
-- Magnus

--
-- Magnus

---

From: Magnus Nyström <[email-masked]>
Date: Mon, 3 May 2021 17:48:35 -0700

Thanks Juan, yes, to me this is at least clearer about the risk. At this
point I leave it to the Security Area directors if they'd like to see any
other modifications.

Thanks!
Magnus

On Mon, May 3, 2021 at 5:17 PM Juan Alcaide (jalcaide) <[email-masked]>
wrote:

> Magnus,
>
>
>
> Yes, they’ll be opening to risk. It’s the risk described in the draft. But
> this risk is necessary for route-servers to work with flowspec. And as
> mentioned, perhaps route servers can be configured to avoid that risk.
>
>
>
>
>
> I wanted to use the term ‘remain’ instead of ‘become’.. We went from
> RFC4271 (rule optional) -> RFC8955 (rule mandatory) -> this draft (rule
> again optional)
>
>    (section 4.2) for Flow Specification Address Family (the rule is no
> longer mandatory as it was specified by [RFC8955]).
>
> *Sent:* Monday, May 3, 2021 9:39 PM
> *To:* Juan Alcaide (jalcaide) <[email-masked]>
> *Cc:* [email-masked]; [email-masked]
> *Subject:* Re: Secdir review of draft-ietf-idr-bgp-flowspec-oid-13
>
>
>
> Thanks Juan.
>

-- 
-- Magnus