From 82ea56abe2e684bb3c247aa3b00a904391bc1662 Mon Sep 17 00:00:00 2001 From: riccardom Date: Thu, 6 Aug 2026 18:04:30 +0200 Subject: [PATCH] [client] pqkem: gate controller re-offer to kick the KEM exactly once When the controller receives the responder's (KEM-less) offer it replies with its own KEM offer instead of answering, so the only transaction that brings the tunnel up is the one that also carries the PSK. Guard that reply with ShouldSendBootstrapOffer so it fires only when no exchange is in flight: without it, every responder offer triggered another offer (an offer-per-offer runaway). The whole behaviour is isolated to the KEM path (config.PQ != nil); non-PQ connections answer as before. --- client/internal/peer/conn.go | 4 ++++ client/internal/peer/conn_pq_test.go | 1 + client/internal/peer/handshaker.go | 13 ++++++++++--- client/internal/pqkem/manager.go | 18 ++++++++++++++++++ client/internal/pqkem_adapter.go | 4 ++++ 5 files changed, 37 insertions(+), 3 deletions(-) diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index f5f8efea4..ac5983c07 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -84,6 +84,10 @@ type PQHandshaker interface { // OfferPayload returns the KEM offer to embed in an outgoing offer (nil if this // peer is not the KEM initiator) and the local PQ data-path port to announce. OfferPayload(remoteKey string) (payload []byte, port int) + // ShouldSendBootstrapOffer reports whether, as the controller, we should reply to a + // received responder offer with our own KEM offer (true only when no exchange is + // already in flight — so we kick the KEM once and ignore further offers). + ShouldSendBootstrapOffer(remoteKey string) bool // AnswerPayload processes a received KEM offer (nil if absent) and returns the KEM // answer to embed in the outgoing answer (nil if none) and the local PQ port. AnswerPayload(remoteKey string, recvOffer []byte) (payload []byte, port int) diff --git a/client/internal/peer/conn_pq_test.go b/client/internal/peer/conn_pq_test.go index 11d81b36e..ffc6e8c32 100644 --- a/client/internal/peer/conn_pq_test.go +++ b/client/internal/peer/conn_pq_test.go @@ -18,6 +18,7 @@ type fakePQ struct { } func (f fakePQ) OfferPayload(string) ([]byte, int) { return nil, 0 } +func (f fakePQ) ShouldSendBootstrapOffer(string) bool { return false } func (f fakePQ) AnswerPayload(string, []byte) ([]byte, int) { return nil, 0 } func (f fakePQ) OnAnswer(string, []byte) {} func (f fakePQ) PSK(string) (wgtypes.Key, bool) { return f.psk, f.ok } diff --git a/client/internal/peer/handshaker.go b/client/internal/peer/handshaker.go index 52d6bf12a..c1715d9c4 100644 --- a/client/internal/peer/handshaker.go +++ b/client/internal/peer/handshaker.go @@ -149,9 +149,16 @@ func (h *Handshaker) Listen(ctx context.Context) { // responder-initiated wake still triggers a KEM offer (no stuck responder). // The re-offer reuses our stable ICE session id, so the peer dedups repeats. if h.config.PQ != nil && isController(h.config) { - h.log.Debugf("pqkem: controller received a responder offer, replying with our KEM offer instead of an answer") - if err := h.sendOffer(); err != nil { - h.log.Errorf("failed to send KEM offer in response to peer offer: %s", err) + // Reply with our KEM offer exactly once, to kick the exchange. If one is + // already in flight, ignore this offer — re-sending on every responder + // offer would be an offer-per-offer runaway. + if h.config.PQ.ShouldSendBootstrapOffer(h.config.Key) { + h.log.Debugf("pqkem: controller received a responder offer, replying with our KEM offer instead of an answer") + if err := h.sendOffer(); err != nil { + h.log.Errorf("failed to send KEM offer in response to peer offer: %s", err) + } + } else { + h.log.Debugf("pqkem: controller received a responder offer but a KEM exchange is already in flight, ignoring") } continue } diff --git a/client/internal/pqkem/manager.go b/client/internal/pqkem/manager.go index 8533833a4..53fd564c4 100644 --- a/client/internal/pqkem/manager.go +++ b/client/internal/pqkem/manager.go @@ -301,6 +301,24 @@ func (m *Manager) SignalOffer(remoteID RemoteID) ([]byte, error) { return m.startExchange(remoteID, true, ExchangeID{}) } +// ShouldSendBootstrapOffer reports whether we should emit a fresh KEM offer to kick a +// bootstrap for this peer. True only if we are the initiator, the peer is not known +// non-capable, and no exchange is already in flight. The host uses this when it (as the +// controller) receives the responder's offer: it replies with a KEM offer exactly once +// to start the exchange, and ignores further responder offers while one is in flight, +// avoiding an offer-per-offer runaway. +func (m *Manager) ShouldSendBootstrapOffer(remoteID RemoteID) bool { + if !m.IsInitiator(remoteID) { + return false + } + m.mu.Lock() + defer m.mu.Unlock() + if capable, ok := m.capable[remoteID]; ok && !capable { + return false + } + return m.exchanges[remoteID] == nil +} + // SignalOnOffer processes a KEM offer the host extracted from an incoming offer and // returns the KEM answer for the host to embed in its outgoing answer. func (m *Manager) SignalOnOffer(remoteID RemoteID, offer []byte) ([]byte, error) { diff --git a/client/internal/pqkem_adapter.go b/client/internal/pqkem_adapter.go index 82d2981de..be5a99c1b 100644 --- a/client/internal/pqkem_adapter.go +++ b/client/internal/pqkem_adapter.go @@ -72,6 +72,10 @@ func (p pqHandshaker) OfferPayload(remoteKey string) ([]byte, int) { return payload, p.announcedPort() } +func (p pqHandshaker) ShouldSendBootstrapOffer(remoteKey string) bool { + return p.mgr.ShouldSendBootstrapOffer(pqkem.RemoteID(remoteKey)) +} + func (p pqHandshaker) AnswerPayload(remoteKey string, recvOffer []byte) ([]byte, int) { if len(recvOffer) == 0 { // Capability signal (responder side): the KEM offer flows initiator->responder,