All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH nf-next] netfilter: x_tables: reject missing '-p' flag and check for INV flag
@ 2026-08-21 13:41 Florian Westphal
  2026-08-21 14:02 ` Pablo Neira Ayuso
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Westphal @ 2026-08-21 13:41 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal, Zhiling Zou

ip6tables traverser doesn't walk ipv6 extension headers unless the PROTO
flag is set, so direct sockopt-users can bypass the check (ip6tables
always sets it if '-p' flag is given.  This is a cleanup patch, it
doesn't fix any real bug.

Reported-by: Zhiling Zou <zhilinz@nebusec.ai>
Fixes: 74f77a6b2b1c ("netfilter: introduce l2tp match extension")
Fixes: af0d29cd2a73 ("netfilter: xtables: add an IPv6 capable version of the ECN match")
Fixes: a89ecb6a2ef7 ("[NETFILTER]: x_tables: unify IPv4/IPv6 multiport match")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Can also be applied to nf, but I don't think this one is urgent.

 net/netfilter/xt_ecn.c       |  3 ++-
 net/netfilter/xt_l2tp.c      | 11 +++++++----
 net/netfilter/xt_multiport.c |  3 +++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/netfilter/xt_ecn.c b/net/netfilter/xt_ecn.c
index a8503f5d26bf..008e7fcbeb22 100644
--- a/net/netfilter/xt_ecn.c
+++ b/net/netfilter/xt_ecn.c
@@ -139,7 +139,8 @@ static int ecn_mt_check6(const struct xt_mtchk_param *par)
 		return -EINVAL;
 
 	if (info->operation & (XT_ECN_OP_MATCH_ECE | XT_ECN_OP_MATCH_CWR) &&
-	    (ip->proto != IPPROTO_TCP || ip->invflags & IP6T_INV_PROTO)) {
+	    (!(ip->flags & IP6T_F_PROTO) ||
+	     ip->proto != IPPROTO_TCP || ip->invflags & IP6T_INV_PROTO)) {
 		pr_info_ratelimited("cannot match TCP bits for non-tcp packets\n");
 		return -EINVAL;
 	}
diff --git a/net/netfilter/xt_l2tp.c b/net/netfilter/xt_l2tp.c
index a61eb81e9f49..ad2d559caa99 100644
--- a/net/netfilter/xt_l2tp.c
+++ b/net/netfilter/xt_l2tp.c
@@ -267,8 +267,9 @@ static int l2tp_mt_check4(const struct xt_mtchk_param *par)
 	if (ret != 0)
 		return ret;
 
-	if ((ip->proto != IPPROTO_UDP) &&
-	    (ip->proto != IPPROTO_L2TP)) {
+	if (ip->invflags & IPT_INV_PROTO ||
+	    (ip->proto != IPPROTO_UDP &&
+	    ip->proto != IPPROTO_L2TP)) {
 		pr_info_ratelimited("missing protocol rule (udp|l2tpip)\n");
 		return -EINVAL;
 	}
@@ -294,8 +295,10 @@ static int l2tp_mt_check6(const struct xt_mtchk_param *par)
 	if (ret != 0)
 		return ret;
 
-	if ((ip->proto != IPPROTO_UDP) &&
-	    (ip->proto != IPPROTO_L2TP)) {
+	if (((ip->flags & IP6T_F_PROTO) == 0 ||
+	     ip->invflags & IP6T_INV_PROTO ||
+	   (ip->proto != IPPROTO_UDP &&
+	    ip->proto != IPPROTO_L2TP))) {
 		pr_info_ratelimited("missing protocol rule (udp|l2tpip)\n");
 		return -EINVAL;
 	}
diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c
index a1691ff405d3..48aa25749063 100644
--- a/net/netfilter/xt_multiport.c
+++ b/net/netfilter/xt_multiport.c
@@ -160,6 +160,9 @@ static int multiport_mt6_check(const struct xt_mtchk_param *par)
 	const struct ip6t_ip6 *ip = par->entryinfo;
 	const struct xt_multiport_v1 *multiinfo = par->matchinfo;
 
+	if (!(ip->flags & IP6T_F_PROTO))
+		return -EINVAL;
+
 	if (!check(ip->proto, ip->invflags, multiinfo->flags, multiinfo->count))
 		return -EINVAL;
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH nf-next] netfilter: x_tables: reject missing '-p' flag and check for INV flag
  2026-08-21 13:41 [PATCH nf-next] netfilter: x_tables: reject missing '-p' flag and check for INV flag Florian Westphal
@ 2026-08-21 14:02 ` Pablo Neira Ayuso
  2026-08-22  8:14   ` Florian Westphal
  0 siblings, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2026-08-21 14:02 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Zhiling Zou

Hi,

On Fri, Aug 21, 2026 at 03:41:36PM +0200, Florian Westphal wrote:
> ip6tables traverser doesn't walk ipv6 extension headers unless the PROTO
> flag is set, so direct sockopt-users can bypass the check (ip6tables
> always sets it if '-p' flag is given.  This is a cleanup patch, it
> doesn't fix any real bug.

I really think it is better to squash the TPROXY fix into this one
too, it is the same logical fix.

Sashiko also hinting to fix ip6_tables core check for this protocol
flag:

https://sashiko.dev/#/patchset/20260821134136.18191-1-fw%40strlen.de

Thanks!

> Reported-by: Zhiling Zou <zhilinz@nebusec.ai>
> Fixes: 74f77a6b2b1c ("netfilter: introduce l2tp match extension")
> Fixes: af0d29cd2a73 ("netfilter: xtables: add an IPv6 capable version of the ECN match")
> Fixes: a89ecb6a2ef7 ("[NETFILTER]: x_tables: unify IPv4/IPv6 multiport match")
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  Can also be applied to nf, but I don't think this one is urgent.
> 
>  net/netfilter/xt_ecn.c       |  3 ++-
>  net/netfilter/xt_l2tp.c      | 11 +++++++----
>  net/netfilter/xt_multiport.c |  3 +++
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/net/netfilter/xt_ecn.c b/net/netfilter/xt_ecn.c
> index a8503f5d26bf..008e7fcbeb22 100644
> --- a/net/netfilter/xt_ecn.c
> +++ b/net/netfilter/xt_ecn.c
> @@ -139,7 +139,8 @@ static int ecn_mt_check6(const struct xt_mtchk_param *par)
>  		return -EINVAL;
>  
>  	if (info->operation & (XT_ECN_OP_MATCH_ECE | XT_ECN_OP_MATCH_CWR) &&
> -	    (ip->proto != IPPROTO_TCP || ip->invflags & IP6T_INV_PROTO)) {
> +	    (!(ip->flags & IP6T_F_PROTO) ||
> +	     ip->proto != IPPROTO_TCP || ip->invflags & IP6T_INV_PROTO)) {
>  		pr_info_ratelimited("cannot match TCP bits for non-tcp packets\n");
>  		return -EINVAL;
>  	}
> diff --git a/net/netfilter/xt_l2tp.c b/net/netfilter/xt_l2tp.c
> index a61eb81e9f49..ad2d559caa99 100644
> --- a/net/netfilter/xt_l2tp.c
> +++ b/net/netfilter/xt_l2tp.c
> @@ -267,8 +267,9 @@ static int l2tp_mt_check4(const struct xt_mtchk_param *par)
>  	if (ret != 0)
>  		return ret;
>  
> -	if ((ip->proto != IPPROTO_UDP) &&
> -	    (ip->proto != IPPROTO_L2TP)) {
> +	if (ip->invflags & IPT_INV_PROTO ||
> +	    (ip->proto != IPPROTO_UDP &&
> +	    ip->proto != IPPROTO_L2TP)) {
>  		pr_info_ratelimited("missing protocol rule (udp|l2tpip)\n");
>  		return -EINVAL;
>  	}
> @@ -294,8 +295,10 @@ static int l2tp_mt_check6(const struct xt_mtchk_param *par)
>  	if (ret != 0)
>  		return ret;
>  
> -	if ((ip->proto != IPPROTO_UDP) &&
> -	    (ip->proto != IPPROTO_L2TP)) {
> +	if (((ip->flags & IP6T_F_PROTO) == 0 ||
> +	     ip->invflags & IP6T_INV_PROTO ||
> +	   (ip->proto != IPPROTO_UDP &&
> +	    ip->proto != IPPROTO_L2TP))) {
>  		pr_info_ratelimited("missing protocol rule (udp|l2tpip)\n");
>  		return -EINVAL;
>  	}
> diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c
> index a1691ff405d3..48aa25749063 100644
> --- a/net/netfilter/xt_multiport.c
> +++ b/net/netfilter/xt_multiport.c
> @@ -160,6 +160,9 @@ static int multiport_mt6_check(const struct xt_mtchk_param *par)
>  	const struct ip6t_ip6 *ip = par->entryinfo;
>  	const struct xt_multiport_v1 *multiinfo = par->matchinfo;
>  
> +	if (!(ip->flags & IP6T_F_PROTO))
> +		return -EINVAL;
> +
>  	if (!check(ip->proto, ip->invflags, multiinfo->flags, multiinfo->count))
>  		return -EINVAL;
>  
> -- 
> 2.54.0
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH nf-next] netfilter: x_tables: reject missing '-p' flag and check for INV flag
  2026-08-21 14:02 ` Pablo Neira Ayuso
@ 2026-08-22  8:14   ` Florian Westphal
  2026-08-24 21:50     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 8+ messages in thread
From: Florian Westphal @ 2026-08-22  8:14 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, Zhiling Zou

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Fri, Aug 21, 2026 at 03:41:36PM +0200, Florian Westphal wrote:
> > ip6tables traverser doesn't walk ipv6 extension headers unless the PROTO
> > flag is set, so direct sockopt-users can bypass the check (ip6tables
> > always sets it if '-p' flag is given.  This is a cleanup patch, it
> > doesn't fix any real bug.
> 
> I really think it is better to squash the TPROXY fix into this one
> too, it is the same logical fix.

I don't mind either way.  I did not do it because unlike TPROXY these
are benign.

> Sashiko also hinting to fix ip6_tables core check for this protocol
> flag:
> 
> https://sashiko.dev/#/patchset/20260821134136.18191-1-fw%40strlen.de

Not so sure.  Maybe just do this?  I don't think proto > 0 and flag
unset makes any sense.

diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -647,6 +647,13 @@ check_entry_size_and_hooks(struct ip6t_entry *e,
 	/* Clear counters and comefrom */
 	e->counters = ((struct xt_counters) { 0, 0 });
 	e->comefrom = 0;
+
+	/* Make sure F_PROTO is set, else ip6_packet_match
+	 * won't do the right thing.
+	 */
+	if (e->ipv6.proto)
+		e->ipv6.flags |= IP6T_F_PROTO;
+
 	return 0;
 }
 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH nf-next] netfilter: x_tables: reject missing '-p' flag and check for INV flag
  2026-08-22  8:14   ` Florian Westphal
@ 2026-08-24 21:50     ` Pablo Neira Ayuso
  2026-08-24 21:55       ` Florian Westphal
  0 siblings, 1 reply; 8+ messages in thread
From: Pablo Neira Ayuso @ 2026-08-24 21:50 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Zhiling Zou

On Sat, Aug 22, 2026 at 10:14:19AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Fri, Aug 21, 2026 at 03:41:36PM +0200, Florian Westphal wrote:
> > > ip6tables traverser doesn't walk ipv6 extension headers unless the PROTO
> > > flag is set, so direct sockopt-users can bypass the check (ip6tables
> > > always sets it if '-p' flag is given.  This is a cleanup patch, it
> > > doesn't fix any real bug.
> > 
> > I really think it is better to squash the TPROXY fix into this one
> > too, it is the same logical fix.
> 
> I don't mind either way.  I did not do it because unlike TPROXY these
> are benign.
>
> > Sashiko also hinting to fix ip6_tables core check for this protocol
> > flag:
> > 
> > https://sashiko.dev/#/patchset/20260821134136.18191-1-fw%40strlen.de
> 
> Not so sure.  Maybe just do this?  I don't think proto > 0 and flag
> unset makes any sense.
> 
> diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
> --- a/net/ipv6/netfilter/ip6_tables.c
> +++ b/net/ipv6/netfilter/ip6_tables.c
> @@ -647,6 +647,13 @@ check_entry_size_and_hooks(struct ip6t_entry *e,
>  	/* Clear counters and comefrom */
>  	e->counters = ((struct xt_counters) { 0, 0 });
>  	e->comefrom = 0;
> +
> +	/* Make sure F_PROTO is set, else ip6_packet_match
> +	 * won't do the right thing.
> +	 */
> +	if (e->ipv6.proto)
> +		e->ipv6.flags |= IP6T_F_PROTO;

Userspace always provides a zeroed e->ipv6.proto when unused, correct?

If so, LGTM.

> +
>  	return 0;
>  }
>  

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH nf-next] netfilter: x_tables: reject missing '-p' flag and check for INV flag
  2026-08-24 21:50     ` Pablo Neira Ayuso
@ 2026-08-24 21:55       ` Florian Westphal
  2026-08-24 22:18         ` Florian Westphal
  2026-08-24 22:23         ` Pablo Neira Ayuso
  0 siblings, 2 replies; 8+ messages in thread
From: Florian Westphal @ 2026-08-24 21:55 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, Zhiling Zou

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > Sashiko also hinting to fix ip6_tables core check for this protocol
> > > flag:
> > > 
> > > https://sashiko.dev/#/patchset/20260821134136.18191-1-fw%40strlen.de
> > 
> > Not so sure.  Maybe just do this?  I don't think proto > 0 and flag
> > unset makes any sense.
> > 
> > diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
> > --- a/net/ipv6/netfilter/ip6_tables.c
> > +++ b/net/ipv6/netfilter/ip6_tables.c
> > @@ -647,6 +647,13 @@ check_entry_size_and_hooks(struct ip6t_entry *e,
> >  	/* Clear counters and comefrom */
> >  	e->counters = ((struct xt_counters) { 0, 0 });
> >  	e->comefrom = 0;
> > +
> > +	/* Make sure F_PROTO is set, else ip6_packet_match
> > +	 * won't do the right thing.
> > +	 */
> > +	if (e->ipv6.proto)
> > +		e->ipv6.flags |= IP6T_F_PROTO;
> 
> Userspace always provides a zeroed e->ipv6.proto when unused, correct?

Yes, ip6tables raises it when it parses '-p' option.

> If so, LGTM.

Alright, I will respin tomorrow, thanks.

Do you want me to include the TPROXY fix too?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH nf-next] netfilter: x_tables: reject missing '-p' flag and check for INV flag
  2026-08-24 21:55       ` Florian Westphal
@ 2026-08-24 22:18         ` Florian Westphal
  2026-08-24 22:24           ` Pablo Neira Ayuso
  2026-08-24 22:23         ` Pablo Neira Ayuso
  1 sibling, 1 reply; 8+ messages in thread
From: Florian Westphal @ 2026-08-24 22:18 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, Zhiling Zou

Florian Westphal <fw@strlen.de> wrote:
> > Userspace always provides a zeroed e->ipv6.proto when unused, correct?
> 
> Yes, ip6tables raises it when it parses '-p' option.
> 
> > If so, LGTM.
> 
> Alright, I will respin tomorrow, thanks.
> 
> Do you want me to include the TPROXY fix too?

Actually nft_compat.c already does:

 if (proto)
   entry->e6.ipv6.flags |= IP6T_F_PROTO;

so the only other necessary fix is the check for 'INV' in xt_l2tp.c.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH nf-next] netfilter: x_tables: reject missing '-p' flag and check for INV flag
  2026-08-24 21:55       ` Florian Westphal
  2026-08-24 22:18         ` Florian Westphal
@ 2026-08-24 22:23         ` Pablo Neira Ayuso
  1 sibling, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2026-08-24 22:23 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Zhiling Zou

On Mon, Aug 24, 2026 at 11:55:59PM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > Sashiko also hinting to fix ip6_tables core check for this protocol
> > > > flag:
> > > > 
> > > > https://sashiko.dev/#/patchset/20260821134136.18191-1-fw%40strlen.de
> > > 
> > > Not so sure.  Maybe just do this?  I don't think proto > 0 and flag
> > > unset makes any sense.
> > > 
> > > diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
> > > --- a/net/ipv6/netfilter/ip6_tables.c
> > > +++ b/net/ipv6/netfilter/ip6_tables.c
> > > @@ -647,6 +647,13 @@ check_entry_size_and_hooks(struct ip6t_entry *e,
> > >  	/* Clear counters and comefrom */
> > >  	e->counters = ((struct xt_counters) { 0, 0 });
> > >  	e->comefrom = 0;
> > > +
> > > +	/* Make sure F_PROTO is set, else ip6_packet_match
> > > +	 * won't do the right thing.
> > > +	 */
> > > +	if (e->ipv6.proto)
> > > +		e->ipv6.flags |= IP6T_F_PROTO;
> > 
> > Userspace always provides a zeroed e->ipv6.proto when unused, correct?
> 
> Yes, ip6tables raises it when it parses '-p' option.
> 
> > If so, LGTM.
> 
> Alright, I will respin tomorrow, thanks.
> 
> Do you want me to include the TPROXY fix too?

I think that is nf-next material, for correctness. Not urgent.

Thanks!

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH nf-next] netfilter: x_tables: reject missing '-p' flag and check for INV flag
  2026-08-24 22:18         ` Florian Westphal
@ 2026-08-24 22:24           ` Pablo Neira Ayuso
  0 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2026-08-24 22:24 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Zhiling Zou

On Tue, Aug 25, 2026 at 12:18:12AM +0200, Florian Westphal wrote:
> Florian Westphal <fw@strlen.de> wrote:
> > > Userspace always provides a zeroed e->ipv6.proto when unused, correct?
> > 
> > Yes, ip6tables raises it when it parses '-p' option.
> > 
> > > If so, LGTM.
> > 
> > Alright, I will respin tomorrow, thanks.
> > 
> > Do you want me to include the TPROXY fix too?
> 
> Actually nft_compat.c already does:
> 
>  if (proto)
>    entry->e6.ipv6.flags |= IP6T_F_PROTO;

So it is only x_tables legacy that has this issue.

> so the only other necessary fix is the check for 'INV' in xt_l2tp.c.

Agreed.

Thanks!

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-24 22:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21 13:41 [PATCH nf-next] netfilter: x_tables: reject missing '-p' flag and check for INV flag Florian Westphal
2026-08-21 14:02 ` Pablo Neira Ayuso
2026-08-22  8:14   ` Florian Westphal
2026-08-24 21:50     ` Pablo Neira Ayuso
2026-08-24 21:55       ` Florian Westphal
2026-08-24 22:18         ` Florian Westphal
2026-08-24 22:24           ` Pablo Neira Ayuso
2026-08-24 22:23         ` Pablo Neira Ayuso

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.