[client] pqkem: re-bootstrap over Signal only while the peer is connected

A failed KEM exchange makes the manager ask for a fresh signalling offer.
For a peer that is registered but offline that offer can never be answered,
so with lazy connections off (the default) each such peer drew a new offer
at every exchange timeout, about every 22 seconds, forever. That runs on top
of the reconnect guard, which already retries the peer with exponential
backoff, and every guard offer starts a fresh KEM exchange anyway. With many
offline peers it is a steady stream of useless Signal traffic.

Send the recovery offer only while the peer is connected. That is where it
is needed: the transport is up but the KEM did not converge (strict mode
stuck before its first PSK, or a broken rotation). An unreachable peer is
left to the guard's backoff.
This commit is contained in:
riccardom
2026-10-19 17:02:15 +02:00
parent 3cfc5c82a8
commit 5f3f5d56d9
2 changed files with 51 additions and 1 deletions
+7 -1
View File
@@ -760,14 +760,20 @@ func (conn *Conn) onGuardEvent() {
// RequestReoffer sends a fresh signalling offer for the peer, re-running the // 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 // 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 // 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() { func (conn *Conn) RequestReoffer() {
conn.mu.Lock() conn.mu.Lock()
h := conn.handshaker h := conn.handshaker
connected := conn.evalStatus() == StatusConnected
conn.mu.Unlock() conn.mu.Unlock()
if h == nil { if h == nil {
return 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 { if err := h.SendOffer(); err != nil {
conn.Log.Debugf("pqkem: recovery re-offer failed: %v", err) conn.Log.Debugf("pqkem: recovery re-offer failed: %v", err)
} }
+44
View File
@@ -2,13 +2,18 @@ package peer
import ( import (
"net/netip" "net/netip"
"strings"
"testing" "testing"
"time" "time"
log "github.com/sirupsen/logrus" log "github.com/sirupsen/logrus"
logtest "github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"golang.zx2c4.com/wireguard/wgctrl/wgtypes" "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 // 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(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") 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")
}