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

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

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.

> >>> 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.

> 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
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.

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 support both of them eventually? Starting by the one above
> > that allows to match at a given position which looks simpler to
> > support to me. Then, look into adding a MPTCP subtype parser later?
> 
> If the second one can be implemented in a simple way, perhaps the first
> one is not needed? If not, yes, good idea to start with the first one,
> which might be enough for most people.

Both solutions are feasible IMO, the second needs a bit more work,
that's all.