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

[John Scudder <[e-mail]>]

Speaking as an individual contributor, I find Bruno’s argument convincing.

Speaking as co-chair, the main thing I want is for the final document to be as unambiguous as it can be about important points like this. I think the conversation so far provides good evidence that knowledgeable and reasonable people can and will disagree about what the document says to do, which is not ideal in a spec.

I suppose one can argue that if one implementation chooses the treat-as-withdraw behavior, and another chooses the session reset behavior, the two are still interoperable and the market can apply corrective pressure as needed. But that’s not a preferred way to do it.

—John

On Sep 29, 2020, at 6:14 AM, [e-mail]<mailto:[e-mail]> wrote:



Hi Jeff, Alvaro, all

>  From: Alvaro Retana [mailto:[e-mail]]
[….]
>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 don’t share the above opinion.
Copy/pasting  below the referenced email on commenting inline

On Wed, Dec 18, 2019 at 08:11:37AM +0100, Christoph Loibl wrote:
> Hi Jeffrey, Alvaro!
>
> I only pick out this single issue from the complete list, because I do not 100% agree with this statement:
>
>
> > On 17.12.2019, at 23:09, Jeffrey Haas <[e-mail]><mailto:&lt;[e-mail]&gt;> 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.
> >
> > There are two options available to us, IMO:
> > 1. Flowspec v2, which we stalled out until 5575bis was done.
> > 2. We decide that for 5575-bis that all future extensions MUST be encoded
> > with a TLV format (length being mandatory).  And perhaps even protect a
> > compliant extension with a capability.  This is a major change, but perhaps
> > provides an upgrade path without waiting on v2.
> >>
> >> 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.
>
> 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.

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.

[Bruno]
OK for the example.
But I don’t think that this specific example support the general claim (that, to be fair, Jeff has not written)

1)      The general claim, that I’m seeing/extrapolating, is that TLV is not a reliable mechanism: it’s not enough to use the length field to skip the unknown type and the unparsable value.
If so, then we should urgently close the LSR WG as we can’t define new reliable (sub)TLVs. We may also consider closing other WGs.
If not, I don’t think that we can make that argument for rfc5575bis.

2)      You are saying that if one don’t see any error but can’t additionally check the validity of the NLRI then we should not trust the NLRI.
If so, we should probably never accept any IPv6 unicast NRLI as we don’t have additional check available to verify that the NLRI is not erroneous: we only rely on the length.

We have seen these bugs before, even in simple RFC 4271 PDUs.

Exactly my point: same thing with  4271 PDU so same conclusion: we couldn’t accept PDU even when the receiver see no error?

3)  Taking one example, and drawing a general conclusion is not a valid analysis of the situation.
If one want to make the analysis, we need to compare at least two scenarios (e.g. go/no go) and analyze all cases taking into account their probability of occurrence and their cost.
Another example: I can probably find a scenario leading to someone committing a fraud in an election. Should we conclude that it’s preferable to avoid election? Or should we accept the result when no fraud is detected?

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.
[Bruno] Yes, the NLRI may be corrupted. But one could state any hypothesis. E.g. the NLRI may be corrupted by Jeff. And I don’t think that we could use any remotely possible hypothesis to conclude that it’s better if Jeff does not approach an NLRI / BGP implementation. (sorry to pick a name. I’m assuming that my example and conclusion is seem as obviously stupid.)

We know that the syntax is correct. The content may be correct or incorrect: we don’t know. Why should we conclude that the content is wrong? (especially against the probabilities)

Thanks for the discussion.
Regards,
Bruno




-- Jeff



From: Alvaro Retana [mailto:[e-mail]]
Sent: Monday, September 28, 2020 9:16 PM
To: DECRAENE Bruno TGI/OLN <[e-mail]<mailto:[e-mail]>>; John Scudder <jgs=[e-mail]<mailto:jgs=[e-mail]>>; Jeffrey Haas <[e-mail]<mailto:[e-mail]>>
Cc: Hares Susan <[e-mail]<mailto:[e-mail]>>; idr@ietf. org <[e-mail]<mailto:[e-mail]>>; [e-mail]<mailto:[e-mail]>
Subject: Re: [Idr] Bug in draft-ietf-idr-rfc5575bis, worth fixing?

[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!X6MVqxYzZKOBij2CXmOCozwSH0sadCC07WqDYxq2kR1EsveHDshZD6-8u-ugOQ$>

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]<mailto: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!X6MVqxYzZKOBij2CXmOCozwSH0sadCC07WqDYxq2kR1EsveHDshZD69d6BidTQ$>] and [RFC4760<https://urldefense.com/v3/__https://tools.ietf.org/html/rfc4760__;!!NEt6yMaO-gk!X6MVqxYzZKOBij2CXmOCozwSH0sadCC07WqDYxq2kR1EsveHDshZD68YbtIlbw$>] 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).

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.