[Idr] Benjamin Kaduk's No Objection on draft-ietf-idr-bgp-flowspec-oid-14: (with COMMENT)

[Benjamin Kaduk via Datatracker <[e-mail]>]

Benjamin Kaduk has entered the following ballot position for
draft-ietf-idr-bgp-flowspec-oid-14: 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 DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-idr-bgp-flowspec-oid/



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

John had some good editorial comments; thank you to him for spotting
them, and to the authors for pulling in the fixes.

It seems to me that the revision to the validation procedures that we
describe here may be broader than needed to satisfy the stated use case,
and that such broadness has a corresponding weakening of the security
properties of the system as a whole.  My understanding is that when
central controllers or route-reflectors are used, they tend to be in
some sense "more trusted" than peer routers in the domain (and,
correspondingly, more tightly secured).  So it seems like the system as
a whole would be less fragile/more secure if "off-path" flow specs were
allowed only from the trusted source, versus from any peer in the same
AS [confederation].  Is there a reason why such a procedure is
infeasible?  (It seems like all peers in the AS could be marked as
"trusted" in this way, if appropriate, which allows for recovering the
current semantics of this draft if needed.)  This is, in essence, the
same topic raised by the secdir reviewer and relates to "fail-closed" vs
"fail-open".  I think it gives a clearer picture on what the safe and
recommended behavior is, if we say to only accept these routes from
"trusted peers in the local domain" (but allow all peers in the local
domain to be trusted if appropriate) than to say to always accept these
routes from peers in the local domain and optionally do strict
enforcement if we know that the peer advertising the route is not a
route server.

Abstract

   This document describes a modification to the validation procedure
   defined for the dissemination of BGP Flow Specifications.  The
   dissemination of BGP Flow Specifications requires that the originator
   of the Flow Specification matches the originator of the best-match
   unicast route for the destination prefix embedded in the Flow
   Specification.  [...]

I'd suggest adding a couple words about "prior to this document
requires" or "as specified in RFC 8955 requires", since that's exactly
what we're changing with this document.

                However, the validation procedure will fail in this
   scenario.  [...]

Similarly, this could also say "the RFC 8955 validation procedure".

Section 4.1

      2.  The AS_PATH attribute of the Flow Specification is empty or
          contains only an AS_CONFED_SEQUENCE segment [RFC5065].

Would there ever be cause to have a knob that allows this condition only
for the empty AS_PATH (and denies this condition when AS_CONFED_SEQUENCE
is present)?  From skimming RFC 5065 I mostly assume not, but have to
ask...

Section 4.2

      For clarity, the AS in the left-most position of the AS_PATH means
      the AS that was last added to the AS_SEQUENCE.

Thank you for including this; it is a big help for readability.

      Using the new rule to validate a Flow Specification route received
      from an External Border Gateway Protocol (eBGP) peer belonging to
      the same local domain (in the case of a confederation) is out of
      the scope of this document.  Note that although it's possible, its
      utility is dubious.  Although it is conceivable that a router in
      the same local domain (both iBGP and eBGP within the same local
      domain) could send a rogue update, only eBGP (outside the local
      domain) risk is considered within this document (in the same
      spirit of the mentioned beforehand AS_PATH validation in
      [RFC4271]).

I'm having (I think) the same trouble John did reading this paragraph.
I'll watch his ballot thread and chime in there if needed; no need to
specifically reply here.

Section 7

We could mention again (per §3) that having this mechanism allows
central SOC or controller systems to be able to propagate flowspecs more
readily during attacks and improves the stability/security of the
network.

   This document updates the route feasibility validation procedures for
   Flow Specifications learned from iBGP peers and through route
   servers.  This change is in line with the procedures described in
   [RFC8955] and, thus, the security characteristics equivalent to the
   existing security properties of BGP unicast routing are maintained.

I would argue that the security characteristics are not (strictly)
"equivalent", but only "roughly equivalent".  With the new procedures, a
malicious or compromised node in the local AS can (e.g.) cause traffic
to be discarded without steering that traffic to/through itself.  There
are plenty of other ways that such a malicious node can cause harm, not
least announcing that prefix and dropping the traffic as it passes
through, so the overall properties of the system are roughly equivalent,
but there are clear differences of the particulars.

   route servers may suppose an operational challenge.  If the condition
   of the peer is unknown, the rule SHOULD not be enforced.

In light of my top-level comment (and the secdir review), I'm
uncomfortable with a SHOULD-level "fail-open" behavior.  Could the
default behavior when the condition of the peer is unkonwn be subject to
a configuration knob?

   BGP updates learned from iBGP peers are considered trusted, so the
   Traffic Flow Specifications contained in BGP updates are also
   considered trusted.  Therefore, it is not required to validate that
   the originator of an intra-domain Traffic Flow Specification matches
   the originator of the best-match unicast route for the destination
   prefix embedded in that Flow Specification.  Note that this
   trustworthy consideration is not absolute and the new possibility
   than an iBGP speaker could send a rogue Flow Specification is
   introduced.

Also in light of my toplevel comment, it's not clear that this paragraph
adds much value.  Specifically, as we note in the last sentence, the
level of trust in a route server is not (in the general case) the same
level of trust in a generic peer in the iBGP domain, even if both peers
are trusted in a general sense.  So it seems that the core sentiment
here is that when we get the flowspec from a trusted source, it's
trusted, and we don't need to be as strict about validating that it's
the same source that we would send the traffic to in the absence of a
flowspec.

Section 9.1

It seems that RFC 7947 is reference in only one location, and not in a
manner that strongly implies a normative relationship.

NITS

Abstract

                For an iBGP received route, the originator is

"IBGP" (with that capitalization) appears on the RFC Editor's list
at https://www.rfc-editor.org/materials/abbrev.expansion.txt but is not
marked as "well-known".  So we should probably use the expanded version
in the abstract and define it on first use in the body.

Section 2

   the same forwarding path as the dest-route.  For the case where AS1
   has thousands of ASBRs, it becomes impractical to originate different
   Flow Specification rules on each ASBR in AS1 based on which ASBR each
   dest-route is learned from.  The objective is to advertise all the
   Flow Specifications from the same route-controller.

"it becomes impractical" does not flow directly to "the objective is";
I'd put some transition phrase in, like "to make the situation more
tenable".

Section 3

   can direct border routers within their AS with specific attack
   mitigation actions (drop the traffic, forward to a clean-pipe center,
   etc.).

Maybe "clean-pipe center" is a term of art I'm not familiar with, but
the natural reading would be that a clean pipe is something that is the
output of a scrubbing center, and that pipe-cleaning is the operation
performed there.  This would suggest rewording to something like
"forward to a scrubbing center", "forward to a pipe-cleaning location",
etc.

   In addition, an operator may extend the requirements above for a
   group of ASes via policy.  This is described in section (b.2.3) of
   the validation procedure.

I suggest a forward reference ("below" or "in Section 4.1") to contrast
to the RFC 8955 procedure.

Section 5

                If the latter applies, a network should be designed so
   it has a congruent topology amongst unicast routes and Flow
   Specification routes.  [...]

This "should" does not give any indication of why the network should be
designed in this manner (e.g., that failing to do so would result in
(b.1) failing).

      Flow Specifications originated in a different local domain sill
      need a congruent topology.  The reason is that the second
      condition (b.2) evaluates to false and only the first condition
      (b.1) is evaluated.

I suggest s/different local domain/different domain/ -- my first reading
of "different local domain" was "a different AS but the same
confederation", and only by contrasting this statement to the previous
paragraph did I realize the intended meaning.
Alternately (or additionally?), this could be clarified by noting that
this scenario will result in an AS_PATH that contains
non-AS_CONFED_SEQUENCE segments.