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

[Benjamin Kaduk <[e-mail]>]

Hi Juan,

Also inline

On Mon, May 24, 2021 at 07:04:05PM +0000, Juan Alcaide (jalcaide) wrote:
> Thanks for following up with the comments.
> Please, check inline
> 
> -J
> 
> -----Original Message-----
> From: Benjamin Kaduk <[e-mail]> 
> Sent: Friday, May 21, 2021 11:39 PM
> To: Juan Alcaide (jalcaide) <[e-mail]>
> Cc: The IESG <[e-mail]>; [e-mail]; [e-mail]; [e-mail]; Susan Hares <[e-mail]>; [e-mail]
> Subject: Re: Benjamin Kaduk's No Objection on draft-ietf-idr-bgp-flowspec-oid-14: (with COMMENT)
> 
> 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.
> 
> [JUAN]: So your proposal would be for only allowing the route-controller to originate the Flow Specification?  That would be an interesting security improvement, but perhaps it should apply to more address-families than FS. It might require more work than just modifying FS validations rules, so I think it's out of scope.

My proposal is that some sites (but definitely not all) might want to only
allow the route-controller to originate the FS while not originating the
actual route.  Yes, it might apply to other address families as well, and
whether or not that means that we should not mention the possibility at all
in this document is not entirely clear to me.  I would (perhaps naively)
hope that we could have just a brief hint or mention of the possibility
even without going into much detail about it.

> 
> > 
> > "
> > 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.
> 
> [JUAN]: Great. Hope it's clear now.

Yes, thank you.

> > 
> > 
> > 
> > "
> > 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.
> 
> [JUAN]: Sorry, I was referring to the paragraph that you didn't understand starting with 
> " 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."
> 
> It's not actually the one you wanted to remove, but the one we discussed in another thread about removing and we compromised to reword it. 

Ah, I understand now; thanks for clarifying.

> Instead of 'roughly/essentially', what about using the last rewrite suggested in the comments
> 
>     The security considerations discussed in <xref target="RFC8955" /> apply to this
>     specification as well.  <<=== I'd like to move this paragraph to the beginning of the 'Security Considerations>
> 
>     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 <xref target="RFC8955" /> and, thus, security
>     characteristics remain essentially equivalent to the existing security properties of BGP
>     unicast routing, except as detailed below.  <<===  After 'except' we list the caveats.

This looks good to me (including reordering the paragraphs); thank you!

> 
> 
> 
> 
> 
> > 
> > "
> >    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.
> 
> [JUAN]: Not sure what is your logic..  When you don't know if the peer is a route-server or a non-route-server, then you **SHOULD NOT** enforce the rule. If you enforce the rule and it happens to be a non-route-server, then everything is ok. If it happens to be a route-server, then that peering breaks. 
> Not sure what you mean 'flowspec without the best route', but I think it's orthogonal to the recommendation here.
> We still may modify a bit this paragraph because of some other comments. I'll cc you if so.

I think we can close this topic; it's basically just the long-standing
divergence where security wants to "fail safe" by denying things that
cannot be determined to be valid or invalid, and routing wants to "fail
safe" by keeping the packets flowing.

Thanks again,

Ben


> > 
> >  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 <[e-mail]>
> > Sent: Friday, May 14, 2021 9:44 PM
> > To: The IESG <[e-mail]>
> > Cc: [e-mail]; [e-mail]; 
> > [e-mail]; Susan Hares <[e-mail]>; [e-mail]; 
> > [e-mail]
> > 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.
> > 
> > 
> >