Merge pull request #434 from dedis/schnorr_checks

Schnorr Verification Checks

group/edwards25519/point.go

@@ -263,3 +263,30 @@ func (P *point) HasSmallOrder() bool {
 
 	return (k>>8)&1 > 0
 }
+
+// IsCanonical determines whether the group element is canonical
+//
+// Checks whether group element s is less than p, according to RFC8032§5.1.3.1
+// https://tools.ietf.org/html/rfc8032#section-5.1.3
+//
+// Taken from
+// https://github.com/jedisct1/libsodium/blob/4744636721d2e420f8bbe2d563f31b1f5e682229/src/libsodium/crypto_core/ed25519/ref10/ed25519_ref10.c#L1113
+//
+// The method accepts a buffer instead of calling `MarshalBinary` on the receiver
+// because that always returns a value modulo `prime`.
+func (P *point) IsCanonical(s []byte) bool {
+	if len(s) != 32 {
+		return false
+	}
+
+	c := (s[31] & 0x7f) ^ 0x7f
+	for i := 30; i > 0; i-- {
+		c |= s[i] ^ 0xff
+	}
+
+	// subtraction might underflow
+	c = byte((uint16(c) - 1) >> 8)
+	d := byte((0xed - 1 - uint16(s[0])) >> 8)
+
+	return 1-(c&d&1) == 1
+}

group/edwards25519/point_test.go

@@ -23,3 +23,42 @@ func TestPoint_HasSmallOrder(t *testing.T) {
 		require.True(t, p.HasSmallOrder(), fmt.Sprintf("%s should be considered to have a small order", hex.EncodeToString(key)))
 	}
 }
+
+// Test_PointIsCanonical ensures that elements >= p are considered
+// non canonical
+func Test_PointIsCanonical(t *testing.T) {
+
+	// buffer stores the candidate points (in little endian) that we'll test
+	// against, starting with `prime`
+	buffer := prime.Bytes()
+	for i, j := 0, len(buffer)-1; i < j; i, j = i+1, j-1 {
+		buffer[i], buffer[j] = buffer[j], buffer[i]
+	}
+
+	// Iterate over the 19*2 finite field elements
+	point := point{}
+	actualNonCanonicalCount := 0
+	expectedNonCanonicalCount := 24
+	for i := 0; i < 19; i++ {
+		buffer[0] = byte(237 + i)
+		buffer[31] = byte(127)
+
+		// Check if it's a valid point on the curve that's
+		// not canonical
+		err := point.UnmarshalBinary(buffer)
+		if err == nil && !point.IsCanonical(buffer) {
+			actualNonCanonicalCount++
+		}
+
+		// flip bit
+		buffer[31] |= 128
+
+		// Check if it's a valid point on the curve that's
+		// not canonical
+		err = point.UnmarshalBinary(buffer)
+		if err == nil && !point.IsCanonical(buffer) {
+			actualNonCanonicalCount++
+		}
+	}
+	require.Equal(t, expectedNonCanonicalCount, actualNonCanonicalCount, "Incorrect number of non canonical points detected")
+}

group/edwards25519/scalar.go

@@ -2230,3 +2230,37 @@ func scReduce(out *[32]byte, s *[64]byte) {
 	out[30] = byte(s11 >> 9)
 	out[31] = byte(s11 >> 17)
 }
+
+// IsCanonical whether the scalar in sb is in the range 0<=s<L as required by RFC8032, Section 5.1.7.
+// Also provides Strong Unforgeability under Chosen Message Attacks (SUF-CMA)
+// See paper https://eprint.iacr.org/2020/823.pdf for definitions and theorems
+// See https://github.com/jedisct1/libsodium/blob/4744636721d2e420f8bbe2d563f31b1f5e682229/src/libsodium/crypto_core/ed25519/ref10/ed25519_ref10.c#L2568
+// for a reference.
+// The method accepts a buffer instead of calling `MarshalBinary` on the receiver since that
+// always returns values modulo `primeOrder`.
+func (s *scalar) IsCanonical(sb []byte) bool {
+	if len(sb) != 32 {
+		return false
+	}
+
+	if sb[31]&0xf0 == 0 {
+		return true
+	}
+
+	L := primeOrder.Bytes()
+	for i, j := 0, 31; i < j; i, j = i+1, j-1 {
+		L[i], L[j] = L[j], L[i]
+	}
+
+	var c byte
+	var n byte = 1
+
+	for i := 31; i >= 0; i-- {
+		// subtraction might lead to an underflow which needs
+		// to be accounted for in the right shift
+		c |= byte((uint16(sb[i])-uint16(L[i]))>>8) & n
+		n &= byte((uint16(sb[i]) ^ uint16(L[i]) - 1) >> 8)
+	}
+
+	return c != 0
+}

group/edwards25519/scalar_test.go

@@ -2,6 +2,7 @@ package edwards25519
 
 import (
 	"fmt"
+	"math/big"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -457,3 +458,24 @@ func scSubFact(s, a, c *[32]byte) {
 
 	scReduceLimbs(limbs)
 }
+
+// Test_ScalarIsCanonical ensures that scalars >= primeOrder are
+// considered non canonical.
+func Test_ScalarIsCanonical(t *testing.T) {
+	candidate := big.NewInt(-2)
+	candidate.Add(candidate, primeOrder)
+
+	candidateBuf := candidate.Bytes()
+	for i, j := 0, len(candidateBuf)-1; i < j; i, j = i+1, j-1 {
+		candidateBuf[i], candidateBuf[j] = candidateBuf[j], candidateBuf[i]
+	}
+
+	expected := []bool{true, true, false, false}
+	scalar := scalar{}
+
+	// We check in range [L-2, L+4)
+	for i := 0; i < 4; i++ {
+		require.Equal(t, expected[i], scalar.IsCanonical(candidateBuf), fmt.Sprintf("`lMinus2 + %d` does not pass canonicality test", i))
+		candidateBuf[0]++
+	}
+}

sign/eddsa/eddsa.go

@@ -7,19 +7,13 @@ import (
 	"crypto/sha512"
 	"errors"
 	"fmt"
-	"math/big"
 
 	"go.dedis.ch/kyber/v3"
 	"go.dedis.ch/kyber/v3/group/edwards25519"
 )
 
 var group = new(edwards25519.Curve)
 
-// TODO: maybe export prime and primeOrder from edwards25519/const or allow it to be
-// retrieved from the curve?
-var prime, _ = new(big.Int).SetString("57896044618658097711785492504343953926634992332820282019728792003956564819949", 10)
-var primeOrder, _ = new(big.Int).SetString("7237005577332262213973186563042994240857116359379907606001950938285454250989", 10)
-
 // EdDSA is a structure holding the data necessary to make a series of
 // EdDSA signatures.
 type EdDSA struct {
@@ -32,15 +26,6 @@ type EdDSA struct {
 	prefix []byte
 }
 
-// edDSAPoint is used to verify signatures
-// with checks around canonicality and group order
-type edDSAPoint interface {
-	kyber.Point
-	// HasSmallOrder checks if the given buffer (in little endian)
-	// represents a point with a small order
-	HasSmallOrder() bool
-}
-
 // NewEdDSA will return a freshly generated key pair to use for generating
 // EdDSA signatures.
 func NewEdDSA(stream cipher.Stream) *EdDSA {
@@ -143,21 +128,28 @@ func VerifyWithChecks(pub, msg, sig []byte) error {
 	if len(sig) != 64 {
 		return fmt.Errorf("signature length invalid, expect 64 but got %v", len(sig))
 	}
-	if !scalarIsCanonical(sig[32:]) {
+
+	type scalarCanCheckCanonical interface {
+		IsCanonical(b []byte) bool
+	}
+
+	if !group.Scalar().(scalarCanCheckCanonical).IsCanonical(sig[32:]) {
 		return fmt.Errorf("signature is not canonical")
 	}
-	if !pointIsCanonical(pub) {
-		return fmt.Errorf("public key is not canonical")
+
+	type pointCanCheckCanonicalAndSmallOrder interface {
+		HasSmallOrder() bool
+		IsCanonical(b []byte) bool
 	}
 
-	if !pointIsCanonical(sig[:32]) {
+	R := group.Point()
+	if !R.(pointCanCheckCanonicalAndSmallOrder).IsCanonical(sig[:32]) {
 		return fmt.Errorf("R is not canonical")
 	}
-	R := group.Point()
 	if err := R.UnmarshalBinary(sig[:32]); err != nil {
 		return fmt.Errorf("got R invalid point: %s", err)
 	}
-	if R.(edDSAPoint).HasSmallOrder() {
+	if R.(pointCanCheckCanonicalAndSmallOrder).HasSmallOrder() {
 		return fmt.Errorf("R has small order")
 	}
 
@@ -167,10 +159,13 @@ func VerifyWithChecks(pub, msg, sig []byte) error {
 	}
 
 	public := group.Point()
+	if !public.(pointCanCheckCanonicalAndSmallOrder).IsCanonical(pub) {
+		return fmt.Errorf("public key is not canonical")
+	}
 	if err := public.UnmarshalBinary(pub); err != nil {
 		return fmt.Errorf("invalid public key: %s", err)
 	}
-	if public.(edDSAPoint).HasSmallOrder() {
+	if public.(pointCanCheckCanonicalAndSmallOrder).HasSmallOrder() {
 		return fmt.Errorf("public key has small order")
 	}
 
@@ -201,59 +196,3 @@ func Verify(public kyber.Point, msg, sig []byte) error {
 	}
 	return VerifyWithChecks(PBuf, msg, sig)
 }
-
-// scalarIsCanonical whether scalar s is in the range 0<=s<L as required by RFC8032, Section 5.1.7.
-// Also provides Strong Unforgeability under Chosen Message Attacks (SUF-CMA)
-// See paper https://eprint.iacr.org/2020/823.pdf for definitions and theorems
-// See https://github.com/jedisct1/libsodium/blob/4744636721d2e420f8bbe2d563f31b1f5e682229/src/libsodium/crypto_core/ed25519/ref10/ed25519_ref10.c#L2568
-// for a reference
-func scalarIsCanonical(sb []byte) bool {
-	if len(sb) != 32 {
-		return false
-	}
-
-	if sb[31]&0xf0 == 0 {
-		return true
-	}
-
-	L := primeOrder.Bytes()
-	for i, j := 0, 31; i < j; i, j = i+1, j-1 {
-		L[i], L[j] = L[j], L[i]
-	}
-
-	var c byte
-	var n byte = 1
-
-	for i := 31; i >= 0; i-- {
-		// subtraction might lead to an underflow which needs
-		// to be accounted for in the right shift
-		c |= byte((uint16(sb[i])-uint16(L[i]))>>8) & n
-		n &= byte((uint16(sb[i]) ^ uint16(L[i]) - 1) >> 8)
-	}
-
-	return c != 0
-}
-
-// pointIsCanonical determines whether the group element is canonical
-//
-// Checks whether group element s is less than p, according to RFC8032§5.1.3.1
-// https://tools.ietf.org/html/rfc8032#section-5.1.3
-//
-// Taken from
-// https://github.com/jedisct1/libsodium/blob/4744636721d2e420f8bbe2d563f31b1f5e682229/src/libsodium/crypto_core/ed25519/ref10/ed25519_ref10.c#L1113
-func pointIsCanonical(s []byte) bool {
-	if len(s) != 32 {
-		return false
-	}
-
-	c := (s[31] & 0x7f) ^ 0x7f
-	for i := 30; i > 0; i-- {
-		c |= s[i] ^ 0xff
-	}
-
-	// subtraction might underflow
-	c = byte((uint16(c) - 1) >> 8)
-	d := byte((0xed - 1 - uint16(s[0])) >> 8)
-
-	return 1-(c&d&1) == 1
-}

sign/eddsa/eddsa_test.go

@@ -6,8 +6,6 @@ import (
 	"compress/gzip"
 	"crypto/cipher"
 	"encoding/hex"
-	"fmt"
-	"math/big"
 	"math/rand"
 	"os"
 	"strings"
@@ -346,60 +344,3 @@ func TestGolden(t *testing.T) {
 		t.Fatalf("error reading test data: %s", err)
 	}
 }
-
-// Test_pointIsCanonical ensures that elements >= p are considered
-// non canonical
-func Test_pointIsCanonical(t *testing.T) {
-
-	// buffer stores the candidate points (in little endian) that we'll test
-	// against, starting with `prime`
-	buffer := prime.Bytes()
-	for i, j := 0, len(buffer)-1; i < j; i, j = i+1, j-1 {
-		buffer[i], buffer[j] = buffer[j], buffer[i]
-	}
-
-	// Iterate over the 19*2 finite field elements
-	point := group.Point()
-	actualNonCanonicalCount := 0
-	expectedNonCanonicalCount := 24
-	for i := 0; i < 19; i++ {
-		buffer[0] = byte(237 + i)
-		buffer[31] = byte(127)
-
-		// Check if it's a valid point on the curve that's
-		// not canonical
-		err := point.UnmarshalBinary(buffer)
-		if err == nil && !pointIsCanonical(buffer) {
-			actualNonCanonicalCount++
-		}
-
-		// flip bit
-		buffer[31] |= 128
-
-		// Check if it's a valid point on the curve that's
-		// not canonical
-		err = point.UnmarshalBinary(buffer)
-		if err == nil && !pointIsCanonical(buffer) {
-			actualNonCanonicalCount++
-		}
-	}
-	require.Equal(t, expectedNonCanonicalCount, actualNonCanonicalCount, "Incorrect number of non canonical points detected")
-}
-
-func Test_scalarIsCanonical(t *testing.T) {
-	candidate := big.NewInt(-2)
-	candidate.Add(candidate, primeOrder)
-
-	candidateBuf := candidate.Bytes()
-	for i, j := 0, len(candidateBuf)-1; i < j; i, j = i+1, j-1 {
-		candidateBuf[i], candidateBuf[j] = candidateBuf[j], candidateBuf[i]
-	}
-
-	expected := []bool{true, true, false, false}
-
-	// We check in range [L-2, L+4)
-	for i := 0; i < 4; i++ {
-		require.Equal(t, expected[i], scalarIsCanonical(candidateBuf), fmt.Sprintf("`lMinus2 + %d` does not pass canonicality test", i))
-		candidateBuf[0]++
-	}
-}