[PATCH nf] netfilter: br_netfilter: disable sabotage_in hook after first suppression

[PATCH nf] netfilter: br_netfilter: disable sabotage_in hook after first suppression

[Florian Westphal]

When using a xfrm interface in a bridged setup (the outgoing device is
bridged), the incoming packets in the xfrm interface are only tracked
in the outgoing direction.

$ brctl show
bridge name     interfaces
br_eth1         eth1

$ conntrack -L
tcp 115 SYN_SENT src=192... dst=192... [UNREPLIED] ...

If br_netfilter is enabled, the first (encrypted) packet is received onR
eth1, conntrack hooks are called from br_netfilter emulation which
allocates nf_bridge info for this skb.

If the packet is for local machine, skb gets passed up the ip stack.
The skb passes through ip prerouting a second time. br_netfilter
ip_sabotage_in supresses the re-invocation of the hooks.

After this, skb gets decrypted in xfrm layer and appears in
network stack a second time (after decyption).

Then, ip_sabotage_in is called again and suppresses netfilter
hook invocation, even though the bridge layer never called them
for the plaintext incarnation of the packet.

Free the bridge info after the first suppression to avoid this.

Reported-and-tested-by: Wolfgang Nothdurft <wolfgang@linogate.de>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/bridge/br_netfilter_hooks.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
index f20f4373ff40..9554abcfd5b4 100644
--- a/net/bridge/br_netfilter_hooks.c
+++ b/net/bridge/br_netfilter_hooks.c
@@ -871,6 +871,7 @@ static unsigned int ip_sabotage_in(void *priv,
 	if (nf_bridge && !nf_bridge->in_prerouting &&
 	    !netif_is_l3_master(skb->dev) &&
 	    !netif_is_l3_slave(skb->dev)) {
+		nf_bridge_info_free(skb);
 		state->okfn(state->net, state->sk, skb);
 		return NF_STOLEN;
 	}
-- 
2.39.1

Re: [PATCH nf] netfilter: br_netfilter: disable sabotage_in hook after first suppression

[Pablo Neira Ayuso]

On Mon, Jan 30, 2023 at 11:39:29AM +0100, Florian Westphal wrote:
> When using a xfrm interface in a bridged setup (the outgoing device is
> bridged), the incoming packets in the xfrm interface are only tracked
> in the outgoing direction.
> 
> $ brctl show
> bridge name     interfaces
> br_eth1         eth1
> 
> $ conntrack -L
> tcp 115 SYN_SENT src=192... dst=192... [UNREPLIED] ...
> 
> If br_netfilter is enabled, the first (encrypted) packet is received onR
> eth1, conntrack hooks are called from br_netfilter emulation which
> allocates nf_bridge info for this skb.
> 
> If the packet is for local machine, skb gets passed up the ip stack.
> The skb passes through ip prerouting a second time. br_netfilter
> ip_sabotage_in supresses the re-invocation of the hooks.
> 
> After this, skb gets decrypted in xfrm layer and appears in
> network stack a second time (after decyption).
> 
> Then, ip_sabotage_in is called again and suppresses netfilter
> hook invocation, even though the bridge layer never called them
> for the plaintext incarnation of the packet.
> 
> Free the bridge info after the first suppression to avoid this.

I'll add this tag (just one sufficiently old):

Fixes: c4b0e771f906 ("netfilter: avoid using skb->nf_bridge directly")

unless you prefer anything else.

Let me know, thanks.

> Reported-and-tested-by: Wolfgang Nothdurft <wolfgang@linogate.de>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  net/bridge/br_netfilter_hooks.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/bridge/br_netfilter_hooks.c b/net/bridge/br_netfilter_hooks.c
> index f20f4373ff40..9554abcfd5b4 100644
> --- a/net/bridge/br_netfilter_hooks.c
> +++ b/net/bridge/br_netfilter_hooks.c
> @@ -871,6 +871,7 @@ static unsigned int ip_sabotage_in(void *priv,
>  	if (nf_bridge && !nf_bridge->in_prerouting &&
>  	    !netif_is_l3_master(skb->dev) &&
>  	    !netif_is_l3_slave(skb->dev)) {
> +		nf_bridge_info_free(skb);
>  		state->okfn(state->net, state->sk, skb);
>  		return NF_STOLEN;
>  	}
> -- 
> 2.39.1
> 

Re: [PATCH nf] netfilter: br_netfilter: disable sabotage_in hook after first suppression

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Jan 30, 2023 at 11:39:29AM +0100, Florian Westphal wrote:
> > When using a xfrm interface in a bridged setup (the outgoing device is
> > bridged), the incoming packets in the xfrm interface are only tracked
> > in the outgoing direction.
> > 
> > $ brctl show
> > bridge name     interfaces
> > br_eth1         eth1
> > 
> > $ conntrack -L
> > tcp 115 SYN_SENT src=192... dst=192... [UNREPLIED] ...
> > 
> > If br_netfilter is enabled, the first (encrypted) packet is received onR
> > eth1, conntrack hooks are called from br_netfilter emulation which
> > allocates nf_bridge info for this skb.
> > 
> > If the packet is for local machine, skb gets passed up the ip stack.
> > The skb passes through ip prerouting a second time. br_netfilter
> > ip_sabotage_in supresses the re-invocation of the hooks.
> > 
> > After this, skb gets decrypted in xfrm layer and appears in
> > network stack a second time (after decyption).
> > 
> > Then, ip_sabotage_in is called again and suppresses netfilter
> > hook invocation, even though the bridge layer never called them
> > for the plaintext incarnation of the packet.
> > 
> > Free the bridge info after the first suppression to avoid this.
> 
> I'll add this tag (just one sufficiently old):
> 
> Fixes: c4b0e771f906 ("netfilter: avoid using skb->nf_bridge directly")
> 
> unless you prefer anything else.

I was unable to figure out where the regression comes from,
as far as i can see br_netfilter always had this problem; i did not
expect that skb is looped again with different headers.

I'm fine with a pseudo-tag.