diff --git a/connection.go b/connection.go index bf261c30a..5555cfd1e 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/path_manager.go b/path_manager.go index aac8103d7..9e2c4a41b 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 } } diff --git a/path_manager_test.go b/path_manager_test.go index a902e93f9..6be3551b2 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) }