From ce391744828cb1e0dbd44ffb2622521a15db5b5d Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Fri, 11 Jul 2025 14:28:30 +0200 Subject: [PATCH] crypto/rsa: check PrivateKey.D for consistency with Dp and Dq MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 LUCI-TryBot-Result: Go LUCI Reviewed-by: Daniel McCarney Reviewed-by: Mark Freeman Reviewed-by: Michael Pratt --- doc/next/6-stdlib/99-minor/crypto/rsa/74115.md | 3 +++ src/crypto/internal/fips140/rsa/rsa.go | 14 ++++++++++++-- src/crypto/rsa/rsa_test.go | 16 ++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) 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 0cda968663..a3647a79f2 100644 --- 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 0bbf701050..c5f9b1c9ff 100644 --- 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 e919479a10..f438b3696d 100644 --- 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)) -- 2.52.0