Re: Re: [PATCH nf-next] netfilter: conntrack: tcp: use UNACK timeout for non-closing RST packets

 Hi Florian,


  Thanks, yes, the explicit CLOSE branch was redundant.


  I tested the narrower version which only uses UNACK when the RST does not
  close the conntrack entry:


          else if (unlikely(index == TCP_RST_SET &&
                            new_state == TCP_CONNTRACK_ESTABLISHED))
                  timeout = timeouts[TCP_CONNTRACK_UNACK];


  With this version, non-exact in-window RSTs stay in ESTABLISHED and get the
  UNACK timeout, while exact RSTs still fall through to timeouts[new_state] and
  therefore keep using the CLOSE timeout.


  Using new_state != TCP_CONNTRACK_ESTABLISHED would also catch the exact-RST
  case and make it use UNACK instead of CLOSE, so I used the ESTABLISHED-only
  condition in v2.


  I have sent v2 with this simplification.


  Thanks,
  Minghao Zhang


> -----Original Messages-----
> From: "Florian Westphal" <[email protected]>
> Send time:Wednesday, 29/07/2026 23:09:03
> To: "Minghao Zhang" <[email protected]>
> Cc: [email protected], [email protected], "Jianjun Chen" <[email protected]>
> Subject: Re: [PATCH nf-next] netfilter: conntrack: tcp: use UNACK timeout for non-closing RST packets
> 
> Minghao Zhang <[email protected]> wrote:
> > Commit be0502a3f2e9 ("netfilter: conntrack: tcp: only close if RST
> > matches exact sequence") keeps an established conntrack entry in
> > ESTABLISHED when an in-window RST does not match the expected sequence
> > number exactly, so the endpoint can validate the RST with a challenge
> > ACK.
> > 
> > The timeout selection nevertheless uses the CLOSE timeout for every RST
> > packet.  The bug is that timeout selection is based on the packet type,
> > not on the state transition result: even when RST validation keeps
> > new_state in ESTABLISHED, the timeout is still forced to
> > TCP_CONNTRACK_CLOSE.
> > 
> > Linux TCP independently rate limits challenge ACKs per socket.  A second
> > non-exact RST can therefore arrive after the first challenge ACK has
> > restored the timeout but before the rate limit expires.  The second RST
> > lowers the timeout to 10 seconds again while the endpoint suppresses the
> > second challenge ACK, allowing the conntrack entry to expire while both
> > TCP endpoints remain established.
> > 
> > Using the ESTABLISHED timeout for non-exact RSTs would avoid this short
> > expiration window, but it could also retain stale entries for the
> > five-day default because conntrack cannot reliably match the endpoint's
> > exact TCP state.
> > 
> > Use the UNACK timeout for RST packets that do not move the conntrack
> > entry to TCP_CONNTRACK_CLOSE.  Exact-match RSTs and accepted RST packet
> > trains still use the CLOSE timeout because their state transition result
> > is CLOSE.  This avoids the exploitable 10-second expiration window for
> > non-exact RSTs while preserving the short timeout for RSTs that
> > conntrack accepts as closing the flow.
> 
> Hmm.
> 
> >  		timeout = timeouts[TCP_CONNTRACK_RETRANS];
> > -	else if (unlikely(index == TCP_RST_SET))
> > +	else if (unlikely(index == TCP_RST_SET &&
> > +			  new_state == TCP_CONNTRACK_CLOSE))
> >  		timeout = timeouts[TCP_CONNTRACK_CLOSE];
> 
> Is this needed?  If new state is TCP_CONNTRACK_CLOSE, the existing
> else branch will:
>         else
>                 timeout = timeouts[new_state];
> 
> > +	else if (unlikely(index == TCP_RST_SET))
> > +		timeout = timeouts[TCP_CONNTRACK_UNACK];
> 
> AFAICS this is enough:
> -       else if (unlikely(index == TCP_RST_SET))
> -               timeout = timeouts[TCP_CONNTRACK_CLOSE];
> +       else if (unlikely(index == TCP_RST_SET && new_state != TCP_CONNTRACK_ESTABLISHED))
> +               timeout = timeouts[TCP_CONNTRACK_UNACK];
> 
> Could you test this?  Thanks!