netdev - Re: [PATCH v14 nf-next 3/3] netfilter: nft_chain_filter: Add bridge double vlan and pppoe

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: <aHEcYTQ2hK1GWlpG@strlen.de>
Date: Fri, 11 Jul 2025 16:14:57 +0200
From: Florian Westphal <fw@...len.de>
To: Eric Woudstra <ericwouds@...il.com>
Cc: Pablo Neira Ayuso <pablo@...filter.org>,
	Jozsef Kadlecsik <kadlec@...filter.org>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	Ido Schimmel <idosch@...dia.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>, netfilter-devel@...r.kernel.org,
	bridge@...ts.linux.dev, netdev@...r.kernel.org
Subject: Re: [PATCH v14 nf-next 3/3] netfilter: nft_chain_filter: Add bridge
 double vlan and pppoe

Eric Woudstra <ericwouds@...il.com> wrote:

[ skb->protocl munging ]

> But in nft_do_chain_bridge() it is needed in the case of matching 'ip
> saddr', 'ip daddr', 'ip6 saddr' or 'ip6 daddr'. I suspect all ip/ip6
> matches are suffering.

Thats because of implicit dependency insertion on userspace side:
# ip saddr 1.2.3.4 counter ip daddr 3.4.5.6
bridge test-bridge input
  [ meta load protocol => reg 1 ]
  [ cmp eq reg 1 0x00000008 ]
  [ payload load 4b @ network header + 12 => reg 1 ]
  ...

So, if userspace would NOT do that it would 'just work'.

Pablo, whats your take on this?
We currently don't have a 'nhproto' field in nft_pktinfo
and there is no space to add one.

We could say that things work as expected, and that
 ip saddr 1.2.3.4

should not magically match packets in e.g. pppoe encap.
I suspect it will start to work if you force it to match in pppoe, e.g.
ether type 0x8864 ip saddr ...

so nft won't silently add the skb->protocol dependency.

Its not a technical issue but about how matching is supposed to work
in a bridge.

If its supposed to work automatically we need to either:
1. munge skb->protocol in kernel, even tough its wrong (we don't strip
   the l2 headers).
2. record the real l3 protocol somewhere and make it accessible, then
   fix the dependency generation in userspace to use the 'new way' (meta
   l3proto)?
3. change the dependency generation to something else.
   But what? 'ether type ip' won't work either for 8021ad etc.
   'ip version' can't be used for arp.

> I haven't found where yet, but It seems nft is checking skb->protocol,
> before it tries to match the ip(6) saddr/daddr.

Yes, userspace inserts this, see 'nft --debug=netlink add rule bridge ..'