Re: [PATCH nf] netfilter: br_netfilter: do not skip all hooks with 0 priority

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Jun 21, 2022 at 06:26:03PM +0200, Florian Westphal wrote:
> When br_netfilter module is loaded, skbs may be diverted to the
> ipv4/ipv6 hooks, just like as if we were routing.
> 
> Unfortunately, bridge filter hooks with priority 0 may be skipped
> in this case.
> 
> Example:
> 1. an nftables bridge ruleset is loaded, with a prerouting
>    hook that has priority 0.
> 2. interface is added to the bridge.
> 3. no tcp packet is ever seen by the bridge prerouting hook.
> 4. flush the ruleset
> 5. load the bridge ruleset again.
> 6. tcp packets are processed as expected.
> 
> After 1) the only registered hook is the bridge prerouting hook, but its
> not called yet because the bridge hasn't been brought up yet.
> 
> After 2), hook order is:
>    0 br_nf_pre_routing // br_netfilter internal hook
>    0 chain bridge f prerouting // nftables bridge ruleset
> 
> The packet is diverted to br_nf_pre_routing.
> If call-iptables is off, the nftables bridge ruleset is called as expected.
> 
> But if its enabled, br_nf_hook_thresh() will skip it because it assumes
> that all 0-priority hooks had been called previously in bridge context.
> 
> To avoid this, check for the br_nf_pre_routing hook itself, we need to
> resume directly after it, even if this hook has a priority of 0.
> 
> Unfortunately, this still results in different packet flow.
> With this fix, the eval order after in 3) is:
> 1. br_nf_pre_routing
> 2. ip(6)tables (if enabled)
> 3. nftables bridge
> 
> but after 5 its the much saner:
> 1. nftables bridge
> 2. br_nf_pre_routing
> 3. ip(6)tables (if enabled)
> 
> Unfortunately I don't see a solution here:
> It would be possible to move br_nf_pre_routing to a higher priority
> so that it will be called later in the pipeline, but this also impacts
> ebtables evaluation order, and would still result in this very ordering
> problem for all nftables-bridge hooks with the same priority as the
> br_nf_pre_routing one.
> 
> Searching back through the git history I don't think this has
> ever behaved in any other way, hence, no fixes-tag.

Applied to nf, thanks