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: <20211019114939.GD28644@breakpoint.cc>
Date:   Tue, 19 Oct 2021 13:49:39 +0200
From:   Florian Westphal <fw@...len.de>
To:     David Ahern <dsahern@...il.com>
Cc:     Florian Westphal <fw@...len.de>,
        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

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?

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.