diff --git a/client/internal/pqkem/capability_test.go b/client/internal/pqkem/capability_test.go index 88ab7b2b9..13acba1bc 100644 --- a/client/internal/pqkem/capability_test.go +++ b/client/internal/pqkem/capability_test.go @@ -141,3 +141,49 @@ func TestManager_OfferErrorMarkerAnsweredWithMarker(t *testing.T) { _, established := dA.PSK("bbbb") assert.False(t, established, "a marker must not derive a PSK") } + +// TestManager_UnreadableOfferAnsweredWithMarker: an offer the responder cannot process, such +// as one from a newer protocol version, must get an error marker. An empty answer would make +// the initiator mark the responder non-capable for good and stay classical with it. +func TestManager_UnreadableOfferAnsweredWithMarker(t *testing.T) { + dA, dB, _, _, _ := pair(t) + defer dB.Stop() + + offer, err := dB.SignalOffer("aaaa") + require.NoError(t, err) + require.NotNil(t, offer) + + newerVersion := append([]byte(nil), offer...) + newerVersion[1] = ProtocolVersion + 1 + answerType := (&ErrorMsg{}).Encode() + answerType[0] = byte(MsgAnswer) + + cases := map[string][]byte{ + "garbage": {1, 2, 3}, + "newer version": newerVersion, + "not an offer": answerType, + "truncated body": offer[:len(offer)-1], + } + for name, in := range cases { + t.Run(name, func(t *testing.T) { + reply, err := dA.SignalOnOffer("bbbb", in) + assert.Error(t, err) + assertErrorMarker(t, reply) + }) + } + + t.Run("manager stopping", func(t *testing.T) { + dA.Stop() + reply, err := dA.SignalOnOffer("bbbb", offer) + assert.Error(t, err) + assertErrorMarker(t, reply) + }) +} + +func assertErrorMarker(t *testing.T, reply []byte) { + t.Helper() + 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 must be an error marker") +} diff --git a/client/internal/pqkem/manager.go b/client/internal/pqkem/manager.go index fdfa856a0..d05b45b3f 100644 --- a/client/internal/pqkem/manager.go +++ b/client/internal/pqkem/manager.go @@ -418,15 +418,18 @@ func (m *Manager) ShouldSendBootstrapOffer(remoteID RemoteID) bool { } // SignalOnOffer processes a KEM offer the host extracted from an incoming offer and -// returns the KEM answer for the host to embed in its outgoing answer. +// returns the KEM answer for the host to embed in its outgoing answer. Every failure +// still returns an error marker: an empty answer means "this peer does not run the KEM" +// and would stop the initiator from ever offering again. func (m *Manager) SignalOnOffer(remoteID RemoteID, offer []byte) ([]byte, error) { if !m.beginCall() { - return nil, fmt.Errorf("manager stopping") + return (&ErrorMsg{}).Encode(), fmt.Errorf("manager stopping") } defer m.wait.Done() typ, msg, err := Decode(offer) if err != nil { - return nil, fmt.Errorf("decode signal offer from %s: %w", remoteID, err) + // Also covers an offer from a newer protocol version this peer cannot read. + return (&ErrorMsg{}).Encode(), fmt.Errorf("decode signal offer from %s: %w", remoteID, err) } if typ == MsgError { // The initiator failed to build its offer and sent a marker. Answer with our own @@ -435,7 +438,7 @@ func (m *Manager) SignalOnOffer(remoteID RemoteID, offer []byte) ([]byte, error) return (&ErrorMsg{}).Encode(), nil } if typ != MsgOffer { - return nil, fmt.Errorf("expected offer from %s, got type %d", remoteID, typ) + return (&ErrorMsg{}).Encode(), fmt.Errorf("expected offer from %s, got type %d", remoteID, typ) } o := msg.(*OfferMsg) answer, err := m.processOffer(remoteID, o, viaSignalLabel)