Skip to content

Commit ce39174

Browse files
FiloSottilegopherbot
authored andcommitted
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 golang#74115 Change-Id: I6a6a6964091817d9aee359cc48932167e55184b9 Reviewed-on: https://go-review.googlesource.com/c/go/+/687836 Auto-Submit: Filippo Valsorda <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Daniel McCarney <[email protected]> Reviewed-by: Mark Freeman <[email protected]> Reviewed-by: Michael Pratt <[email protected]>
1 parent 5d9d051 commit ce39174

File tree

3 files changed

+31
-2
lines changed

3 files changed

+31
-2
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,5 @@
11
If [PrivateKey] fields are modified after calling [PrivateKey.Precompute],
22
[PrivateKey.Validate] now fails.
3+
4+
[PrivateKey.D] is now checked for consistency with precomputed values, even if
5+
it is not used.

src/crypto/internal/fips140/rsa/rsa.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,7 @@ func checkPrivateKey(priv *PrivateKey) error {
233233
// It also implies that a^de ≡ a mod p as a^(p-1) ≡ 1 mod p. Thus a^de ≡ a
234234
// mod n for all a coprime to n, as required.
235235
//
236-
// This checks dP, dQ, and e. We don't check d because it is not actually
237-
// used in the RSA private key operation.
236+
// This checks dP, dQ, and e.
238237
pMinus1, err := bigmod.NewModulus(p.Nat().SubOne(p).Bytes(p))
239238
if err != nil {
240239
return errors.New("crypto/rsa: invalid prime")
@@ -274,6 +273,17 @@ func checkPrivateKey(priv *PrivateKey) error {
274273
return errors.New("crypto/rsa: invalid CRT coefficient")
275274
}
276275

276+
// Check d against dP and dQ, even though we never actually use d,
277+
// to make sure the key is consistent.
278+
dP1 := bigmod.NewNat().Mod(priv.d, pMinus1)
279+
if dP1.Equal(dP) != 1 {
280+
return errors.New("crypto/rsa: d does not match dP")
281+
}
282+
dQ1 := bigmod.NewNat().Mod(priv.d, qMinus1)
283+
if dQ1.Equal(dQ) != 1 {
284+
return errors.New("crypto/rsa: d does not match dQ")
285+
}
286+
277287
// Check that |p - q| > 2^(nlen/2 - 100).
278288
//
279289
// If p and q are very close to each other, then N=pq can be trivially

src/crypto/rsa/rsa_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,6 +1244,22 @@ func TestModifiedPrivateKey(t *testing.T) {
12441244
})
12451245
})
12461246

1247+
t.Run("D+2", func(t *testing.T) {
1248+
testModifiedPrivateKey(t, func(k *PrivateKey) {
1249+
k.D = new(big.Int).Add(k.D, big.NewInt(2))
1250+
})
1251+
})
1252+
t.Run("D=0", func(t *testing.T) {
1253+
testModifiedPrivateKey(t, func(k *PrivateKey) {
1254+
k.D = new(big.Int)
1255+
})
1256+
})
1257+
t.Run("D is nil", func(t *testing.T) {
1258+
testModifiedPrivateKey(t, func(k *PrivateKey) {
1259+
k.D = nil
1260+
})
1261+
})
1262+
12471263
t.Run("N+2", func(t *testing.T) {
12481264
testModifiedPrivateKey(t, func(k *PrivateKey) {
12491265
k.N = new(big.Int).Add(k.N, big.NewInt(2))

0 commit comments

Comments
 (0)