[client] pqkem: answer an unreadable offer with an error marker

An empty answer tells the initiator the peer does not run the KEM, and it
stops offering for good. A decode or version error is not that: a newer
client talking to an older one would silently stay classical.

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")
+}

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)