Re: Netfilter: match "tcp option" with the same type present multiple times

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

Hi Matthieu,

On Wed, Sep 30, 2026 at 08:33:36PM +0200, Matthieu Baerts wrote:
> Hi Pablo,
> 
> Thank you for your reply!
> 
> On 29/09/2026 14:03, Pablo Neira Ayuso wrote:
> > On Tue, Sep 29, 2026 at 12:40:47PM +0200, Matthieu Baerts wrote:
> >> On 29/09/2026 12:00, Pablo Neira Ayuso wrote:
> >>> On Tue, Sep 29, 2026 at 11:21:15AM +0200, Matthieu Baerts wrote:
> >>>> Hi Pablo,
> >>>>
> >>>> Thank you for your reply!
> >>>>
> >>>> On 28/09/2026 23:52, Pablo Neira Ayuso wrote:
> >>>>> Hi Mattieu,
> >>>>>
> >>>>> On Mon, Sep 28, 2026 at 02:13:16PM +0200, Matthieu Baerts wrote:
> >>>>>> Hello Netfilter devs,
> >>>>>>
> >>>>>> First, thank you for maintaining Netfilter in the kernel and the
> >>>>>> userspace tools.
> >>>>>>
> >>>>>> The MPTCP selftests are switching from IPTables to NFTables, and
> >>>>>> Clashiko reported that this part of a rule wouldn't match anything:
> >>>>>>
> >>>>>>   tcp option mptcp subtype remove-addr drop
> >>>>>>
> >>>>>> When an MPTCP REMOVE_ADDR suboption (type 0x30, len >=4, subtype 0x4) is
> >>>>>> added to the TCP options, it is added after an MPTCP DSS option (type
> >>>>>> 0x30, len >= 8, subtype 0x2). In other words, there will be two MPTCP
> >>>>>> (type 30) options in the TCP options. It looks like Netfilter doesn't
> >>>>>> handle that, because it stops processing other TCP options when the
> >>>>>> expected type is found:
> >>>>>>
> >>>>>> net/netfilter/nft_exthdr.c:nft_exthdr_tcp_eval() {
> >>>>>>     ...
> >>>>>> 	for (i = sizeof(*tcph); i < tcphdr_len - 1; i += optl) {
> >>>>>> 		optl = optlen(opt, i);
> >>>>>>
> >>>>>> 		if (priv->type != opt[i])
> >>>>>> 			continue;
> >>>>>> 		...
> >>>>>> 		return;  // <== it will look at the first MPTCP option
> >>>>>> 	}
> >>>>>>     ...
> >>>>>> }
> >>>>>>
> >>>>>> It looks like it shouldn't stop if the wrong subtype is found, but the
> >>>>>> subtype is not compared there if I'm not mistaken. Should there be a fix
> >>>>>> to support this case?
> >>>>>
> >>>>> Would it work for you if you can specify what TCP option you want to
> >>>>> match? ie.
> >>>>>  
> >>>>>     tcp option[2] mptcp subtype remove-addr drop
> >>>>>                ^
> >>>>>                |
> >>>>>                |
> >>>>>        allow to specify a match at a given tcp option
> >>>>
> >>>> In my specific use-case, yes it would work. Some MPTCP sub-options will
> >>>> always be after another one... when the Linux stack is used.
> >>>
> >>> This would require a smaller patch.
> >>
> >> I understand. But I guess that still means adding a new option.
> >>
> >> In my case, it would work, but this might be confusing for people who
> >> want to use this filter: they need to know if a suboption can be used
> >> with another one. I guess they could have multiple rules to check the
> >> presence of a suboption in the first matched type, and in a second one,
> >> but that seems confusing. Or maybe not?
> > 
> > You mean, the might want to validate the entire list of suboptions in
> > a particular order, correct? ie. first suboption A then suboption B.
> 
> Yes, that's one possibility. But initially, I was thinking about having
> two rules (or a set?) to match "the MPTCP subtype is either in the first
> MPTCP option, or the second one (if any)". (Or nft could always say with
> MPTCP by default, check also for a second MPTCP option, if any)
>
> >>>>> Or would this be too strict for your use-case and you would prefer
> >>>>> that you can match it anywhere?
> >>>>
> >>>> I think it would make more sense to match it anywhere. For example, an
> >>>> MP_RESET can be used alone, or after an MP_FASTCLOSE. Plus some stacks
> >>>> could reorder the options as there is no imposed order.
> >>>
> >>> I think this would require a MPTCP subtype parser, ie. make kernel
> >>> aware of MPTCP subtypes.
> >>
> >> In nft_exthdr_tcp_eval(), why can the comparison not be done there
> >> instead of copying the data in a register, and compare later on? If the
> >> comparison is done directly for a given type and is different, the code
> >> could continue and check for the same type. (Or store in multiple
> >> registers.)
> > 
> > In nftables, the fetch then cmp instructions are splitted, ie. you
> > first retrieve then you compare (or make a set lookup with it).
> > Builtin comparison should be possible, but it defeats the integration
> > with the set infrastructure.
> 
> OK, I see.
> 
> If someone gives ...
> 
>   tcp option mptcp subtype { mp-capable, mp-join, remove-addr } drop
> 
> ... a bitmap could be used, but then that's specific to MPTCP I suppose.

I think this makes sense from user perspective.

The nft_exthdr expression needs iterate over the whole list of tcp
options, then if type is 30, make a set lookup to check if the subtype
value is in the set.

Something like:

 - loop
 |  r1 <- exthdr     [ if no more tcp options, break loop ]
 |  r2 <- lookup(r1) [ if found, break loop ]

but exthdr need to be taugh to resume from the last visited tcp
option. A new loop expression would wrap these two exthdr and lookup
expression (similar to nft_inner) and store the iteration context (ie.
last visited tcp option to continue from there).

exthdr tcp option support needs advertise a new NFT_EXPR_LOOP flag, so
it can be used with within this new nft_loop expression.

Another question: Would you still like to validate that DSS comes
before REMOVE_ADDR?

> >> I don't know if you need something specific to MPTCP, maybe just a way
> >> to check all options with the same type.
> > 
> > Going back to this:
> > 
> > "When an MPTCP REMOVE_ADDR suboption (type 0x30, len >=4, subtype 0x4) is
> > added to the TCP options, it is added after an MPTCP DSS option (type
> > 0x30, len >= 8, subtype 0x2)"
> > 
> > Would you like to check for this particular sequence, right? Then
> 
> In my case, no particularly -- I just want to count/drop REMOVE_ADDR to
> catch kernel regressions in the selftests -- but I can.
> 
> > position-based solution would work, because this would allow to match
> > strictly:
> > 
> > pos = 0, suboption DSS
> > pos = 1, suboption REMOVE_ADDR
> > 
> > This ruleset might break the protocol if there is ever a suboption
> > before DSS.
> 
> Indeed. Technically, with MPTCP, we could send the REMOVE_ADDR alone
> (without DSS), or before the DSS, or with another one in between if
> there is room.

Position matching is too strict.

You can still make it with the raw expression, but I think the native
matching representation should be more flexible too as you describe.

> > Loose mode is more protocol designer friendly as it does not constrain
> > future updates in the protocol:
> > 
> > "please find MP-TCP suboption REMOVE_ADDR for me"
> > 
> > but security folks might probably say this is ... loose, because it
> > does not validate DSS before and REMOVE_ADDR might come anywhere.
> > Protocol designer would say that this is good for them because this
> > firewall does not constrain future enhancements of the protocol (still
> > raw expressions are possible, then this last statement does not hold
> > anymore).
> > 
> > Which one would you pick? :-)
> 
> I would first prefer if people don't drop packets with MPTCP options :-D
> 
> Honestly, I'm not sure what people would be interested in doing in
> production, but I guess it would be more: "please match packet with
> MPTCP suboption X, no matter the order".

Yes, as discussed, this is more protocol friendly too in terms of
extensibility.

> If people are interested in a specific packet where MPTCP suboptions
> are in a specific order, with a specific length, I *think* they can
> use a raw payload expression instead, no?
> 
> In my case, I just wanted to drop packets with a specific MPTCP
> suboption. It happens that I know exactly which packet I want to block,
> and I can use a raw payload expression, but a simpler rule would be
> welcome :)

Makes sense.

This would need an extension in the kernel as Florian has anticipated.