mbox series

[net-next,0/2] tcp: fix ISN selection in TIMEWAIT -> SYN_RECV

Message ID 20240407093322.3172088-1-edumazet@google.com (mailing list archive)
Headers show
Series tcp: fix ISN selection in TIMEWAIT -> SYN_RECV | expand

Message

Eric Dumazet April 7, 2024, 9:33 a.m. UTC
TCP can transform a TIMEWAIT socket into a SYN_RECV one from
a SYN packet, and the ISN of the SYNACK packet is normally
generated using TIMEWAIT tw_snd_nxt.

This SYN packet also bypasses normal checks against listen queue
being full or not.

Unfortunately this has been broken almost one decade ago.

This series fixes the issue, in two patches.

First patch refactors code to add tcp_tw_isn as a parameter
to ->route_req(), to make the second patch smaller.

Second patch fixes the issue, by no longer using TCP_SKB_CB(skb)
to store the tcp_tw_isn.

Following packetdrill test passes after this series:

// Set up a server listening socket.
    0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
   +0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
   +0 bind(3, ..., ...) = 0
   +0 listen(3, 1) = 0

// Establish connection
   +0 < S 0:0(0) win 32792 <mss 1460,nop,nop,sackOK>
   +0 > S. 0:0(0) ack 1    <mss 1460,nop,nop,sackOK>
 +.01 < . 1:1(0) ack 1 win 32792

   +0 accept(3, ..., ...) = 4

// We close(), send a FIN, and get an ACK and FIN, in order to get into TIME_WAIT.

 +.01 close(4) = 0
   +0 > F. 1:1(0) ack 1
 +.01 < F. 1:1(0) ack 2 win 32792
   +0 > . 2:2(0) ack 2

// SYN hitting a TIME_WAIT -> should use an ISN based on TIMEWAIT tw_snd_nxt

 +.01 < S 1000:1000(0) win 65535 <mss 1460,nop,nop,sackOK>
   +0 > S. 65539:65539(0) ack 1001 <mss 1460,nop,nop,sackOK>


Eric Dumazet (2):
  tcp: propagate tcp_tw_isn via an extra parameter to ->route_req()
  tcp: replace TCP_SKB_CB(skb)->tcp_tw_isn with a per-cpu field

 include/net/tcp.h        | 13 +++++++------
 net/ipv4/tcp.c           |  3 +++
 net/ipv4/tcp_input.c     | 28 +++++++++++++++++-----------
 net/ipv4/tcp_ipv4.c      |  8 +++++---
 net/ipv4/tcp_minisocks.c |  4 ++--
 net/ipv6/tcp_ipv6.c      | 15 +++++++++------
 net/mptcp/subflow.c      | 10 ++++++----
 7 files changed, 49 insertions(+), 32 deletions(-)

Comments

patchwork-bot+netdevbpf@kernel.org April 9, 2024, 11:20 a.m. UTC | #1
Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Sun,  7 Apr 2024 09:33:20 +0000 you wrote:
> TCP can transform a TIMEWAIT socket into a SYN_RECV one from
> a SYN packet, and the ISN of the SYNACK packet is normally
> generated using TIMEWAIT tw_snd_nxt.
> 
> This SYN packet also bypasses normal checks against listen queue
> being full or not.
> 
> [...]

Here is the summary with links:
  - [net-next,1/2] tcp: propagate tcp_tw_isn via an extra parameter to ->route_req()
    https://git.kernel.org/netdev/net-next/c/b9e810405880
  - [net-next,2/2] tcp: replace TCP_SKB_CB(skb)->tcp_tw_isn with a per-cpu field
    https://git.kernel.org/netdev/net-next/c/41eecbd712b7

You are awesome, thank you!