Re: [Idr] Benjamin Kaduk's Discuss on draft-ietf-idr-rfc5575bis-22: (with DISCUSS and COMMENT)

Christoph Loibl <c@tix.at> Thu, 23 April 2020 08:53 UTC

Return-Path: <c@tix.at>
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 468C03A1733; Thu, 23 Apr 2020 01:53:27 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.099
X-Spam-Level:
X-Spam-Status: No, score=-2.099 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=tix.at
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 KRHwq7RIX0sD; Thu, 23 Apr 2020 01:53:23 -0700 (PDT)
Received: from mail.fbsd.host (mail.fbsd.host [IPv6:2001:858:58::22]) (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 7C5A43A1731; Thu, 23 Apr 2020 01:53:22 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tix.at; s=rev1; h=To:References:Message-Id:Content-Transfer-Encoding:Cc:Date: In-Reply-To:From:Subject:Mime-Version:Content-Type:Sender:Reply-To:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe :List-Post:List-Owner:List-Archive; bh=ABw2C1jixwNUfFX4fFiWoioqWUBJeRAIrM/aPgVT5Js=; b=uk/AUOjuwgMYHvB9ScJY3qIofA 36PZuARVL8simcSlr3lmDKwhujl5J5IfSsDJ4gFt5pb6F9BaTxJSjBPcOuTK6hgcSZJtVadjpuz8s fMrnuyG4h23t36JRZcA7YRSwldX4vC+Ntp9Cb6py/eddxEp3Ctx5NUgoCIstMq/NQmDSjwFKY9Pim BNhoMD1o0CaHmEOI2vTxpRfQ0TCiAZBXsiyGztXlc8S4M9CgXvBWmc2nlPxajHjMiA0zGYjeJy22X bmfQj5cUW6GWNjltcgs0jChueDhUlSMsupDdQ0u3Tk7Sa25qd6/cYvc6ZQbwDBBx7u/t5E0Gx+rsB 2dyyQLXQ==;
Received: from [89.144.223.235] (helo=[192.168.88.217]) by mail.fbsd.host with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92.3) (envelope-from <c@tix.at>) id 1jRXbi-000AA7-FH; Thu, 23 Apr 2020 10:53:18 +0200
Content-Type: text/plain; charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3608.60.0.2.5\))
From: Christoph Loibl <c@tix.at>
In-Reply-To: <158760254746.26942.2049621502527864651@ietfa.amsl.com>
Date: Thu, 23 Apr 2020 10:53:12 +0200
Cc: The IESG <iesg@ietf.org>, draft-ietf-idr-rfc5575bis@ietf.org, idr-chairs@ietf.org, idr@ietf.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <95C65325-EE11-42A1-8BAE-967BA2A0E448@tix.at>
References: <158760254746.26942.2049621502527864651@ietfa.amsl.com>
To: Benjamin Kaduk <kaduk@mit.edu>
X-Mailer: Apple Mail (2.3608.60.0.2.5)
X-Scanned-By: primary on mail.fbsd.host (78.142.178.22); Thu, 23 Apr 2020 10:53:18 +0200
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/nLTWvBQ0hRUmmbnVpCq1tBwUwes>
Subject: Re: [Idr] Benjamin Kaduk's Discuss on draft-ietf-idr-rfc5575bis-22: (with DISCUSS and 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: Thu, 23 Apr 2020 08:53:28 -0000

Hi Benjamin,

Thanks for your review. I opened cases for all of the issues you raised in the github repo:

	https://github.com/stoffi92/rfc5575bis

The discuss item you raised is this:

	https://github.com/stoffi92/rfc5575bis/issues/184

To resolve this I suggest the following change to the document:

I suggest to change it to the following (I hope that Robert and Sue can also have a look if this is still correct to the desired behaviour):

OLD:
   Contrary to the behavior specified for the non-VPN NLRI, Flow
   Specifications are accepted by default, when received from remote PE
   routers.

   The validation procedure (Section 6) and Traffic Filtering Actions
   (Section 7) are the same as for IPv4.

NEW:
   Contrary to the behavior specified for the non-VPN NLRI, Flow
   Specifications are accepted by default, when received from remote PE
   routers. The validation procedure (Section 6) when Flow Specifications
   are received from a remote CE are the same as for IPv4. 

   Traffic Filtering Actions (Section 7) are the same as for IPv4.


Cheers Christoph

-- 
Christoph Loibl
c@tix.at | CL8-RIPE | PGP-Key-ID: 0x4B2C0055 | http://www.nextlayer.at



> On 23.04.2020, at 02:42, Benjamin Kaduk via Datatracker <noreply@ietf.org> wrote:
> 
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-idr-rfc5575bis-22: Discuss
> 
> 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 IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-idr-rfc5575bis/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> There might be a minor internal inconsistency to tidy up (or I might be
> misunderstanding things).  Section 8 states that:
> 
>   Contrary to the behavior specified for the non-VPN NLRI, Flow
>   Specifications are accepted by default, when received from remote PE
>   routers.
> 
> As far as I can tell, this is referring to the text in Section 6 where (for
> the non-VPN case) "By default a Flow Specification NLRI MUST be validated
> such that it is considered feasible if and only if all of the below is true
> [...]".  But immediately following what I quote above is a statement that
> "the validation proceure (section 6) [...] [is] the same as for IPv4", which
> seems to be in conflict with this statement ("contrary to" vs. "the same
> as").
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> The Abstract is perhaps pushing the length limits appropriate for an
> Abstract (vs. Introduction).
> 
> Abstract
> 
>   Additionally, it defines two applications of that encoding format:
>   one that can be used to automate inter-domain coordination of traffic
>   filtering, such as what is required in order to mitigate
>   (distributed) denial-of-service attacks, and a second application to
> 
> This makes me think of the DOTS WG (but it's far from clear that we should
> reference it in this text).
> 
> Section 1
> 
>   This document obsoletes "Dissemination of Flow Specification Rules"
>   [RFC5575], the differences can be found in Appendix B.  This document
> 
> nit: comma splice
> 
>   A Flow Specification received from an external autonomous system will
>   need to be validated against unicast routing before being accepted
>   (Section 6).  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.  If the aggregate traffic flow defined
> 
> (Just to check, there's no particular harm in also validating iBGP
> flowspecs, just the extra computation burden of doing so, right?)
> 
>   systems that are able to detect malicious traffic flows.  When
>   automated systems are used, care should be taken to ensure their
>   correctness as well as the limitations of the systems that receive
>   and process the advertised Flow Specifications (see also Section 12).
> 
> This seems to discuss making sure that the recipients of automated
> information handle it robustly; is it also worth some considerations on
> robustness of generating such automated information (something akin to a
> circuit breaker that would shut off the source if it started producing
> nonsensical output)?
> Also, nit: I think I parse this as "care should be taken to ensure [...] the
> limitations of the systems that receive and process [flowspecs]", which
> doesn't quite seem like what was intended.
> 
> Section 4
> 
>   The NLRI field of the MP_REACH_NLRI and MP_UNREACH_NLRI is encoded as
>   one or more 2-tuples of the form <length, NLRI value>.  It consists
>   of a 1- or 2-octet length field followed by a variable-length NLRI
>   value.  The length is expressed in octets.
> 
> RFC 4760 only shows the one-octet length form; is there a good reference for
> the 2-octet length?  (Or is it "new" with 5575?)
> 
> Section 4.2
> 
> I think we should say something about how multi-byte values are encoded,
> especially given that we implicitly allow for different encodings of a given
> value (e.g., IP protocol could have padding, and small port numbers don't
> need to be encoded in 2 bytes).  Even just making it more explicit that
> multiple encodings are allowed (and what the semantics are for any such
> "padding") would be worthwhile.  Is there a maximum encoded length for
> anything other than the constraints of the 2-bit 'len' field?
> Interestingly, for DCSP we are much more strict -- why is the strictness
> needed there but not in general?  Is this just to faithfully reproduce RFC
> 5575, as implied by the discussion in Appendix B?
> (I would make this a DISCUSS point if I didn't think that we're acting as if
> we're required to preserve the RFC 5575 behavior.)
> 
>   A NLRI value not encoded as specified in Section 4.2 is considered
> 
> This *is* Section 4.2; perhaps "specified here" or "specified below"?
> 
> Section 4.2.1.1, 4.2.1.2
> 
> While I don't anticipate needing a future extension to numeric_op, having
> the '0' field only be "SHOULD be set to 0" (vs. "MUST") seems to hamper any
> future extensibility efforts.
> (Similarly for the "Traffic-action" field in Section 7.3, and
> "Traffic-marking" from Section 7.5.)
> 
> Section 4.2.2.3
> 
>   This component uses the Numeric Operator (numeric_op) described in
>   Section 4.2.1.1.  Type 3 component values SHOULD be encoded as single
>   octet (numeric_op len=00).
> 
> Is it well-defined how I would encode the value if I ignored the SHOULD?
> I, for one, am not sure what I would do...
> 
> Section 4.2.2.4, etc.
> 
> (I agree with my esteemed colleagues that hardcoding TCP and UDP as
> "the only protocols with ports" is unnecessary.)
> 
>   system is unable to locate the transport header.  Different
>   implementations may or may not be able to decode the transport header
>   in the presence of IP options or Encapsulating Security Payload (ESP)
>   NULL [RFC4303] encryption.
> 
> If an implementation cannot decode the transport header does it treat all
> such packets as matching or not matching?
> 
> Section 4.2.2.7
> 
>   In case of the presence of the ICMP type (code) component only ICMP
>   packets can match the entire Flow Specification.  The ICMP type
> 
> I'm not sure why the parenthetical is needed given that there's a separate
> NRLI type for "ICMP code".
> 
> Section 5.1
> 
>   component type.  If the component types are the same, then a type-
>   specific comparison is performed (see below) if the types are equal
>   the algorithm continues with the next component.
> 
> nit: there seems to be some missing punctuation or conjunction here.
> 
>   For all other component types, unless otherwise specified, the
>   comparison is performed by comparing the component data as a binary
>   string using the memcmp() function as defined by [ISO_IEC_9899].  For
>   strings with equal lengths the lowest string (memcmp) has higher
>   precedence.  For strings of different lengths, the common prefix is
>   compared.  If the common prefix is not equal the string with the
>   lowest prefix has higher precedence.  If the common prefix is equal,
>   the longest string is considered to have higher precedence than the
>   shorter one.
> 
> It is surprising to me that the comparison operator can return "not-equal"
> for different encodings of a quantity that have the same semantic value
> (viz my previous note about encoding flexibility).  That said, this
> procedure is well-defined and BGP speakers are not going to be re-encoding
> the NRLI values as they propagate, so I don't see an interop problem.
> 
> Section 6
> 
>   By default a Flow Specification NLRI MUST be validated such that it
>   is considered feasible if and only if all of the below is true:
> 
> Perhaps "in the absence of explicit configuration otherwise," to more
> closely parallel the other case?
> 
>   BGP implementations MUST also enforce that the AS_PATH attribute of a
>   route received via the External Border Gateway Protocol (eBGP)
>   contains the neighboring AS in the left-most position of the AS_PATH
>   attribute.  While this rule is optional in the BGP specification, it
>   becomes necessary to enforce it for security reasons.
> 
> nit: missing word (e.g., "necessary to enforce it here" or "necessary to
> enforce it in processing flow specifications").
> 
>   The best-match unicast route may change over the time independently
>   of the Flow Specification NLRI.  Therefore, a revalidation of the
>   Flow Specification NLRI MUST be performed whenever unicast routes
>   change.  Revalidation is defined as retesting that clause a and
>   clause b above are true.
> 
> IMPORTANT: Why does clause c not need to be retested?
> 
>   The neighboring AS is the immediate destination of the traffic
>   described by the Flow Specification.  If it requests these flows to
>   be dropped, that request can be honored without concern that it
>   represents a denial of service in itself.  Supposedly, the traffic is
>   being dropped by the downstream autonomous system, and there is no
>   added value in carrying the traffic to it.
> 
> (This presumes that there is some integrity protection applied to the
> received data, which might be worth making more explicit.)
> Also, nit: I'd suggest s/Supposedly,/The reasoning is that this is as if/
> 
> Section 7
> 
> Please be consistent about "ASN" vs. "AS" in Table 2.
> 
> Should the "traffic-action" and "traffic-marking" actions list the encoded
> length for the reader's convenience?
> 
> Section 7.1
> 
>   The remaining 4 octets carry the maximum rate information in IEEE
>   floating point [IEEE.754.1985] format, units being bytes per second.
> 
> Thank you for being clear about the units!
> 
> Section 7.2
> 
> Is there anything to say about what time interval a non-integer packet rate
> should be evaluated over?
> 
> Section 7.3
> 
>   o  T: Terminal Action (bit 47): When this bit is set, the traffic
>      filtering engine will evaluate any subsequent Flow Specifications
>      (as defined by the ordering procedure Section 5.1).  If not set,
>      the evaluation of the traffic filters stops when this Flow
>      Specification is evaluated.
> 
> Just to check I'm reading this right: when the "terminal action" bit is set
> to one, it is *not* the terminal action, and the action *is* the terminal
> action when the "terminal action" bit is set to zero?  That sounds
> backwards to my intuition (and maybe others'?).
> 
> Section 7.4
> 
>   Interferes with: No other BGP Flow Specification Traffic Filtering
>   Action in this document.
> 
> Do the different possible 'type's for this 'sub-type' interfere with each
> other?
> 
> Section 7.6
> 
>   Implementations should provide mechanisms that map an arbitrary BGP
>   community value (normal or extended) to Traffic Filtering Actions
>   that require different mappings in different systems in the network.
> 
> nit(?): to my eye this reads better as "on different systems" (vs. "in").
> 
>   For instance, providing packets with a worse-than-best-effort, per-
>   hop behavior is a functionality that is likely to be implemented
> 
> nit: no comma after worst-than-best-effort.
> 
> Section 7.7
> 
>   other (e.g. there may be more than one conflicting traffic-rate-bytes
>   Traffic Filtering Action associated with a single Flow
>   Specification).  Traffic Filtering Action interference has no impact
> 
> Maybe we should revise the earlier text about "Interferes with: No other
> [...]" to be explicit that it *does* self-interfere.
> 
>   behaviour (ie. match - replace - delete communities on administrative
>   boundaries).  See also Section 12.
> 
> nit: could this parenthetical be expanded a little bit more?  I feel like
> it's expected to be common knowledge among the main readership and so the
> current wording just serves as a trigger for this "well-known" (but not to
> me) concept, as opposed to standing on its own.
> 
> Section 8
> 
> [side note: I'd love for us to eventually stop using "VPN" for things that
> don't encrypt the data passing over them.  This doesn't seem like the right
> place to start, though.]
> 
> Section 9
> 
> Should either/both of these mechanisms be on or off by default?
> 
> Section 12
> 
> I'd consider mentioning again that some of the NRLI components won't work
> properly on fragmented traffic and that unexpected fragmentation can cause
> DoS.  (This is particularly relevant for IPv4, as opposed to IPv6, where
> fragmentation can occur anywhere on the path.)
> 
> Is it worth saying anything about how the RT-redirect actions can direct
> traffic to an entity not expecting it (and, as such, potentially DoS them)?
> 
> It also struck me as noteworthy that we're letting DSCP values creep out of
> a single administrative scope -- the filtering of inbound DSCP markings
> should probably be consistent with the traffic markings advertised to that
> peer.  (There is perhaps also some "information leakage" as to what
> semantics are assigned to given DSCP values within the network in question,
> but I find it hard to get too worked up about that, especially since the
> inter-AS relationship is inherently pretty trusted.)
> 
>   As long as Flow Specifications are restricted to match the
>   corresponding unicast routing paths for the relevant prefixes
>   (Section 6), the security characteristics of this proposal are
> 
> [There's an implicit "and both are received over trustworthy channels" in
> here.  Perhaps it's okay to remain implicit, given how well-known BGP's
> security properties are...]
> 
>   Where the above mechanisms are not in place, this could open the door
>   to further denial-of-service attacks such as unwanted traffic
>   filtering, remarking or redirection.
> 
> nit(?): I might just say "In particular, relaxing the validation procedure
> could [...]".
> 
>   additional filtering.  For example, the use of [RFC6811] to enhance
>   filtering within an AS confederation.
> 
> nit: incomplete sentence.
> 
>   Inter-provider routing is based on a web of trust.  Neighboring
>   autonomous systems are trusted to advertise valid reachability
>   information.  If this trust model is violated, a neighboring
>   autonomous system may cause a denial-of-service attack by advertising
>   reachability information for a given prefix for which it does not
> 
> I guess it's also a well-known attack that malicious neighbor could also
> draw traffic to itself for snooping purposes without actually dropping the
> traffic.  But I'm not sure if there are any flowspec-specific considerations
> relating to that scenario.
> 
>   provide service (unfiltered address space hijack).  Since validation
>   of the Flow Specification is tied to the announcement of the best
>   unicast route, this may also cause this validation to fail and
>   consequently prevent Flow Specifications from being accepted by a
>   peer.  Possible mitigations are [RFC6811] and [RFC8205].
> 
> nit: there's a lot of pronouns ("this") in here; it might be worth
> disambiguating a couple.
> 
>   Flow Specification BGP speakers (e.g. automated DDoS controllers) not
>   properly programmed, algorithms that are not performing as expected,
>   or simply rogue systems may announce unintended Flow Specifications,
>   send updates at a high rate or generate a high number of Flow
>   Specifications.  This may stress the receiving systems, exceed their
>   maximum capacity or may lead to unwanted Traffic Filtering Actions
>   being applied to flows.
> 
> Is there any relevant guidance to give to receiving systems?
> 
> Appendix A
> 
> I don't know that just this one function is useful without some description
> of what format of input it requires.  For example, if a.components and/or
> b.components have elements that evaluate to "false", the first two checks
> could return incorrect results (the perhaps-more-obvious case where both
> comp_a and comp_b are None cannot happen due to the nature of zip_longest).
> 
> Appendix B
> 
>      Section 7 defines all Traffic Filtering Action Extended
>      communities as transitive extended communities.  [RFC5575] defined
>      the traffic-rate action to be non-transitive and did not define
>      the transitivity of the other Traffic Filtering Action communities
>      at all.
> 
> I assume the consequences of changing a community from non-transitive to
> transitive are well-known (but not to me) and that any operational
> considerations for mixed deployments are already stated somewhere prominent
> to someone working with BGP.  (Where is that?)
> 
>      Section 7.7 contains general considerations on interfering traffic
>      actions.  Section 7.3 also cross-references this section.
> 
> nit(?): the last "this section" refers to 7.7, not the location of the text
> I'm quoting from (Appendix B).  I misread it the first time.
> 
> 
> 
> _______________________________________________
> Idr mailing list
> Idr@ietf.org
> https://www.ietf.org/mailman/listinfo/idr