diff --git a/client/internal/peer/conn_test.go b/client/internal/peer/conn_test.go index c6e82c06e..0092cf503 100644 --- a/client/internal/peer/conn_test.go +++ b/client/internal/peer/conn_test.go @@ -296,16 +296,12 @@ func TestConn_presharedKey_RosenpassManaged(t *testing.T) { // When Rosenpass has already initialized the PSK for this peer, // presharedKey must return nil to avoid UpdatePeer overwriting it. conn.rosenpassInitializedPresharedKeyValidator = func(peerKey string) bool { return true } - if k := conn.presharedKey([]byte("remote"), nil); k != nil { - t.Fatalf("expected nil presharedKey when Rosenpass manages PSK, got %v", k) - } + assert.Nil(t, conn.presharedKey([]byte("remote"), nil), "expected nil presharedKey when Rosenpass manages PSK") // When Rosenpass hasn't taken over yet, presharedKey should provide // a non-nil initial key (deterministic or from NetBird PSK). conn.rosenpassInitializedPresharedKeyValidator = func(peerKey string) bool { return false } - if k := conn.presharedKey([]byte("remote"), nil); k == nil { - t.Fatalf("expected non-nil presharedKey before Rosenpass manages PSK") - } + assert.NotNil(t, conn.presharedKey([]byte("remote"), nil), "expected non-nil presharedKey before Rosenpass manages PSK") } func newWGTimeoutTestConn(rosenpassEnabled bool, disconnected *[]string) *Conn { diff --git a/client/internal/peer/handshaker.go b/client/internal/peer/handshaker.go index deb1f0a8d..fe8d1d0ec 100644 --- a/client/internal/peer/handshaker.go +++ b/client/internal/peer/handshaker.go @@ -171,10 +171,11 @@ func (h *Handshaker) handleRemoteOffer(remoteOfferAnswer OfferAnswer) { // Derive+store the KEM PSK (inside sendAnswer's AnswerPayload) BEFORE bringing up the // connection: the relay/ICE workers configure the WG endpoint, which pulls the PSK // for the first handshake. Notifying them first would race the KEM exchange and hand - // the first handshake a not-yet-derived key. + // the first handshake a not-yet-derived key. A failure to signal the answer is + // transient (the peer retries), so still bring the local transport up over relay/ICE + // instead of aborting the whole setup. if err := h.sendAnswer(&remoteOfferAnswer); err != nil { h.log.Errorf("failed to send remote offer confirmation: %s", err) - return } h.notifyListeners(&remoteOfferAnswer) } diff --git a/client/internal/pqkem/convergence.go b/client/internal/pqkem/convergence.go index e740a16e5..9c8d893a0 100644 --- a/client/internal/pqkem/convergence.go +++ b/client/internal/pqkem/convergence.go @@ -67,7 +67,6 @@ func (m *Manager) startExchangeLocked(remoteID RemoteID, viaSignal bool, ackID E m.exchanges[remoteID] = &exchangeCtl{ id: id, state: stateAwaitingAnswer, - startedAt: time.Now(), cancel: cancel, lastSent: raw, initiator: init, @@ -118,11 +117,19 @@ func (m *Manager) processOffer(remoteID RemoteID, o *OfferMsg, via string) ([]by return last, nil } // Reserve the slot so a concurrent duplicate offer bails. - m.exchanges[remoteID] = &exchangeCtl{id: o.ExchangeID, state: stateReserved, startedAt: time.Now()} + m.exchanges[remoteID] = &exchangeCtl{id: o.ExchangeID, state: stateReserved} m.mu.Unlock() answerBytes, psk, err := Respond(o.KEMOffer, m.binding(remoteID)) if err != nil { + // Respond failed before the PSK was committed: clear the reservation so a + // retransmission of this offer (a transient or malformed first packet) can retry + // instead of hitting the stuck reserved slot forever. + m.mu.Lock() + if cur := m.exchanges[remoteID]; cur != nil && cur.id == o.ExchangeID && cur.state == stateReserved { + delete(m.exchanges, remoteID) + } + m.mu.Unlock() return nil, err } raw, err := (&AnswerMsg{ExchangeID: o.ExchangeID, KEMAnswer: answerBytes}).Encode() @@ -235,7 +242,6 @@ func (m *Manager) ackConverged(remoteID RemoteID, ackID ExchangeID) { delete(m.exchanges, remoteID) m.established[remoteID] = true m.failures[remoteID] = 0 - _ = time.Since(ex.startedAt) // convergence latency (metrics hook, later step) m.mu.Unlock() m.trace("pqkem: previous exchange confirmed by ack", "peer", remoteID, "exchange", idHex(ackID)) diff --git a/client/internal/pqkem/env.go b/client/internal/pqkem/env.go index 6eaa9e129..19b5bf76f 100644 --- a/client/internal/pqkem/env.go +++ b/client/internal/pqkem/env.go @@ -43,15 +43,16 @@ const EnvStrict = "NB_PQ_MLKEM_STRICT" // Strict reports whether strict (fail-closed) mode is enabled via the environment. // An empty or unrecognized value is treated as disabled (opportunistic). func Strict() bool { - switch strings.ToLower(strings.TrimSpace(os.Getenv(EnvStrict))) { + raw := strings.ToLower(strings.TrimSpace(os.Getenv(EnvStrict))) + switch raw { case "on": return true case "", "off": return false } - enabled, err := strconv.ParseBool(strings.TrimSpace(os.Getenv(EnvStrict))) + enabled, err := strconv.ParseBool(raw) if err != nil { - log.Warnf("failed to parse %s value %q: %v", EnvStrict, os.Getenv(EnvStrict), err) + log.Warnf("failed to parse %s value %q: %v", EnvStrict, raw, err) return false } return enabled diff --git a/client/internal/pqkem/manager.go b/client/internal/pqkem/manager.go index e49790b43..7e4b411d2 100644 --- a/client/internal/pqkem/manager.go +++ b/client/internal/pqkem/manager.go @@ -80,7 +80,6 @@ const ( type exchangeCtl struct { id ExchangeID state exchangeState - startedAt time.Time cancel context.CancelFunc lastSent []byte initiator *Initiator @@ -402,7 +401,7 @@ func (m *Manager) OnDataPathMessage(remoteID RemoteID, raw []byte) error { } return m.pushDataPath(remoteID, answer) case MsgAnswer: - return m.processAnswer(remoteID, msg.(*AnswerMsg), viaSignalLabel) + return m.processAnswer(remoteID, msg.(*AnswerMsg), viaDataPathLabel) default: return fmt.Errorf("unhandled data-path message type %d from %s", typ, remoteID) } diff --git a/client/status/status.go b/client/status/status.go index e5c83c995..96527fa04 100644 --- a/client/status/status.go +++ b/client/status/status.go @@ -793,7 +793,7 @@ func peerQuantumResistanceStatus(established, rosenpassEnabled, rosenpassPermiss } switch { case mlkemEnabled && mlkemStrict: - return "false (ML-KEM strict: blocking peer traffic until the exchange converges)" + return "false (ML-KEM strict: no post-quantum PSK established for this peer yet)" case mlkemEnabled: return "false (ML-KEM: not converged yet, or peer does not run the exchange)" case rosenpassEnabled && rosenpassPermissive: