crypto/elliptic: upgrade from generic curve impl to specific if available

This change alters the CurveParam methods to upgrade from the generic
curve implementation to the specific P224 or P256 implementations when
called on the embedded CurveParams. This removes the trap of using
elliptic.P224().Params() instead of elliptic.P224(), for example, which
results in using the generic implementation instead of the optimized
constant time one. For P224 this is done for all of the CurveParams
methods, except Params, as the optimized implementation covers all
these methods. For P256 this is only done for ScalarMult and
ScalarBaseMult, as despite having implementations of addition and
doubling they aren't exposed and instead the generic implementation is
used. For P256 an additional check that there actually is a specific
implementation is added, as unlike the P224 implementation the P256 one
is only available on certain platforms.

This change takes the simple, fast approach to checking this, it simply
compares pointers. This removes the most obvious class of mistakes
people make, but still allows edge cases where the embedded CurveParams
pointer has been dereferenced (as seen in the unit tests) or when someone
has manually constructed their own CurveParams that matches one of the
standard curves. A more complex approach could be taken to also address
these cases, but it would require directly comparing all of the
CurveParam fields which would, in the worst case, require comparing
against two standard CurveParam sets in the ScalarMult and
ScalarBaseMult paths, which are likely to be the hottest already.

Updates #34648

Change-Id: I82d752f979260394632905c15ffe4f65f4ffa376
Reviewed-on: https://go-review.googlesource.com/c/go/+/233939
Trust: Roland Shoemaker <roland@golang.org>
Trust: Katie Hockman <katie@golang.org>
Run-TryBot: Roland Shoemaker <roland@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>

diff --git a/src/crypto/elliptic/elliptic.go b/src/crypto/elliptic/elliptic.go
index 85d105419b431383bd1ddfb047c1308ba0458053..b8e5a3097d261f2312e6042cb72a94be371475f1 100644 (file)
--- a/src/crypto/elliptic/elliptic.go
+++ b/src/crypto/elliptic/elliptic.go
@@ -40,6 +40,15 @@ type Curve interface {
        ScalarBaseMult(k []byte) (x, y *big.Int)
 }
 
+func matchesSpecificCurve(params *CurveParams, available ...Curve) (Curve, bool) {
+       for _, c := range available {
+               if params == c.Params() {
+                       return c, true
+               }
+       }
+       return nil, false
+}
+
 // CurveParams contains the parameters of an elliptic curve and also provides
 // a generic, non-constant time implementation of Curve.
 type CurveParams struct {
@@ -71,6 +80,12 @@ func (curve *CurveParams) polynomial(x *big.Int) *big.Int {
 }
 
 func (curve *CurveParams) IsOnCurve(x, y *big.Int) bool {
+       // If there is a dedicated constant-time implementation for this curve operation,
+       // use that instead of the generic one.
+       if specific, ok := matchesSpecificCurve(curve, p224, p521); ok {
+               return specific.IsOnCurve(x, y)
+       }
+
        // y² = x³ - 3x + b
        y2 := new(big.Int).Mul(y, y)
        y2.Mod(y2, curve.P)
@@ -108,6 +123,12 @@ func (curve *CurveParams) affineFromJacobian(x, y, z *big.Int) (xOut, yOut *big.
 }
 
 func (curve *CurveParams) Add(x1, y1, x2, y2 *big.Int) (*big.Int, *big.Int) {
+       // If there is a dedicated constant-time implementation for this curve operation,
+       // use that instead of the generic one.
+       if specific, ok := matchesSpecificCurve(curve, p224, p521); ok {
+               return specific.Add(x1, y1, x2, y2)
+       }
+
        z1 := zForAffine(x1, y1)
        z2 := zForAffine(x2, y2)
        return curve.affineFromJacobian(curve.addJacobian(x1, y1, z1, x2, y2, z2))
@@ -192,6 +213,12 @@ func (curve *CurveParams) addJacobian(x1, y1, z1, x2, y2, z2 *big.Int) (*big.Int
 }
 
 func (curve *CurveParams) Double(x1, y1 *big.Int) (*big.Int, *big.Int) {
+       // If there is a dedicated constant-time implementation for this curve operation,
+       // use that instead of the generic one.
+       if specific, ok := matchesSpecificCurve(curve, p224, p521); ok {
+               return specific.Double(x1, y1)
+       }
+
        z1 := zForAffine(x1, y1)
        return curve.affineFromJacobian(curve.doubleJacobian(x1, y1, z1))
 }
@@ -258,6 +285,12 @@ func (curve *CurveParams) doubleJacobian(x, y, z *big.Int) (*big.Int, *big.Int,
 }
 
 func (curve *CurveParams) ScalarMult(Bx, By *big.Int, k []byte) (*big.Int, *big.Int) {
+       // If there is a dedicated constant-time implementation for this curve operation,
+       // use that instead of the generic one.
+       if specific, ok := matchesSpecificCurve(curve, p224, p256, p521); ok {
+               return specific.ScalarMult(Bx, By, k)
+       }
+
        Bz := new(big.Int).SetInt64(1)
        x, y, z := new(big.Int), new(big.Int), new(big.Int)
 
@@ -275,6 +308,12 @@ func (curve *CurveParams) ScalarMult(Bx, By *big.Int, k []byte) (*big.Int, *big.
 }
 
 func (curve *CurveParams) ScalarBaseMult(k []byte) (*big.Int, *big.Int) {
+       // If there is a dedicated constant-time implementation for this curve operation,
+       // use that instead of the generic one.
+       if specific, ok := matchesSpecificCurve(curve, p224, p256, p521); ok {
+               return specific.ScalarBaseMult(k)
+       }
+
        return curve.ScalarMult(curve.Gx, curve.Gy, k)
 }
 

diff --git a/src/crypto/elliptic/elliptic_test.go b/src/crypto/elliptic/elliptic_test.go
index 0d43b736f9ae5dac936d26b467d8c442b2a7f156..183861a54b52fe2678f9165b171a38b471e9b40f 100644 (file)
--- a/src/crypto/elliptic/elliptic_test.go
+++ b/src/crypto/elliptic/elliptic_test.go
@@ -12,19 +12,29 @@ import (
        "testing"
 )
 
+// genericParamsForCurve returns the dereferenced CurveParams for
+// the specified curve. This is used to avoid the logic for
+// upgrading a curve to it's specific implementation, forcing
+// usage of the generic implementation. This is only relevant
+// for the P224, P256, and P521 curves.
+func genericParamsForCurve(c Curve) *CurveParams {
+       d := *(c.Params())
+       return &d
+}
+
 func testAllCurves(t *testing.T, f func(*testing.T, Curve)) {
        tests := []struct {
                name  string
                curve Curve
        }{
                {"P256", P256()},
-               {"P256/Params", P256().Params()},
+               {"P256/Params", genericParamsForCurve(P256())},
                {"P224", P224()},
-               {"P224/Params", P224().Params()},
+               {"P224/Params", genericParamsForCurve(P224())},
                {"P384", P384()},
-               {"P384/Params", P384().Params()},
+               {"P384/Params", genericParamsForCurve(P384())},
                {"P521", P521()},
-               {"P521/Params", P521().Params()},
+               {"P521/Params", genericParamsForCurve(P521())},
        }
        if testing.Short() {
                tests = tests[:1]

diff --git a/src/crypto/elliptic/p256_asm.go b/src/crypto/elliptic/p256_asm.go
index 08dbd2ea54807d9f5d03d66b31fa1a4300cd86fa..9a808f260ac09535d488dfa390b61584506beba4 100644 (file)
--- a/src/crypto/elliptic/p256_asm.go
+++ b/src/crypto/elliptic/p256_asm.go
@@ -29,9 +29,7 @@ type (
        }
 )
 
-var (
-       p256 p256Curve
-)
+var p256 p256Curve
 
 func initP256() {
        // See FIPS 186-3, section D.2.3

diff --git a/src/crypto/elliptic/p256_generic.go b/src/crypto/elliptic/p256_generic.go
index 8ad56638e9519cc6da679a47221b8ddcfb7c00d1..25762a8f768b3b4825c0e59de47ffac0b68e9da8 100644 (file)
--- a/src/crypto/elliptic/p256_generic.go
+++ b/src/crypto/elliptic/p256_generic.go
@@ -7,9 +7,7 @@
 
 package elliptic
 
-var (
-       p256 p256Curve
-)
+var p256 p256Curve
 
 func initP256Arch() {
        // Use pure Go implementation.