crypto/x509: keep RSA CRT values in ParsePKCS1PrivateKey

Turns out that recomputing them (and qInv in particular) in constant
time is expensive, so let's not throw them away when they are available.
They are much faster to check, so we now do that on precompute.

Also, thanks to the opaque crypto/internal/fips140/rsa.PrivateKey type,
we now have some assurance that the values we use are always ones we
checked.

Recovers most of the performance loss since CL 630516 in the happy path.
Also, since now we always use the CRT, if necessary by running a
throwaway Precompute, which is now cheap if PrecomputedValues is filled
out, we effectively fixed the JSON round-trip slowdown (#59695).

goos: darwin
goarch: arm64
pkg: crypto/rsa
cpu: Apple M2
                            │ 3b42687c56  │          f017604bc6-dirty           │
                            │   sec/op    │   sec/op     vs base                │
ParsePKCS8PrivateKey/2048-8   26.76µ ± 1%   65.99µ ± 1%  +146.64% (p=0.002 n=6)

Fixes #59695
Updates #69799
For #69536

Change-Id: I507f8c5a32e69ab28990a3bf78959836b9b08cc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/632478
Auto-Submit: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
This commit is contained in:
Filippo Valsorda 2024-11-29 15:38:48 +01:00 committed by Gopher Robot
parent acd54c9985
commit c5c4f3dd5f
9 changed files with 208 additions and 34 deletions

View File

@ -208,6 +208,11 @@ X25519MLKEM768 by default. The default can be reverted using the
[`tlsmlkem` setting](/pkg/crypto/tls/#Config.CurvePreferences).
Go 1.24 also removed X25519Kyber768Draft00 and the Go 1.23 `tlskyber` setting.
Go 1.24 made [`ParsePKCS1PrivateKey`](/pkg/crypto/x509/#ParsePKCS1PrivateKey)
use and validate the CRT parameters in the encoded private key. This behavior
can be controlled with the `x509rsacrt` setting. Using `x509rsacrt=0` restores
the Go 1.23 behavior.
### Go 1.23
Go 1.23 changed the channels created by package time to be unbuffered

View File

@ -1,3 +1,7 @@
[MarshalPKCS8PrivateKey] now returns an error instead of marshaling an invalid
RSA key. ([MarshalPKCS1PrivateKey] doesn't have an error return, and its behavior
when provided invalid keys continues to be undefined.)
[ParsePKCS1PrivateKey] and [ParsePKCS8PrivateKey] now use and validate the
encoded CRT values, so might reject invalid keys that were previously accepted.
Use `GODEBUG=x509rsacrt=0` to revert to recomputing them.

View File

@ -104,6 +104,43 @@ func newPrivateKey(n *bigmod.Modulus, e int, d *bigmod.Nat, p, q *bigmod.Modulus
return pk, nil
}
// NewPrivateKeyWithPrecomputation creates a new RSA private key from the given
// parameters, which include precomputed CRT values.
func NewPrivateKeyWithPrecomputation(N []byte, e int, d, P, Q, dP, dQ, qInv []byte) (*PrivateKey, error) {
n, err := bigmod.NewModulus(N)
if err != nil {
return nil, err
}
p, err := bigmod.NewModulus(P)
if err != nil {
return nil, err
}
q, err := bigmod.NewModulus(Q)
if err != nil {
return nil, err
}
dN, err := bigmod.NewNat().SetBytes(d, n)
if err != nil {
return nil, err
}
qInvNat, err := bigmod.NewNat().SetBytes(qInv, p)
if err != nil {
return nil, err
}
pk := &PrivateKey{
pub: PublicKey{
N: n, E: e,
},
d: dN, p: p, q: q,
dP: dP, dQ: dQ, qInv: qInvNat,
}
if err := checkPrivateKey(pk); err != nil {
return nil, err
}
return pk, nil
}
// NewPrivateKeyWithoutCRT creates a new RSA private key from the given parameters.
//
// This is meant for deprecated multi-prime keys, and is not FIPS 140 compliant.
@ -157,37 +194,64 @@ func checkPrivateKey(priv *PrivateKey) error {
}
N := priv.pub.N
Π := bigmod.NewNat().ExpandFor(N)
for _, prime := range []*bigmod.Modulus{priv.p, priv.q} {
p := prime.Nat().ExpandFor(N)
if p.IsZero() == 1 || p.IsOne() == 1 {
return errors.New("crypto/rsa: invalid prime")
}
Π.Mul(p, N)
p := priv.p
q := priv.q
// Check that de ≡ 1 mod p-1, for each prime.
// This implies that e is coprime to each p-1 as e has a multiplicative
// inverse. Therefore e is coprime to lcm(p-1,q-1,r-1,...) =
// exponent(/n). 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.
pMinus1, err := bigmod.NewModulus(p.SubOne(N).Bytes(N))
if err != nil {
return errors.New("crypto/rsa: invalid prime")
}
e := bigmod.NewNat().SetUint(uint(priv.pub.E)).ExpandFor(pMinus1)
de := bigmod.NewNat()
de.Mod(priv.d, pMinus1)
de.Mul(e, pMinus1)
if de.IsOne() != 1 {
return errors.New("crypto/rsa: invalid exponents")
}
// Check that pq ≡ 1 mod N (and that pN < N and q < N).
pN := bigmod.NewNat().ExpandFor(N)
if _, err := pN.SetBytes(p.Nat().Bytes(p), N); err != nil {
return errors.New("crypto/rsa: invalid prime")
}
// Check that Πprimes == n.
if Π.IsZero() != 1 {
return errors.New("crypto/rsa: invalid modulus")
qN := bigmod.NewNat().ExpandFor(N)
if _, err := qN.SetBytes(q.Nat().Bytes(q), N); err != nil {
return errors.New("crypto/rsa: invalid prime")
}
if pN.Mul(qN, N).IsZero() != 1 {
return errors.New("crypto/rsa: p * q != n")
}
// Check that de ≡ 1 mod p-1, and de ≡ 1 mod q-1.
//
// This implies that e is coprime to each p-1 as e has a multiplicative
// inverse. Therefore e is coprime to lcm(p-1,q-1,r-1,...) = exponent(/n).
// 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.
pMinus1, err := bigmod.NewModulus(p.Nat().SubOne(p).Bytes(p))
if err != nil {
return errors.New("crypto/rsa: invalid prime")
}
dP, err := bigmod.NewNat().SetBytes(priv.dP, pMinus1)
if err != nil {
return errors.New("crypto/rsa: invalid CRT exponent")
}
de := bigmod.NewNat()
de.SetUint(uint(priv.pub.E)).ExpandFor(pMinus1)
de.Mul(dP, pMinus1)
if de.IsOne() != 1 {
return errors.New("crypto/rsa: invalid CRT exponent")
}
qMinus1, err := bigmod.NewModulus(q.Nat().SubOne(q).Bytes(q))
if err != nil {
return errors.New("crypto/rsa: invalid prime")
}
dQ, err := bigmod.NewNat().SetBytes(priv.dQ, qMinus1)
if err != nil {
return errors.New("crypto/rsa: invalid CRT exponent")
}
de.SetUint(uint(priv.pub.E)).ExpandFor(qMinus1)
de.Mul(dQ, qMinus1)
if de.IsOne() != 1 {
return errors.New("crypto/rsa: invalid CRT exponent")
}
// Check that qInv * q ≡ 1 mod p.
one := q.Nat().Mul(priv.qInv, p)
if one.IsOne() != 1 {
return errors.New("crypto/rsa: invalid CRT coefficient")
}
return nil

View File

@ -519,6 +519,20 @@ func (priv *PrivateKey) precompute() (PrecomputedValues, error) {
return precomputed, errors.New("crypto/rsa: prime Q is nil")
}
// If the CRT values are already set, use them.
if priv.Precomputed.Dp != nil && priv.Precomputed.Dq != nil && priv.Precomputed.Qinv != nil {
k, err := rsa.NewPrivateKeyWithPrecomputation(priv.N.Bytes(), priv.E, priv.D.Bytes(),
priv.Primes[0].Bytes(), priv.Primes[1].Bytes(),
priv.Precomputed.Dp.Bytes(), priv.Precomputed.Dq.Bytes(), priv.Precomputed.Qinv.Bytes())
if err != nil {
return precomputed, err
}
precomputed = priv.Precomputed
precomputed.fips = k
precomputed.CRTValues = make([]CRTValue, 0)
return precomputed, nil
}
k, err := rsa.NewPrivateKey(priv.N.Bytes(), priv.E, priv.D.Bytes(),
priv.Primes[0].Bytes(), priv.Primes[1].Bytes())
if err != nil {

View File

@ -8,6 +8,7 @@ import (
"crypto/rsa"
"encoding/asn1"
"errors"
"internal/godebug"
"math/big"
)
@ -19,10 +20,9 @@ type pkcs1PrivateKey struct {
D *big.Int
P *big.Int
Q *big.Int
// We ignore these values, if present, because rsa will calculate them.
Dp *big.Int `asn1:"optional"`
Dq *big.Int `asn1:"optional"`
Qinv *big.Int `asn1:"optional"`
Dp *big.Int `asn1:"optional"`
Dq *big.Int `asn1:"optional"`
Qinv *big.Int `asn1:"optional"`
AdditionalPrimes []pkcs1AdditionalRSAPrime `asn1:"optional,omitempty"`
}
@ -41,9 +41,16 @@ type pkcs1PublicKey struct {
E int
}
// x509rsacrt, if zero, makes ParsePKCS1PrivateKey ignore and recompute invalid
// CRT values in the RSA private key.
var x509rsacrt = godebug.New("x509rsacrt")
// ParsePKCS1PrivateKey parses an [RSA] private key in PKCS #1, ASN.1 DER form.
//
// This kind of key is commonly encoded in PEM blocks of type "RSA PRIVATE KEY".
//
// Before Go 1.24, the CRT parameters were ignored and recomputed. To restore
// the old behavior, use the GODEBUG=x509rsacrt=0 environment variable.
func ParsePKCS1PrivateKey(der []byte) (*rsa.PrivateKey, error) {
var priv pkcs1PrivateKey
rest, err := asn1.Unmarshal(der, &priv)
@ -64,7 +71,8 @@ func ParsePKCS1PrivateKey(der []byte) (*rsa.PrivateKey, error) {
return nil, errors.New("x509: unsupported private key version")
}
if priv.N.Sign() <= 0 || priv.D.Sign() <= 0 || priv.P.Sign() <= 0 || priv.Q.Sign() <= 0 {
if priv.N.Sign() <= 0 || priv.D.Sign() <= 0 || priv.P.Sign() <= 0 || priv.Q.Sign() <= 0 ||
priv.Dp.Sign() <= 0 || priv.Dq.Sign() <= 0 || priv.Qinv.Sign() <= 0 {
return nil, errors.New("x509: private key contains zero or negative value")
}
@ -78,6 +86,9 @@ func ParsePKCS1PrivateKey(der []byte) (*rsa.PrivateKey, error) {
key.Primes = make([]*big.Int, 2+len(priv.AdditionalPrimes))
key.Primes[0] = priv.P
key.Primes[1] = priv.Q
key.Precomputed.Dp = priv.Dp
key.Precomputed.Dq = priv.Dq
key.Precomputed.Qinv = priv.Qinv
for i, a := range priv.AdditionalPrimes {
if a.Prime.Sign() <= 0 {
return nil, errors.New("x509: private key contains zero or negative prime")
@ -89,6 +100,19 @@ func ParsePKCS1PrivateKey(der []byte) (*rsa.PrivateKey, error) {
key.Precompute()
if err := key.Validate(); err != nil {
// If x509rsacrt=0 is set, try dropping the CRT values and
// rerunning precomputation and key validation.
if x509rsacrt.Value() == "0" {
key.Precomputed.Dp = nil
key.Precomputed.Dq = nil
key.Precomputed.Qinv = nil
key.Precompute()
if err := key.Validate(); err == nil {
x509rsacrt.IncNonDefault()
return key, nil
}
}
return nil, err
}

View File

@ -32,6 +32,9 @@ type pkcs8 struct {
// in the future.
//
// This kind of key is commonly encoded in PEM blocks of type "PRIVATE KEY".
//
// Before Go 1.24, the CRT parameters of RSA keys were ignored and recomputed.
// To restore the old behavior, use the GODEBUG=x509rsacrt=0 environment variable.
func ParsePKCS8PrivateKey(der []byte) (key any, err error) {
var privKey pkcs8
if _, err := asn1.Unmarshal(der, &privKey); err != nil {

View File

@ -253,6 +253,61 @@ func TestMarshalRSAPrivateKey(t *testing.T) {
priv.Primes[2].Cmp(priv2.Primes[2]) != 0 {
t.Errorf("wrong priv:\ngot %+v\nwant %+v", priv2, priv)
}
if priv.Precomputed.Dp == nil {
t.Fatalf("Precomputed.Dp is nil")
}
}
func TestMarshalRSAPrivateKeyInvalid(t *testing.T) {
block, _ := pem.Decode([]byte(strings.ReplaceAll(
`-----BEGIN RSA TESTING KEY-----
MIIEowIBAAKCAQEAsPnoGUOnrpiSqt4XynxA+HRP7S+BSObI6qJ7fQAVSPtRkqso
tWxQYLEYzNEx5ZSHTGypibVsJylvCfuToDTfMul8b/CZjP2Ob0LdpYrNH6l5hvFE
89FU1nZQF15oVLOpUgA7wGiHuEVawrGfey92UE68mOyUVXGweJIVDdxqdMoPvNNU
l86BU02vlBiESxOuox+dWmuVV7vfYZ79Toh/LUK43YvJh+rhv4nKuF7iHjVjBd9s
B6iDjj70HFldzOQ9r8SRI+9NirupPTkF5AKNe6kUhKJ1luB7S27ZkvB3tSTT3P59
3VVJvnzOjaA1z6Cz+4+eRvcysqhrRgFlwI9TEwIDAQABAoIBAEEYiyDP29vCzx/+
dS3LqnI5BjUuJhXUnc6AWX/PCgVAO+8A+gZRgvct7PtZb0sM6P9ZcLrweomlGezI
FrL0/6xQaa8bBr/ve/a8155OgcjFo6fZEw3Dz7ra5fbSiPmu4/b/kvrg+Br1l77J
aun6uUAs1f5B9wW+vbR7tzbT/mxaUeDiBzKpe15GwcvbJtdIVMa2YErtRjc1/5B2
BGVXyvlJv0SIlcIEMsHgnAFOp1ZgQ08aDzvilLq8XVMOahAhP1O2A3X8hKdXPyrx
IVWE9bS9ptTo+eF6eNl+d7htpKGEZHUxinoQpWEBTv+iOoHsVunkEJ3vjLP3lyI/
fY0NQ1ECgYEA3RBXAjgvIys2gfU3keImF8e/TprLge1I2vbWmV2j6rZCg5r/AS0u
pii5CvJ5/T5vfJPNgPBy8B/yRDs+6PJO1GmnlhOkG9JAIPkv0RBZvR0PMBtbp6nT
Y3yo1lwamBVBfY6rc0sLTzosZh2aGoLzrHNMQFMGaauORzBFpY5lU50CgYEAzPHl
u5DI6Xgep1vr8QvCUuEesCOgJg8Yh1UqVoY/SmQh6MYAv1I9bLGwrb3WW/7kqIoD
fj0aQV5buVZI2loMomtU9KY5SFIsPV+JuUpy7/+VE01ZQM5FdY8wiYCQiVZYju9X
Wz5LxMNoz+gT7pwlLCsC4N+R8aoBk404aF1gum8CgYAJ7VTq7Zj4TFV7Soa/T1eE
k9y8a+kdoYk3BASpCHJ29M5R2KEA7YV9wrBklHTz8VzSTFTbKHEQ5W5csAhoL5Fo
qoHzFFi3Qx7MHESQb9qHyolHEMNx6QdsHUn7rlEnaTTyrXh3ifQtD6C0yTmFXUIS
CW9wKApOrnyKJ9nI0HcuZQKBgQCMtoV6e9VGX4AEfpuHvAAnMYQFgeBiYTkBKltQ
XwozhH63uMMomUmtSG87Sz1TmrXadjAhy8gsG6I0pWaN7QgBuFnzQ/HOkwTm+qKw
AsrZt4zeXNwsH7QXHEJCFnCmqw9QzEoZTrNtHJHpNboBuVnYcoueZEJrP8OnUG3r
UjmopwKBgAqB2KYYMUqAOvYcBnEfLDmyZv9BTVNHbR2lKkMYqv5LlvDaBxVfilE0
2riO4p6BaAdvzXjKeRrGNEKoHNBpOSfYCOM16NjL8hIZB1CaV3WbT5oY+jp7Mzd5
7d56RZOE+ERK2uz/7JX9VSsM/LbH9pJibd4e8mikDS9ntciqOH/3
-----END RSA TESTING KEY-----`, "TESTING KEY", "PRIVATE KEY")))
testRSA2048, _ := ParsePKCS1PrivateKey(block.Bytes)
broken := *testRSA2048
broken.Precomputed.Dp = new(big.Int).SetUint64(42)
parsed, err := ParsePKCS1PrivateKey(MarshalPKCS1PrivateKey(&broken))
if err == nil {
t.Errorf("expected error, got success")
}
t.Setenv("GODEBUG", "x509rsacrt=0")
parsed, err = ParsePKCS1PrivateKey(MarshalPKCS1PrivateKey(&broken))
if err != nil {
t.Fatalf("expected success, got error: %v", err)
}
// Dp should have been recomputed.
if parsed.Precomputed.Dp.Cmp(testRSA2048.Precomputed.Dp) != 0 {
t.Errorf("Dp recomputation failed: got %v, want %v", parsed.Precomputed.Dp, testRSA2048.Precomputed.Dp)
}
}
func TestMarshalRSAPublicKey(t *testing.T) {

View File

@ -62,6 +62,7 @@ var All = []Info{
{Name: "winsymlink", Package: "os", Changed: 22, Old: "0"},
{Name: "x509keypairleaf", Package: "crypto/tls", Changed: 23, Old: "0"},
{Name: "x509negativeserial", Package: "crypto/x509", Changed: 23, Old: "1"},
{Name: "x509rsacrt", Package: "crypto/x509", Changed: 24, Old: "0"},
{Name: "x509usefallbackroots", Package: "crypto/x509"},
{Name: "x509usepolicies", Package: "crypto/x509", Changed: 24, Old: "0"},
{Name: "zipinsecurepath", Package: "archive/zip"},

View File

@ -362,6 +362,10 @@ Below is the full list of supported metrics, ordered lexicographically.
package due to a non-default GODEBUG=x509negativeserial=...
setting.
/godebug/non-default-behavior/x509rsacrt:events
The number of non-default behaviors executed by the crypto/x509
package due to a non-default GODEBUG=x509rsacrt=... setting.
/godebug/non-default-behavior/x509usefallbackroots:events
The number of non-default behaviors executed by the crypto/x509
package due to a non-default GODEBUG=x509usefallbackroots=...