From c4a91bd4f524f10d064139674cf55852e055ad01 Mon Sep 17 00:00:00 2001
From: Adam Langley <agl@golang.org>
Date: Tue, 13 Mar 2018 12:10:02 -0700
Subject: [PATCH] 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 <bradfitz@golang.org>
---
 ed25519/ed25519.go                            | 13 +++++++---
 ed25519/ed25519_test.go                       | 24 +++++++++++++++++++
 ed25519/internal/edwards25519/edwards25519.go | 22 +++++++++++++++++
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/ed25519/ed25519.go b/ed25519/ed25519.go
index 4f26b49b6a..a57771a1ed 100644
--- a/ed25519/ed25519.go
+++ b/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)
diff --git a/ed25519/ed25519_test.go b/ed25519/ed25519_test.go
index e272f8a557..5f946e996e 100644
--- a/ed25519/ed25519_test.go
+++ b/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++ {
diff --git a/ed25519/internal/edwards25519/edwards25519.go b/ed25519/internal/edwards25519/edwards25519.go
index 5f8b994787..fd03c252af 100644
--- a/ed25519/internal/edwards25519/edwards25519.go
+++ b/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
+}