diff --git a/goodkey/good_key.go b/goodkey/good_key.go index d8efd703ddc..fe99aa80e59 100644 --- a/goodkey/good_key.go +++ b/goodkey/good_key.go @@ -34,8 +34,15 @@ var smallPrimeInts = []int64{ // singleton defines the object of a Singleton pattern var ( + // These are lazily initialized on first use to a value that can be used to + // quickly compute whether an RSA key has a small prime as one of its factors. smallPrimesSingleton sync.Once smallPrimesProduct *big.Int + + // These are lazily initialized on first use to a map that can be used to + // quickly check whether an ECDSA key has a small private scalar. + smallScalarsSingleton sync.Once + smallScalarXs map[*elliptic.CurveParams]map[string]struct{} ) type Config struct { @@ -180,9 +187,40 @@ func (policy *KeyPolicy) goodKeyECDSA(key *ecdsa.PublicKey) (err error) { return err } - // Key validation routine adapted from NIST SP800-56A § 5.6.2.3.2. - // - // + // Check compliance with NIST SP800-56A Section 5.6.2.3.2 + err = policy.nistSP800(key) + if err != nil { + return err + } + + // Check for small private scalars. + err = checkSmallPrivateScalar(key) + if err != nil { + return err + } + + return nil +} + +// GoodCurve determines if an elliptic curve meets our requirements. +func (policy *KeyPolicy) goodCurve(c elliptic.Curve) (err error) { + // Simply use a whitelist for now. + params := c.Params() + switch { + case policy.allowedKeys.ECDSAP256 && params == elliptic.P256().Params(): + return nil + case policy.allowedKeys.ECDSAP384 && params == elliptic.P384().Params(): + return nil + case policy.allowedKeys.ECDSAP521 && params == elliptic.P521().Params(): + return nil + default: + return badKey("ECDSA curve %v not allowed", params.Name) + } +} + +// nistSP800 implements the key validation routine adapted from NIST SP800-56A § 5.6.2.3.2. +// +func (policy *KeyPolicy) nistSP800(key *ecdsa.PublicKey) error { // Assuming a prime field since a) we are only allowing such curves and b) // crypto/elliptic only supports prime curves. Where this assumption // simplifies the code below, it is explicitly stated and explained. If ever @@ -267,20 +305,41 @@ func isPointAtInfinityNISTP(x, y *big.Int) bool { return x.Sign() == 0 && y.Sign() == 0 } -// GoodCurve determines if an elliptic curve meets our requirements. -func (policy *KeyPolicy) goodCurve(c elliptic.Curve) (err error) { - // Simply use a whitelist for now. - params := c.Params() - switch { - case policy.allowedKeys.ECDSAP256 && params == elliptic.P256().Params(): - return nil - case policy.allowedKeys.ECDSAP384 && params == elliptic.P384().Params(): - return nil - case policy.allowedKeys.ECDSAP521 && params == elliptic.P521().Params(): - return nil - default: - return badKey("ECDSA curve %v not allowed", params.Name) +// checkSmallPrivateScalar checks to see if the ECDSA private key can be +// trivially recovered by brute-forcing small values of the private scalar. +// +// On first run we precompute the x-coordinates of k*G for all small k on each +// supported curve. On each subsequent run, we simply do a lookup of the key's +// x-coordinate in that set. +// +// This function must only be called on keys which have already passed goodCurve +// and nistSP800, as it won't catch small private scalars on other curves or +// with bad encodings. Note that this function does use deprecated operations +// (curve.Add and key.X). These are deprecated because they are unsafe; using +// them to catch keys which were generated unsafely is an appropriate use. +func checkSmallPrivateScalar(key *ecdsa.PublicKey) error { + maxSmallScalar := 255 + + smallScalarsSingleton.Do(func() { + smallScalarXs = make(map[*elliptic.CurveParams]map[string]struct{}) + for _, curve := range []elliptic.Curve{elliptic.P256(), elliptic.P384(), elliptic.P521()} { + params := curve.Params() + xs := make(map[string]struct{}, maxSmallScalar) + x, y := params.Gx, params.Gy + for k := 1; k <= maxSmallScalar; k++ { + xs[string(x.Bytes())] = struct{}{} + x, y = curve.Add(x, y, params.Gx, params.Gy) + } + smallScalarXs[params] = xs + } + }) + + _, found := smallScalarXs[key.Params()][string(key.X.Bytes())] + if found { + return badKey("key has a small private scalar and is trivially recoverable") } + + return nil } // GoodKeyRSA determines if a RSA pubkey meets our requirements diff --git a/goodkey/good_key_test.go b/goodkey/good_key_test.go index 133b6ac11ef..da9270757e8 100644 --- a/goodkey/good_key_test.go +++ b/goodkey/good_key_test.go @@ -244,6 +244,53 @@ func TestECDSAIdentity(t *testing.T) { } } +func TestECDSASmallPrivateScalar(t *testing.T) { + for _, curve := range validCurves { + n := curve.Params().N + testCases := []struct { + name string + k *big.Int + wantErr bool + }{ + // Boundary conditions. + {name: "1", k: big.NewInt(1), wantErr: true}, + {name: "255", k: big.NewInt(255), wantErr: true}, + {name: "256", k: big.NewInt(256), wantErr: false}, + {name: "n-1", k: new(big.Int).Sub(n, big.NewInt(1)), wantErr: true}, + {name: "n-255", k: new(big.Int).Sub(n, big.NewInt(255)), wantErr: true}, + {name: "n-256", k: new(big.Int).Sub(n, big.NewInt(256)), wantErr: false}, + // Known-compromised keys. + {name: "4", k: big.NewInt(4), wantErr: true}, + {name: "56", k: big.NewInt(56), wantErr: true}, + {name: "66", k: big.NewInt(66), wantErr: true}, + {name: "115", k: big.NewInt(115), wantErr: true}, + {name: "155", k: big.NewInt(155), wantErr: true}, + {name: "156", k: big.NewInt(156), wantErr: true}, + {name: "230", k: big.NewInt(230), wantErr: true}, + {name: "237", k: big.NewInt(237), wantErr: true}, + } + for _, tc := range testCases { + t.Run(fmt.Sprintf("%s/k=%s", curve.Params().Name, tc.name), func(t *testing.T) { + x, y := curve.ScalarBaseMult(tc.k.Bytes()) + err := testingPolicy.GoodKey(t.Context(), &ecdsa.PublicKey{Curve: curve, X: x, Y: y}) + if !tc.wantErr { + if err != nil { + t.Errorf("GoodKey() = %q, want nil", err) + } + return + } + if err == nil { + t.Fatalf("GoodKey() = nil, want error for small private scalar") + } + want := "key has a small private scalar and is trivially recoverable" + if err.Error() != want { + t.Errorf("GoodKey() = %q, want %q", err, want) + } + }) + } + } +} + func TestNonRefKey(t *testing.T) { private, err := rsa.GenerateKey(rand.Reader, 2048) test.AssertNotError(t, err, "Error generating key")