diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index 74ebff54c..f2b5e75e5 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -3,6 +3,7 @@ package peer import ( "context" "fmt" + "math" "net" "net/netip" "runtime" @@ -26,6 +27,7 @@ import ( "github.com/netbirdio/netbird/client/internal/portforward" "github.com/netbirdio/netbird/client/internal/rosenpass" "github.com/netbirdio/netbird/client/internal/stdnet" + "github.com/netbirdio/netbird/monotime" "github.com/netbirdio/netbird/route" relayClient "github.com/netbirdio/netbird/shared/relay/client" ) @@ -94,8 +96,10 @@ type PQHandshaker interface { // its WG overlay IP with the advertised pq UDP port. SetRemoteAddr(remoteKey string, addr netip.AddrPort) // OnDataPathRekeyed signals a fresh WireGuard handshake for the peer; it clocks the - // next chained PSK rotation pushed over the data path. - OnDataPathRekeyed(remoteKey string) + // next chained PSK rotation pushed over the data path. sinceActivity is how long + // ago the peer last exchanged real user data, so the rotation can be skipped for + // idle tunnels. + OnDataPathRekeyed(remoteKey string, sinceActivity time.Duration) // OnDataPathDown signals the peer's tunnel went down. OnDataPathDown(remoteKey string) } @@ -980,12 +984,27 @@ func (conn *Conn) onWGCheckSuccess() { conn.wgTimeouts = 0 conn.mu.Unlock() - // A fresh WireGuard handshake is the clock for the post-quantum PSK rotation. + // A fresh WireGuard handshake clocks the post-quantum PSK rotation. Pass how long + // ago the peer last exchanged real user data (keepalives excluded) so the pqkem + // manager can skip rotation on idle tunnels — rotating then would push data-path + // traffic that keeps the lazy connection artificially active. if conn.config.PQ != nil { - conn.config.PQ.OnDataPathRekeyed(conn.config.Key) + conn.config.PQ.OnDataPathRekeyed(conn.config.Key, conn.dataActivityAge()) } } +// dataActivityAge returns how long ago the peer last exchanged real user data +// (WireGuard keepalives excluded), per the same LastActivities signal the +// lazy-connection inactivity monitor uses. It reports a very large duration when no +// activity has ever been recorded, so the peer is treated as idle. +func (conn *Conn) dataActivityAge() time.Duration { + last, ok := conn.config.WgConfig.WgInterface.LastActivities()[conn.config.WgConfig.RemoteKey] + if !ok { + return time.Duration(math.MaxInt64) + } + return monotime.Since(last) +} + // recordConnectionMetrics records connection stage timestamps as metrics func (conn *Conn) recordConnectionMetrics() { if conn.metricsRecorder == nil { diff --git a/client/internal/peer/iface.go b/client/internal/peer/iface.go index 678396e61..47780a4d0 100644 --- a/client/internal/peer/iface.go +++ b/client/internal/peer/iface.go @@ -10,6 +10,7 @@ import ( "github.com/netbirdio/netbird/client/iface/configurer" "github.com/netbirdio/netbird/client/iface/wgaddr" "github.com/netbirdio/netbird/client/iface/wgproxy" + "github.com/netbirdio/netbird/monotime" ) type WGIface interface { @@ -19,4 +20,7 @@ type WGIface interface { GetProxy() wgproxy.Proxy Address() wgaddr.Address RemoveEndpointAddress(key string) error + // LastActivities returns the last real-data activity time per peer (WireGuard + // keepalives excluded), used to gate post-quantum PSK rotation on active tunnels. + LastActivities() map[string]monotime.Time } diff --git a/client/internal/pqkem/convergence_test.go b/client/internal/pqkem/convergence_test.go index e67491509..f4ce251c4 100644 --- a/client/internal/pqkem/convergence_test.go +++ b/client/internal/pqkem/convergence_test.go @@ -52,8 +52,8 @@ func TestManager_RekeyToleratesKFailures(t *testing.T) { // Establish: bootstrap + data-path-rekeyed so B becomes established and its data // path is usable. bootstrap(t, dA, dB) - dA.OnDataPathRekeyed("bbbb") - dB.OnDataPathRekeyed("aaaa") + dA.OnDataPathRekeyed("bbbb", 0) + dB.OnDataPathRekeyed("aaaa", 0) require.NotEqual(t, PSK{}, wgB.psk("aaaa")) // Drop B's outbound so rekeys can no longer converge. diff --git a/client/internal/pqkem/manager.go b/client/internal/pqkem/manager.go index 9cf4a189b..bb3aae48d 100644 --- a/client/internal/pqkem/manager.go +++ b/client/internal/pqkem/manager.go @@ -20,6 +20,14 @@ const ( // DefaultMaxRekeyFailures is how many consecutive rekey (non-initial) failures // are tolerated before OnRekeyFailed. The initial exchange fails immediately. DefaultMaxRekeyFailures = 3 + + // rotationActivityWindow gates rotation on recent real-data activity: a rekey + // clocks a rotation only if the peer exchanged user data within this window. It + // must stay shorter than the data path's rekey interval (WireGuard + // REKEY_AFTER_TIME ~120s) so the rotation's own traffic — which itself renews the + // activity signal — ages out before the next rekey, letting an idle tunnel stop + // rotating instead of self-sustaining. + rotationActivityWindow = 90 * time.Second ) // LocalID and RemoteID are peer identity keys (e.g. WireGuard public keys). They are @@ -319,7 +327,17 @@ func (m *Manager) OnDataPathMessage(remoteID RemoteID, raw []byte) error { // initiator that just derived a PSK, it chains the next exchange: a fresh offer over // the data path that acknowledges the just-completed one (its arrival under the new // key proves to the responder that the key works). -func (m *Manager) OnDataPathRekeyed(remoteID RemoteID) { +// OnDataPathRekeyed clocks the next chained PSK rotation on a fresh data-path rekey. +// sinceActivity is how long ago the peer last exchanged real user data; when it +// exceeds rotationActivityWindow the tunnel is treated as idle and rotation is +// skipped — an idle tunnel has nothing to protect, and rotating would emit data-path +// traffic that keeps the peer artificially active (see conn.onWGCheckSuccess). +func (m *Manager) OnDataPathRekeyed(remoteID RemoteID, sinceActivity time.Duration) { + if sinceActivity >= rotationActivityWindow { + m.trace("pqkem: peer idle, skipping data-path rotation", "peer", remoteID, "since_activity", sinceActivity) + return + } + m.mu.Lock() ex := m.exchanges[remoteID] chain := ex != nil && ex.state == stateAwaitingRekey diff --git a/client/internal/pqkem/manager_test.go b/client/internal/pqkem/manager_test.go index bbe8ab161..eab264478 100644 --- a/client/internal/pqkem/manager_test.go +++ b/client/internal/pqkem/manager_test.go @@ -143,8 +143,8 @@ func TestManager_ChainRotatesAndAcks(t *testing.T) { // Data path up: B (initiator) chains the next offer over the data path, which // rotates both to a fresh PSK and acknowledges A. - dA.OnDataPathRekeyed("bbbb") - dB.OnDataPathRekeyed("aaaa") + dA.OnDataPathRekeyed("bbbb", 0) + dB.OnDataPathRekeyed("aaaa", 0) psk2A := wgA.psk("bbbb") psk2B := wgB.psk("aaaa") @@ -152,6 +152,30 @@ func TestManager_ChainRotatesAndAcks(t *testing.T) { require.NotEqual(t, psk1, psk2B, "the chain rotated to a new PSK") } +func TestManager_RotationSkippedWhenIdle(t *testing.T) { + dA, dB, wgA, wgB, _ := pair(t) + defer dA.Stop() + defer dB.Stop() + + bootstrap(t, dA, dB) + psk1 := wgB.psk("aaaa") + require.NotEqual(t, PSK{}, psk1) + + // Idle: the peer's last real-data activity is older than the window, so a rekey + // must NOT clock a rotation. + dA.OnDataPathRekeyed("bbbb", rotationActivityWindow) + dB.OnDataPathRekeyed("aaaa", rotationActivityWindow) + require.Equal(t, psk1, wgB.psk("aaaa"), "idle peer must not rotate the PSK") + require.Equal(t, psk1, wgA.psk("bbbb"), "idle peer must not rotate the PSK") + + // Active: activity within the window clocks the rotation as usual. + dA.OnDataPathRekeyed("bbbb", rotationActivityWindow-1) + dB.OnDataPathRekeyed("aaaa", rotationActivityWindow-1) + psk2 := wgB.psk("aaaa") + require.NotEqual(t, psk1, psk2, "recent activity must clock a rotation") + require.Equal(t, psk2, wgA.psk("bbbb"), "both sides converge on the rotated PSK") +} + func TestManager_NonInitiatorReturnsNoOffer(t *testing.T) { dA := NewManager("aaaa", newFakeWG(), nil) defer dA.Stop() diff --git a/client/internal/pqkem_adapter.go b/client/internal/pqkem_adapter.go index 074039b9e..fe41082ad 100644 --- a/client/internal/pqkem_adapter.go +++ b/client/internal/pqkem_adapter.go @@ -2,6 +2,7 @@ package internal import ( "net/netip" + "time" log "github.com/sirupsen/logrus" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" @@ -92,8 +93,10 @@ func (p pqHandshaker) SetRemoteAddr(remoteKey string, addr netip.AddrPort) { } // OnDataPathRekeyed clocks the next chained PSK rotation on a fresh WG handshake. -func (p pqHandshaker) OnDataPathRekeyed(remoteKey string) { - p.mgr.OnDataPathRekeyed(pqkem.RemoteID(remoteKey)) +// sinceActivity is how long ago the peer last exchanged real user data; the manager +// skips rotation for idle tunnels. +func (p pqHandshaker) OnDataPathRekeyed(remoteKey string, sinceActivity time.Duration) { + p.mgr.OnDataPathRekeyed(pqkem.RemoteID(remoteKey), sinceActivity) } // OnDataPathDown signals the peer's tunnel went down.