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

[Robert Raszuk <[e-mail]>]

Hi John,

I admit I have missed that change or even if I noticed it apparently I did
not realize the magnitude and consequences this change brings.

IMHO it would be a fine change for FSv2 however I am quite sceptical for
rolling it into 5575bis.

Many thx,
R.


On Tue, Sep 29, 2020 at 5:20 PM John Scudder <[e-mail]> wrote:

> Hi Robert,
>
> You are right that RFC 5575 is very clear about that decision:
>
>    We define a "Flow Specification" NLRI type that may include several
>    components such as destination prefix, source prefix, protocol,
>    ports, etc.  This NLRI is treated as an opaque bit string prefix by
>    BGP.  Each bit string identifies a key to a database entry with which
>    a set of attributes can be associated.
>
>
> It’s also true that rfc5575bis retracts it. Appendix B makes it clear this
> wasn’t an accident:
>
>       Section 1 <https://tools.ietf.org/html/draft-ietf-idr-rfc5575bis-26#section-1> introduces the Flow Specification NLRI.  In [RFC5575 <https://tools.ietf.org/html/rfc5575>]
>       this NLRI was defined as an opaque-key in BGPs database.  This
>       specification has removed all references to an opaque-key
>       property.  BGP implementations are able to understand the NLRI
>       encoding.
>
>
> I haven’t gone back to check, but given that this was made so explicit in
> the draft, I’m going to suppose that it was discussed in the WG before the
> WGLC concluded, so I’m not very excited to re-litigate it.
>
> I do think that it would be good to commence work on FSv2 soon, while we
> still have this experience fresh in our minds.
>
> —John
>
> On Sep 29, 2020, at 2:57 AM, Robert Raszuk <[e-mail]> wrote:
>
>
>
> 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
>> <https://urldefense.com/v3/__http://www.nextlayer.at__;!!NEt6yMaO-gk!X4eMxV72EsYddrA45y5RcVvdurOK06WI3jav90JT_G28wjdKzOfMHr3s65ZeQA$>
>>
>>
>>
>>
>>
>>
>>
>> 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/
>> <https://urldefense.com/v3/__https://mailarchive.ietf.org/arch/msg/idr/FHC-Vz26LZfam-o5Y-9-WSrEm2g/__;!!NEt6yMaO-gk!X4eMxV72EsYddrA45y5RcVvdurOK06WI3jav90JT_G28wjdKzOfMHr29g9EyWw$>
>>
>>
>>
>>
>> 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://urldefense.com/v3/__https://tools.ietf.org/html/rfc7606__;!!NEt6yMaO-gk!X4eMxV72EsYddrA45y5RcVvdurOK06WI3jav90JT_G28wjdKzOfMHr00REP3-Q$>] and [RFC4760 <https://urldefense.com/v3/__https://tools.ietf.org/html/rfc4760__;!!NEt6yMaO-gk!X4eMxV72EsYddrA45y5RcVvdurOK06WI3jav90JT_G28wjdKzOfMHr324tDOCw$>] 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).*
>>
>>
>>
>>
>