Repository: quic-go/quic-go
Pull request #5034 (2025-04-12 06:51:02)

evict old paths when the client probes a new path
Part of #234.

Old paths are only evicted after a timeout of 5s. This prevents an attack where the attacker duplicates packets and sends them with spoofed source addresses: As long as the client's path probing packet arrives first, we have plenty of time to send the PATH_CHALLENGE and receive the client's PATH_RESPONSE. Since during the path timeout new paths are ignored (once we've reached `maxPaths` tracked paths), the spoofed packets don't cause any new path validations.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/quic-go/quic-go/pull/5034?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) Report
Attention: Patch coverage is `90.90909%` with `3 lines` in your changes missing coverage. Please review.
> Project coverage is 84.05%. Comparing base [(`213b54b`)](https://app.codecov.io/gh/quic-go/quic-go/commit/213b54b010bf2cf2f218f5a27cdb363f79642b48?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) to head [(`4b70169`)](https://app.codecov.io/gh/quic-go/quic-go/commit/4b70169d512d496284d424a56d04e187e5549c25?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).
> Report is 1 commits behind head on master.

| [Files with missing lines](https://app.codecov.io/gh/quic-go/quic-go/pull/5034?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) | Patch % | Lines |
|---|---|---|
| [path\_manager.go](https://app.codecov.io/gh/quic-go/quic-go/pull/5034?src=pr&el=tree&filepath=path_manager.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go#diff-cGF0aF9tYW5hZ2VyLmdv) | 90.62% | [2 Missing and 1 partial :warning: ](https://app.codecov.io/gh/quic-go/quic-go/pull/5034?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #5034      +/-   ##
==========================================
- Coverage   84.17%   84.05%   -0.12%     
==========================================
  Files         153      150       -3     
  Lines       16932    16603     -329     
==========================================
- Hits        14251    13954     -297     
+ Misses       2152     2134      -18     
+ Partials      529      515      -14     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/quic-go/quic-go/pull/5034?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=quic-go).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/connection.go b/connection.go
index bf261c30..5555cfd1 100644
--- a/connection.go
+++ b/connection.go
@@ -1075,7 +1075,7 @@ func (s *connection) handleShortHeaderPacket(p receivedPacket, isCoalesced bool)
 			s.logger,
 		)
 	}
-	destConnID, frames, shouldSwitchPath := s.pathManager.HandlePacket(p.remoteAddr, pathChallenge, isNonProbing)
+	destConnID, frames, shouldSwitchPath := s.pathManager.HandlePacket(p.remoteAddr, p.rcvTime, pathChallenge, isNonProbing)
 	if len(frames) > 0 {
 		probe, buf, err := s.packer.PackPathProbePacket(destConnID, frames, s.version)
 		if err != nil {
diff --git a/interface.go b/interface.go
index f70ae85e..343f0142 100644
--- a/interface.go
+++ b/interface.go
@@ -323,10 +323,10 @@ type Config struct {
 	// If set to 0, then no keep alive is sent. Otherwise, the keep alive is sent on that period (or at most
 	// every half of MaxIdleTimeout, whichever is smaller).
 	KeepAlivePeriod time.Duration
-	// InitialPacketSize is the initial size (and the lower limit) for packets sent.
-	// Under most circumstances, it is not necessary to manually set this value,
-	// since path MTU discovery quickly finds the path's MTU.
-	// If set too high, the path might not support packets of that size, leading to a timeout of the QUIC handshake.
+	// InitialPacketSize is the initial size of packets sent.
+	// It is usually not necessary to manually set this value,
+	// since Path MTU discovery very quickly finds the path's MTU.
+	// If set too high, the path might not support packets that large, leading to a timeout of the QUIC handshake.
 	// Values below 1200 are invalid.
 	InitialPacketSize uint16
 	// DisablePathMTUDiscovery disables Path MTU Discovery (RFC 8899).
diff --git a/path_manager.go b/path_manager.go
index aac8103d..9e2c4a41 100644
--- a/path_manager.go
+++ b/path_manager.go
@@ -3,6 +3,8 @@ package quic
 import (
 	"crypto/rand"
 	"net"
+	"slices"
+	"time"
 
 	"github.com/quic-go/quic-go/internal/ackhandler"
 	"github.com/quic-go/quic-go/internal/protocol"
@@ -14,10 +16,21 @@ type pathID int64
 
 const invalidPathID pathID = -1
 
+// Maximum number of paths to keep track of.
+// If the peer probes another path (before the pathTimeout of an existing path expires),
+// this probing attempt is ignored.
 const maxPaths = 3
 
+// If no packet is received for a path for pathTimeout,
+// the path can be evicted when the peer probes another path.
+// This prevents an attacker from churning through paths by duplicating packets and
+// sending them with spoofed source addresses.
+const pathTimeout = 5 * time.Second
+
 type path struct {
+	id             pathID
 	addr           net.Addr
+	lastPacketTime time.Time
 	pathChallenge  [8]byte
 	validated      bool
 	rcvdNonProbing bool
@@ -25,7 +38,8 @@ type path struct {
 
 type pathManager struct {
 	nextPathID pathID
-	paths      map[pathID]*path
+	// ordered by lastPacketTime, with the most recently used path at the end
+	paths []*path
 
 	getConnID    func(pathID) (_ protocol.ConnectionID, ok bool)
 	retireConnID func(pathID)
@@ -39,7 +53,7 @@ func newPathManager(
 	logger utils.Logger,
 ) *pathManager {
 	return &pathManager{
-		paths:        make(map[pathID]*path),
+		paths:        make([]*path, 0, maxPaths+1),
 		getConnID:    getConnID,
 		retireConnID: retireConnID,
 		logger:       logger,
@@ -50,15 +64,15 @@ func newPathManager(
 // May return nil.
 func (pm *pathManager) HandlePacket(
 	remoteAddr net.Addr,
+	t time.Time,
 	pathChallenge *wire.PathChallengeFrame, // may be nil if the packet didn't contain a PATH_CHALLENGE
 	isNonProbing bool,
 ) (_ protocol.ConnectionID, _ []ackhandler.Frame, shouldSwitch bool) {
 	var p *path
-	pathID := pm.nextPathID
-	for id, path := range pm.paths {
+	for i, path := range pm.paths {
 		if addrsEqual(path.addr, remoteAddr) {
 			p = path
-			pathID = id
+			p.lastPacketTime = t
 			// already sent a PATH_CHALLENGE for this path
 			if isNonProbing {
 				path.rcvdNonProbing = true
@@ -67,6 +81,11 @@ func (pm *pathManager) HandlePacket(
 				pm.logger.Debugf("received packet for path %s that was already probed, validated: %t", remoteAddr, path.validated)
 			}
 			shouldSwitch = path.validated && path.rcvdNonProbing
+			if i != len(pm.paths)-1 {
+				// move the path to the end of the list
+				pm.paths = slices.Delete(pm.paths, i, i+1)
+				pm.paths = append(pm.paths, p)
+			}
 			if pathChallenge == nil {
 				return protocol.ConnectionID{}, nil, shouldSwitch
 			}
@@ -74,10 +93,22 @@ func (pm *pathManager) HandlePacket(
 	}
 
 	if len(pm.paths) >= maxPaths {
-		if pm.logger.Debug() {
-			pm.logger.Debugf("received packet for previously unseen path %s, but already have %d paths", remoteAddr, len(pm.paths))
+		if pm.paths[0].lastPacketTime.Add(pathTimeout).After(t) {
+			if pm.logger.Debug() {
+				pm.logger.Debugf("received packet for previously unseen path %s, but already have %d paths", remoteAddr, len(pm.paths))
+			}
+			return protocol.ConnectionID{}, nil, shouldSwitch
 		}
-		return protocol.ConnectionID{}, nil, shouldSwitch
+		// evict the oldest path, if the last packet was received more than pathTimeout ago
+		pm.retireConnID(pm.paths[0].id)
+		pm.paths = pm.paths[1:]
+	}
+
+	var pathID pathID
+	if p != nil {
+		pathID = p.id
+	} else {
+		pathID = pm.nextPathID
 	}
 
 	// previously unseen path, initiate path validation by sending a PATH_CHALLENGE
@@ -92,16 +123,18 @@ func (pm *pathManager) HandlePacket(
 		var pathChallengeData [8]byte
 		rand.Read(pathChallengeData[:])
 		p = &path{
+			id:             pm.nextPathID,
 			addr:           remoteAddr,
+			lastPacketTime: t,
 			rcvdNonProbing: isNonProbing,
 			pathChallenge:  pathChallengeData,
 		}
+		pm.nextPathID++
+		pm.paths = append(pm.paths, p)
 		frames = append(frames, ackhandler.Frame{
 			Frame:   &wire.PathChallengeFrame{Data: p.pathChallenge},
 			Handler: (*pathManagerAckHandler)(pm),
 		})
-		pm.paths[pm.nextPathID] = p
-		pm.nextPathID++
 		pm.logger.Debugf("enqueueing PATH_CHALLENGE for new path %s", remoteAddr)
 	}
 	if pathChallenge != nil {
@@ -127,14 +160,15 @@ func (pm *pathManager) HandlePathResponseFrame(f *wire.PathResponseFrame) {
 // SwitchToPath is called when the connection switches to a new path
 func (pm *pathManager) SwitchToPath(addr net.Addr) {
 	// retire all other paths
-	for id := range pm.paths {
-		if addrsEqual(pm.paths[id].addr, addr) {
-			pm.logger.Debugf("switching to path %d (%s)", id, addr)
+	for _, path := range pm.paths {
+		if addrsEqual(path.addr, addr) {
+			pm.logger.Debugf("switching to path %d (%s)", path.id, addr)
 			continue
 		}
-		pm.retireConnID(id)
+		pm.retireConnID(path.id)
 	}
 	clear(pm.paths)
+	pm.paths = pm.paths[:0]
 }
 
 type pathManagerAckHandler pathManager
@@ -147,10 +181,10 @@ func (pm *pathManagerAckHandler) OnAcked(f wire.Frame) {}
 func (pm *pathManagerAckHandler) OnLost(f wire.Frame) {
 	// TODO: retransmit the packet the first time it is lost
 	pc := f.(*wire.PathChallengeFrame)
-	for id, path := range pm.paths {
+	for i, path := range pm.paths {
 		if path.pathChallenge == pc.Data {
-			delete(pm.paths, id)
-			pm.retireConnID(id)
+			pm.paths = slices.Delete(pm.paths, i, i+1)
+			pm.retireConnID(path.id)
 			break
 		}
 	}


Test patch:
diff --git a/path_manager_test.go b/path_manager_test.go
index a902e93f..6be3551b 100644
--- a/path_manager_test.go
+++ b/path_manager_test.go
@@ -4,6 +4,7 @@ import (
 	"crypto/rand"
 	"net"
 	"testing"
+	"time"
 
 	"github.com/quic-go/quic-go/internal/ackhandler"
 	"github.com/quic-go/quic-go/internal/protocol"
@@ -28,8 +29,10 @@ func TestPathManagerIntentionalMigration(t *testing.T) {
 		func(id pathID) { retiredConnIDs = append(retiredConnIDs, connIDs[id]) },
 		utils.DefaultLogger,
 	)
+	now := time.Now()
 	connID, frames, shouldSwitch := pm.HandlePacket(
 		&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000},
+		now,
 		&wire.PathChallengeFrame{Data: [8]byte{1, 2, 3, 4, 5, 6, 7, 8}},
 		false,
 	)
@@ -46,6 +49,7 @@ func TestPathManagerIntentionalMigration(t *testing.T) {
 	// receiving another packet for the same path doesn't trigger another PATH_CHALLENGE
 	connID, frames, shouldSwitch = pm.HandlePacket(
 		&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000},
+		now,
 		nil,
 		false,
 	)
@@ -55,7 +59,7 @@ func TestPathManagerIntentionalMigration(t *testing.T) {
 
 	// receiving a packet for a different path triggers another PATH_CHALLENGE
 	addr2 := &net.UDPAddr{IP: net.IPv4(5, 6, 7, 8), Port: 1000}
-	connID, frames, shouldSwitch = pm.HandlePacket(addr2, nil, false)
+	connID, frames, shouldSwitch = pm.HandlePacket(addr2, now, nil, false)
 	require.Equal(t, connIDs[1], connID)
 	require.Len(t, frames, 1)
 	require.IsType(t, &wire.PathChallengeFrame{}, frames[0].Frame)
@@ -67,6 +71,7 @@ func TestPathManagerIntentionalMigration(t *testing.T) {
 	frames[0].Handler.OnAcked(frames[0].Frame)
 	connID, frames, shouldSwitch = pm.HandlePacket(
 		&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000},
+		now,
 		nil,
 		false,
 	)
@@ -76,7 +81,7 @@ func TestPathManagerIntentionalMigration(t *testing.T) {
 
 	// receiving a PATH_RESPONSE for the second path confirms the path
 	pm.HandlePathResponseFrame(&wire.PathResponseFrame{Data: pc2.Data})
-	connID, frames, shouldSwitch = pm.HandlePacket(addr2, nil, false)
+	connID, frames, shouldSwitch = pm.HandlePacket(addr2, now, nil, false)
 	require.Zero(t, connID)
 	require.Empty(t, frames)
 	require.False(t, shouldSwitch) // no non-probing packet received yet
@@ -85,6 +90,7 @@ func TestPathManagerIntentionalMigration(t *testing.T) {
 	// confirming the path doesn't remove other paths
 	connID, frames, shouldSwitch = pm.HandlePacket(
 		&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000},
+		now,
 		nil,
 		false,
 	)
@@ -95,6 +101,7 @@ func TestPathManagerIntentionalMigration(t *testing.T) {
 	// now receive a non-probing packet for the new path
 	connID, frames, shouldSwitch = pm.HandlePacket(
 		&net.UDPAddr{IP: net.IPv4(5, 6, 7, 8), Port: 1000},
+		now,
 		nil,
 		true,
 	)
@@ -106,7 +113,7 @@ func TestPathManagerIntentionalMigration(t *testing.T) {
 	pm.SwitchToPath(&net.UDPAddr{IP: net.IPv4(5, 6, 7, 8), Port: 1000})
 
 	// switching to the path removes other paths
-	connID, frames, shouldSwitch = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000}, nil, false)
+	connID, frames, shouldSwitch = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000}, now, nil, false)
 	require.Equal(t, connIDs[2], connID)
 	require.NotEmpty(t, frames)
 	require.NotEqual(t, frames[0].Frame.(*wire.PathChallengeFrame).Data, pc1.Data)
@@ -123,9 +130,11 @@ func TestPathManagerMultipleProbes(t *testing.T) {
 		func(id pathID) {},
 		utils.DefaultLogger,
 	)
+	now := time.Now()
 	// first receive a packet without a PATH_CHALLENGE
 	connID, frames, shouldSwitch := pm.HandlePacket(
 		&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000},
+		now,
 		nil,
 		false,
 	)
@@ -137,6 +146,7 @@ func TestPathManagerMultipleProbes(t *testing.T) {
 	// now receive a packet on the same path with a PATH_CHALLENGE
 	connID, frames, shouldSwitch = pm.HandlePacket(
 		&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000},
+		now,
 		&wire.PathChallengeFrame{Data: [8]byte{1, 2, 3, 4, 5, 6, 7, 8}},
 		false,
 	)
@@ -148,6 +158,7 @@ func TestPathManagerMultipleProbes(t *testing.T) {
 	// now receive an other packet on the same path with a PATH_RESPONSE
 	connID, frames, shouldSwitch = pm.HandlePacket(
 		&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000},
+		now,
 		&wire.PathChallengeFrame{Data: [8]byte{8, 7, 6, 5, 4, 3, 2, 1}},
 		false,
 	)
@@ -171,7 +182,8 @@ func TestPathManagerNATRebinding(t *testing.T) {
 		utils.DefaultLogger,
 	)
 
-	connID, frames, shouldSwitch := pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000}, nil, true)
+	now := time.Now()
+	connID, frames, shouldSwitch := pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000}, now, nil, true)
 	require.Equal(t, connIDs[0], connID)
 	require.Len(t, frames, 1)
 	require.IsType(t, &wire.PathChallengeFrame{}, frames[0].Frame)
@@ -182,7 +194,7 @@ func TestPathManagerNATRebinding(t *testing.T) {
 	// receiving a PATH_RESPONSE for the second path confirms the path
 	pm.HandlePathResponseFrame(&wire.PathResponseFrame{Data: pc1.Data})
 	// we now switch to the new path, as soon as the next packet on that path is received
-	connID, frames, shouldSwitch = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000}, nil, false)
+	connID, frames, shouldSwitch = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000}, now, nil, false)
 	require.Zero(t, connID)
 	require.Empty(t, frames)
 	require.True(t, shouldSwitch)
@@ -190,7 +202,7 @@ func TestPathManagerNATRebinding(t *testing.T) {
 
 func TestPathManagerLimits(t *testing.T) {
 	var connIDs []protocol.ConnectionID
-	for range 2*maxPaths + 1 {
+	for range 2*maxPaths + 2 {
 		b := make([]byte, 8)
 		rand.Read(b)
 		connIDs = append(connIDs, protocol.ParseConnectionID(b))
@@ -202,29 +214,48 @@ func TestPathManagerLimits(t *testing.T) {
 		utils.DefaultLogger,
 	)
 
+	now := time.Now()
+	firstPathTime := now
+	var firstPathConnID protocol.ConnectionID
+	require.Greater(t, pathTimeout, maxPaths*time.Second)
 	for i := range maxPaths {
-		connID, frames, _ := pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000 + i}, nil, true)
+		connID, frames, _ := pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000 + i}, now, nil, true)
 		require.NotEmpty(t, frames)
 		require.Equal(t, connIDs[i], connID)
+		if i == 0 {
+			firstPathConnID = connID
+		}
+		now = now.Add(time.Second)
 	}
 	// the maximum number of paths is already being probed
-	connID, frames, _ := pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 2000}, nil, true)
+	now = firstPathTime.Add(pathTimeout).Add(-time.Nanosecond)
+	connID, frames, _ := pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 2000}, now, nil, true)
 	require.Zero(t, connID)
 	require.Empty(t, frames)
 
+	// receiving another packet after the pathTimeout of the first path evicts the first path
+	now = firstPathTime.Add(pathTimeout)
+	connIDIndex := maxPaths
+	connID, frames, _ = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000 + maxPaths}, now, nil, true)
+	require.NotEmpty(t, frames)
+	require.Equal(t, connIDs[connIDIndex], connID)
+	require.Equal(t, []protocol.ConnectionID{firstPathConnID}, retiredConnIDs)
+	connIDIndex++
+
 	// switching to a new path frees is up all paths
 	var f1 []ackhandler.Frame
 	pm.SwitchToPath(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 1000})
 	for i := range maxPaths {
-		connID, frames, _ := pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 3000 + i}, nil, true)
+		connID, frames, _ := pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 3000 + i}, now, nil, true)
 		if i == 0 {
 			f1 = frames
 		}
 		require.NotEmpty(t, frames)
-		require.Equal(t, connIDs[maxPaths+i], connID)
+		require.Equal(t, connIDs[connIDIndex], connID)
+		connIDIndex++
 	}
 	// again, the maximum number of paths is already being probed
-	connID, frames, _ = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 2000}, nil, true)
+	connID, frames, _ = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 2000}, now, nil, true)
 	require.Zero(t, connID)
 	require.Empty(t, frames)
 
@@ -232,10 +263,10 @@ func TestPathManagerLimits(t *testing.T) {
 	f1[0].Handler.OnLost(f1[0].Frame)
 
 	// we can open exactly one more path
-	connID, frames, _ = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 4000}, nil, true)
+	connID, frames, _ = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 4000}, now, nil, true)
 	require.NotEmpty(t, frames)
-	require.Equal(t, connIDs[2*maxPaths], connID)
-	connID, frames, _ = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 4001}, nil, true)
+	require.Equal(t, connIDs[connIDIndex], connID)
+	connID, frames, _ = pm.HandlePacket(&net.UDPAddr{IP: net.IPv4(1, 2, 3, 4), Port: 4001}, now, nil, true)
 	require.Zero(t, connID)
 	require.Empty(t, frames)
 }