netdev - Re: Commit 09e856d54bda5f288ef8437a90ab2b9b3eab83d1r "vrf: Reset skb conntrack connection on VRF rcv" breaks expected netfilter behaviour

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: <c0279807-2f5b-4fe4-d7f5-d545b95860a7@gmail.com>
Date:   Tue, 19 Oct 2021 08:34:01 -0600
From:   David Ahern <dsahern@...il.com>
To:     Florian Westphal <fw@...len.de>
Cc:     Eugene Crosser <crosser@...rage.org>, netdev@...r.kernel.org,
        netfilter-devel@...r.kernel.org,
        Lahav Schlesinger <lschlesinger@...venets.com>,
        David Ahern <dsahern@...nel.org>
Subject: Re: Commit 09e856d54bda5f288ef8437a90ab2b9b3eab83d1r "vrf: Reset skb
 conntrack connection on VRF rcv" breaks expected netfilter behaviour

On 10/19/21 5:49 AM, Florian Westphal wrote:
> David Ahern <dsahern@...il.com> wrote:
>> Thanks for the detailed summary and possible solutions.
>>
>> NAT/MASQ rules with VRF were not really thought about during
>> development; it was not a use case (or use cases) Cumulus or other NOS
>> vendors cared about. Community users were popping up fairly early and
>> patches would get sent, but no real thought about how to handle both
>> sets of rules - VRF device and port devices.
>>
>> What about adding an attribute on the VRF device to declare which side
>> to take -- rules against the port device or rules against the VRF device
>> and control the nf resets based on it?
> 
> This would need a way to suppress the NF_HOOK invocation from the
> normal IP path.  Any idea on how to do that?  AFAICS there is no way to
> get to the vrf device at that point, so no way to detect the toggle.
> 
> Or did you mean to only suppress the 2nd conntrack round?

My thought was that the newly inserted nf_reset_ct fixed one use case
and breaks another, so the new attribute would control that call.

> 
> For packets that get forwarded we'd always need to run those in the vrf
> context, afaics, because doing an nf_reset() may create a new conntrack
> entry (if flow has DNAT, then incoming address has been reversed
> already, so it won't match existing REPLY entry in the conntrack table anymore).
> 
> For locally generated packets, we could skip conntrack for VRF context
> via 'skb->_nfct = UNTRACKED' + nf_reset_ct before xmit to lower device,
> and for lower device by eliding the reset entirely.
> 

ok.