From 9edf2f4dea1cf5b7f6d3db26f6e818c7f8d4ddb2 Mon Sep 17 00:00:00 2001 From: riccardom Date: Wed, 22 Jul 2026 09:46:34 +0200 Subject: [PATCH] [squash] isInitial and answered can be inferred without state variables --- client/internal/pqkem/convergence.go | 28 +++++++++++++++++----------- client/internal/pqkem/driver.go | 13 ++++++------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/client/internal/pqkem/convergence.go b/client/internal/pqkem/convergence.go index 8b2ac93bb..3e1368ef7 100644 --- a/client/internal/pqkem/convergence.go +++ b/client/internal/pqkem/convergence.go @@ -25,7 +25,6 @@ func (d *Driver) handleOffer(remoteWgKey string, o *OfferMsg) error { d.exchanges[remoteWgKey] = &exchangeCtl{ id: o.ExchangeID, startedAt: time.Now(), - isInitial: !d.established[remoteWgKey], } d.mu.Unlock() @@ -51,15 +50,15 @@ func (d *Driver) handleOffer(remoteWgKey string, o *OfferMsg) error { // exchange then switches its retransmit payload to the confirm. Stale or duplicate // answers are ignored. func (d *Driver) handleAnswer(remoteWgKey string, a *AnswerMsg) error { - // Claim the answer under the lock (set answered) so a concurrent/duplicate answer - // bails before calling the Manager. + // Ignore stale answers, answers to a responder-side exchange (cancel == nil), and + // duplicates once we are already in the confirm phase. A duplicate that still + // slips through is caught by the Manager, whose HandleAnswer bails under its lock. d.mu.Lock() ex := d.exchanges[remoteWgKey] - if ex == nil || ex.id != a.ExchangeID || ex.cancel == nil || ex.answered { + if ex == nil || ex.id != a.ExchangeID || ex.cancel == nil || isConfirmPhase(ex.lastSent) { d.mu.Unlock() return nil } - ex.answered = true d.mu.Unlock() psk, confirm, err := d.mgr.HandleAnswer(remoteWgKey, a) @@ -132,10 +131,10 @@ func (d *Driver) initiatorLoop(ctx context.Context, remoteWgKey string, id Excha return } - if !ex.answered { + if !isConfirmPhase(ex.lastSent) { if offerAttempts >= d.maxRetries { delete(d.exchanges, remoteWgKey) - fail := d.registerFailureLocked(remoteWgKey, ex.isInitial) + fail := d.registerFailureLocked(remoteWgKey) d.mu.Unlock() d.raiseFailure(remoteWgKey, fail) return @@ -161,11 +160,12 @@ func (d *Driver) initiatorLoop(ctx context.Context, remoteWgKey string, id Excha } // registerFailureLocked applies policy B and reports whether OnRekeyFailed is due: -// the initial exchange fails immediately; a rekey tolerates up to maxRekeyFailures -// consecutive misses (we stay on the still-valid previous PSK) before failing. +// an initial exchange (peer never established) fails immediately; a rekey tolerates +// up to maxRekeyFailures consecutive misses (we stay on the still-valid previous +// PSK) before failing. Whether it is initial is read live from established. // Assumes d.mu is held. -func (d *Driver) registerFailureLocked(remoteWgKey string, isInitial bool) bool { - if isInitial { +func (d *Driver) registerFailureLocked(remoteWgKey string) bool { + if !d.established[remoteWgKey] { return true } d.failures[remoteWgKey]++ @@ -176,6 +176,12 @@ func (d *Driver) registerFailureLocked(remoteWgKey string, isInitial bool) bool return false } +// isConfirmPhase reports whether the initiator's outstanding message is the confirm +// (as opposed to the offer), derived from the message's type byte. +func isConfirmPhase(lastSent []byte) bool { + return len(lastSent) > 0 && lastSent[0] == byte(MsgConfirm) +} + func (d *Driver) raiseFailure(remoteWgKey string, fail bool) { if !fail { d.logger.Warn("pqkem rekey attempt timed out, will retry next cycle", "peer", remoteWgKey) diff --git a/client/internal/pqkem/driver.go b/client/internal/pqkem/driver.go index 8e0980dbc..b4b6c4514 100644 --- a/client/internal/pqkem/driver.go +++ b/client/internal/pqkem/driver.go @@ -35,17 +35,17 @@ type Transport interface { } // exchangeCtl tracks one in-flight exchange for a peer. A single lastSent holds the -// message currently being (re)transmitted: for the initiator it is the offer and -// then, once answered, the confirm; for the responder it is the answer, resent on a -// duplicate offer. Only the initiator runs a retransmit loop (cancel != nil); the -// responder is purely reactive. +// message currently being (re)transmitted: for the initiator the offer and then, +// once answered, the confirm; for the responder the answer, resent on a duplicate +// offer. Its type byte (lastSent[0]) also encodes the initiator's phase, so no +// separate "answered" flag is kept. Only the initiator runs a retransmit loop +// (cancel != nil); the responder is purely reactive. Whether an exchange is the +// peer's first is read live from d.established, not snapshotted here. type exchangeCtl struct { id ExchangeID startedAt time.Time - isInitial bool lastSent []byte cancel context.CancelFunc - answered bool // initiator: the answer arrived; retransmit phase is now the confirm } // Driver ties the pure Manager to the outside world: per-peer rekey timer, inbound @@ -186,7 +186,6 @@ func (d *Driver) initiateRekey(remoteWgKey string) error { d.exchanges[remoteWgKey] = &exchangeCtl{ id: offer.ExchangeID, startedAt: time.Now(), - isInitial: !d.established[remoteWgKey], lastSent: raw, cancel: cancel, }