From 15ae046599c45c337c56c9d40099c60f52dbae7c Mon Sep 17 00:00:00 2001 From: riccardom Date: Fri, 28 Aug 2026 16:11:05 +0200 Subject: [PATCH] [client] pqkem: don't drop a responder's offer when the controller has no re-offer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pqControllerReoffer returned true whenever we are the controller running the KEM, even when ShouldSendBootstrapOffer was false — which is the steady state for every PQ peer past its bootstrap (exchange in awaitingRekey) and for non-capable peers. handleRemoteOffer then returned early, dropping the ICE credentials and relay info carried in the peer's offer, so a responder-initiated reconnect (the normal path under lazy connections) stalled: the controller only recovered on its own guard timing. Return true only when a re-offer was actually sent; otherwise fall through to the normal offer handling (sendAnswer + notifyListeners) so the connection can come up on the retained PSK. A stale PSK still self-heals via the WG watcher, which triggers a fresh signal offer that re-bootstraps. --- client/internal/peer/handshaker.go | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/client/internal/peer/handshaker.go b/client/internal/peer/handshaker.go index e64c9c299..e01dc47fa 100644 --- a/client/internal/peer/handshaker.go +++ b/client/internal/peer/handshaker.go @@ -203,18 +203,27 @@ func (h *Handshaker) handleRemoteAnswer(remoteOfferAnswer OfferAnswer) { // one is in flight are ignored (re-sending on every responder offer would be a runaway). // The re-offer reuses our stable ICE session id, so the peer dedups repeats. // -// Returns true when it took ownership of the offer (the caller must not answer it). +// Returns true ONLY when it actually sent a re-offer and thus took ownership of the peer's +// offer (the caller must not answer it). When there is nothing to re-offer — the peer is +// non-capable, or an exchange is already in flight (e.g. every PQ peer past its bootstrap, +// which is the steady state under lazy connections) — it returns false so the caller keeps +// handling the offer normally (sendAnswer + notifyListeners). Returning true unconditionally +// would drop the ICE credentials and relay info the responder's offer carries, stalling a +// responder-initiated reconnect. func (h *Handshaker) pqControllerReoffer() bool { if h.config.PQ == nil || !isController(h.config) { return false } - 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") + if !h.config.PQ.ShouldSendBootstrapOffer(h.config.Key) { + h.log.Debugf("pqkem: controller received a responder offer with no fresh KEM offer to send, handling it normally") + return false + } + h.log.Debugf("pqkem: controller received a responder offer, replying with our KEM offer instead of an answer") + if err := h.sendOffer(); err != nil { + // The re-offer did not go out; fall through to normal handling rather than + // silently dropping the peer's offer. + h.log.Errorf("failed to send KEM offer in response to peer offer, handling it normally: %s", err) + return false } return true }