Re: [Idr] AD Review of draft-ietf-idr-rfc5575bis-17 -> Updated Version -18 and Flowspec v6

[Alvaro Retana <[e-mail]>]

On December 18, 2019 at 7:08:06 AM, Jeffrey Haas wrote:

Jeff/Christoph:

Hi!

As I read the responses it sounded to me as if we were all going in
the same direction: discard if an unknown component is found.  But
reading more into it, I think we're talking about different cases...

> On Wed, Dec 18, 2019 at 08:11:37AM +0100, Christoph Loibl wrote:
...
> > > On 17.12.2019, at 23:09, Jeffrey Haas wrote:
> > >
> > > > If an unknown Flow Specification component exists, then the entire
> > > > NLRI cannot be "successfully parsed"...which results in not being able
> > > > to use treat-as-withdraw. The text above leaves us with AFI/SAFI
> > > > disable, which is not extension-friendly.
> > >
> > > This is exactly my largest concern about flowspec right now. There is no
> > > safe way to extend the spec at this point.
...
> > > > NEW (suggestion)>
> > > > An advertisement containing an unknown Flow Specification component
> > > > should be discarded as specified in Section 5.4 of [RFC7606].
> > >
> > > You can't parse it, therefore it's malformed.

Jeff: I interpreted this response as agreeing that if the NLRI can't
be parsed then it should be discarded (even maybe buying the Typed
NLRI argument).  It sounds like we're agreeing.  Is that true?


> > In case of an unknown flowspec component the parser can run until decoding
> > the component type (always the first byte of the component). When it reads
> > a unknown type-value it can easily jump over the remaining bytes of the
> > entire NLRI (the length of the the complete NLRI is encoded in the first
> > bytes of the NLRI) without needing to understand what is in there, because
> > it should treat it as withdraw. I do not see the problem. It is supposed to
> > “ignore” the entire NLRI not only a single component of it.

Christoph: It seems to me that we're getting to the same behavior: I
call it "discard", you call it "treat it as withdraw...“ignore”".
While discarding, ignoring and treat-as-withdraw are different things,
the effect seems to be the same: we won't use the NLRI.  We can find
the right words...  Do you also think we're going in the same
direction?


> Sigh. Into example-land we go:
>
> You receive the following byte-stream as NLRI in a single BGP PDU:
> NLRI-1, length X components 1,2,3
> NLRI-2, length Y components 1,2,3,?,?,?
> NLRI-3, length Z components 1,2,3
>
> In the case of NLRI-1,3 if you had received these individually, you'd know
> they were correct because the implementation understands them.
>
> In NLRI-2, you don't know what some of the components are starting at first ?.
>
> The _best_ you can do in these circumstances is walk the list of NLRI by
> length and see if you can arrive at NLRI boundaries and have a valid BGP
> Update PDU.
>
> However, you also don't know if you actually have an NLRI-3 or not. It
> might actually be a random byte stream that just happens to parse as an NLRI
> and accidentally lands on a packet boundary. NLRI-2 may be corrupt and the
> length mis-calculated.
>
> We have seen these bugs before, even in simple RFC 4271 PDUs.

In theory, if the length is set correctly, and the unknown components
are "well behaved" (i.e. they are actually correct, the receiver just
doesn't understand them), then it seems to me that we could safely
just discard NLRI-2.  In theory...

I think that we are talking about two different actions when bumping
into an unknown component: discarding an NLRI in the list (e.g. just
NLRI-2 above), vs discarding the whole advertisement (i.e. all NLRIs
to protect against potential bugs).

Going back to rfc7606/§3, it talks about parsing the "entire NLRI
field" (not just a single entry).  But the Typed NLRI behavior
(rfc7606/§5.4) talks about discarding "routes with unrecognized NLRI
types" (not the entire advertisement).

Personal opinion:  If we (WG) don't buy into the Typed NLRI-like
argument, then we deal with the "entire NLRI field", but we're also
stuck with AFI/SAFI disable (or session reset).  If we buy into the
Typed NLRI-like argument, then it seems like we have the option to
specify either discard behavior.

???

Alvaro.