From f1a604cffb8df6e69ebe97f17a9d334c75ec6b96 Mon Sep 17 00:00:00 2001 From: riccardom Date: Fri, 7 Aug 2026 16:08:52 +0200 Subject: [PATCH] Addresses CI fixes --- client/internal/pqkem/convergence.go | 24 +++++++++++++++++++++--- client/internal/pqkem/kem.go | 5 +++++ client/internal/pqkem/message.go | 2 +- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/client/internal/pqkem/convergence.go b/client/internal/pqkem/convergence.go index 0d6085f73..e504cbfbd 100644 --- a/client/internal/pqkem/convergence.go +++ b/client/internal/pqkem/convergence.go @@ -4,6 +4,7 @@ import ( "context" "crypto/sha256" "encoding/hex" + "fmt" "time" ) @@ -22,7 +23,14 @@ func pskFingerprint(psk PSK) string { // bootstrap) and returns the framed offer for the caller to send — pushed over the // data path for a chained rekey, or handed to the host for signalling when viaSignal // is set. Any previous in-flight exchange for the peer is cancelled. -func (m *Manager) startExchange(remoteID RemoteID, viaSignal bool, ackID ExchangeID) ([]byte, error) { +// startExchangeLocked must be called with m.mu held: the caller's idempotency check and +// the exchange install stay under one lock acquisition so two concurrent starts for the +// same peer cannot both create an exchange. It also refuses to start (and to Add to the +// wait group) once the manager is stopping, so it never races Manager.Stop's Wait. +func (m *Manager) startExchangeLocked(remoteID RemoteID, viaSignal bool, ackID ExchangeID) ([]byte, error) { + if m.stopping { + return nil, fmt.Errorf("manager stopping") + } init, err := NewInitiator() if err != nil { return nil, err @@ -37,7 +45,6 @@ func (m *Manager) startExchange(remoteID RemoteID, viaSignal bool, ackID Exchang } ctx, cancel := context.WithCancel(m.rootCtx) - m.mu.Lock() if old := m.exchanges[remoteID]; old != nil && old.cancel != nil { old.cancel() } @@ -50,7 +57,6 @@ func (m *Manager) startExchange(remoteID RemoteID, viaSignal bool, ackID Exchang initiator: init, viaSignal: viaSignal, } - m.mu.Unlock() m.wait.Add(1) go m.initiatorLoop(ctx, remoteID, id) @@ -146,6 +152,18 @@ func (m *Manager) processAnswer(remoteID RemoteID, a *AnswerMsg) error { psk, err := init.Finish(a.KEMAnswer, m.binding(remoteID)) if err != nil { + // The state already advanced to stateAwaitingRekey and the initiator was cleared, + // so initiatorLoop would exit its default branch without registering a failure — + // leaving the peer desynced (the responder committed its PSK in processOffer). + // Drop the exchange and raise the failure so recovery re-bootstraps. + m.mu.Lock() + if cur := m.exchanges[remoteID]; cur != nil && cur.id == a.ExchangeID { + delete(m.exchanges, remoteID) + } + initial := !m.established[remoteID] + fail := m.registerFailureLocked(remoteID) + m.mu.Unlock() + m.raiseFailure(remoteID, fail, initial) return err } diff --git a/client/internal/pqkem/kem.go b/client/internal/pqkem/kem.go index cd3c80d09..90c638a7a 100644 --- a/client/internal/pqkem/kem.go +++ b/client/internal/pqkem/kem.go @@ -149,6 +149,11 @@ func Respond(offer []byte, b Binding) (answer []byte, psk PSK, err error) { // canonicalised peer identities, so the PSK cannot be transplanted to another peer // pair or a different exchange. func derivePSK(ssMLKEM, ssX, offer, answer []byte, b Binding) (PSK, error) { + // A PSK not bound to both peer identities could be transplanted to a different peer + // pair, so refuse to derive one from an empty binding. + if len(b.LocalID) == 0 || len(b.RemoteID) == 0 { + return PSK{}, fmt.Errorf("empty peer identity binding") + } lo, hi := canonicalPair(b.LocalID, b.RemoteID) ikm := make([]byte, 0, len(ssMLKEM)+len(ssX)) diff --git a/client/internal/pqkem/message.go b/client/internal/pqkem/message.go index 312279d80..e483e9d72 100644 --- a/client/internal/pqkem/message.go +++ b/client/internal/pqkem/message.go @@ -40,7 +40,7 @@ const ( // that acknowledges nothing, i.e. the first exchange of a connection). type ExchangeID [ExchangeIDSize]byte -// OfferMsg carries the initiator's public material (X25519 pub ‖ ML-KEM encap key) +// OfferMsg carries the initiator's public material (ML-KEM encap key ‖ X25519 pub) // and AckID, the id of the previous exchange this offer acknowledges (zero if none). type OfferMsg struct { ExchangeID ExchangeID