Dissemination of Flow Specification Rules
draft-ietf-idr-rfc5575bis-27

I hope all routing documents are this easy to follow.  All I have are minor nits:

Section 1:
* "... manually as well as automatically.  The latter by systems that ..." -- change ". The" to ", the", because the second half of this isn't a sentence by itself.  Just join them.

Section 7.2:
The "TBD" here feels like it's dangling.  I realize it's dealt with in the IANA considerations, which is Section 11.3, but I had to jump around to make sure.  I imagine I'm more accustomed to an RFC Editor note or some such when I run into these.  You might consider saying in Section 11.3 that this is the thing described in Section 7.2, or vice versa.

Appendix A:
Should the copyright date be updated?

Thanks for this approachable update.  One item of feedback:

Appendix A.  This python code is referenced in Section 5.1.  Is it intended to be normative?  If not, then please say so.  If it is, I’d recommend include the full snippet of flowspec-cmp code in the document in addition to making the github reference.  Likewise, please provide a reference for Python 3.

Thank you for the work put into this document. The document is clear, easy to read (I appreciated the given examples). 

After discussion with the responsible AD, Alvaro Retana, and the IDR WG chairs, I am clearing my DISCUSS (kept below for archival purpose) based on the promise that the IPv6 companion document will be 'expedite processed' by the WG chairs, document shepherd, and responsible AD + request for prompt processing by the RFC Editor. The final goal is to have this document and its IPv6 companion published roughly at the same time. (I still wonder why making two documents though ;-) ).

Thanks to all for quickly updating the list of IPv6 implementations, that opened the gate for publication.

-éric

-- below is no more current as the DISCUSS is cleared --

Why having two different documents ? One for IPv4 (with the core elements of the protocol) and one for IPv6 (with only the IPv6 specifics)... I am more than surprized to say the least... hence my DISCUSS...

This blocking DISCUSS can easily be fixed: e.g., with a RFC Editor note to make a cluster of this document and draft-ietf-idr-flow-spec-v6 so that they are published together with adjacent RFC numbers. Merging the two documents would be preferred but I understand that this is more work (albeit a missed opportunity).

Please find below a couple on non-blocking COMMENTs.

I hope that this helps to improve the document,

Regards,

-éricI also second Erik K.'s comment on non-TCP/UDP ports.

Thanks for addressing my DISCUSS.

All of this is editorial:

— Abstract —

   Other applications (ie. centralized control of traffic in a SDN or
   NFV context) are also possible.

I don’t think you mean “i.e.” here, but “e.g.”  I would avoid the Latin (for exactly this reason: many people don’t understand it and misuse it) and say, “Other applications, such as centralized control of traffic in an SDN or NFV context, are also possible.”

— Section 1 —

   Modern IP routers contain both the capability to forward traffic
   according to IP prefixes as well as to classify, shape, rate limit,
   filter, or redirect packets based on administratively defined

The two things that come after “both” have to be parallel, and they’re not.  This will read better without the word “both”, like this:

NEW
   Modern IP routers have the capabilities to forward traffic
   according to IP prefixes and to classify, shape, rate limit,
   filter, or redirect packets based on administratively defined
END

   Section 4 of this document defines a general procedure to encode Flow
   Specification for aggregated traffic flows so that they can be
   distributed as a BGP [RFC4271] NLRI.

I think this has to say “Flow Specifications”, plural, yes?  That matches “they”.

   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

How does “as well as the limitations...” fit into the sentence?  There seems to be something missing here.

— Section 3 —

   prefix as well as community matching and manipulation, must apply to
   the Flow Specification defined NLRI-type

I can’t tell whether “Flow Specification defined” is meant to be a compound modifier for “NLRI-type” (in which case the former needs hyphens and the latter does not), or this is meant to describe a defined NLRI tyoe called “Flow Specification”.  I think you mean the latter.  Can you re-word this or re-punctuate it to make it clear?

— Section 5 —

   In order to achieve this goal, this document specifies two
   application specific NLRI identifiers that provide traffic filters,
   and a set of actions encoding in BGP Extended Communities.  The two
   application specific NLRI identifiers are:

Both instances of “application-specific” should be hyphenated.

— 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:

Make it “are true”.

      c) There are no more specific unicast routes

Hyphenate “more-specific” to distinguish it from “there are no more (specific unicast routes)”.

   By originator of a BGP route, we mean either the address of the

But nothing else says “originator of a BGP route”.  (b) does say “originator of the best-match unicast route”... is that what you mean here?  Can you make this match up so it’s clearer?

On thinking more about this text, I think maybe putting quotation marks around “originator” in the quoted sentence is all that’s needed.

   Specification information that conveys a more or equally specific
   destination prefix.

This needs awkward hyphenation ti be correct.  I suggest avoiding that by saying, “Specification information that conveys a destination prefix that is more or equally specific.”

   Thus, as long as there are no more specific
   unicast routes, received from a different neighboring AS, which would
   be affected by that Flow Specification.

As above, hyphenate “more-specific”.  And then fix the sentence, as it doesn’t appear to be a complete sentence.

— Section 7 —

   treated as interfering Traffic filtering actions (see below).

Please capitalize “Filtering Actions”, to be consistent.

   Some Traffic Filtering Actions may interfere with each other even
   contradict.

Make it “may interfere with or even contradict each other.”

— Section 7.7 —

   behaviour (ie. match - replace - delete communities on administrative
   boundaries).

I can’t understand what’s in the parentheses, nor even whether “i.e.” is correct or it should be “e.g.”.

— Section 9 —

   statistics facilities.  While this is an implementation specific

Hyphenate “implementation-specific”.

— Section 12 —

   This may stress the receiving systems, exceed their
   maximum capacity or may lead to unwanted Traffic Filtering Actions
   being applied to flows.

Change “capacity or may lead” to “capacity, or lead” — the “may” is already there at the start of the sentence, and the Oxford comma helps readability here.

[cleared Discuss, as I was indeed misunderstanding things]

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.

[Clearing discuss, for Alvaro to close on the final details].

Hi,

I also had a few minor comments on this document:

4.1.  Length Encoding

   o  If the NLRI length is smaller than 240 (0xf0 hex) octets, the
      length field can be encoded as a single octet.

   o  Otherwise, it is encoded as an extended-length 2-octet value in
      which the most significant nibble of the first octet is all ones.

This describes the first nibble in binary, and then later it is shown in hex.  It might be more clear to write ... in which the most significant nibble has the hex value 0xf.

   In Figure 1 above, values less-than 240 are encoded using two hex
   digits (0xnn).  Values above 239 are encoded using 3 hex digits
   (0xfnnn).  The highest value that can be represented with this
   encoding is 4095.  For example the length value of 239 is encoded as
   0xef (single octet) while 240 is encoded as 0xf0f0 (2-octet).


4.2.  NLRI Value Encoding

   Components MUST follow strict type ordering by increasing numerical
   order.  A given component type may (exactly once) or may not be
   present in the Flow Specification.  If present, it MUST precede any
   component of higher numeric type value.

I wasn't sure, but wondering whether "may (exactly once) or may not be" should be "MAY (exactly once) be"?


5.1.  Ordering of Flow Specifications

   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.

I think that the intended comparison here is clear, but I was wondering whether this text should flag endian concerns at all.  E.g. if the component data has been stored in memory and any numeric values have had endian conversion performed on them then a naive memcmp() could give a different answer.

Previous discuss comment:
I don't know if this is a valid discuss point, so happy to be educated that it is always written this way and I'll remove my discuss ...

I note that in 5 places this document has text that states the equivalent to "SHOULD be set to 0 on encoding, and MUST be ignored during decoding." (example given below).

Doesn't this make extending this in future more risky because if new meaning are given to these bits then there could be senders already transmitting non 0 values which a receiver might then misinterpret?

Hence, I was surprised that the constraints did not also include a MUST on the encoding side (i.e. be strict in what you send ...), i.e. "MUST be set to 0 on encoding, and MUST be ignored during decoding."

Example:
   The extended is encoded as follows:

      0                   1                   2                   3
      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |   reserved    |   reserved    |   reserved    |   reserved    |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |   reserved    | r.|    DSCP   |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

           Figure 6: Traffic Marking Extended Community Encoding

   o  DSCP: new DSCP value for the transiting IP packet.

   o  reserved, r.: SHOULD be set to 0 on encoding, and MUST be ignored
      during decoding.