diff --git a/crypto/bn256/google/alias_test.go b/crypto/bn256/google/alias_test.go new file mode 100644 index 000000000000..38c5b1a0af5a --- /dev/null +++ b/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 +} diff --git a/crypto/bn256/google/bn256.go b/crypto/bn256/google/bn256.go index e427b8bf42a3..7e872ba7badd 100644 --- a/crypto/bn256/google/bn256.go +++ b/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) diff --git a/crypto/bn256/google/curve.go b/crypto/bn256/google/curve.go index 819cb81da7ab..081d8a180d4c 100644 --- a/crypto/bn256/google/curve.go +++ b/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_) diff --git a/crypto/bn256/google/twist.go b/crypto/bn256/google/twist.go index 631d1ca8df0b..3e535777314e 100644 --- a/crypto/bn256/google/twist.go +++ b/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,6 +188,11 @@ 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) @@ -194,9 +200,6 @@ func (c *twistPoint) Double(a *twistPoint, pool *bnPool) { 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() }