does nft 'tcp option ... exists' work?

does nft 'tcp option ... exists' work?

[Maciej Żenczykowski]

(ran into this while debugging
https://bugzilla.redhat.com/show_bug.cgi?id=2252550 &
https://forum.openwrt.org/t/how-to-block-outbound-ipv4-tcp-fast-open/179518
)

CC'ing Florian directly based on:
https://lore.kernel.org/all/20211119152847.18118-6-fw@strlen.de/

f39vm# tcpdump -i eth0 -nn 'ip and tcp and dst port 853 and
(tcp[tcpflags] & (tcp-syn|tcp-ack|tcp-fin|tcp-rst) == tcp-syn)'
dropped privs to tcpdump
tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
listening on eth0, link-type EN10MB (Ethernet), snapshot length 262144 bytes

03:59:51.539225 IP 192.168.10.2.34324 > 8.8.4.4.853: Flags [S], seq
2804210339:2804210653, win 32120, options [mss 1460,sackOK,TS val
417951723 ecr 0,nop,wscale 7,tfo  cookie d2f9ee39dc952129,nop,nop],
length 314

and yet on the OpenWrt 22.03.5, r20134-5f15225c1e (5.10.176) router I see:

meta nfproto ipv4 oifname "464-xlat" tcp flags syn / fin,syn,rst,ack
counter packets 1 bytes 386 tcp option fastopen exists counter packets
0 bytes 0 drop comment "Drop Outbound IPv4 TCP FastOpen"

so AFAICT it sees the SYN, but not the option.

(and yes, if I run it longer the first counter increments exactly when
tcpdump shows an outbound syn with fastopen, the second counter never
increments)

btw. this doesn't appear to be limited to the fastopen option.
Changing 'fastopen' to 'mss'/'maxseg' or 'sack-perm' or 'nop' also
does not appear to result in it matching and the post match counter
does not increment...

I understand "tcp option fastopen exists" translates to:
inet
  [ exthdr load tcpopt 1b @ 34 + 0 present => reg 1 ]
  [ cmp eq reg 1 0x00000001 ]
(but I don't know how to read that)

Is there perhaps some minimal kernel version dependency for the above?
(but if so... why does the ruleset even load into the kernel)

- Maciej

Re: does nft 'tcp option ... exists' work?

[Maciej Żenczykowski]

FYI, I upgraded the router to OpenWrt 23.05.2 with 5.15.137 and it
doesn't appear to have changed anything, ie. 'tcp option fastopen
exists' still does not appear to match.

Also note that I'm putting this in table inet filter postrouting like
below... but that shouldn't matter should it?

root@mf286a:/usr/share/nftables.d/table-post# cat disable-ipv4-fastopen.nft
chain postrouting {
type filter hook postrouting priority filter; policy accept;
meta nfproto ipv4 oifname "464-xlat" tcp dport 853 tcp flags syn /
fin,syn,rst,ack counter tcp option fastopen exists counter drop
comment "Drop Outbound IPv4 TCP FastOpen"
}

On Sun, Dec 3, 2023 at 4:24 AM Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
>
> (ran into this while debugging
> https://bugzilla.redhat.com/show_bug.cgi?id=2252550 &
> https://forum.openwrt.org/t/how-to-block-outbound-ipv4-tcp-fast-open/179518
> )
>
> CC'ing Florian directly based on:
> https://lore.kernel.org/all/20211119152847.18118-6-fw@strlen.de/
>
> f39vm# tcpdump -i eth0 -nn 'ip and tcp and dst port 853 and
> (tcp[tcpflags] & (tcp-syn|tcp-ack|tcp-fin|tcp-rst) == tcp-syn)'
> dropped privs to tcpdump
> tcpdump: verbose output suppressed, use -v[v]... for full protocol decode
> listening on eth0, link-type EN10MB (Ethernet), snapshot length 262144 bytes
>
> 03:59:51.539225 IP 192.168.10.2.34324 > 8.8.4.4.853: Flags [S], seq
> 2804210339:2804210653, win 32120, options [mss 1460,sackOK,TS val
> 417951723 ecr 0,nop,wscale 7,tfo  cookie d2f9ee39dc952129,nop,nop],
> length 314
>
> and yet on the OpenWrt 22.03.5, r20134-5f15225c1e (5.10.176) router I see:
>
> meta nfproto ipv4 oifname "464-xlat" tcp flags syn / fin,syn,rst,ack
> counter packets 1 bytes 386 tcp option fastopen exists counter packets
> 0 bytes 0 drop comment "Drop Outbound IPv4 TCP FastOpen"
>
> so AFAICT it sees the SYN, but not the option.
>
> (and yes, if I run it longer the first counter increments exactly when
> tcpdump shows an outbound syn with fastopen, the second counter never
> increments)
>
> btw. this doesn't appear to be limited to the fastopen option.
> Changing 'fastopen' to 'mss'/'maxseg' or 'sack-perm' or 'nop' also
> does not appear to result in it matching and the post match counter
> does not increment...
>
> I understand "tcp option fastopen exists" translates to:
> inet
>   [ exthdr load tcpopt 1b @ 34 + 0 present => reg 1 ]
>   [ cmp eq reg 1 0x00000001 ]
> (but I don't know how to read that)
>
> Is there perhaps some minimal kernel version dependency for the above?
> (but if so... why does the ruleset even load into the kernel)
>
> - Maciej

Re: does nft 'tcp option ... exists' work?

[Florian Westphal]

Maciej Żenczykowski <zenczykowski@gmail.com> wrote:
> FYI, I upgraded the router to OpenWrt 23.05.2 with 5.15.137 and it
> doesn't appear to have changed anything, ie. 'tcp option fastopen
> exists' still does not appear to match.
> 
> Also note that I'm putting this in table inet filter postrouting like
> below... but that shouldn't matter should it?

No, this is an endianess bug, on BE the compared byte is always 0.

I suspect this will fix it:

diff --git a/net/netfilter/nft_exthdr.c b/net/netfilter/nft_exthdr.c
--- a/net/netfilter/nft_exthdr.c
+++ b/net/netfilter/nft_exthdr.c
@@ -214,7 +214,7 @@ static void nft_exthdr_tcp_eval(const struct nft_expr *expr,
 
 		offset = i + priv->offset;
 		if (priv->flags & NFT_EXTHDR_F_PRESENT) {
-			*dest = 1;
+			nft_reg_store8(dest, 1);
 		} else {
 			if (priv->len % NFT_REG32_SIZE)
 				dest[priv->len / NFT_REG32_SIZE] = 0;
@@ -461,7 +461,7 @@ static void nft_exthdr_dccp_eval(const struct nft_expr *expr,
 		type = bufp[0];
 
 		if (type == priv->type) {
-			*dest = 1;
+			nft_reg_store8(dest, 1);
 			return;
 		}
 
diff --git a/net/netfilter/nft_fib.c b/net/netfilter/nft_fib.c
--- a/net/netfilter/nft_fib.c
+++ b/net/netfilter/nft_fib.c
@@ -145,11 +145,15 @@ void nft_fib_store_result(void *reg, const struct nft_fib *priv,
 	switch (priv->result) {
 	case NFT_FIB_RESULT_OIF:
 		index = dev ? dev->ifindex : 0;
-		*dreg = (priv->flags & NFTA_FIB_F_PRESENT) ? !!index : index;
+		if (priv->flags & NFTA_FIB_F_PRESENT)
+			nft_reg_store8(dreg, !!index);
+		else
+			*dreg = index;
+
 		break;
 	case NFT_FIB_RESULT_OIFNAME:
 		if (priv->flags & NFTA_FIB_F_PRESENT)
-			*dreg = !!dev;
+			nft_reg_store8(dreg, !!dev);
 		else
 			strscpy_pad(reg, dev ? dev->name : "", IFNAMSIZ);
 		break;

Re: does nft 'tcp option ... exists' work?

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> Maciej Żenczykowski <zenczykowski@gmail.com> wrote:
> > FYI, I upgraded the router to OpenWrt 23.05.2 with 5.15.137 and it
> > doesn't appear to have changed anything, ie. 'tcp option fastopen
> > exists' still does not appear to match.
> > 
> > Also note that I'm putting this in table inet filter postrouting like
> > below... but that shouldn't matter should it?
> 
> No, this is an endianess bug, on BE the compared byte is always 0.

We could fix this from userspace too:

... exists  -> reg32 != 0
... missing -> reg32 == 0

currently nftables uses &boolean_type, so the
compare is for 1 byte.  We could switch this to
32 bit integer type, this way it will no longer
matter if the kernel stores the number at offset 0 or 3.

Phil, Pablo, what do you think?

Re: does nft 'tcp option ... exists' work?

[Pablo Neira Ayuso]

On Mon, Dec 04, 2023 at 10:43:51AM +0100, Florian Westphal wrote:
> Florian Westphal <fw@strlen.de> wrote:
> > Maciej Żenczykowski <zenczykowski@gmail.com> wrote:
> > > FYI, I upgraded the router to OpenWrt 23.05.2 with 5.15.137 and it
> > > doesn't appear to have changed anything, ie. 'tcp option fastopen
> > > exists' still does not appear to match.
> > > 
> > > Also note that I'm putting this in table inet filter postrouting like
> > > below... but that shouldn't matter should it?
> > 
> > No, this is an endianess bug, on BE the compared byte is always 0.
> 
> We could fix this from userspace too:
> 
> ... exists  -> reg32 != 0
> ... missing -> reg32 == 0
> 
> currently nftables uses &boolean_type, so the
> compare is for 1 byte.  We could switch this to
> 32 bit integer type, this way it will no longer
> matter if the kernel stores the number at offset 0 or 3.

This simplifies things.

> Phil, Pablo, what do you think?

Just make sure this does not break backward compatibility. When used
from set declarations with typeof, for example.

Re: does nft 'tcp option ... exists' work?

[Maciej Żenczykowski]

On Mon, Dec 4, 2023 at 11:48 AM Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Dec 04, 2023 at 10:43:51AM +0100, Florian Westphal wrote:
> > Florian Westphal <fw@strlen.de> wrote:
> > > Maciej Żenczykowski <zenczykowski@gmail.com> wrote:
> > > > FYI, I upgraded the router to OpenWrt 23.05.2 with 5.15.137 and it
> > > > doesn't appear to have changed anything, ie. 'tcp option fastopen
> > > > exists' still does not appear to match.
> > > >
> > > > Also note that I'm putting this in table inet filter postrouting like
> > > > below... but that shouldn't matter should it?
> > >
> > > No, this is an endianess bug, on BE the compared byte is always 0.
> >
> > We could fix this from userspace too:
> >
> > ... exists  -> reg32 != 0
> > ... missing -> reg32 == 0
> >
> > currently nftables uses &boolean_type, so the
> > compare is for 1 byte.  We could switch this to
> > 32 bit integer type, this way it will no longer
> > matter if the kernel stores the number at offset 0 or 3.
>
> This simplifies things.
>
> > Phil, Pablo, what do you think?
>
> Just make sure this does not break backward compatibility. When used
> from set declarations with typeof, for example.

I can confirm the box in question is Big Endian:
root@mf286a:~# uname -a; cat /proc/net/tcp | egrep '0100007F|7F000001'
Linux mf286a 5.15.137 #0 Tue Nov 14 13:38:11 2023 mips GNU/Linux
   3: 7F000001:0035 00000000:0000 0A 00000000:00000000 00:00000000
00000000     0        0 3519 1 2ca55a24 100 0 0
^ 7F000001 - big endian
^ 0100007F - little endian

(I'm guessing 'mips' is always BE, vs mipsel being LE or something,
but not actually 100% sure of that)

wrt. the fix, perhaps this should be fixed both in the kernel and in userspace?
it seems wrong to have unpredictable endian-ness dependent kernel logic,
but a userspace fix/workaround would be easier to deploy...

Is there some way I could feed raw nf bytecode in via nft syntax (if
no... should support for this be added?) ?
(ie. I'm not too sure how to rebuild/flash the kernel, changing
userspace nft binary would be easier, but doing something without
rebuilding either, to confirm this will indeed fix it would be even
better...)

Re: does nft 'tcp option ... exists' work?

[Florian Westphal]

Maciej Żenczykowski <zenczykowski@gmail.com> wrote:
> wrt. the fix, perhaps this should be fixed both in the kernel and in userspace?
> it seems wrong to have unpredictable endian-ness dependent kernel logic,
> but a userspace fix/workaround would be easier to deploy...

Right.

> Is there some way I could feed raw nf bytecode in via nft syntax (if
> no... should support for this be added?) ?

You could try this:

tcp option @34,8,8 == 34

(where 34 is the kind/option you are looking for).

Re: does nft 'tcp option ... exists' work?

[Maciej Żenczykowski]

On Mon, Dec 4, 2023 at 5:01 AM Florian Westphal <fw@strlen.de> wrote:
> Maciej Żenczykowski <zenczykowski@gmail.com> wrote:
> > wrt. the fix, perhaps this should be fixed both in the kernel and in userspace?
> > it seems wrong to have unpredictable endian-ness dependent kernel logic,
> > but a userspace fix/workaround would be easier to deploy...
>
> Right.
>
> > Is there some way I could feed raw nf bytecode in via nft syntax (if
> > no... should support for this be added?) ?
>
> You could try this:
>
> tcp option @34,8,8 == 34

So this seems to mean @number(34),offset(8),length(8) == 34
And I understand the idea, but don't understand where the two 8's are
coming from.
Is this counting bits? bytes?
( http://netfilter.org/projects/nftables/manpage.html "RAW PAYLOAD
EXPRESSION" seems to suggest it counts bits)
If it's counting bits, shouldn't it be @34,0,8 == 34
My understanding is that tcp options (except for eol[0] and nop[1])
are (u8 kind >=2, u8 length >=2, length-2 data bytes...),
so kind is at offset 0...

> (where 34 is the kind/option you are looking for).

I tried replacing 'tcp option fastopen exists' with 'tcp option
fastopen length ge 0' and that seems to work.
(I also tried 'tcp option fastopen kind eq 34' but I guess the nft
binary is too new and no longer supports 'kind')

Furthermore, I realized that really mangle postrouting 'reset tcp
option fastopen' is a better solution to my particular problem.
(I think stripping out the tcp fastopen option from the syn disables
fastopen without causing a blackhole and thus an extra RTT roundtrip)

I'll give your suggestion a try too.

Re: does nft 'tcp option ... exists' work?

[Florian Westphal]

Maciej Żenczykowski <zenczykowski@gmail.com> wrote:
> > You could try this:
> >
> > tcp option @34,8,8 == 34
> 
> So this seems to mean @number(34),offset(8),length(8) == 34
> And I understand the idea, but don't understand where the two 8's are
> coming from.

Yes, its wrong, it should be 0,8 as you found out.

> Is this counting bits? bytes?

Bits.

> Furthermore, I realized that really mangle postrouting 'reset tcp
> option fastopen' is a better solution to my particular problem.

Ah, yes, that will nop it out.

Re: does nft 'tcp option ... exists' work?

[Maciej Żenczykowski]

On Mon, Dec 4, 2023 at 5:33 AM Florian Westphal <fw@strlen.de> wrote:
> Maciej Żenczykowski <zenczykowski@gmail.com> wrote:
> > > You could try this:
> > >
> > > tcp option @34,8,8 == 34
> >
> > So this seems to mean @number(34),offset(8),length(8) == 34
> > And I understand the idea, but don't understand where the two 8's are
> > coming from.
>
> Yes, its wrong, it should be 0,8 as you found out.
>
> > Is this counting bits? bytes?
>
> Bits.
>
> > Furthermore, I realized that really mangle postrouting 'reset tcp
> > option fastopen' is a better solution to my particular problem.
>
> Ah, yes, that will nop it out.

So... new problem - turns out there's an experimental and an official
tcp fastopen option.

And it looks like numeric options segfault:

root@mf286a:~# cat
/usr/share/nftables.d/chain-pre/mangle_postrouting/nop-out-tcp-fastopen.nft
#meta nfproto ipv4 oifname "464-xlat" tcp flags syn / fin,syn,rst,ack
tcp option 254      length ge 4 counter drop comment "drop outbound
IPv4 TCP Exp FastOpen";
#meta nfproto ipv6 oifname "wwan0"    tcp flags syn / fin,syn,rst,ack
tcp option 254      length ge 4 counter drop comment "drop outbound
IPv6 TCP Exp FastOpen";
meta nfproto ipv4 oifname "464-xlat" tcp flags syn / fin,syn,rst,ack
tcp option fastopen length ge 2 reset tcp option fastopen counter
comment "NOP out outbound IPv4 TCP FastOpen";
meta nfproto ipv6 oifname "wwan0"    tcp flags syn / fin,syn,rst,ack
tcp option fastopen length ge 2 reset tcp option fastopen counter
comment "NOP out outbound IPv6 TCP FastOpen"

works, but if I uncomment things then 'fw4 check' hits a 'Segmentation
fault' in nft:
[122865.227686] do_page_fault(): sending SIGSEGV to nft for invalid
read access from 0000003d
[122865.236361] epc = 77d0aa0d in libnftables.so.1.1.0[77cf0000+a4000]
[122865.242935] ra  = 77d0c7b5 in libnftables.so.1.1.0[77cf0000+a4000]

root@mf286a:~# opkg search /usr/sbin/nft
nftables-json - 1.0.8-1
root@mf286a:~# opkg search /usr/lib/libnftables.so.1.1.0
nftables-json - 1.0.8-1

the issue is (total guess) related to 254 failing to convert back into
a string, since using '34' works...

(I can make things work if I use 'tcp option @254,0,32 == 0xFE0CF989'
instead, which is better anyway... but still)

[PATCH nft] parser: tcpopt: fix tcp option parsing with NUM + length field

[Florian Westphal]

"tcp option 254 length ge 4" segfaults.

The crash bug is that tcpopt_expr_alloc() can return NULL if we cannot
find a suitable template for the requested kind + field combination,
so add the needed error handling in the bison parser.

However, we can handle this.  NOP and EOL have templates, all other
options (known or unknown) must also have a length field at opt[1].

Add a fallback template to handle both kind and length, even if only a
numeric option is given that nft doesn't recognize.

Don't bother with output, above will be printed via raw syntax, i.e.
tcp option @254,0,8 >= 4.

Fixes: 24d8da308342 ("tcpopt: allow to check for presence of any tcp option")
Reported-by: Maciej Żenczykowski <zenczykowski@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_bison.y                            |  4 ++
 src/tcpopt.c                                  | 19 ++++++++-
 .../packetpath/dumps/tcp_options.nft          | 14 +++++++
 tests/shell/testcases/packetpath/tcp_options  | 39 +++++++++++++++++++
 4 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 tests/shell/testcases/packetpath/dumps/tcp_options.nft
 create mode 100755 tests/shell/testcases/packetpath/tcp_options

diff --git a/src/parser_bison.y b/src/parser_bison.y
index ee7e9e14c1f2..1a3d64f794cb 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -5828,6 +5828,10 @@ tcp_hdr_expr		:	TCP	tcp_hdr_field
 			|	TCP	OPTION	tcp_hdr_option_kind_and_field
 			{
 				$$ = tcpopt_expr_alloc(&@$, $3.kind, $3.field);
+				if ($$ == NULL) {
+					erec_queue(error(&@1, "Could not find a tcp option template"), state->msgs);
+					YYERROR;
+				}
 			}
 			|	TCP	OPTION	AT	close_scope_at	tcp_hdr_option_type	COMMA	NUM	COMMA	NUM
 			{
diff --git a/src/tcpopt.c b/src/tcpopt.c
index 3fcb2731ae73..aee2327bc24a 100644
--- a/src/tcpopt.c
+++ b/src/tcpopt.c
@@ -118,6 +118,13 @@ static const struct exthdr_desc tcpopt_mptcp = {
 		[TCPOPT_MPTCP_SUBTYPE]  = PHT("subtype", 16, 4),
 	},
 };
+
+static const struct exthdr_desc tcpopt_fallback = {
+	.templates	= {
+		[TCPOPT_COMMON_KIND]	= PHT("kind",   0, 8),
+		[TCPOPT_COMMON_LENGTH]	= PHT("length", 8, 8),
+	},
+};
 #undef PHT
 
 const struct exthdr_desc *tcpopt_protocols[] = {
@@ -182,13 +189,21 @@ struct expr *tcpopt_expr_alloc(const struct location *loc,
 		desc = tcpopt_protocols[kind];
 
 	if (!desc) {
-		if (field != TCPOPT_COMMON_KIND || kind > 255)
+		if (kind > 255)
+			return NULL;
+
+		switch (field) {
+		case TCPOPT_COMMON_KIND:
+		case TCPOPT_COMMON_LENGTH:
+			break;
+		default:
 			return NULL;
+		}
 
 		expr = expr_alloc(loc, EXPR_EXTHDR, &integer_type,
 				  BYTEORDER_BIG_ENDIAN, 8);
 
-		desc = tcpopt_protocols[TCPOPT_NOP];
+		desc = &tcpopt_fallback;
 		tmpl = &desc->templates[field];
 		expr->exthdr.desc   = desc;
 		expr->exthdr.tmpl   = tmpl;
diff --git a/tests/shell/testcases/packetpath/dumps/tcp_options.nft b/tests/shell/testcases/packetpath/dumps/tcp_options.nft
new file mode 100644
index 000000000000..b540fe2597f0
--- /dev/null
+++ b/tests/shell/testcases/packetpath/dumps/tcp_options.nft
@@ -0,0 +1,14 @@
+table inet t {
+	chain c {
+		type filter hook output priority filter; policy accept;
+		tcp dport != 22345 accept
+		tcp flags syn / fin,syn,rst,ack tcp option @254,0,8 >= 4 counter packets 0 bytes 0 drop
+		tcp flags syn / fin,syn,rst,ack tcp option fastopen length >= 2 reset tcp option fastopen counter packets 0 bytes 0
+		tcp flags syn / fin,syn,rst,ack tcp option sack-perm missing counter packets 0 bytes 0 drop
+		tcp flags syn / fin,syn,rst,ack tcp option sack-perm exists counter packets 1 bytes 60
+		tcp flags syn / fin,syn,rst,ack tcp option maxseg size > 1400 counter packets 1 bytes 60
+		tcp flags syn / fin,syn,rst,ack tcp option nop missing counter packets 0 bytes 0
+		tcp flags syn / fin,syn,rst,ack tcp option nop exists counter packets 1 bytes 60
+		tcp flags syn / fin,syn,rst,ack drop
+	}
+}
diff --git a/tests/shell/testcases/packetpath/tcp_options b/tests/shell/testcases/packetpath/tcp_options
new file mode 100755
index 000000000000..0f1ca2644655
--- /dev/null
+++ b/tests/shell/testcases/packetpath/tcp_options
@@ -0,0 +1,39 @@
+#!/bin/bash
+
+have_socat="no"
+socat -h > /dev/null && have_socat="yes"
+
+ip link set lo up
+
+$NFT -f /dev/stdin <<EOF
+table inet t {
+	chain c {
+		type filter hook output priority 0;
+		tcp dport != 22345 accept
+		tcp flags syn / fin,syn,rst,ack tcp option 254  length ge 4 counter drop
+		tcp flags syn / fin,syn,rst,ack tcp option fastopen length ge 2 reset tcp option fastopen counter
+		tcp flags syn / fin,syn,rst,ack tcp option sack-perm missing counter drop
+		tcp flags syn / fin,syn,rst,ack tcp option sack-perm exists counter
+		tcp flags syn / fin,syn,rst,ack tcp option maxseg size gt 1400 counter
+		tcp flags syn / fin,syn,rst,ack tcp option nop missing counter
+		tcp flags syn / fin,syn,rst,ack tcp option nop exists counter
+		tcp flags syn / fin,syn,rst,ack drop
+	}
+}
+EOF
+
+if [ $? -ne 0 ]; then
+	exit 1
+fi
+
+if [ $have_socat != "yes" ]; then
+	echo "Ran partial test, socat not available (skipped)"
+	exit 77
+fi
+
+# This will fail (drop in output -> connect fails with eperm)
+socat -t 3 -u STDIN TCP:127.0.0.1:22345,connect-timeout=1 < /dev/null > /dev/null
+
+# Indicate success, dump file has incremented packet counter where its
+# expected to match.
+exit 0
-- 
2.41.0

Re: [PATCH nft] parser: tcpopt: fix tcp option parsing with NUM + length field

[Pablo Neira Ayuso]

On Tue, Dec 05, 2023 at 02:00:01AM +0100, Florian Westphal wrote:
[...]
> @@ -182,13 +189,21 @@ struct expr *tcpopt_expr_alloc(const struct location *loc,
>  		desc = tcpopt_protocols[kind];
>  
>  	if (!desc) {
> -		if (field != TCPOPT_COMMON_KIND || kind > 255)
> +		if (kind > 255)
> +			return NULL;
> +
> +		switch (field) {
> +		case TCPOPT_COMMON_KIND:
> +		case TCPOPT_COMMON_LENGTH:
> +			break;
> +		default:
>  			return NULL;
> +		}
>  
>  		expr = expr_alloc(loc, EXPR_EXTHDR, &integer_type,
>  				  BYTEORDER_BIG_ENDIAN, 8);
>  
> -		desc = tcpopt_protocols[TCPOPT_NOP];
> +		desc = &tcpopt_fallback;
>  		tmpl = &desc->templates[field];
>  		expr->exthdr.desc   = desc;
>  		expr->exthdr.tmpl   = tmpl;

I believe this is missing in this patch:

                expr->exthdr.offset = tmpl->offset;

so it matches at offset 1, not 0:

  [ exthdr load tcpopt 1b @ 255 + 1 => reg 1 ]
  [ cmp eq reg 1 0x00000004 ]