diff --git a/client/internal/engine.go b/client/internal/engine.go index 73fca0893..89768da54 100644 --- a/client/internal/engine.go +++ b/client/internal/engine.go @@ -613,9 +613,9 @@ func (e *Engine) startPQKEMManager(publicKey wgtypes.Key) error { // On a persistent rekey failure, re-bootstrap the KEM over Signal: a fresh // signalling offer starts a new exchange that overwrites the stalled PSK on both // sides, recovering from a data-path desync. - reoffer: func(remoteKey string) { + recoverPQKeyOverSignal: func(remoteKey string) { if conn, ok := e.peerStore.PeerConn(remoteKey); ok { - conn.RequestReoffer() + conn.RecoverPQKeyOverSignal() } }, } diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index d688f1096..ca341cc11 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -757,12 +757,13 @@ 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 +// RecoverPQKeyOverSignal re-runs the post-quantum bootstrap over Signal after the ML-KEM +// exchange failed (the initial exchange, or repeated rotation failures): a fresh +// signalling offer starts a new exchange that overwrites the stalled PSK on both sides. +// It is not part of normal key rotation, which runs over the data path. No-op if the // 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) RecoverPQKeyOverSignal() { conn.mu.Lock() h := conn.handshaker connected := conn.evalStatus() == StatusConnected @@ -775,7 +776,7 @@ func (conn *Conn) RequestReoffer() { return } if err := h.SendOffer(); err != nil { - conn.Log.Debugf("pqkem: recovery re-offer failed: %v", err) + conn.Log.Debugf("pqkem: PQ key recovery over signal failed: %v", err) } } @@ -806,7 +807,7 @@ func (conn *Conn) onWGDisconnected(watcherCtx context.Context) { key := conn.config.Key conn.mu.Unlock() - // Signal the PQ manager outside conn.mu: it may re-enter Conn (reoffer) under + // Signal the PQ manager outside conn.mu: it may re-enter Conn (PQ key recovery) under // conn.mu, so calling it while holding the lock would invert the lock order. if pq != nil { pq.OnDataPathDown(key) @@ -1135,7 +1136,7 @@ func (conn *Conn) AgentVersionString() string { // pqPSK returns the post-quantum PSK derived for this peer, if the ML-KEM exchange has // produced one. It reads the manager WITHOUT conn.mu, so callers fetch it before taking // conn.mu: the lock order is always conn.mu -> manager, never the reverse (the manager's -// reoffer callback re-enters Conn under conn.mu). +// PQ key recovery callback re-enters Conn under conn.mu). func (conn *Conn) pqPSK() (*wgtypes.Key, bool) { if conn.config.PQ == nil { return nil, false diff --git a/client/internal/peer/conn_pq_test.go b/client/internal/peer/conn_pq_test.go index a010ceabb..3aa7cfa46 100644 --- a/client/internal/peer/conn_pq_test.go +++ b/client/internal/peer/conn_pq_test.go @@ -126,16 +126,16 @@ func TestConn_IdlePresharedKey(t *testing.T) { 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 +// TestConn_RecoverPQKeyOverSignalOnlyWhenConnected 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) { +// own backed-off offers. The recovery offer must only be sent while the peer is connected. +func TestConn_RecoverPQKeyOverSignalOnlyWhenConnected(t *testing.T) { newConn := func() (*Conn, *logtest.Hook) { logger, hook := logtest.NewNullLogger() logger.SetLevel(log.DebugLevel) - entry := logger.WithField("peer", "reoffer-test") + entry := logger.WithField("peer", "pq-recovery-test") // The signal client is never ready, so an attempted offer fails fast and is logged. signaler := NewSignaler(&signal.MockClient{}, wgtypes.Key{}) c := &Conn{ @@ -148,7 +148,7 @@ func TestConn_RequestReofferOnlyWhenConnected(t *testing.T) { } offerAttempted := func(hook *logtest.Hook) bool { for _, e := range hook.AllEntries() { - if strings.Contains(e.Message, "recovery re-offer failed") { + if strings.Contains(e.Message, "PQ key recovery over signal failed") { return true } } @@ -156,11 +156,11 @@ func TestConn_RequestReofferOnlyWhenConnected(t *testing.T) { } c, hook := newConn() - c.RequestReoffer() + c.RecoverPQKeyOverSignal() 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") + c.RecoverPQKeyOverSignal() + assert.True(t, offerAttempted(hook), "a connected peer must get a recovery offer") } diff --git a/client/internal/pqkem/capability_test.go b/client/internal/pqkem/capability_test.go index af30ff19e..0471cdb3f 100644 --- a/client/internal/pqkem/capability_test.go +++ b/client/internal/pqkem/capability_test.go @@ -9,7 +9,7 @@ import ( // TestManager_NonCapablePeerNotOffered: a peer known not to run the KEM (it answered our // offer with an empty KEM payload over signalling) is never offered an exchange, and no -// failure is raised — this is what stops the reoffer storm against non-PQ peers (e.g. +// failure is raised — this is what stops the recovery storm against non-PQ peers (e.g. // Rosenpass peers). func TestManager_NonCapablePeerNotOffered(t *testing.T) { wg := newFakeWG() diff --git a/client/internal/pqkem/manager.go b/client/internal/pqkem/manager.go index 1fee7da0e..f2ba97031 100644 --- a/client/internal/pqkem/manager.go +++ b/client/internal/pqkem/manager.go @@ -364,7 +364,7 @@ func (m *Manager) SignalOffer(remoteID RemoteID) ([]byte, error) { m.mu.Lock() if capable, ok := m.capable[remoteID]; ok && !capable { m.mu.Unlock() - return nil, nil // peer does not run the KEM; do not offer (avoids a failure/reoffer loop) + return nil, nil // peer does not run the KEM; do not offer (avoids a failure/recovery loop) } // Idempotent ONLY while a signalling bootstrap is still in flight (awaitingAnswer): // a repeat call returns the SAME offer instead of starting a duplicate exchange. Once diff --git a/client/internal/pqkem_adapter.go b/client/internal/pqkem_adapter.go index ad65a895c..8ec4a326c 100644 --- a/client/internal/pqkem_adapter.go +++ b/client/internal/pqkem_adapter.go @@ -61,9 +61,9 @@ type pqCallbackHandler struct { // applied drops an out-of-order PSK apply so a stale callback can't overwrite a newer // key. Nil disables the guard. applied *appliedGenerations - // reoffer re-bootstraps the KEM over Signal for a peer (a fresh signalling offer) - // to recover from a persistent data-path rekey failure. Nil disables recovery. - reoffer func(remoteKey string) + // recoverPQKeyOverSignal re-bootstraps the KEM over Signal for a peer (a fresh + // signalling offer) after its exchange failed. Nil disables recovery. + recoverPQKeyOverSignal func(remoteKey string) } // OnNewPSKReady programs the freshly derived PSK for the peer (updateOnly: a no-op @@ -109,8 +109,8 @@ func (h pqCallbackHandler) OnNewPSKReady(remoteID pqkem.RemoteID, gen uint64, ps // broken data path). func (h pqCallbackHandler) OnRekeyFailed(remoteID pqkem.RemoteID) error { log.Warnf("pqkem: post-quantum rekey failed for peer %s, re-bootstrapping over signal", remoteID) - if h.reoffer != nil { - h.reoffer(string(remoteID)) + if h.recoverPQKeyOverSignal != nil { + h.recoverPQKeyOverSignal(string(remoteID)) } return nil } @@ -176,7 +176,7 @@ func (p pqHandshaker) OnAnswer(remoteKey string, recvAnswer []byte) { if len(recvAnswer) == 0 { // Capability signal (initiator side): the KEM answer flows responder->initiator, // so an empty answer to our offer means the peer does not run the KEM — mark it - // non-capable to stop offering (no failure/reoffer storm). Only meaningful when + // non-capable to stop offering (no failure/recovery storm). Only meaningful when // we are the initiator: as the responder we also receive an (empty) answer to // our own non-KEM offer from a perfectly capable peer, which must not be flagged. if p.mgr.IsInitiator(pqkem.RemoteID(remoteKey)) {