crypto/elliptic: move P-521 group logic to internal/nistec

This abstracts the clunky and not constant time math/big elliptic.Curve
compatibility layer away from the pure fiat-backed group logic.

Change-Id: I3b7a7495034d0c569b21c442ae36958763b8b2d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/320074
Trust: Filippo Valsorda <filippo@golang.org>
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Julie Qiu <julie@golang.org>

src/crypto/elliptic/elliptic.go

@@ -21,9 +21,12 @@ import (
 
 // A Curve represents a short-form Weierstrass curve with a=-3.
 //
-// Note that the point at infinity (0, 0) is not considered on the curve, and
-// although it can be returned by Add, Double, ScalarMult, or ScalarBaseMult, it
-// can't be marshaled or unmarshaled, and IsOnCurve will return false for it.
+// The output of Add, Double, and ScalarMult when the input is not a point on
+// the curve is undefined.
+//
+// Note that the conventional point at infinity (0, 0) is not considered on the
+// curve, although it can be returned by Add, Double, ScalarMult, or
+// ScalarBaseMult (but not Unmarshal or UnmarshalCompressed).
 type Curve interface {
 	// Params returns the parameters for the curve.
 	Params() *CurveParams

src/crypto/elliptic/elliptic_test.go

@@ -109,6 +109,15 @@ func testInfinity(t *testing.T, curve Curve) {
 	if curve.IsOnCurve(x, y) {
 		t.Errorf("IsOnCurve(∞) == true")
 	}
+
+	if xx, yy := Unmarshal(curve, Marshal(curve, x, y)); xx != nil || yy != nil {
+		t.Errorf("Unmarshal(Marshal(∞)) did not return an error")
+	}
+	// We don't test UnmarshalCompressed(MarshalCompressed(∞)) because there are
+	// two valid points with x = 0.
+	if xx, yy := Unmarshal(curve, []byte{0x00}); xx != nil || yy != nil {
+		t.Errorf("Unmarshal(∞) did not return an error")
+	}
 }
 
 func TestMarshal(t *testing.T) {
@@ -274,3 +283,29 @@ func BenchmarkScalarMult(b *testing.B) {
 		}
 	})
 }
+
+func BenchmarkMarshalUnmarshal(b *testing.B) {
+	benchmarkAllCurves(b, func(b *testing.B, curve Curve) {
+		_, x, y, _ := GenerateKey(curve, rand.Reader)
+		b.Run("Uncompressed", func(b *testing.B) {
+			b.ReportAllocs()
+			for i := 0; i < b.N; i++ {
+				buf := Marshal(curve, x, y)
+				xx, yy := Unmarshal(curve, buf)
+				if xx.Cmp(x) != 0 || yy.Cmp(y) != 0 {
+					b.Error("Unmarshal output different from Marshal input")
+				}
+			}
+		})
+		b.Run("Compressed", func(b *testing.B) {
+			b.ReportAllocs()
+			for i := 0; i < b.N; i++ {
+				buf := Marshal(curve, x, y)
+				xx, yy := Unmarshal(curve, buf)
+				if xx.Cmp(x) != 0 || yy.Cmp(y) != 0 {
+					b.Error("Unmarshal output different from Marshal input")
+				}
+			}
+		})
+	})
+}

src/crypto/elliptic/internal/fiat/p521.go

@@ -53,28 +53,40 @@ func (e *P521Element) Set(t *P521Element) *P521Element {
 	return e
 }
 
-// Bytes returns the 66-byte little-endian encoding of e.
+// Bytes returns the 66-byte big-endian encoding of e.
 func (e *P521Element) Bytes() []byte {
-	// This function must be inlined to move the allocation to the parent and
-	// save it from escaping to the heap.
+	// This function is outlined to make the allocations inline in the caller
+	// rather than happen on the heap.
 	var out [66]byte
-	p521ToBytes(&out, &e.x)
+	return e.bytes(&out)
+}
+
+func (e *P521Element) bytes(out *[66]byte) []byte {
+	p521ToBytes(out, &e.x)
+	invertEndianness(out[:])
 	return out[:]
 }
 
-// SetBytes sets e = v, where v is a little-endian 66-byte encoding, and returns
+// SetBytes sets e = v, where v is a big-endian 66-byte encoding, and returns
 // e. If v is not 66 bytes or it encodes a value higher than 2^521 - 1, SetBytes
 // returns nil and an error, and e is unchanged.
 func (e *P521Element) SetBytes(v []byte) (*P521Element, error) {
-	if len(v) != 66 || v[65] > 1 {
+	if len(v) != 66 || v[0] > 1 {
 		return nil, errors.New("invalid P-521 field encoding")
 	}
 	var in [66]byte
 	copy(in[:], v)
+	invertEndianness(in[:])
 	p521FromBytes(&e.x, &in)
 	return e, nil
 }
 
+func invertEndianness(v []byte) {
+	for i := 0; i < len(v)/2; i++ {
+		v[i], v[len(v)-1-i] = v[len(v)-1-i], v[i]
+	}
+}
+
 // Add sets e = t1 + t2, and returns e.
 func (e *P521Element) Add(t1, t2 *P521Element) *P521Element {
 	p521Add(&e.x, &t1.x, &t2.x)

src/crypto/elliptic/internal/fiat/p521_test.go

@@ -15,7 +15,7 @@ func p521Random(t *testing.T) *fiat.P521Element {
 	if _, err := rand.Read(buf); err != nil {
 		t.Fatal(err)
 	}
-	buf[65] &= 1
+	buf[0] &= 1
 	e, err := new(fiat.P521Element).SetBytes(buf)
 	if err != nil {
 		t.Fatal(err)