One test for decaf, unmarshaling straight-line code, and check for er…

…rors.

ecc/goldilocks/constants.go

@@ -15,8 +15,9 @@ const (
 	DecafEncodingSize = fp.Size
 )
 
+// All these values are in RFC-7748 (https://tools.ietf.org/html/rfc7748).
 var (
-	// genX is the x-coordintate of the generator of Goldilocks curve.
+	// genX is the x-coordinate of the generator of Goldilocks curve.
 	genX = fp.Elt{
 		0x5e, 0xc0, 0x0c, 0xc7, 0x2b, 0xa8, 0x26, 0x26,
 		0x8e, 0x93, 0x00, 0x8b, 0xe1, 0x80, 0x3b, 0x43,
@@ -26,7 +27,7 @@ var (
 		0xda, 0x36, 0xbf, 0x22, 0xa6, 0x15, 0x1d, 0x22,
 		0xed, 0x0d, 0xed, 0x6b, 0xc6, 0x70, 0x19, 0x4f,
 	}
-	// genY is the y-coordintate of the generator of Goldilocks curve.
+	// genY is the y-coordinate of the generator of Goldilocks curve.
 	genY = fp.Elt{
 		0x14, 0xfa, 0x30, 0xf2, 0x5b, 0x79, 0x08, 0x98,
 		0xad, 0xc8, 0xd7, 0x4e, 0x2c, 0x13, 0xbd, 0xfd,

ecc/goldilocks/decaf.go

@@ -54,53 +54,51 @@ func (e *Elt) IsEqual(a *Elt) bool {
 }
 
 // UnmarshalBinary if succeeds returns a Decaf element by decoding the first
-// DecafEncodingSize bytes of b.
-func (e *Elt) UnmarshalBinary(b []byte) error {
-	if len(b) < DecafEncodingSize {
+// DecafEncodingSize bytes of data.
+func (e *Elt) UnmarshalBinary(data []byte) error {
+	if len(data) < DecafEncodingSize {
 		return errInvalidDecoding
 	}
 
 	s := &fp.Elt{}
-	copy(s[:], b[:DecafEncodingSize])
-	isNeg := fp.Parity(s)
-	modulus := fp.P()
-	if isNeg == 1 || !isLessThan(b[:DecafEncodingSize], modulus[:]) {
-		return errInvalidDecoding
-	}
+	copy(s[:], data[:DecafEncodingSize])
+	isPositiveS := fp.Parity(s) == 0
+	p := fp.P()
+	isLessThanP := isLessThan(s[:], p[:])
 
-	one, s2, den, num := &fp.Elt{}, &fp.Elt{}, &fp.Elt{}, &fp.Elt{}
+	s2, den, num := &fp.Elt{}, &fp.Elt{}, &fp.Elt{}
 	isr, altx := &fp.Elt{}, &fp.Elt{}
 	t0, t1 := &fp.Elt{}, &fp.Elt{}
-	fp.SetOne(one)
-	fp.Sqr(s2, s)                    // s2  = s^2
-	fp.Sub(den, one, s2)             // den = 1 + a*s^2
-	fp.Mul(t1, s2, &paramDTwist)     // t1 = d*s^2
-	fp.Add(t1, t1, t1)               // t1 = 2*d*s^2
-	fp.Add(t1, t1, t1)               // t1 = 4*d*s^2
-	fp.Sqr(t0, den)                  // num = (1 + a*s^2)^2
-	fp.Sub(num, t0, t1)              // num = (1 + a*s^2)^2 - 4*d*s^2
-	fp.Mul(t0, t0, num)              // t0 = num*den^2
-	isQR := fp.InvSqrt(isr, one, t0) // v = 1/sqrt(num*den^2)
-	if !isQR {
-		return errInvalidDecoding
-	}
+	x, y := &fp.Elt{}, &fp.Elt{}
+	one := fp.One()
+	fp.Sqr(s2, s)                       // s2  = s^2
+	fp.Sub(den, &one, s2)               // den = 1 + a*s^2
+	fp.Mul(t1, s2, &paramDTwist)        // t1 = d*s^2
+	fp.Add(t1, t1, t1)                  // t1 = 2*d*s^2
+	fp.Add(t1, t1, t1)                  // t1 = 4*d*s^2
+	fp.Sqr(t0, den)                     // num = (1 + a*s^2)^2
+	fp.Sub(num, t0, t1)                 // num = (1 + a*s^2)^2 - 4*d*s^2
+	fp.Mul(t0, t0, num)                 // t0 = num*den^2
+	isQR := fp.InvSqrt(isr, &one, t0)   // v = 1/sqrt(num*den^2)
 	fp.Mul(t1, den, isr)                // altx = isr*den
 	fp.Mul(t1, t1, s)                   // altx = s*isr*den
 	fp.Add(t1, t1, t1)                  // t1   = 2*s*isr*den
 	fp.Mul(altx, t1, &sqrtAMinusDTwist) // altx = 2*s*isr*den*sqrt(A-D)
-	isNeg = fp.Parity(altx)             // isNeg = sgn(altx)
+	isNegX := fp.Parity(altx)           // isNeg = sgn(altx)
 	fp.Neg(t0, isr)                     // t0 = -isr
-	fp.Cmov(isr, t0, uint(isNeg))       // if altx is negative then isr = -isr
-	fp.Sqr(&e.p.x, isr)                 // x = isr^2
-	fp.Mul(&e.p.x, &e.p.x, den)         // x = isr^2*den
-	fp.Mul(&e.p.x, &e.p.x, num)         // x = isr^2*den*num
-	fp.Mul(&e.p.x, &e.p.x, s)           // x = s*isr^2*den*num
-	fp.Add(&e.p.x, &e.p.x, &e.p.x)      // x = 2*s*isr^2*den*num
-	fp.Mul(&e.p.y, isr, den)            // y = isr*den
-	fp.Add(t0, one, s2)                 // t0 = 1 - a*s^2
-	fp.Mul(&e.p.y, &e.p.y, t0)          // y = (1 - a*s^2)*isr*den
-	e.p.ta, e.p.tb = e.p.x, e.p.y       // T = Ta*Tb = x*y
-	fp.SetOne(&e.p.z)
+	fp.Cmov(isr, t0, uint(isNegX))      // if altx is negative then isr = -isr
+	fp.Sqr(x, isr)                      // x = isr^2
+	fp.Mul(x, x, den)                   // x = isr^2*den
+	fp.Mul(x, x, num)                   // x = isr^2*den*num
+	fp.Mul(x, x, s)                     // x = s*isr^2*den*num
+	fp.Add(x, x, x)                     // x = 2*s*isr^2*den*num
+	fp.Mul(y, isr, den)                 // y = isr*den
+	fp.Add(t0, &one, s2)                // t0 = 1 - a*s^2
+	fp.Mul(y, y, t0)                    // y = (1 - a*s^2)*isr*den
+	if !(isPositiveS && isLessThanP && isQR) {
+		return errInvalidDecoding
+	}
+	e.p.x, e.p.y, e.p.ta, e.p.tb, e.p.z = *x, *y, *x, *y, one
 	return nil
 }
 

ecc/goldilocks/decaf_test.go

@@ -100,6 +100,25 @@ func TestDecafv1_1(t *testing.T) {
 	}
 }
 
+func TestDecafRandom(t *testing.T) {
+	const testTimes = 1 << 10
+	var e goldilocks.Elt
+	var enc [goldilocks.DecafEncodingSize]byte
+
+	for i := 0; i < testTimes; i++ {
+		for found := false; !found; {
+			_, _ = rand.Read(enc[:])
+			err := e.UnmarshalBinary(enc[:])
+			found = err == nil
+		}
+		got, err := e.MarshalBinary()
+		want := enc[:]
+		if err != nil || !bytes.Equal(got, want) {
+			test.ReportError(t, got, want, enc)
+		}
+	}
+}
+
 func BenchmarkDecaf(b *testing.B) {
 	var d goldilocks.Decaf
 	var k, l goldilocks.Scalar

ecc/goldilocks/point.go

@@ -1,7 +1,6 @@
 package goldilocks
 
 import (
-	"errors"
 	"fmt"
 
 	fp "github.com/cloudflare/circl/math/fp448"
@@ -14,14 +13,14 @@ func (P *Point) String() string {
 	return fmt.Sprintf("x: %v\ny: %v\nz: %v\nta: %v\ntb: %v", P.x, P.y, P.z, P.ta, P.tb)
 }
 
-// FromAffine creates a point from affine coordinates.
-func FromAffine(x, y *fp.Elt) (*Point, error) {
-	P := &Point{x: *x, y: *y, z: fp.One(), ta: *x, tb: *y}
-	if !(Curve{}).IsOnCurve(P) {
-		return nil, errors.New("point not on curve")
-	}
-	return P, nil
-}
+// // FromAffine creates a point from affine coordinates.
+// func FromAffine(x, y *fp.Elt) (*Point, error) {
+// 	P := &Point{x: *x, y: *y, z: fp.One(), ta: *x, tb: *y}
+// 	if !(Curve{}).IsOnCurve(P) {
+// 		return nil, errors.New("point not on curve")
+// 	}
+// 	return P, nil
+// }
 
 // isLessThan returns true if 0 <= x < y, and assumes that slices are of the
 // same length and are interpreted in little-endian order.
@@ -108,38 +107,32 @@ func (P *Point) Add(Q *Point) {
 }
 
 // UnmarshalBinary if succeeds constructs a point by decoding the first
-// CurveEncodingSize bytes of in.
-func (P *Point) UnmarshalBinary(in []byte) error {
-	if len(in) < CurveEncodingSize {
+// CurveEncodingSize bytes of data.
+func (P *Point) UnmarshalBinary(data []byte) error {
+	if len(data) < CurveEncodingSize {
 		return errInvalidDecoding
 	}
-	signX := in[fp.Size] >> 7
-	copy(P.y[:], in[:fp.Size])
+
+	x, y := &fp.Elt{}, &fp.Elt{}
+	signX := data[fp.Size] >> 7
+	copy(y[:], data[:fp.Size])
 	p := fp.P()
-	if !isLessThan(P.y[:], p[:]) {
-		return errInvalidDecoding
-	}
+	isLessThanP := isLessThan(y[:], p[:])
 
 	u, v := &fp.Elt{}, &fp.Elt{}
 	one := fp.One()
-	fp.Sqr(u, &P.y)                // u = y^2
-	fp.Mul(v, u, &paramD)          // v = dy^2
-	fp.Sub(u, u, &one)             // u = y^2-1
-	fp.Sub(v, v, &one)             // v = dy^2-1
-	isQR := fp.InvSqrt(&P.x, u, v) // x = sqrt(u/v)
-	if !isQR {
+	fp.Sqr(u, y)                // u = y^2
+	fp.Mul(v, u, &paramD)       // v = dy^2
+	fp.Sub(u, u, &one)          // u = y^2-a
+	fp.Sub(v, v, &one)          // v = dy^2-a
+	isQR := fp.InvSqrt(x, u, v) // x = sqrt(u/v)
+	isValidXSign := !(fp.IsZero(x) && signX == 1)
+	fp.Neg(u, x)                        // u = -x
+	fp.Cmov(x, u, uint(signX^(x[0]&1))) // if signX != x mod 2
+	if !(isLessThanP && isQR && isValidXSign) {
 		return errInvalidDecoding
 	}
-	fp.Modp(&P.x) // x = x mod p
-	if fp.IsZero(&P.x) && signX == 1 {
-		return errInvalidDecoding
-	}
-	if signX != (P.x[0] & 1) {
-		fp.Neg(&P.x, &P.x)
-	}
-	P.ta = P.x
-	P.tb = P.y
-	P.z = fp.One()
+	P.x, P.y, P.ta, P.tb, P.z = *x, *y, *x, *y, one
 	return nil
 }
 

ecc/goldilocks/point_test.go

@@ -58,18 +58,6 @@ func TestPointNeg(t *testing.T) {
 	}
 }
 
-func TestPointAffine(t *testing.T) {
-	const testTimes = 1 << 10
-	for i := 0; i < testTimes; i++ {
-		got := randomPoint()
-		x, y := got.ToAffine()
-		want, err := goldilocks.FromAffine(&x, &y)
-		if !got.IsEqual(want) || err != nil {
-			test.ReportError(t, got, want)
-		}
-	}
-}
-
 func TestPointMarshal(t *testing.T) {
 	const testTimes = 1 << 10
 	var want error

sign/ed448/ed448.go

@@ -98,7 +98,10 @@ func NewKeyFromSeed(seed PrivateKey) *KeyPair {
 	copy(pair.private[:], seed)
 	P := &goldilocks.Point{}
 	goldilocks.Curve{}.ScalarBaseMult(P, s)
-	enc, _ := P.MarshalBinary()
+	enc, err := P.MarshalBinary()
+	if err != nil {
+		panic(err)
+	}
 	copy(pair.public[:], enc)
 	return pair
 }
@@ -194,7 +197,10 @@ func Verify(public PublicKey, message, context, signature []byte) bool {
 	P.Neg()
 	Q := &goldilocks.Point{}
 	goldilocks.Curve{}.CombinedMult(Q, S, k, P)
-	encR, _ := Q.MarshalBinary()
+	encR, err := Q.MarshalBinary()
+	if err != nil {
+		return false
+	}
 	return bytes.Equal(R, encR)
 }