kem/hybrid: ensure X25519 hybrids fails with low order points

kem/hybrid/ckem.go

@@ -159,6 +159,9 @@ func (pk *cPublicKey) X(sk *cPrivateKey) []byte {
 
 	sharedKey, err := sk.key.ECDH(pk.key)
 	if err != nil {
+		// ECDH cannot fail for NIST curves as NewPublicKey rejects
+		// invalid points and the point in infinity, and NewPrivateKey
+		// rejects invalid scalars and the zero value.
 		panic(err)
 	}
 	return sharedKey

kem/hybrid/xkem.go

@@ -131,7 +131,7 @@ func (sch *xScheme) Encapsulate(pk kem.PublicKey) (ct, ss []byte, err error) {
 	return sch.EncapsulateDeterministically(pk, seed)
 }
 
-func (pk *xPublicKey) X(sk *xPrivateKey) []byte {
+func (pk *xPublicKey) X(sk *xPrivateKey) ([]byte, error) {
 	if pk.scheme != sk.scheme {
 		panic(kem.ErrTypeMismatch)
 	}
@@ -141,14 +141,18 @@ func (pk *xPublicKey) X(sk *xPrivateKey) []byte {
 		var ss2, pk2, sk2 x25519.Key
 		copy(pk2[:], pk.key)
 		copy(sk2[:], sk.key)
-		x25519.Shared(&ss2, &sk2, &pk2)
-		return ss2[:]
+		if !x25519.Shared(&ss2, &sk2, &pk2) {
+			return nil, kem.ErrPubKey
+		}
+		return ss2[:], nil
 	case x448.Size:
 		var ss2, pk2, sk2 x448.Key
 		copy(pk2[:], pk.key)
 		copy(sk2[:], sk.key)
-		x448.Shared(&ss2, &sk2, &pk2)
-		return ss2[:]
+		if !x448.Shared(&ss2, &sk2, &pk2) {
+			return nil, kem.ErrPubKey
+		}
+		return ss2[:], nil
 	}
 	panic(kem.ErrTypeMismatch)
 }
@@ -165,7 +169,10 @@ func (sch *xScheme) EncapsulateDeterministically(
 	}
 
 	pk2, sk2 := sch.DeriveKeyPair(seed)
-	ss = pub.X(sk2.(*xPrivateKey))
+	ss, err = pub.X(sk2.(*xPrivateKey))
+	if err != nil {
+		return nil, nil, err
+	}
 	ct, _ = pk2.MarshalBinary()
 	return
 }
@@ -185,8 +192,7 @@ func (sch *xScheme) Decapsulate(sk kem.PrivateKey, ct []byte) ([]byte, error) {
 		return nil, err
 	}
 
-	ss := pk.(*xPublicKey).X(priv)
-	return ss, nil
+	return pk.(*xPublicKey).X(priv)
 }
 
 func (sch *xScheme) UnmarshalBinaryPublicKey(buf []byte) (kem.PublicKey, error) {

kem/hybrid/xkem_test.go

@@ -0,0 +1,68 @@
+package hybrid
+
+import (
+	"encoding/hex"
+	"testing"
+
+	"github.com/cloudflare/circl/kem"
+)
+
+func mustDecodeString(s string) []byte {
+	b, err := hex.DecodeString(s)
+	if err != nil {
+		panic(err)
+	}
+	return b
+}
+
+// patchHybridWithLowOrderX25519 replaces the last half of the given ciphertext
+// or public key by a 32-byte Curve25519 public key with a point of low order.
+func patchHybridWithLowOrderX25519(hybridKey []byte) {
+	// order 8
+	xPub := mustDecodeString("e0eb7a7c3b41b8ae1656e3faf19fc46ada098deb9c32b1fd866205165f49b800")
+	copy(hybridKey[len(hybridKey)-len(xPub):], xPub)
+}
+
+func patchHybridPublicKeyWithLowOrderX25519(pub kem.PublicKey) (kem.PublicKey, error) {
+	packed, err := pub.MarshalBinary()
+	if err != nil {
+		return nil, err
+	}
+	patchHybridWithLowOrderX25519(packed)
+	return pub.Scheme().UnmarshalBinaryPublicKey(packed)
+}
+
+func TestLowOrderX25519PointEncapsulate(t *testing.T) {
+	scheme := X25519MLKEM768()
+	pk, _, err := scheme.GenerateKeyPair()
+	if err != nil {
+		t.Fatalf("X25519MLKEM768 keygen: %s", err)
+	}
+	badPk, err := patchHybridPublicKeyWithLowOrderX25519(pk)
+	if err != nil {
+		t.Fatalf("patching X25519 key failed: %s", err)
+	}
+	_, _, err = scheme.Encapsulate(badPk)
+	want := kem.ErrPubKey
+	if err != want {
+		t.Fatalf("Encapsulate error: expected %v; got %v", want, err)
+	}
+}
+
+func TestLowOrderX25519PointDecapsulate(t *testing.T) {
+	scheme := X25519MLKEM768()
+	pk, sk, err := scheme.GenerateKeyPair()
+	if err != nil {
+		t.Fatalf("X25519MLKEM768 keygen: %s", err)
+	}
+	ct, _, err := scheme.Encapsulate(pk)
+	if err != nil {
+		t.Fatalf("Encapsulate failed: %s", err)
+	}
+	patchHybridWithLowOrderX25519(ct)
+	_, err = scheme.Decapsulate(sk, ct)
+	want := kem.ErrPubKey
+	if err != want {
+		t.Fatalf("Decapsulate error: expected %v; got %v", want, err)
+	}
+}

kem/xwing/xwing.go

@@ -252,6 +252,9 @@ func (pk *PublicKey) EncapsulateTo(ct, ss, seed []byte) {
 	copy(ekx[:], seed[32:])
 
 	x25519.KeyGen(&ctx, &ekx)
+	// A peer public key with low order points results in an all-zeroes
+	// shared secret. Ignored for now pending clarification in the spec,
+	// https://github.com/dconnolly/draft-connolly-cfrg-xwing-kem/issues/28
 	x25519.Shared(&ssx, &ekx, &pk.x)
 	pk.m.EncapsulateTo(ct[:mlkem768.CiphertextSize], ssm[:], seedm[:])
 
@@ -283,6 +286,9 @@ func (sk *PrivateKey) DecapsulateTo(ss, ct []byte) {
 	copy(ctx[:], ct[mlkem768.CiphertextSize:])
 
 	sk.m.DecapsulateTo(ssm[:], ctm)
+	// A peer public key with low order points results in an all-zeroes
+	// shared secret. Ignored for now pending clarification in the spec,
+	// https://github.com/dconnolly/draft-connolly-cfrg-xwing-kem/issues/28
 	x25519.Shared(&ssx, &sk.x, &ctx)
 	combiner(ss, &ssm, &ssx, &ctx, &sk.xpk)
 }