From 6c1237da03bf52d51284e15e6c6c2b6776cd7da6 Mon Sep 17 00:00:00 2001
From: Arthur Sfez <asfez@google.com>
Date: Mon, 4 Oct 2021 13:55:19 -0700
Subject: [PATCH] Reply to invalid ACKs even when accept queue is full

Before checking if there is space in the accept queue, the listener
should verify that the cookie is valid. If it is not, instead of
silently dropping the packet, reply with an RST.

Fixes #6683

PiperOrigin-RevId: 400807346
---
 pkg/tcpip/transport/tcp/accept.go             | 36 +++++++++----------
 .../tests/tcp_listen_backlog_test.go          | 15 +++-----
 2 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/pkg/tcpip/transport/tcp/accept.go b/pkg/tcpip/transport/tcp/accept.go
index 95fcdc1b6..caf14b0dc 100644
--- a/pkg/tcpip/transport/tcp/accept.go
+++ b/pkg/tcpip/transport/tcp/accept.go
@@ -583,23 +583,6 @@ func (e *endpoint) handleListenSegment(ctx *listenContext, s *segment) tcpip.Err
 		return nil
 
 	case s.flags.Contains(header.TCPFlagAck):
-		// Keep hold of acceptMu until the new endpoint is in the accept queue (or
-		// if there is an error), to guarantee that we will keep our spot in the
-		// queue even if another handshake from the syn queue completes.
-		e.acceptMu.Lock()
-		if e.acceptQueue.isFull() {
-			// Silently drop the ack as the application can't accept
-			// the connection at this point. The ack will be
-			// retransmitted by the sender anyway and we can
-			// complete the connection at the time of retransmit if
-			// the backlog has space.
-			e.acceptMu.Unlock()
-			e.stack.Stats().TCP.ListenOverflowAckDrop.Increment()
-			e.stats.ReceiveErrors.ListenOverflowAckDrop.Increment()
-			e.stack.Stats().DroppedPackets.Increment()
-			return nil
-		}
-
 		iss := s.ackNumber - 1
 		irs := s.sequenceNumber - 1
 
@@ -615,7 +598,6 @@ func (e *endpoint) handleListenSegment(ctx *listenContext, s *segment) tcpip.Err
 		// Validate the cookie.
 		data, ok := ctx.isCookieValid(s.id, iss, irs)
 		if !ok || int(data) >= len(mssTable) {
-			e.acceptMu.Unlock()
 			e.stack.Stats().TCP.ListenOverflowInvalidSynCookieRcvd.Increment()
 			e.stack.Stats().DroppedPackets.Increment()
 
@@ -636,6 +618,24 @@ func (e *endpoint) handleListenSegment(ctx *listenContext, s *segment) tcpip.Err
 			// ACK was received from the sender.
 			return replyWithReset(e.stack, s, e.sendTOS, e.ttl)
 		}
+
+		// Keep hold of acceptMu until the new endpoint is in the accept queue (or
+		// if there is an error), to guarantee that we will keep our spot in the
+		// queue even if another handshake from the syn queue completes.
+		e.acceptMu.Lock()
+		if e.acceptQueue.isFull() {
+			// Silently drop the ack as the application can't accept
+			// the connection at this point. The ack will be
+			// retransmitted by the sender anyway and we can
+			// complete the connection at the time of retransmit if
+			// the backlog has space.
+			e.acceptMu.Unlock()
+			e.stack.Stats().TCP.ListenOverflowAckDrop.Increment()
+			e.stats.ReceiveErrors.ListenOverflowAckDrop.Increment()
+			e.stack.Stats().DroppedPackets.Increment()
+			return nil
+		}
+
 		e.stack.Stats().TCP.ListenOverflowSynCookieRcvd.Increment()
 		// Create newly accepted endpoint and deliver it.
 		rcvdSynOptions := header.TCPSynOptions{
diff --git a/test/packetimpact/tests/tcp_listen_backlog_test.go b/test/packetimpact/tests/tcp_listen_backlog_test.go
index 4b0f2e31f..e124002f6 100644
--- a/test/packetimpact/tests/tcp_listen_backlog_test.go
+++ b/test/packetimpact/tests/tcp_listen_backlog_test.go
@@ -150,17 +150,10 @@ func TestTCPListenBacklog(t *testing.T) {
 		conn := dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{})
 		defer conn.Close(t)
 		conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)})
-		if !dut.Uname.IsLinux() {
-			// TODO(https://gvisor.dev/issues/6683): Expect a RST.
-			if got, err := conn.Expect(t, testbench.TCP{}, time.Second); err == nil {
-				t.Errorf("expected no TCP frame, got %s", got)
-			}
-		} else {
-			if got, err := conn.Expect(t, testbench.TCP{}, time.Second); err != nil {
-				t.Errorf("expected TCP frame: %s", err)
-			} else if got, want := *got.Flags, header.TCPFlagRst; got != want {
-				t.Errorf("got %s, want %s", got, want)
-			}
+		if got, err := conn.Expect(t, testbench.TCP{}, time.Second); err != nil {
+			t.Errorf("expected TCP frame: %s", err)
+		} else if got, want := *got.Flags, header.TCPFlagRst; got != want {
+			t.Errorf("got %s, want %s", got, want)
 		}
 	}()