crypto/bn256/google: fix receiver aliasing in point doubling

crypto/bn256/google/alias_test.go

@@ -0,0 +1,131 @@
+// Copyright 2012 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package bn256
+
+import (
+	"bytes"
+	"crypto/rand"
+	"math/big"
+	"testing"
+)
+
+// TestG1AddAliasing checks that G1.Add tolerates a receiver that aliases one of
+// its operands. The case that used to break is a+b with a and b the same point:
+// Add dispatches to curvePoint.Double, which computed z3 = 2*y1*z1 after it had
+// already overwritten c.y, so an aliased receiver fed it the wrong y1.
+func TestG1AddAliasing(t *testing.T) {
+	a := new(G1).ScalarBaseMult(big.NewInt(7))
+	b := new(G1).ScalarBaseMult(big.NewInt(11))
+
+	doubled := new(G1).ScalarMult(a, big.NewInt(2)).Marshal()
+	sum := new(G1).Add(a, b).Marshal()
+
+	got := new(G1).ScalarBaseMult(big.NewInt(7))
+	if got.Add(got, got); !bytes.Equal(got.Marshal(), doubled) {
+		t.Errorf("got.Add(got, got) = %x, want %x", got.Marshal(), doubled)
+	}
+	if got = new(G1).Add(a, a); !bytes.Equal(got.Marshal(), doubled) {
+		t.Errorf("new(G1).Add(a, a) = %x, want %x", got.Marshal(), doubled)
+	}
+	got = new(G1).ScalarBaseMult(big.NewInt(7))
+	if got.Add(got, b); !bytes.Equal(got.Marshal(), sum) {
+		t.Errorf("got.Add(got, b) = %x, want %x", got.Marshal(), sum)
+	}
+	got = new(G1).ScalarBaseMult(big.NewInt(11))
+	if got.Add(a, got); !bytes.Equal(got.Marshal(), sum) {
+		t.Errorf("got.Add(a, got) = %x, want %x", got.Marshal(), sum)
+	}
+}
+
+// TestG2AddAliasing is TestG1AddAliasing for the twist.
+func TestG2AddAliasing(t *testing.T) {
+	a := new(G2).ScalarBaseMult(big.NewInt(7))
+	b := new(G2).ScalarBaseMult(big.NewInt(11))
+
+	doubled := new(G2).ScalarMult(a, big.NewInt(2)).Marshal()
+	sum := new(G2).Add(a, b).Marshal()
+
+	got := new(G2).ScalarBaseMult(big.NewInt(7))
+	if got.Add(got, got); !bytes.Equal(got.Marshal(), doubled) {
+		t.Errorf("got.Add(got, got) = %x, want %x", got.Marshal(), doubled)
+	}
+	if got = new(G2).Add(a, a); !bytes.Equal(got.Marshal(), doubled) {
+		t.Errorf("new(G2).Add(a, a) = %x, want %x", got.Marshal(), doubled)
+	}
+	got = new(G2).ScalarBaseMult(big.NewInt(7))
+	if got.Add(got, b); !bytes.Equal(got.Marshal(), sum) {
+		t.Errorf("got.Add(got, b) = %x, want %x", got.Marshal(), sum)
+	}
+	got = new(G2).ScalarBaseMult(big.NewInt(11))
+	if got.Add(a, got); !bytes.Equal(got.Marshal(), sum) {
+		t.Errorf("got.Add(a, got) = %x, want %x", got.Marshal(), sum)
+	}
+}
+
+// TestPointDoubleAliasing exercises the underlying Double and Negative directly,
+// on random points, with and without an aliased receiver.
+func TestPointDoubleAliasing(t *testing.T) {
+	pool := new(bnPool)
+	for i := 0; i < 10; i++ {
+		k, err := rand.Int(rand.Reader, Order)
+		if err != nil {
+			t.Fatal(err)
+		}
+		cp := newCurvePoint(pool).Mul(curveGen, k, pool)
+		want := newCurvePoint(pool)
+		want.Double(cp, pool)
+		want.MakeAffine(pool)
+		got := newCurvePoint(pool)
+		got.Set(cp)
+		got.Double(got, pool)
+		got.MakeAffine(pool)
+		if got.x.Cmp(want.x) != 0 || got.y.Cmp(want.y) != 0 {
+			t.Fatalf("%d: aliased curvePoint.Double = (%s, %s), want (%s, %s)", i, got.x, got.y, want.x, want.y)
+		}
+
+		want.Negative(cp)
+		want.MakeAffine(pool)
+		got.Set(cp)
+		got.Negative(got)
+		got.MakeAffine(pool)
+		if got.x.Cmp(want.x) != 0 || got.y.Cmp(want.y) != 0 {
+			t.Fatalf("%d: aliased curvePoint.Negative = (%s, %s), want (%s, %s)", i, got.x, got.y, want.x, want.y)
+		}
+
+		tp := newTwistPoint(pool).Mul(twistGen, k, pool)
+		wantT := newTwistPoint(pool)
+		wantT.Double(tp, pool)
+		wantT.MakeAffine(pool)
+		gotT := newTwistPoint(pool)
+		gotT.Set(tp)
+		gotT.Double(gotT, pool)
+		gotT.MakeAffine(pool)
+		if !twistEqual(gotT, wantT) {
+			t.Fatalf("%d: aliased twistPoint.Double disagrees with the unaliased result", i)
+		}
+
+		wantT.Negative(tp, pool)
+		wantT.MakeAffine(pool)
+		gotT.Set(tp)
+		gotT.Negative(gotT, pool)
+		gotT.MakeAffine(pool)
+		if !twistEqual(gotT, wantT) {
+			t.Fatalf("%d: aliased twistPoint.Negative disagrees with the unaliased result", i)
+		}
+	}
+}
+
+// twistEqual compares two affine twist points.
+func twistEqual(a, b *twistPoint) bool {
+	if a.IsInfinity() || b.IsInfinity() {
+		return a.IsInfinity() == b.IsInfinity()
+	}
+	a.x.Minimal()
+	a.y.Minimal()
+	b.x.Minimal()
+	b.y.Minimal()
+	return a.x.x.Cmp(b.x.x) == 0 && a.x.y.Cmp(b.x.y) == 0 &&
+		a.y.x.Cmp(b.y.x) == 0 && a.y.y.Cmp(b.y.y) == 0
+}

crypto/bn256/google/bn256.go

@@ -83,8 +83,7 @@ func (e *G1) ScalarMult(a *G1, k *big.Int) *G1 {
 	return e
 }
 
-// Add sets e to a+b and then returns e.
-// BUG(agl): this function is not complete: a==b fails.
+// Add sets e to a+b and then returns e. e may alias a or b.
 func (e *G1) Add(a, b *G1) *G1 {
 	if e.p == nil {
 		e.p = newCurvePoint(nil)
@@ -213,8 +212,7 @@ func (e *G2) ScalarMult(a *G2, k *big.Int) *G2 {
 	return e
 }
 
-// Add sets e to a+b and then returns e.
-// BUG(agl): this function is not complete: a==b fails.
+// Add sets e to a+b and then returns e. e may alias a or b.
 func (e *G2) Add(a, b *G2) *G2 {
 	if e.p == nil {
 		e.p = newTwistPoint(nil)

crypto/bn256/google/curve.go

@@ -180,6 +180,7 @@ func (c *curvePoint) Add(a, b *curvePoint, pool *bnPool) {
 	pool.Put(t6)
 }
 
+// Double sets c to 2a. c may alias a.
 func (c *curvePoint) Double(a *curvePoint, pool *bnPool) {
 	// See http://hyperelliptic.org/EFD/g1p/auto-code/shortw/jacobian-0/doubling/dbl-2009-l.op3
 	A := pool.Get().Mul(a.x, a.x)
@@ -203,6 +204,12 @@ func (c *curvePoint) Double(a *curvePoint, pool *bnPool) {
 	t.Add(d, d)
 	c.x.Sub(f, t)
 
+	// z3 = 2*y1*z1 is the last use of a.y, so it has to be computed before
+	// c.y is overwritten below: c is allowed to alias a.
+	t.Mul(a.y, a.z)
+	t.Mod(t, P)
+	c.z.Add(t, t)
+
 	t.Add(C_, C_)
 	t2.Add(t, t)
 	t.Add(t2, t2)
@@ -211,10 +218,6 @@ func (c *curvePoint) Double(a *curvePoint, pool *bnPool) {
 	t2.Mod(t2, P)
 	c.y.Sub(t2, t)
 
-	t.Mul(a.y, a.z)
-	t.Mod(t, P)
-	c.z.Add(t, t)
-
 	pool.Put(A)
 	pool.Put(B)
 	pool.Put(C_)

crypto/bn256/google/twist.go

@@ -169,6 +169,7 @@ func (c *twistPoint) Add(a, b *twistPoint, pool *bnPool) {
 	t6.Put(pool)
 }
 
+// Double sets c to 2a. c may alias a.
 func (c *twistPoint) Double(a *twistPoint, pool *bnPool) {
 	// See http://hyperelliptic.org/EFD/g1p/auto-code/shortw/jacobian-0/doubling/dbl-2009-l.op3
 	A := newGFp2(pool).Square(a.x, pool)
@@ -187,16 +188,18 @@ func (c *twistPoint) Double(a *twistPoint, pool *bnPool) {
 	t.Add(d, d)
 	c.x.Sub(f, t)
 
+	// z3 = 2*y1*z1 is the last use of a.y, so it has to be computed before
+	// c.y is overwritten below: c is allowed to alias a.
+	t.Mul(a.y, a.z, pool)
+	c.z.Add(t, t)
+
 	t.Add(C_, C_)
 	t2.Add(t, t)
 	t.Add(t2, t2)
 	c.y.Sub(d, c.x)
 	t2.Mul(e, c.y, pool)
 	c.y.Sub(t2, t)
 
-	t.Mul(a.y, a.z, pool)
-	c.z.Add(t, t)
-
 	A.Put(pool)
 	B.Put(pool)
 	C_.Put(pool)
@@ -258,8 +261,7 @@ func (c *twistPoint) MakeAffine(pool *bnPool) *twistPoint {
 
 func (c *twistPoint) Negative(a *twistPoint, pool *bnPool) {
 	c.x.Set(a.x)
-	c.y.SetZero()
-	c.y.Sub(c.y, a.y)
+	c.y.Negative(a.y)
 	c.z.Set(a.z)
 	c.t.SetZero()
 }