Fix listen backlog handling to be in parity with Linux

- Change the accept queue full condition for a listening endpoint
  to only honor completed (and delivered) connections.
- Use syncookies if the number of incomplete connections is beyond
  listen backlog. This also cleans up the SynThreshold option code
  as that is no longer used with this change.
- Added a new stack option to unconditionally generate syncookies.
  Similar to sysctl -w net.ipv4.tcp_syncookies=2 on Linux.
- Enable keeping of incomplete connections beyond listen backlog.
- Drop incoming SYNs only if the accept queue is filled up.
- Drop incoming ACKs that complete handshakes when accept queue is full
- Enable the stack to accept one more connection than programmed by
  listen backlog.
- Handle backlog argument being zero, negative for listen, as Linux.
- Add syscall and packetimpact tests to reflect the changes above.
- Remove TCPConnectBacklog test which is polling for completed
  connections on the client side which is not reflective of whether
  the accept queue is filled up by the test. The modified syscall test
  in this CL addresses testing of connecting sockets.

Fixes #3153

PiperOrigin-RevId: 366935921

3 files changed, 176 insertions, 0 deletions

diff --git a/test/packetimpact/tests/BUILD b/test/packetimpact/tests/BUILD
index 92103c1e9..83ff70951 100644
--- a/test/packetimpact/tests/BUILD
+++ b/test/packetimpact/tests/BUILD
@@ -385,6 +385,26 @@ packetimpact_testbench(
     ],
 )
 
+packetimpact_testbench(
+    name = "tcp_listen_backlog",
+    srcs = ["tcp_listen_backlog_test.go"],
+    deps = [
+        "//pkg/tcpip/header",
+        "//test/packetimpact/testbench",
+        "@org_golang_x_sys//unix:go_default_library",
+    ],
+)
+
+packetimpact_testbench(
+    name = "tcp_syncookie",
+    srcs = ["tcp_syncookie_test.go"],
+    deps = [
+        "//pkg/tcpip/header",
+        "//test/packetimpact/testbench",
+        "@org_golang_x_sys//unix:go_default_library",
+    ],
+)
+
 validate_all_tests()
 
 [packetimpact_go_test(
diff --git a/test/packetimpact/tests/tcp_listen_backlog_test.go b/test/packetimpact/tests/tcp_listen_backlog_test.go
new file mode 100644
index 000000000..26c812d0a
--- /dev/null
+++ b/test/packetimpact/tests/tcp_listen_backlog_test.go
@@ -0,0 +1,86 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package tcp_listen_backlog_test
+
+import (
+	"flag"
+	"testing"
+	"time"
+
+	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/tcpip/header"
+	"gvisor.dev/gvisor/test/packetimpact/testbench"
+)
+
+func init() {
+	testbench.Initialize(flag.CommandLine)
+}
+
+// TestTCPListenBacklog tests for a listening endpoint behavior:
+// (1) reply to more SYNs than what is configured as listen backlog
+// (2) ignore ACKs (that complete a handshake) when the accept queue is full
+// (3) ignore incoming SYNs when the accept queue is full
+func TestTCPListenBacklog(t *testing.T) {
+	dut := testbench.NewDUT(t)
+
+	// Listening endpoint accepts one more connection than the listen backlog.
+	listenFd, remotePort := dut.CreateListener(t, unix.SOCK_STREAM, unix.IPPROTO_TCP, 0 /*backlog*/)
+
+	var establishedConn testbench.TCPIPv4
+	var incompleteConn testbench.TCPIPv4
+
+	// Test if the DUT listener replies to more SYNs than listen backlog+1
+	for i, conn := range []*testbench.TCPIPv4{&establishedConn, &incompleteConn} {
+		*conn = dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{SrcPort: &remotePort})
+		// Expect dut connection to have transitioned to SYN-RCVD state.
+		conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn)})
+		if _, err := conn.ExpectData(t, &testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn | header.TCPFlagAck)}, nil, time.Second); err != nil {
+			t.Fatalf("expected SYN-ACK for %d connection, %s", i, err)
+		}
+	}
+	defer establishedConn.Close(t)
+	defer incompleteConn.Close(t)
+
+	// Send the ACK to complete handshake.
+	establishedConn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)})
+	dut.PollOne(t, listenFd, unix.POLLIN, time.Second)
+
+	// Send the ACK to complete handshake, expect this to be ignored by the
+	// listener.
+	incompleteConn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)})
+
+	// Drain the accept queue to enable poll for subsequent connections on the
+	// listener.
+	dut.Accept(t, listenFd)
+
+	// The ACK for the incomplete connection should be ignored by the
+	// listening endpoint and the poll on listener should now time out.
+	if pfds := dut.Poll(t, []unix.PollFd{{Fd: listenFd, Events: unix.POLLIN}}, time.Second); len(pfds) != 0 {
+		t.Fatalf("got dut.Poll(...) = %#v", pfds)
+	}
+
+	// Re-send the ACK to complete handshake and re-fill the accept-queue.
+	incompleteConn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)})
+	dut.PollOne(t, listenFd, unix.POLLIN, time.Second)
+
+	// Now initiate a new connection when the accept queue is full.
+	connectingConn := dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{SrcPort: &remotePort})
+	defer connectingConn.Close(t)
+	// Expect dut connection to drop the SYN and let the client stay in SYN_SENT state.
+	connectingConn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn)})
+	if got, err := connectingConn.ExpectData(t, &testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn | header.TCPFlagAck)}, nil, time.Second); err == nil {
+		t.Fatalf("expected no SYN-ACK, but got %s", got)
+	}
+}
diff --git a/test/packetimpact/tests/tcp_syncookie_test.go b/test/packetimpact/tests/tcp_syncookie_test.go
new file mode 100644
index 000000000..1c21c62ff
--- /dev/null
+++ b/test/packetimpact/tests/tcp_syncookie_test.go
@@ -0,0 +1,70 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package tcp_syncookie_test
+
+import (
+	"flag"
+	"testing"
+	"time"
+
+	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/tcpip/header"
+	"gvisor.dev/gvisor/test/packetimpact/testbench"
+)
+
+func init() {
+	testbench.Initialize(flag.CommandLine)
+}
+
+// TestSynCookie test if the DUT listener is replying back using syn cookies.
+// The test does not complete the handshake by not sending the ACK to SYNACK.
+// When syncookies are not used, this forces the listener to retransmit SYNACK.
+// And when syncookies are being used, there is no such retransmit.
+func TestTCPSynCookie(t *testing.T) {
+	dut := testbench.NewDUT(t)
+
+	// Listening endpoint accepts one more connection than the listen backlog.
+	_, remotePort := dut.CreateListener(t, unix.SOCK_STREAM, unix.IPPROTO_TCP, 1 /*backlog*/)
+
+	var withoutSynCookieConn testbench.TCPIPv4
+	var withSynCookieConn testbench.TCPIPv4
+
+	// Test if the DUT listener replies to more SYNs than listen backlog+1
+	for _, conn := range []*testbench.TCPIPv4{&withoutSynCookieConn, &withSynCookieConn} {
+		*conn = dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{SrcPort: &remotePort})
+	}
+	defer withoutSynCookieConn.Close(t)
+	defer withSynCookieConn.Close(t)
+
+	checkSynAck := func(t *testing.T, conn *testbench.TCPIPv4, expectRetransmit bool) {
+		// Expect dut connection to have transitioned to SYN-RCVD state.
+		conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn)})
+		if _, err := conn.ExpectData(t, &testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn | header.TCPFlagAck)}, nil, time.Second); err != nil {
+			t.Fatalf("expected SYN-ACK, but got %s", err)
+		}
+
+		// If the DUT listener is using syn cookies, it will not retransmit SYNACK
+		got, err := conn.ExpectData(t, &testbench.TCP{SeqNum: testbench.Uint32(uint32(*conn.RemoteSeqNum(t) - 1)), Flags: testbench.TCPFlags(header.TCPFlagSyn | header.TCPFlagAck)}, nil, 2*time.Second)
+		if expectRetransmit && err != nil {
+			t.Fatalf("expected retransmitted SYN-ACK, but got %s", err)
+		}
+		if !expectRetransmit && err == nil {
+			t.Fatalf("expected no retransmitted SYN-ACK, but got %s", got)
+		}
+	}
+
+	t.Run("without syncookies", func(t *testing.T) { checkSynAck(t, &withoutSynCookieConn, true /*expectRetransmit*/) })
+	t.Run("with syncookies", func(t *testing.T) { checkSynAck(t, &withSynCookieConn, false /*expectRetransmit*/) })
+}