diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index c22b34bca..d688f1096 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -760,14 +760,20 @@ func (conn *Conn) onGuardEvent() { // RequestReoffer sends a fresh signalling offer for the peer, re-running the // post-quantum bootstrap over Signal. Used to recover from a persistent data-path // rekey failure: a new exchange overwrites the stalled PSK on both sides. No-op if the -// connection is not open yet. +// connection is not open yet, or if the peer is not connected: then the reconnect guard +// already retries it with backoff, and each of its offers starts a fresh exchange. func (conn *Conn) RequestReoffer() { conn.mu.Lock() h := conn.handshaker + connected := conn.evalStatus() == StatusConnected conn.mu.Unlock() if h == nil { return } + if !connected { + conn.Log.Debugf("pqkem: peer not connected, leaving the KEM re-bootstrap to the reconnect guard") + return + } if err := h.SendOffer(); err != nil { conn.Log.Debugf("pqkem: recovery re-offer failed: %v", err) } diff --git a/client/internal/peer/conn_pq_test.go b/client/internal/peer/conn_pq_test.go index f62d2d6e6..a010ceabb 100644 --- a/client/internal/peer/conn_pq_test.go +++ b/client/internal/peer/conn_pq_test.go @@ -2,13 +2,18 @@ package peer import ( "net/netip" + "strings" "testing" "time" log "github.com/sirupsen/logrus" + logtest "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" + + "github.com/netbirdio/netbird/client/internal/peer/worker" + signal "github.com/netbirdio/netbird/shared/signal/client" ) // fakePQ is a minimal PQHandshaker: only PSK is exercised by presharedKey, the rest @@ -120,3 +125,42 @@ func TestConn_IdlePresharedKey(t *testing.T) { assert.Nil(t, newConn(fakePQ{ok: false}, false).IdlePresharedKey(), "non-strict mode leaves the idle key unchanged") assert.Nil(t, newConn(nil, false).IdlePresharedKey(), "without ML-KEM the idle key is left unchanged") } + +// TestConn_RequestReofferOnlyWhenConnected covers the PQ recovery path. A failed KEM +// exchange makes the manager ask for a fresh signalling offer. For a peer with no +// transport (offline) that offer cannot be answered: with lazy connections off it was +// repeated at every exchange timeout, a fixed-rate stream on top of the reconnect guard's +// own backed-off offers. The re-offer must only be sent while the peer is connected. +func TestConn_RequestReofferOnlyWhenConnected(t *testing.T) { + newConn := func() (*Conn, *logtest.Hook) { + logger, hook := logtest.NewNullLogger() + logger.SetLevel(log.DebugLevel) + entry := logger.WithField("peer", "reoffer-test") + // The signal client is never ready, so an attempted offer fails fast and is logged. + signaler := NewSignaler(&signal.MockClient{}, wgtypes.Key{}) + c := &Conn{ + Log: entry, + statusRelay: worker.NewAtomicStatus(), + statusICE: worker.NewAtomicStatus(), + } + c.handshaker = NewHandshaker(entry, ConnConfig{}, signaler, nil, nil, nil) + return c, hook + } + offerAttempted := func(hook *logtest.Hook) bool { + for _, e := range hook.AllEntries() { + if strings.Contains(e.Message, "recovery re-offer failed") { + return true + } + } + return false + } + + c, hook := newConn() + c.RequestReoffer() + assert.False(t, offerAttempted(hook), "a disconnected peer must be left to the reconnect guard") + + c, hook = newConn() + c.statusRelay.SetConnected() + c.RequestReoffer() + assert.True(t, offerAttempted(hook), "a connected peer must be re-offered") +}