ed25519: require canonical signatures

https://tools.ietf.org/html/rfc8032#section-5.1.7 requires that s be in
the range [0, order) in order to prevent signature malleability. This is
a new requirement in the RFC so the ed25519 package predates it. This
change aligns the code with the RFC.

The linked bug says that libsodium is also enforcing this check by
default.

See golang/go#24350

Change-Id: Ib69ce7c9e5a58971cbe225318d9fd87660bd5e4b
Reviewed-on: https://go-review.googlesource.com/100436
Reviewed-by: Brad Fitzpatrick <[email protected]>

Author: agl

ed25519/ed25519.go

@@ -171,9 +171,16 @@ func Verify(publicKey PublicKey, message, sig []byte) bool {
 	edwards25519.ScReduce(&hReduced, &digest)
 
 	var R edwards25519.ProjectiveGroupElement
-	var b [32]byte
-	copy(b[:], sig[32:])
-	edwards25519.GeDoubleScalarMultVartime(&R, &hReduced, &A, &b)
+	var s [32]byte
+	copy(s[:], sig[32:])
+
+	// https://tools.ietf.org/html/rfc8032#section-5.1.7 requires that s be in
+	// the range [0, order) in order to prevent signature malleability.
+	if !edwards25519.ScMinimal(&s) {
+		return false
+	}
+
+	edwards25519.GeDoubleScalarMultVartime(&R, &hReduced, &A, &s)
 
 	var checkR [32]byte
 	R.ToBytes(&checkR)

ed25519/ed25519_test.go

@@ -146,6 +146,30 @@ func TestGolden(t *testing.T) {
 	}
 }
 
+func TestMalleability(t *testing.T) {
+	// https://tools.ietf.org/html/rfc8032#section-5.1.7 adds an additional test
+	// that s be in [0, order). This prevents someone from adding a multiple of
+	// order to s and obtaining a second valid signature for the same message.
+	msg := []byte{0x54, 0x65, 0x73, 0x74}
+	sig := []byte{
+		0x7c, 0x38, 0xe0, 0x26, 0xf2, 0x9e, 0x14, 0xaa, 0xbd, 0x05, 0x9a,
+		0x0f, 0x2d, 0xb8, 0xb0, 0xcd, 0x78, 0x30, 0x40, 0x60, 0x9a, 0x8b,
+		0xe6, 0x84, 0xdb, 0x12, 0xf8, 0x2a, 0x27, 0x77, 0x4a, 0xb0, 0x67,
+		0x65, 0x4b, 0xce, 0x38, 0x32, 0xc2, 0xd7, 0x6f, 0x8f, 0x6f, 0x5d,
+		0xaf, 0xc0, 0x8d, 0x93, 0x39, 0xd4, 0xee, 0xf6, 0x76, 0x57, 0x33,
+		0x36, 0xa5, 0xc5, 0x1e, 0xb6, 0xf9, 0x46, 0xb3, 0x1d,
+	}
+	publicKey := []byte{
+		0x7d, 0x4d, 0x0e, 0x7f, 0x61, 0x53, 0xa6, 0x9b, 0x62, 0x42, 0xb5,
+		0x22, 0xab, 0xbe, 0xe6, 0x85, 0xfd, 0xa4, 0x42, 0x0f, 0x88, 0x34,
+		0xb1, 0x08, 0xc3, 0xbd, 0xae, 0x36, 0x9e, 0xf5, 0x49, 0xfa,
+	}
+
+	if Verify(publicKey, msg, sig) {
+		t.Fatal("non-canonical signature accepted")
+	}
+}
+
 func BenchmarkKeyGeneration(b *testing.B) {
 	var zero zeroReader
 	for i := 0; i < b.N; i++ {

ed25519/internal/edwards25519/edwards25519.go

@@ -4,6 +4,8 @@
 
 package edwards25519
 
+import "encoding/binary"
+
 // This code is a port of the public domain, “ref10” implementation of ed25519
 // from SUPERCOP.
 
@@ -1769,3 +1771,23 @@ func ScReduce(out *[32]byte, s *[64]byte) {
 	out[30] = byte(s11 >> 9)
 	out[31] = byte(s11 >> 17)
 }
+
+// order is the order of Curve25519 in little-endian form.
+var order = [4]uint64{0x5812631a5cf5d3ed, 0x14def9dea2f79cd6, 0, 0x1000000000000000}
+
+// ScMinimal returns true if the given scalar is less than the order of the
+// curve.
+func ScMinimal(scalar *[32]byte) bool {
+	for i := 3; ; i-- {
+		v := binary.LittleEndian.Uint64(scalar[i*8:])
+		if v > order[i] {
+			return false
+		} else if v < order[i] {
+			break
+		} else if i == 0 {
+			return false
+		}
+	}
+
+	return true
+}