Re: [Idr] [BULK] Bug in draft-ietf-idr-rfc5575bis, worth fixing?

[Robert Raszuk <[e-mail]>]

Hi Jakob,

> I personally prefer that BGP be nothing more than a transport for
flowspec.

100%

- - -

Can you take a look if XR, XE & NX all parse today's shipping FS NLRIs and
validate each type and when applicable length to be "valid" ?

What all three of those implementations do when they:

a) run into the case of unknown type ?

b) run into the case of invalid value of any type carried inside the NLRI ?

Many thx,
R.



On Wed, Sep 30, 2020 at 8:34 PM Jakob Heitz (jheitz) <[e-mail]>
wrote:

> In that case, I'm fine with it.
>
> I personally prefer that BGP be nothing more than a transport for flowspec.
>
> That makes it possible to change the flowspec updates without modifying BGP
>
> or upgrading speakers that don't install the flowspecs themselves.
>
>
>
> Regards,
>
> Jakob.
>
>
>
> *From:* Dongjie (Jimmy) <[e-mail]>
> *Sent:* Wednesday, September 30, 2020 12:53 AM
> *To:* Robert Raszuk <[e-mail]>; Jakob Heitz (jheitz) <
> [e-mail]>
> *Cc:* idr@ietf. org <[e-mail]>; John Scudder <jgs=
> [e-mail]>; [e-mail];
> [e-mail]; Hares Susan <[e-mail]>
> *Subject:* RE: [Idr] [BULK] Bug in draft-ietf-idr-rfc5575bis, worth
> fixing?
>
>
>
> Hi Robert and Jakob,
>
>
>
> Please note that section 3 of 5575bis says:
>
>
>
> “BGP itself treats the NLRI as a key to an entry in its databases. Entries
> that are placed in the Loc-RIB are then associated with a given set of
> semantics, which is application dependent.”
>
>
>
> Although comparing with RFC5575 the word “opaque” is removed, the NLRI is
> still treated as the key.
>
>
>
> Best regards,
>
> Jie
>
>
>
> *From:* Idr [mailto:[e-mail] <[e-mail]>] *On
> Behalf Of *Robert Raszuk
> *Sent:* Tuesday, September 29, 2020 2:58 PM
> *To:* Jakob Heitz (jheitz) <[e-mail]>
> *Cc:* idr@ietf. org <[e-mail]>; John Scudder <
> jgs=[e-mail]>; [e-mail];
> [e-mail]; Hares Susan <[e-mail]>
> *Subject:* Re: [Idr] [BULK] Bug in draft-ietf-idr-rfc5575bis, worth
> fixing?
>
>
>
> Hi Jakob,
>
>
>
> > Flowspec does not AFAIK have any such NLRI... yet.
>
>
>
> I am not quite sure what are you trying to say above.
>
>
>
> Today in RFC5575 NLRI is clearly defined as a numeric value with a given
> length. It was by design not parsable by BGP. The entire NLRI is a key.
>
>
>
> So updates and withdraws processing happens on the key as defined.
>
>
>
> Now if we start to parse that value (except maybe for validation which I
> am also now pretty sceptical about) and dissecting part of that calling
> some types to be a key and some not - at min IMHO we should use different
> SAFI not to create complete deployment issues.
>
>
>
> Many thx,
>
> R.
>
>
>
>
>
> On Tue, Sep 29, 2020 at 1:23 AM Jakob Heitz (jheitz) <[e-mail]>
> wrote:
>
> Also to consider that in BGP, there are several examples of NLRI
>
> where the NLRI key is not the whole NLRI, starting with RFC 3107,
>
> where the label is not part of the key.
>
>
>
> A speaker that receives an update with an unknown NLRI does not
>
> know if that unknown NLRI servers to withdraw a (seemingly)
>
> different unknown NLRI, if the parts that are different are not
>
> key material.
>
>
>
> Therefore, a BGP speaker must always be able to completely
>
> parse and understand received NLRI.
>
>
>
> Flowspec does not AFAIK have any such NLRI... yet.
>
>
>
> Regards,
>
> Jakob.
>
>
>
> *From:* Idr <[e-mail]> *On Behalf Of *Robert Raszuk
> *Sent:* Monday, September 28, 2020 1:44 PM
> *To:* Christoph Loibl <[e-mail]>
> *Cc:* idr@ietf. org <[e-mail]>; [e-mail];
> John Scudder <jgs=[e-mail]>; [e-mail];
> Hares Susan <[e-mail]>
> *Subject:* Re: [Idr] [BULK] Bug in draft-ietf-idr-rfc5575bis, worth
> fixing?
>
>
>
> All,
>
>
>
> To me the real question is if we really should validate the content of
> NLRI or just make sure that NLRI boundaries meet BGP MP_REACH
> definition and treat NLRI values completely opaque to BGP (as per original
> RFC5575).
>
>
>
> If NLRI is really malformed resulting in MP_REACH attribute being
> malformed I do not see that much harm in session reset.
>
>
>
> But if we dive into each atomic type parsing of the NLRI value itself at
> each BGP speaker talking SAFI 133/134 I think this is going to be a mess.
>
>
>
> Best,
>
> R.
>
>
>
>
>
>
>
> On Mon, Sep 28, 2020 at 9:41 PM Christoph Loibl <[e-mail]> wrote:
>
> Hi,
>
>
>
> I do not really want to repeat the whole discussion here (seems that we
> are going in circles) if not needed. I think that we agreed that if the
> NLRI is malformed the only option is to reset (+send notification) the
> session (even if we consider rfc7606). And from draft-rfc5575bis it is
> clear that we are talking about a malformed NLRI:
>
>
>
>    A NLRI value not encoded as specified specified here is considered
>
>    malformed and error handling according to Section 10 is performed.
>
>
>
> -> I think that adding the small term that John suggested is sufficient.
>
>
>
> Cheers Christoph
>
>
>
> --
> Christoph Loibl
> [e-mail] | CL8-RIPE | PGP-Key-ID: 0x4B2C0055 | http://www.nextlayer.at
>
>
>
>
>
>
>
> On 28.09.2020, at 21:16, Alvaro Retana <[e-mail]> wrote:
>
>
>
> [Explicitly adding Jeff to the To list.]
>
>
>
> Hi!
>
>
>
> During my AD review there was a discussion on the list about this point,
> and whether we could avoid resetting the session.  Jeff presented some
> examples and, I think, made a very convincing case of why we really can’t:
> even if we can look at the length, we would still be guessing.
>
>
>
> I think this is one of the messages:
> https://mailarchive.ietf.org/arch/msg/idr/FHC-Vz26LZfam-o5Y-9-WSrEm2g/
>
>
>
> Jeff: if you get a chance, please chime in.
>
>
>
> Thanks!
>
>
>
> Alvaro.
>
>
>
> On September 28, 2020 at 2:16:38 PM, John Scudder (
> jgs=[e-mail]) wrote:
>
>
>
> I think that is the right thing, too: FS uses T,V and not T,L,V for its
> component types, the lengths are implicit. So, if my parser encounters an
> unknown type code it is impossible for me to know how to skip over that
> type code. At that point, parsing breaks down.
>
> [Bruno] I had in mind the higher level of hierarchy:
>
>
>
>    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.
>
>
>
>                      +-------------------------------+
>
>                      |    length (0xnn or 0xfnnn)    |
>
>                      +-------------------------------+
>
>                      |    NLRI value   (variable)    |
>
>                      +-------------------------------+
>
>
>
>                 Figure 1: Flow Specification NLRI for IPv4
>
>
>
> At this level, assuming that the NLRI value is found semantically
> incorrect, it seems to me that one could:
>
> -          Skip this NLRI (thanks to the ‘length’ field) and continue
> with the next NLRI
>
> -          Read the ‘NLRI value’ as an opaque data, and treat this NLRI
> as withdraw. (In the context of the discussion, this NLRI would never had
> been accepted, so ‘treat-as-withdraw’  could even be replaced with ‘ignore
> ’. But I’m _*not*_ calling for this).
>
> Hence it seems to me that treat-as-withdraw is technically possible.
>
>
>
> Fair enough. It’s a little unfortunate that the draft contains this
> ambiguity; in retrospect it would have been better to be explicit about the
> error-handling strategy chosen rather than simply referencing RFC 7606.
> Whether or not we want to respin the draft in order to clarify it, is a
> question for the WG. If we were to make a change, it could potentially be
> the addition of this sentence, in Section 10:
>
>
>
>    Error handling according to [RFC7606 <https://tools.ietf.org/html/rfc7606>] and [RFC4760 <https://tools.ietf.org/html/rfc4760>] applies to this
>
>    specification. *Notably, an NLRI that is found to be semantically*
>
>    *incorrect (for example due to an unknown component type) MUST be*
>
>    *handled using the “treat-as-withdraw” strategy (which in this case,*
>
>    *it must be noted, works out to be the same as skipping over the NLRI).*
>
>
>
>