conntrack: account for window scaling

Conntrack was not reading the window scale TCP option and thus could reject valid packets for being beyond the receive window. Addresses #6734. PiperOrigin-RevId: 411932393
2026-05-22 17:12:49 -07:00 · 2021-11-23 17:43:24 -08:00
parent 654af2af2e
commit 5e984d5aa2
3 changed files with 375 additions and 34 deletions
@@ -160,7 +160,9 @@ go_test(
        "//pkg/tcpip/buffer",
        "//pkg/tcpip/faketime",
        "//pkg/tcpip/header",
+        "//pkg/tcpip/seqnum",
        "//pkg/tcpip/testutil",
+        "//pkg/tcpip/transport/tcpconntrack",
        "@com_github_google_go_cmp//cmp:go_default_library",
        "@com_github_google_go_cmp//cmp/cmpopts:go_default_library",
    ],
@@ -17,9 +17,13 @@ package stack
 import (
 	"testing"

+	"gvisor.dev/gvisor/pkg/tcpip"
+	"gvisor.dev/gvisor/pkg/tcpip/buffer"
 	"gvisor.dev/gvisor/pkg/tcpip/faketime"
 	"gvisor.dev/gvisor/pkg/tcpip/header"
+	"gvisor.dev/gvisor/pkg/tcpip/seqnum"
 	"gvisor.dev/gvisor/pkg/tcpip/testutil"
+	"gvisor.dev/gvisor/pkg/tcpip/transport/tcpconntrack"
 )

 func TestReap(t *testing.T) {
@@ -31,15 +35,16 @@ func TestReap(t *testing.T) {
 	ct.init()
 	ct.checkNumTuples(t, 0)

-	// Simulate sending a SYN. This will get the connection into conntrack, but
-	// the connection won't be considered established. Thus the timeout for
-	// reaping is unestablishedTimeout.
-	pkt1 := genTCPPacket()
-	pkt1.tuple = ct.getConnAndUpdate(pkt1)
 	// We set rt.routeInfo.Loop to avoid a panic when handlePacket calls
 	// rt.RequiresTXTransportChecksum.
 	var rt Route
 	rt.routeInfo.Loop = PacketLoop
+
+	// Simulate sending a SYN. This will get the connection into conntrack, but
+	// the connection won't be considered established. Thus the timeout for
+	// reaping is unestablishedTimeout.
+	pkt1 := genTCPPacket(genTCPOpts{})
+	pkt1.tuple = ct.getConnAndUpdate(pkt1)
 	if pkt1.tuple.conn.handlePacket(pkt1, Output, &rt) {
 		t.Fatal("handlePacket() shouldn't perform any NAT")
 	}
@@ -48,7 +53,7 @@ func TestReap(t *testing.T) {
 	// Travel a little into the future and send the same SYN. This should update
 	// lastUsed, but per #6748 didn't.
 	clock.Advance(unestablishedTimeout / 2)
-	pkt2 := genTCPPacket()
+	pkt2 := genTCPPacket(genTCPOpts{})
 	pkt2.tuple = ct.getConnAndUpdate(pkt2)
 	if pkt2.tuple.conn.handlePacket(pkt2, Output, &rt) {
 		t.Fatal("handlePacket() shouldn't perform any NAT")
@@ -68,31 +73,286 @@ func TestReap(t *testing.T) {
 	ct.checkNumTuples(t, 0)
 }

+func TestWindowScaling(t *testing.T) {
+	tcs := []struct {
+		name        string
+		windowSize  uint16
+		synScale    uint8
+		synAckScale uint8
+		dataLen     int
+		finalSeq    uint32
+	}{
+		{
+			name:       "no scale, full overlap",
+			windowSize: 4,
+			dataLen:    2,
+			finalSeq:   2,
+		},
+		{
+			name:       "no scale, partial overlap",
+			windowSize: 4,
+			dataLen:    8,
+			finalSeq:   4,
+		},
+		{
+			name:        "scale, full overlap",
+			windowSize:  4,
+			synScale:    1,
+			synAckScale: 1,
+			dataLen:     6,
+			finalSeq:    6,
+		},
+		{
+			name:        "scale, partial overlap",
+			windowSize:  4,
+			synScale:    1,
+			synAckScale: 1,
+			dataLen:     10,
+			finalSeq:    8,
+		},
+		{
+			name:        "SYN scale larger",
+			windowSize:  4,
+			synScale:    2,
+			synAckScale: 1,
+			dataLen:     10,
+			finalSeq:    8,
+		},
+		{
+			name:        "SYN/ACK scale larger",
+			windowSize:  4,
+			synScale:    1,
+			synAckScale: 2,
+			dataLen:     10,
+			finalSeq:    10,
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			testWindowScaling(t, tc.windowSize, tc.synScale, tc.synAckScale, tc.dataLen, tc.finalSeq)
+		})
+	}
+}
+
+// testWindowScaling performs a TCP handshake with the given parameters,
+// attaching dataLen bytes as the payload to the final ACK.
+func testWindowScaling(t *testing.T, windowSize uint16, synScale, synAckScale uint8, dataLen int, finalSeq uint32) {
+	// Initialize conntrack.
+	clock := faketime.NewManualClock()
+	ct := ConnTrack{
+		clock: clock,
+	}
+	ct.init()
+	ct.checkNumTuples(t, 0)
+
+	// We set rt.routeInfo.Loop to avoid a panic when handlePacket calls
+	// rt.RequiresTXTransportChecksum.
+	var rt Route
+	rt.routeInfo.Loop = PacketLoop
+
+	var (
+		rwnd           = windowSize
+		seqOrig        = uint32(10)
+		seqRepl        = uint32(20)
+		flags          = header.TCPFlags(header.TCPFlagSyn)
+		originatorAddr = testutil.MustParse4("1.0.0.1")
+		responderAddr  = testutil.MustParse4("1.0.0.2")
+		originatorPort = uint16(5555)
+		responderPort  = uint16(6666)
+	)
+
+	// Send SYN outbound through conntrack, simulating the Output hook.
+	synPkt := genTCPPacket(genTCPOpts{
+		windowSize:  &rwnd,
+		windowScale: synScale,
+		seqNum:      &seqOrig,
+		flags:       &flags,
+		srcAddr:     &originatorAddr,
+		dstAddr:     &responderAddr,
+		srcPort:     &originatorPort,
+		dstPort:     &responderPort,
+	})
+	synPkt.tuple = ct.getConnAndUpdate(synPkt)
+	if synPkt.tuple.conn.handlePacket(synPkt, Output, &rt) {
+		t.Fatal("handlePacket() shouldn't perform any NAT")
+	}
+	ct.checkNumTuples(t, 1)
+
+	// Simulate the Postrouting hook.
+	synPkt.tuple.conn.finalize()
+	conn := synPkt.tuple.conn
+	synPkt.tuple = nil
+	ct.checkNumTuples(t, 2)
+	conn.stateMu.Lock()
+	if got, want := conn.tcb.State(), tcpconntrack.ResultConnecting; got != want {
+		t.Fatalf("connection in state %v, but wanted %v", got, want)
+	}
+	conn.stateMu.Unlock()
+	conn.checkOriginalSeq(t, seqOrig+1)
+
+	// Send SYN/ACK, simulating the Prerouting hook.
+	seqOrig++
+	flags |= header.TCPFlagAck
+	synAckPkt := genTCPPacket(genTCPOpts{
+		windowSize:  &windowSize,
+		windowScale: synAckScale,
+		seqNum:      &seqRepl,
+		ackNum:      &seqOrig,
+		flags:       &flags,
+		srcAddr:     &responderAddr,
+		dstAddr:     &originatorAddr,
+		srcPort:     &responderPort,
+		dstPort:     &originatorPort,
+	})
+	synAckPkt.tuple = ct.getConnAndUpdate(synAckPkt)
+	if synAckPkt.tuple.conn.handlePacket(synAckPkt, Prerouting, &rt) {
+		t.Fatal("handlePacket() shouldn't perform any NAT")
+	}
+	ct.checkNumTuples(t, 2)
+
+	// Simulate the Input hook.
+	synAckPkt.tuple.conn.finalize()
+	synAckPkt.tuple = nil
+	ct.checkNumTuples(t, 2)
+	conn.stateMu.Lock()
+	if got, want := conn.tcb.State(), tcpconntrack.ResultAlive; got != want {
+		t.Fatalf("connection in state %v, but wanted %v", got, want)
+	}
+	conn.stateMu.Unlock()
+	conn.checkReplySeq(t, seqRepl+1)
+
+	// Send ACK with a payload, simulating the Output hook.
+	seqRepl++
+	flags = header.TCPFlagAck
+	ackPkt := genTCPPacket(genTCPOpts{
+		windowSize: &windowSize,
+		seqNum:     &seqOrig,
+		ackNum:     &seqRepl,
+		flags:      &flags,
+		data:       make([]byte, dataLen),
+		srcAddr:    &originatorAddr,
+		dstAddr:    &responderAddr,
+		srcPort:    &originatorPort,
+		dstPort:    &responderPort,
+	})
+	ackPkt.tuple = ct.getConnAndUpdate(ackPkt)
+	if ackPkt.tuple.conn.handlePacket(ackPkt, Output, &rt) {
+		t.Fatal("handlePacket() shouldn't perform any NAT")
+	}
+	ct.checkNumTuples(t, 2)
+
+	// Simulate the Postrouting hook.
+	ackPkt.tuple.conn.finalize()
+	ackPkt.tuple = nil
+	ct.checkNumTuples(t, 2)
+	conn.stateMu.Lock()
+	if got, want := conn.tcb.State(), tcpconntrack.ResultAlive; got != want {
+		t.Fatalf("connection in state %v, but wanted %v", got, want)
+	}
+	conn.stateMu.Unlock()
+	// Depending on the test, all or a fraction of dataLen will go towards
+	// advancing the sequence number.
+	conn.checkOriginalSeq(t, finalSeq+seqOrig)
+
+	// Go into the future to make sure we don't reap active connections quickly.
+	clock.Advance(unestablishedTimeout * 2)
+	ct.reapEverything()
+	ct.checkNumTuples(t, 2)
+
+	// Go way into the future to make sure we eventually reap active connections.
+	clock.Advance(establishedTimeout)
+	ct.reapEverything()
+	ct.checkNumTuples(t, 0)
+}
+
+type genTCPOpts struct {
+	windowSize  *uint16
+	windowScale uint8
+	seqNum      *uint32
+	ackNum      *uint32
+	flags       *header.TCPFlags
+	data        []byte
+	srcAddr     *tcpip.Address
+	dstAddr     *tcpip.Address
+	srcPort     *uint16
+	dstPort     *uint16
+}
+
 // genTCPPacket returns an initialized IPv4 TCP packet.
-func genTCPPacket() *PacketBuffer {
-	const packetLen = header.IPv4MinimumSize + header.TCPMinimumSize
+func genTCPPacket(opts genTCPOpts) *PacketBuffer {
+	// Get values from opts.
+	windowSize := uint16(50000)
+	if opts.windowSize != nil {
+		windowSize = *opts.windowSize
+	}
+	tcpHdrSize := uint8(header.TCPMinimumSize)
+	if opts.windowScale != 0 {
+		tcpHdrSize += 4 // 3 bytes of window scale plus 1 of padding.
+	}
+	seqNum := uint32(7777)
+	if opts.seqNum != nil {
+		seqNum = *opts.seqNum
+	}
+	ackNum := uint32(8888)
+	if opts.ackNum != nil {
+		ackNum = *opts.ackNum
+	}
+	flags := header.TCPFlagSyn
+	if opts.flags != nil {
+		flags = *opts.flags
+	}
+	srcAddr := testutil.MustParse4("1.0.0.1")
+	if opts.srcAddr != nil {
+		srcAddr = *opts.srcAddr
+	}
+	dstAddr := testutil.MustParse4("1.0.0.2")
+	if opts.dstAddr != nil {
+		dstAddr = *opts.dstAddr
+	}
+	srcPort := uint16(5555)
+	if opts.srcPort != nil {
+		srcPort = *opts.srcPort
+	}
+	dstPort := uint16(6666)
+	if opts.dstPort != nil {
+		dstPort = *opts.dstPort
+	}
+
+	// Initialize the PacketBuffer.
+	packetLen := header.IPv4MinimumSize + uint16(tcpHdrSize)
 	pkt := NewPacketBuffer(PacketBufferOptions{
-		ReserveHeaderBytes: packetLen,
+		ReserveHeaderBytes: int(packetLen),
+		Data:               buffer.NewVectorisedView(len(opts.data), []buffer.View{opts.data}),
 	})
 	pkt.NetworkProtocolNumber = header.IPv4ProtocolNumber
 	pkt.TransportProtocolNumber = header.TCPProtocolNumber
-	tcpHdr := header.TCP(pkt.TransportHeader().Push(header.TCPMinimumSize))
-	tcpHdr.Encode(&header.TCPFields{
-		SrcPort:    5555,
-		DstPort:    6666,
-		SeqNum:     7777,
-		AckNum:     8888,
-		DataOffset: header.TCPMinimumSize,
-		Flags:      header.TCPFlagSyn,
-		WindowSize: 50000,
+
+	// Craft the TCP header, including the window scale option if necessary.
+	tcpHdr := header.TCP(pkt.TransportHeader().Push(int(tcpHdrSize)))
+	tcpHdr[:header.TCPMinimumSize].Encode(&header.TCPFields{
+		SrcPort:    srcPort,
+		DstPort:    dstPort,
+		SeqNum:     seqNum,
+		AckNum:     ackNum,
+		DataOffset: tcpHdrSize,
+		Flags:      flags,
+		WindowSize: windowSize,
 		Checksum:   0, // Conntrack doesn't verify the checksum.
 	})
+	if opts.windowScale != 0 {
+		// Set the window scale option, which is 3 bytes long. The option is
+		// properly padded because the final remaining byte is already zeroed.
+		_ = header.EncodeWSOption(int(opts.windowScale), tcpHdr[header.TCPMinimumSize:])
+	}
+
+	// Craft an IPv4 header.
 	ipHdr := header.IPv4(pkt.NetworkHeader().Push(header.IPv4MinimumSize))
 	ipHdr.Encode(&header.IPv4Fields{
 		TotalLength: packetLen,
 		Protocol:    uint8(header.TCPProtocolNumber),
-		SrcAddr:     testutil.MustParse4("1.0.0.1"),
-		DstAddr:     testutil.MustParse4("1.0.0.2"),
+		SrcAddr:     srcAddr,
+		DstAddr:     dstAddr,
 		Checksum:    0, // Conntrack doesn't verify the checksum.
 	})

@@ -130,3 +390,23 @@ func (ct *ConnTrack) reapEverything() {
 		bucket = newBucket
 	}
 }
+
+func (cn *conn) checkOriginalSeq(t *testing.T, seq uint32) {
+	t.Helper()
+	cn.stateMu.Lock()
+	defer cn.stateMu.Unlock()
+
+	if got, want := cn.tcb.OriginalSendSequenceNumber(), seqnum.Value(seq); got != want {
+		t.Fatalf("checkOriginalSeq: got %d, wanted %d", got, want)
+	}
+}
+
+func (cn *conn) checkReplySeq(t *testing.T, seq uint32) {
+	t.Helper()
+	cn.stateMu.Lock()
+	defer cn.stateMu.Unlock()
+
+	if got, want := cn.tcb.ReplySendSequenceNumber(), seqnum.Value(seq); got != want {
+		t.Fatalf("checkReplySeq: got %d, wanted %d", got, want)
+	}
+}
@@ -49,6 +49,10 @@ const (
 	ResultClosedByOriginator
 )

+// maxWindowShift is the maximum shift value of the per the windows scale
+// option defined by RFC 1323.
+const maxWindowShift = 14
+
 // TCB is a TCP Control Block. It holds state necessary to keep track of a TCP
 // connection and inform the caller when the connection has been closed.
 type TCB struct {
@@ -74,8 +78,12 @@ func (t *TCB) Init(initialSyn header.TCP, dataLen int) Result {

 	iss := seqnum.Value(initialSyn.SequenceNumber())
 	t.original.una = iss
-	t.original.nxt = iss.Add(logicalLen(initialSyn, dataLen))
+	t.original.nxt = iss.Add(logicalLenSyn(initialSyn, dataLen))
 	t.original.end = t.original.nxt
+	// TODO(gvisor.dev/issue/6734): Cache TCP options instead of re-parsing them.
+	// Because original and reply are streams, scale applies to the reply; it is
+	// the receive window in the reply direction.
+	t.reply.shiftCnt = header.ParseSynOptions(initialSyn.Options(), false /* isAck */).WS

 	// Even though "end" is a sequence number, we don't know the initial
 	// receive sequence number yet, so we store the window size until we get
@@ -175,13 +183,37 @@ func synSentStateReply(t *TCB, tcp header.TCP, dataLen int) Result {
 		return ResultConnecting
 	}

+	// TODO(gvisor.dev/issue/6734): Cache TCP options instead of re-parsing them.
+	// Because original and reply are streams, scale applies to the reply; it is
+	// the receive window in the original direction.
+	t.original.shiftCnt = header.ParseSynOptions(tcp.Options(), ackPresent).WS
+
+	// Window scaling works only when both ends use the scale option.
+	if t.original.shiftCnt != -1 && t.reply.shiftCnt != -1 {
+		// Per RFC 1323 section 2.3:
+		//
+		//  "If a Window Scale option is received with a shift.cnt value exceeding
+		//  14, the TCP should log the error but use 14 instead of the specified
+		//  value."
+		if t.original.shiftCnt > maxWindowShift {
+			t.original.shiftCnt = maxWindowShift
+		}
+		if t.reply.shiftCnt > maxWindowShift {
+			t.original.shiftCnt = maxWindowShift
+		}
+	} else {
+		t.original.shiftCnt = 0
+		t.reply.shiftCnt = 0
+	}
 	// Update state informed by this SYN.
 	irs := seqnum.Value(tcp.SequenceNumber())
 	t.reply.una = irs
-	t.reply.nxt = irs.Add(logicalLen(tcp, dataLen))
-	t.reply.end += irs
+	t.reply.nxt = irs.Add(logicalLen(tcp, dataLen, seqnum.Size(t.reply.end) /* end currently holds the receive window size */))
+	t.reply.end <<= t.reply.shiftCnt
+	t.reply.end.UpdateForward(seqnum.Size(irs))

-	t.original.end = t.original.una.Add(seqnum.Size(tcp.WindowSize()))
+	windowSize := t.original.windowSize(tcp)
+	t.original.end = t.original.una.Add(windowSize)

 	// If the ACK was set (it is acceptable), update our unacknowledgement
 	// tracking.
@@ -191,7 +223,7 @@ func synSentStateReply(t *TCB, tcp header.TCP, dataLen int) Result {
 			t.original.una = ack
 		}

-		if end := ack.Add(seqnum.Size(tcp.WindowSize())); t.original.end.LessThan(end) {
+		if end := ack.Add(seqnum.Size(windowSize)); t.original.end.LessThan(end) {
 			t.original.end = end
 		}
 	}
@@ -207,8 +239,7 @@ func synSentStateReply(t *TCB, tcp header.TCP, dataLen int) Result {
 // connection is in SYN-SENT state.
 func synSentStateOriginal(t *TCB, tcp header.TCP, _ int) Result {
 	// Drop original segments that aren't retransmits of the original one.
-	if tcp.Flags() != header.TCPFlagSyn ||
-		tcp.SequenceNumber() != uint32(t.original.una) {
+	if tcp.Flags() != header.TCPFlagSyn || tcp.SequenceNumber() != uint32(t.original.una) {
 		return ResultDrop
 	}

@@ -253,12 +284,12 @@ func update(tcp header.TCP, reply, original *stream, firstFin **stream, dataLen
 		original.una = ack
 	}

-	if end := ack.Add(seqnum.Size(tcp.WindowSize())); original.end.LessThan(end) {
+	if end := ack.Add(original.windowSize(tcp)); original.end.LessThan(end) {
 		original.end = end
 	}

 	// Advance the "nxt" index of the reply stream.
-	end := s.Add(logicalLen(tcp, dataLen))
+	end := s.Add(logicalLen(tcp, dataLen, reply.rwndSize()))
 	if reply.nxt.LessThan(end) {
 		reply.nxt = end
 	}
@@ -311,6 +342,11 @@ type stream struct {

 	// rstSeen indicates if a RST has already been sent on this stream.
 	rstSeen bool
+
+	// shiftCnt is the shift of the window scale of the receiver of the stream,
+	// i.e. in a stream from A to B it is B's receive window scale. It cannot be
+	// greater than maxWindowScale.
+	shiftCnt int
 }

 // acceptable determines if the segment with the given sequence number and data
@@ -327,16 +363,39 @@ func (s *stream) closed() bool {
 	return s.finSeen && s.fin.LessThan(s.una)
 }

-// logicalLen calculates the logical length of the TCP segment.
-func logicalLen(tcp header.TCP, dataLen int) seqnum.Size {
+// rwndSize returns the stream's receive window size.
+func (s *stream) rwndSize() seqnum.Size {
+	return s.una.Size(s.end)
+}
+
+// windowSize returns the stream's window size accounting for scale.
+func (s *stream) windowSize(tcp header.TCP) seqnum.Size {
+	return seqnum.Size(tcp.WindowSize()) << s.shiftCnt
+}
+
+// logicalLenSyn calculates the logical length of a SYN (without ACK) segment.
+// It is similar to logicalLen, but does not impose a window size requirement
+// because of the SYN.
+func logicalLenSyn(tcp header.TCP, dataLen int) seqnum.Size {
+	length := seqnum.Size(dataLen)
 	flags := tcp.Flags()
 	if flags&header.TCPFlagSyn != 0 {
-		dataLen++
+		length++
 	}
 	if flags&header.TCPFlagFin != 0 {
-		dataLen++
+		length++
 	}
-	return seqnum.Size(dataLen)
+	return length
+}
+
+// logicalLen calculates the logical length of the TCP segment.
+func logicalLen(tcp header.TCP, dataLen int, windowSize seqnum.Size) seqnum.Size {
+	// If the segment is too large, TCP trims the payload per RFC 793 page 70.
+	length := logicalLenSyn(tcp, dataLen)
+	if length > windowSize {
+		length = windowSize
+	}
+	return length
 }

 // IsEmpty returns true if tcb is not initialized.