netdev - Re: [PATCH v1 net] tcp: Don't drop SYN+ACK for simultaneous connect().

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <05cf321aa7d33124a17b4c75d92d5f8c67286871.camel@redhat.com>
Date: Thu, 04 Jul 2024 10:14:14 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: Kuniyuki Iwashima <kuniyu@...zon.com>, "David S. Miller"
	 <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, David Ahern
	 <dsahern@...nel.org>, Lawrence Brakmo <brakmo@...com>, Kuniyuki Iwashima
	 <kuni1840@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH v1 net] tcp: Don't drop SYN+ACK for simultaneous
 connect().

On Thu, 2024-07-04 at 10:03 +0200, Eric Dumazet wrote:
> On Thu, Jul 4, 2024 at 10:01 AM Paolo Abeni <pabeni@...hat.com> wrote:
> > 
> > On Wed, 2024-07-03 at 20:57 -0700, Kuniyuki Iwashima wrote:
> > > RFC 9293 states that in the case of simultaneous connect(), the connection
> > > gets established when SYN+ACK is received. [0]
> > > 
> > >       TCP Peer A                                       TCP Peer B
> > > 
> > >   1.  CLOSED                                           CLOSED
> > >   2.  SYN-SENT     --> <SEQ=100><CTL=SYN>              ...
> > >   3.  SYN-RECEIVED <-- <SEQ=300><CTL=SYN>              <-- SYN-SENT
> > >   4.               ... <SEQ=100><CTL=SYN>              --> SYN-RECEIVED
> > >   5.  SYN-RECEIVED --> <SEQ=100><ACK=301><CTL=SYN,ACK> ...
> > >   6.  ESTABLISHED  <-- <SEQ=300><ACK=101><CTL=SYN,ACK> <-- SYN-RECEIVED
> > >   7.               ... <SEQ=100><ACK=301><CTL=SYN,ACK> --> ESTABLISHED
> > > 
> > > However, since commit 0c24604b68fc ("tcp: implement RFC 5961 4.2"), such a
> > > SYN+ACK is dropped in tcp_validate_incoming() and responded with Challenge
> > > ACK.
> > > 
> > > For example, the write() syscall in the following packetdrill script fails
> > > with -EAGAIN, and wrong SNMP stats get incremented.
> > > 
> > >    0 socket(..., SOCK_STREAM|SOCK_NONBLOCK, IPPROTO_TCP) = 3
> > >   +0 connect(3, ..., ...) = -1 EINPROGRESS (Operation now in progress)
> > > 
> > >   +0 > S  0:0(0) <mss 1460,sackOK,TS val 1000 ecr 0,nop,wscale 8>
> > >   +0 < S  0:0(0) win 1000 <mss 1000>
> > >   +0 > S. 0:0(0) ack 1 <mss 1460,sackOK,TS val 3308134035 ecr 0,nop,wscale 8>
> > >   +0 < S. 0:0(0) ack 1 win 1000
> > > 
> > >   +0 write(3, ..., 100) = 100
> > >   +0 > P. 1:101(100) ack 1
> > > 
> > >   --
> > > 
> > >   # packetdrill cross-synack.pkt
> > >   cross-synack.pkt:13: runtime error in write call: Expected result 100 but got -1 with errno 11 (Resource temporarily unavailable)
> > >   # nstat
> > >   ...
> > >   TcpExtTCPChallengeACK           1                  0.0
> > >   TcpExtTCPSYNChallenge           1                  0.0
> > > 
> > > That said, this is no big deal because the Challenge ACK finally let the
> > > connection state transition to TCP_ESTABLISHED in both directions.  If the
> > > peer is not using Linux, there might be a small latency before ACK though.
> > 
> > I'm curious to learn in which scenarios the peer is not running Linux:
> > out of sheer ignorance on my side I thought simult-connect was only
> > possible - or at least made any sense - only on loopback.
> 
> This is the case in the scenario used in the packetdrill test included
> in this changelog,
> but in general simultaneous connect() can be attempted from two different hosts.

I understand that. I also thought such thing belonged to protocol's
edge cases nobody would dare to really use. Why doing that instead of
more usual client-server connection?

Thanks,

Paolo