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

Benjamin Kaduk <kaduk@mit.edu> Fri, 21 May 2021 21:39 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: idr@ietfa.amsl.com
Delivered-To: idr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 6CB333A2198; Fri, 21 May 2021 14:39:01 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.897
X-Spam-Level:
X-Spam-Status: No, score=-1.897 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_NONE=0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eEtKwfpQXobf; Fri, 21 May 2021 14:38:56 -0700 (PDT)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 001E63A219A; Fri, 21 May 2021 14:38:55 -0700 (PDT)
Received: from kduck.mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 14LLce2o030361 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 21 May 2021 17:38:50 -0400
Date: Fri, 21 May 2021 14:38:40 -0700
From: Benjamin Kaduk <kaduk@mit.edu>
To: "Juan Alcaide (jalcaide)" <jalcaide@cisco.com>
Cc: The IESG <iesg@ietf.org>, "draft-ietf-idr-bgp-flowspec-oid@ietf.org" <draft-ietf-idr-bgp-flowspec-oid@ietf.org>, "idr-chairs@ietf.org" <idr-chairs@ietf.org>, "idr@ietf.org" <idr@ietf.org>, Susan Hares <shares@ndzh.com>, "aretana.ietf@gmail.com" <aretana.ietf@gmail.com>
Message-ID: <20210521213840.GW32395@kduck.mit.edu>
References: <162102145453.12250.16239592060339839431@ietfa.amsl.com> <BL1PR11MB5416E108FB35105DFDB020E0CD2C9@BL1PR11MB5416.namprd11.prod.outlook.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <BL1PR11MB5416E108FB35105DFDB020E0CD2C9@BL1PR11MB5416.namprd11.prod.outlook.com>
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/vM-sxMgDOmhqsj21GnLwlXLIRjo>
Subject: Re: [Idr] Benjamin Kaduk's No Objection on draft-ietf-idr-bgp-flowspec-oid-14: (with COMMENT)
X-BeenThere: idr@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Inter-Domain Routing <idr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/idr>, <mailto:idr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/idr/>
List-Post: <mailto:idr@ietf.org>
List-Help: <mailto:idr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/idr>, <mailto:idr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 21 May 2021 21:39:02 -0000

Hi Juan,

On Tue, May 18, 2021 at 12:49:56AM +0000, Juan Alcaide (jalcaide) wrote:
> Thanks Bejamin for all your comments. I'll incorporate them in the draft.
> I have however concerns about some specific observations of yours. Namely:
> 
> 
> 
> 
> About your toplevel comment, I'm not sure if I understand. Both route controllers and other routers are considered in this document equally trusted. They are
> just BGP speakers propagating Flow Specifications alltoguether. So it doesn't matter which one is more trusted. The overall trust is always the trust of
> the weakest link. We tried to add some security considerations for when an iBGP peer is not trusted for FS. Also note that if we don't trust iBGP peers, even
> with just unicast, the security of the network is highly diminished. It's nothing specific to FS.

I recognize that it's a common scenario for all BGP speakers in the
AS/confederation to be treated as equally trusted.  I don't think it's the
only possible configuration, though, and I'm pretty sure I have heard of
setups that give the controller a "first among equals" treatment.

My remark here is coming from the bedrock security principle of "least
privilege", where we grant just the entities that need it just enough
authority to do what needs to be done.  They don't get more authority, and
other parties don't get authority they don't need.  Applied to this
document, we're granting new authority to originate flowspecs without
originating the best route.  This is pretty small new privilege, since
the other routers can always originate flowspec if they also originate the
best route, and there's basically nothing to stop them from doing that.
But in some scenarios it can have a leveraged impact, so my proposal is
that we allow for implementations/deployments to be able to apply the
principle of least privilege if it makes sense for them.  I am not trying
to impede anyone else from using this document in a way where all the BGP
speakers are equally trusted.

> 
> "
> 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...
> "
> 
> I guess you mean if it's possible that an AS_PATH with just AS_CONFED_SEQUENCE is not automatically validated (and we have to still validate
> with unicast paths). As long as all the peers belonging to the confederations of ASes are trusted, there is no need to validate with unicast paths and
> better not to do this because then the congruent topologies assumption described later would fail.
> I guess it's conceivable that a confederation of ASes is formed by subAS not trusted between each other (perhaps during some weird AS adquisitions?)
> and you may need a knob. But I'd think this is out of scope. IT's a bit related to the other paragraph you are having rpboelms with "Using the new rule ...".
> I'll remove it because it's confusing while discussing just an uncommon configuration.

I don't think you should take my comment as a suggestion to remove the
AS_CONFED_SEQUENCE case, my apologies if I gave that impression.  Part of
my job as a security reviewer is to look for edge cases, and this is one
that I had to ask for more data about before I was confident enough to
dismiss it as irrelevant.

> 
> 
> 
> "
> 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 think this is explained later. We are assuming iBGP peers as trusted as a first assumption,
> but later we explained that if that assumption is broken, then we actually are introducing 
> a new security risk. I thought it was clear that way. That's the paragraph you want to remove, so I am not sure
> how you want to say both things:  equally secure in the common case where iBGP peers are secure, less secure if
> iBGP peers are not secure.

I wasn't saying we should remove this text!  I just wanted to add a word,
so that we have security characteristics "roughly equivalent" or
"essentially equivalent" to the existing security properties.

> 
> "
>    route servers may suppose an operational challenge.  If the condition
>    of the peer is unknown, the rule SHOULD not be enforced.
> "
> 
> For several other comments, the agreement/compromise was to rewrite it as follows
> 
> "This risk
>    is impossible to prevent if the Flow Specification route is received
>    from a route server peer.
>    If configuration (or other means beyond the scope of this document) 
> indicates that the peer is not a route server, that optional rule 
> SHOULD be enforced. If the indication is that the peer is not a route server or there is no conclusive indication, that optional rule SHOULD NOT be enforced.
> "

This is probably tolerable.  (I prefer to have "no conclusive indication"
imply "SHOULD enforce the rule" but expect few others to agree.)
I was hopeful that we could use a phrasing that is not specific to just "a
route server peer" and instead talked about a peer that is trusted in a
specific way, since that seems to me to be the most important part.  Many
of the phrasings that I can come up with are pretty long and banal, so I
won't suggest them.  The most promising one I've come up with (which is a
relative term only) involves introducing a new logical capability, the
ability to advertise flowspecs without the best route, and then talking
about peers that are trusted with that capability.  It would still be
pretty long, though.

> 
>  I think [RFC9747] describes the behavior of route-servers, and it's important to 'design' the rules mentioned in this draft. You could say the same about [RFC5065] for instance
> 
> 
> I'd like to use eBGP/iBGP as in [RFC8955]. There are other discrepancies (left-most vs leftmost , ..) using inconsistently on different rfc. But unlike there is opposition, I'd like
> to follow the same style in [RFC8955] for anything where there is doubt.

Okay.

> 
> "
>       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.
> "
> 
> We plan to clarify by defining the exact meaning of "Local Domain"

I saw the thread with John, it looks good :)

Thanks,

Ben

> 
> 
> 
> 
> -----Original Message-----
> From: Benjamin Kaduk via Datatracker <noreply@ietf.org> 
> Sent: Friday, May 14, 2021 9:44 PM
> To: The IESG <iesg@ietf.org>
> Cc: draft-ietf-idr-bgp-flowspec-oid@ietf.org; idr-chairs@ietf.org; idr@ietf.org; Susan Hares <shares@ndzh.com>om>; aretana.ietf@gmail.com; shares@ndzh.com
> Subject: Benjamin Kaduk's No Objection on draft-ietf-idr-bgp-flowspec-oid-14: (with COMMENT)
> 
> 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.
> 
> 
>