diff --git a/client/internal/pqkem/capability_test.go b/client/internal/pqkem/capability_test.go index 0471cdb3f..aabf2edd3 100644 --- a/client/internal/pqkem/capability_test.go +++ b/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") +} diff --git a/client/internal/pqkem/concurrency_test.go b/client/internal/pqkem/concurrency_test.go index bcf389404..e81265ee2 100644 --- a/client/internal/pqkem/concurrency_test.go +++ b/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 diff --git a/client/internal/pqkem/kem_test.go b/client/internal/pqkem/kem_test.go index 9a2c9c8f9..da5e98c1e 100644 --- a/client/internal/pqkem/kem_test.go +++ b/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") } diff --git a/client/internal/pqkem/message_test.go b/client/internal/pqkem/message_test.go index 0ce9be69d..d061b9528 100644 --- a/client/internal/pqkem/message_test.go +++ b/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") }