[client] pqkem: make weak tests fail on the regressions they name

The recovery test could time out its own bootstraps under CI load, the
non-capable test passed with the cancellation removed, and the decoder
rejection test only exercised the encoder. The timing spike ran on every
go test; it is a benchmark now.

client/internal/pqkem/capability_test.go

@@ -1,6 +1,7 @@
 package pqkem
 
 import (
+	"sync/atomic"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -39,9 +40,28 @@ func TestManager_MarkNonCapableCancelsInFlight(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, offer)
 
+	// Record the cancellation of the in-flight exchange's loop. Without it the loop would
+	// time out and raise OnRekeyFailed, the re-bootstrap storm MarkNonCapable prevents.
+	var cancelled atomic.Bool
+	d.mu.Lock()
+	ex := d.exchanges["aaaa"]
+	require.NotNil(t, ex, "precondition: the bootstrap exchange is in flight")
+	loopCancel := ex.cancel
+	ex.cancel = func() {
+		cancelled.Store(true)
+		loopCancel()
+	}
+	d.mu.Unlock()
+
 	// Now we learn the peer is non-PQ: the exchange must be dropped.
 	d.MarkNonCapable("aaaa")
 
+	assert.True(t, cancelled.Load(), "the in-flight exchange's loop must be cancelled")
+	d.mu.Lock()
+	_, stillTracked := d.exchanges["aaaa"]
+	d.mu.Unlock()
+	assert.False(t, stillTracked, "the in-flight exchange must be dropped")
+
 	next, err := d.SignalOffer("aaaa")
 	require.NoError(t, err)
 	assert.Nil(t, next, "after learning non-capability the peer is no longer offered")
@@ -97,3 +117,23 @@ func TestManager_ErrorMarkerIsBenign(t *testing.T) {
 	assert.Equal(t, wgB.psk("aaaa"), wgA.psk("bbbb"), "the exchange must still converge after a benign error marker")
 	assert.NotEqual(t, PSK{}, wgB.psk("aaaa"))
 }
+
+// TestManager_OfferErrorMarkerAnsweredWithMarker covers the other direction: the initiator
+// failed to build its offer and sent a marker. The responder must answer with a marker of
+// its own, never an empty answer, which the initiator would read as "peer has no KEM".
+func TestManager_OfferErrorMarkerAnsweredWithMarker(t *testing.T) {
+	dA, dB, _, _, _ := pair(t)
+	defer dA.Stop()
+	defer dB.Stop()
+
+	reply, err := dA.SignalOnOffer("bbbb", (&ErrorMsg{}).Encode())
+	require.NoError(t, err)
+	require.NotEmpty(t, reply, "an empty reply would mark the responder non-capable")
+
+	typ, _, err := Decode(reply)
+	require.NoError(t, err)
+	assert.Equal(t, MsgError, typ, "the reply to an offer marker must be a marker")
+
+	_, established := dA.PSK("bbbb")
+	assert.False(t, established, "a marker must not derive a PSK")
+}

client/internal/pqkem/concurrency_test.go

@@ -17,11 +17,12 @@ func TestConcurrency_RecoversViaResignalAfterDataPathBreak(t *testing.T) {
 	defer dA.Stop()
 	defer dB.Stop()
 
-	// Tighten B's timings and make a single rotation miss raise OnRekeyFailed. Set
-	// before any exchange loop spawns (the loop reads these fields).
+	// A fast retransmit tick, set before any exchange loop spawns because the loop reads it
+	// unguarded at start. The retry budget stays generous for the two bootstraps, which
+	// must succeed: with a tight budget the loop could time an exchange out while the test
+	// goroutine is descheduled between the offer and the answer.
 	dB.retryInterval = 5 * time.Millisecond
-	dB.maxRetries = 2
-	dB.maxRekeyFailures = 1
+	setRetryBudget(dB, 1000, 1)
 
 	bootstrap(t, dA, dB)
 	dA.OnDataPathRekeyed("bbbb", 0)
@@ -30,7 +31,9 @@ func TestConcurrency_RecoversViaResignalAfterDataPathBreak(t *testing.T) {
 	require.NotEqual(t, PSK{}, psk1)
 	require.Equal(t, psk1, wgA.psk("bbbb"), "converged on the same PSK after bootstrap+rotation")
 
-	// Data path breaks: the rotation can no longer converge -> OnRekeyFailed.
+	// Data path breaks: the rotation can no longer converge, and with a budget of two
+	// retransmits a single miss raises OnRekeyFailed.
+	setRetryBudget(dB, 2, 1)
 	lbB.drop.Store(true)
 	_, err := dB.startExchangeTest("aaaa", false, ExchangeID{})
 	require.NoError(t, err)
@@ -38,12 +41,22 @@ func TestConcurrency_RecoversViaResignalAfterDataPathBreak(t *testing.T) {
 
 	// Recovery: re-bootstrap over signalling with the data path STILL broken. It must
 	// still converge (signal is independent of the data path) on a fresh PSK.
+	setRetryBudget(dB, 1000, 1)
 	bootstrap(t, dA, dB)
 	psk2 := wgB.psk("aaaa")
 	require.NotEqual(t, psk1, psk2, "recovery derived a fresh PSK")
 	require.Equal(t, psk2, wgA.psk("bbbb"), "both sides resync after recovery")
 }
 
+// setRetryBudget changes the retry and failure budgets of a running manager. The exchange
+// loops read both under the manager lock, so they can change between phases of a test.
+func setRetryBudget(m *Manager, maxRetries, maxRekeyFailures int) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	m.maxRetries = maxRetries
+	m.maxRekeyFailures = maxRekeyFailures
+}
+
 // TestConcurrency_ConcurrentRekeysNoRace hammers both managers with concurrent rotation
 // clocks from many goroutines. Its primary job (with -race) is to prove the single-lock
 // state machine has no data races or deadlocks under contention; a final deterministic

client/internal/pqkem/kem_test.go

@@ -2,7 +2,6 @@ package pqkem
 
 import (
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/require"
 )
@@ -74,33 +73,17 @@ func TestExchange_RejectsMalformedMessages(t *testing.T) {
 	require.Error(t, err)
 }
 
-// TestExchange_ReportSizesAndTiming is a spike measurement, not a pass/fail gate.
-// Run with: go test -run TestExchange_ReportSizesAndTiming -v ./client/internal/pqkem/
-func TestExchange_ReportSizesAndTiming(t *testing.T) {
-	const iters = 200
-
-	var tInit, tResp, tFinish time.Duration
-	for i := 0; i < iters; i++ {
-		s0 := time.Now()
+// BenchmarkExchange measures a full hybrid exchange (keygen, encapsulation, decapsulation)
+// and reports its on-wire size.
+// Run with: go test -run '^$' -bench BenchmarkExchange ./client/internal/pqkem/
+func BenchmarkExchange(b *testing.B) {
+	for i := 0; i < b.N; i++ {
 		init, err := NewInitiator()
-		require.NoError(t, err)
-		tInit += time.Since(s0)
-
-		s1 := time.Now()
+		require.NoError(b, err)
 		answer, _, err := Respond(init.Offer(), Binding{LocalID: wgB, RemoteID: wgA})
-		require.NoError(t, err)
-		tResp += time.Since(s1)
-
-		s2 := time.Now()
+		require.NoError(b, err)
 		_, err = init.Finish(answer, Binding{LocalID: wgA, RemoteID: wgB})
-		require.NoError(t, err)
-		tFinish += time.Since(s2)
+		require.NoError(b, err)
 	}
-
-	t.Logf("wire sizes:   offer=%d B   answer=%d B   (Rosenpass static pubkey ~524160 B)", OfferSize, AnswerSize)
-	t.Logf("total on-wire per handshake: %d B  (~%.0fx smaller than RP static key)", OfferSize+AnswerSize, 524160.0/float64(OfferSize+AnswerSize))
-	t.Logf("avg NewInitiator (keygen): %s", tInit/iters)
-	t.Logf("avg Respond (encaps+dh):   %s", tResp/iters)
-	t.Logf("avg Finish (decaps+dh):    %s", tFinish/iters)
-	t.Logf("avg full handshake CPU:    %s", (tInit+tResp+tFinish)/iters)
+	b.ReportMetric(float64(OfferSize+AnswerSize), "wire-B/exchange")
 }

client/internal/pqkem/message_test.go

@@ -60,7 +60,27 @@ func TestDecodeRejects(t *testing.T) {
 	_, _, err = Decode(bad2)
 	require.Error(t, err)
 
-	// offer with wrong payload size
+	// the encoder refuses an offer with the wrong payload size
 	_, err = (&OfferMsg{KEMOffer: []byte{1, 2, 3}}).Encode()
 	require.Error(t, err)
+
+	// the decoder refuses a framed offer, answer or error marker whose payload has the
+	// wrong size
+	short := make([]byte, headerSize+ExchangeIDSize+OfferSize-1)
+	short[0] = byte(MsgOffer)
+	short[1] = ProtocolVersion
+	_, _, err = Decode(short)
+	require.Error(t, err, "a truncated offer must be rejected")
+
+	long := make([]byte, headerSize+AnswerSize+1)
+	long[0] = byte(MsgAnswer)
+	long[1] = ProtocolVersion
+	_, _, err = Decode(long)
+	require.Error(t, err, "an oversized answer must be rejected")
+
+	marker := make([]byte, headerSize+1)
+	marker[0] = byte(MsgError)
+	marker[1] = ProtocolVersion
+	_, _, err = Decode(marker)
+	require.Error(t, err, "an error marker with a payload must be rejected")
 }