linux-kernel - Re: [RFC PATCH v2 0/3] l3mdev icmp error route lookup fixes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <730d8a09-7d3b-1033-4131-520dc42e8855@efficios.com>
Date:   Wed, 23 Sep 2020 12:04:27 -0400
From:   Michael Jeanson <mjeanson@...icios.com>
To:     David Ahern <dsahern@...il.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc:     David <davem@...emloft.net>, netdev <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v2 0/3] l3mdev icmp error route lookup fixes

On 2020-09-22 21 h 59, David Ahern wrote:
> On 9/22/20 7:52 AM, Michael Jeanson wrote:
>>>>
>>>> the test setup is bad. You have r1 dropping the MTU in VRF red, but not
>>>> telling VRF red how to send back the ICMP. e.g., for IPv4 add:
>>>>
>>>>     ip -netns r1 ro add vrf red 172.16.1.0/24 dev blue
>>>>
>>>> do the same for v6.
>>>>
>>>> Also, I do not see a reason for r2; I suggest dropping it. What you are
>>>> testing is icmp crossing VRF with route leaking, so there should not be
>>>> a need for r2 which leads to asymmetrical routing (172.16.1.0 via r1 and
>>>> the return via r2).
>>
>> The objective of the test was to replicate a clients environment where
>> packets are crossing from a VRF which has a route back to the source to
>> one which doesn't while reaching a ttl of 0. If the route lookup for the
>> icmp error is done on the interface in the first VRF, it can be routed to
>> the source but not on the interface in the second VRF which is the
>> current behaviour for icmp errors generated while crossing between VRFs.
>>
>> There may be a better test case that doesn't involve asymmetric routing
>> to test this but it's the only way I found to replicate this.
>>
> 
> It should work without asymmetric routing; adding the return route to
> the second vrf as I mentioned above fixes the FRAG_NEEDED problem. It
> should work for TTL as well.
> 
> Adding a second pass on the tests with the return through r2 is fine,
> but add a first pass for the more typical case.

Hi,

Before writing new tests I just want to make sure we are trying to fix 
the same issue. If I add a return route to the red VRF then we don't
need this patchset because whether the ICMP error are routed using the
table from the source or destination interface they will reach the 
source host.

The issue for which this patchset was sent only happens when the 
destination interface's VRF doesn't have a route back to the source 
host. I guess we might question if this is actually a bug or not.

So the question really is, when a packet is forwarded between VRFs 
through route leaking and an icmp error is generated, which table should 
be used for the route lookup? And does it depend on the type of icmp 
error? (e.g. TTL=1 happens before forwarding, but fragmentation needed 
happens after when on the destination interface)

Cheers,

Michael