Don't rotate PQ keys if data path is idle for ~90s (less than a WG handhshake time

This commit is contained in:
riccardom
2026-07-31 10:27:32 +02:00
parent 46c12f3d01
commit ba2d9abbdc
6 changed files with 79 additions and 11 deletions

View File

@@ -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 {

View File

@@ -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
}

View File

@@ -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.

View File

@@ -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

View File

@@ -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()

View File

@@ -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.