crypto/rsa: check PrivateKey.D for consistency with Dp and Dq

This unfortunately nearly doubles the runtime of
NewPrivateKeyWithPrecomputation. It would be nice to find an alternative
way to check it.

fips140: off
goos: darwin
goarch: arm64
pkg: crypto/rsa
cpu: Apple M2
                            │ 6aeb841faf  │             62ec3e34f3              │
                            │   sec/op    │    sec/op     vs base               │
ParsePKCS8PrivateKey/2048-8   70.28µ ± 0%   116.16µ ± 0%  +65.28% (p=0.002 n=6)

Fixes #74115

Change-Id: I6a6a6964091817d9aee359cc48932167e55184b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/687836
Auto-Submit: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Daniel McCarney <daniel@binaryparadox.net>
Reviewed-by: Mark Freeman <markfreeman@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>

diff --git a/doc/next/6-stdlib/99-minor/crypto/rsa/74115.md b/doc/next/6-stdlib/99-minor/crypto/rsa/74115.md
index 0cda96866309857d1f4b531df8bc06c1f7159d45..a3647a79f2efd8b67d9fd309976e8ba8b84d9f76 100644 (file)
--- a/doc/next/6-stdlib/99-minor/crypto/rsa/74115.md
+++ b/doc/next/6-stdlib/99-minor/crypto/rsa/74115.md
@@ -1,2 +1,5 @@
 If [PrivateKey] fields are modified after calling [PrivateKey.Precompute],
 [PrivateKey.Validate] now fails.
+
+[PrivateKey.D] is now checked for consistency with precomputed values, even if
+it is not used.

diff --git a/src/crypto/internal/fips140/rsa/rsa.go b/src/crypto/internal/fips140/rsa/rsa.go
index 0bbf7010506107da359f38727e1e9489ecd9c753..c5f9b1c9ff126b19867b6878124b713792c8e274 100644 (file)
--- a/src/crypto/internal/fips140/rsa/rsa.go
+++ b/src/crypto/internal/fips140/rsa/rsa.go
@@ -233,8 +233,7 @@ func checkPrivateKey(priv *PrivateKey) error {
        // It also implies that a^de ≡ a mod p as a^(p-1) ≡ 1 mod p. Thus a^de ≡ a
        // mod n for all a coprime to n, as required.
        //
-       // This checks dP, dQ, and e. We don't check d because it is not actually
-       // used in the RSA private key operation.
+       // This checks dP, dQ, and e.
        pMinus1, err := bigmod.NewModulus(p.Nat().SubOne(p).Bytes(p))
        if err != nil {
                return errors.New("crypto/rsa: invalid prime")
@@ -274,6 +273,17 @@ func checkPrivateKey(priv *PrivateKey) error {
                return errors.New("crypto/rsa: invalid CRT coefficient")
        }
 
+       // Check d against dP and dQ, even though we never actually use d,
+       // to make sure the key is consistent.
+       dP1 := bigmod.NewNat().Mod(priv.d, pMinus1)
+       if dP1.Equal(dP) != 1 {
+               return errors.New("crypto/rsa: d does not match dP")
+       }
+       dQ1 := bigmod.NewNat().Mod(priv.d, qMinus1)
+       if dQ1.Equal(dQ) != 1 {
+               return errors.New("crypto/rsa: d does not match dQ")
+       }
+
        // Check that |p - q| > 2^(nlen/2 - 100).
        //
        // If p and q are very close to each other, then N=pq can be trivially

diff --git a/src/crypto/rsa/rsa_test.go b/src/crypto/rsa/rsa_test.go
index e919479a10258802c3c0ba3bbde250bb61127d91..f438b3696db07db2e34553a815ecaeb2f7b1096f 100644 (file)
--- a/src/crypto/rsa/rsa_test.go
+++ b/src/crypto/rsa/rsa_test.go
@@ -1244,6 +1244,22 @@ func TestModifiedPrivateKey(t *testing.T) {
                })
        })
 
+       t.Run("D+2", func(t *testing.T) {
+               testModifiedPrivateKey(t, func(k *PrivateKey) {
+                       k.D = new(big.Int).Add(k.D, big.NewInt(2))
+               })
+       })
+       t.Run("D=0", func(t *testing.T) {
+               testModifiedPrivateKey(t, func(k *PrivateKey) {
+                       k.D = new(big.Int)
+               })
+       })
+       t.Run("D is nil", func(t *testing.T) {
+               testModifiedPrivateKey(t, func(k *PrivateKey) {
+                       k.D = nil
+               })
+       })
+
        t.Run("N+2", func(t *testing.T) {
                testModifiedPrivateKey(t, func(k *PrivateKey) {
                        k.N = new(big.Int).Add(k.N, big.NewInt(2))