Re: [PATCH 3/3 nf v5] netfilter: xtables: fix L4 header parsing for non-first fragments

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

On Thu, Apr 30, 2026 at 04:53:14PM +0200, Fernando Fernandez Mancera wrote:
> On 4/30/26 8:08 AM, Pablo Neira Ayuso wrote:
> > Hi Fernando,
> > 
> > On Tue, Apr 28, 2026 at 12:25:48PM +0200, Fernando Fernandez Mancera wrote:
> > > Multiple targets and matches relies on L4 header to operate. For
> > > fragmented packets, every fragment carries the transport protocol
> > > identifier, but only the first fragment contains the L4 header.
> > > 
> > > As the 'raw' table can be configured to run at priority -450 (before
> > > defragmentation at -400), the target/match can be reached before
> > > reassembly. In this case, non-first fragments have their payload
> > > incorrectly parsed as a TCP/UDP header. This would be of course a
> > > misconfiguration scenario. In most of the cases this just lead to a
> > > unreliable behavior for fragmented traffic.
> > > 
> > > Add a fragment check to ensure target/match only evaluates unfragmented
> > > packets or the first fragment in the stream.
> > 
> > One more little issue here: There seems to be an issue in
> > xt_hashlimit, hashlimit_init_dst() drops packets via hotdrop if it
> > returns -1.
> 
> Hi Pablo, I do not follow here. I think a hotdrop is the right thing to do.
> 
> xt_hashlimit creates the hash and later checks whether we are over the limit
> or not. The verdict is set based on that and the INVERT flag.. I don't think
> we should match or not match packets that we cannot parse correctly, we
> should just drop them.
> 
> This is the current behavior for example when protoff < 0 (because no L4
> header is found).
> 
> What do you think?

Your reasoning makes sense.

Currently if protoff < 0, the packet is dropped, therefore, fragments
are already being dropped.

Let's stick to this approach, thanks for explaining. I will take this
series as is then.