fix(tls): validate RSA keys before marshaling (#23295)

Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>
This commit is contained in:
Ville Vesilehto 2025-06-06 14:05:41 +03:00 committed by GitHub
parent e1195fd931
commit 109cd6c382
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 88 additions and 2 deletions

View file

@ -192,6 +192,11 @@ func publicKey(priv any) any {
func pemBlockForKey(priv any) *pem.Block {
switch k := priv.(type) {
case *rsa.PrivateKey:
// In Go 1.24+, MarshalPKCS1PrivateKey calls Precompute() which can panic
// if the key is invalid. Validate the key first.
if k == nil || k.Validate() != nil {
return nil
}
return &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(k)}
case *ecdsa.PrivateKey:
b, err := x509.MarshalECPrivateKey(k)
@ -297,7 +302,11 @@ func generatePEM(opts CertOptions) ([]byte, []byte, error) {
return nil, nil, err
}
certpem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certBytes})
keypem := pem.EncodeToMemory(pemBlockForKey(privateKey))
keyBlock := pemBlockForKey(privateKey)
if keyBlock == nil {
return nil, nil, errors.New("failed to encode private key")
}
keypem := pem.EncodeToMemory(keyBlock)
return certpem, keypem, nil
}
@ -320,7 +329,11 @@ func EncodeX509KeyPair(cert tls.Certificate) ([]byte, []byte) {
for _, certtmp := range cert.Certificate {
certpem = append(certpem, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certtmp})...)
}
keypem := pem.EncodeToMemory(pemBlockForKey(cert.PrivateKey))
keyBlock := pemBlockForKey(cert.PrivateKey)
if keyBlock == nil {
return certpem, []byte{}
}
keypem := pem.EncodeToMemory(keyBlock)
return certpem, keypem
}

View file

@ -1,11 +1,13 @@
package tls
import (
"crypto/rsa"
"crypto/tls"
"crypto/x509"
"encoding/pem"
"errors"
"fmt"
"math/big"
"os"
"strings"
"testing"
@ -458,3 +460,74 @@ func TestLoadX509CertPool(t *testing.T) {
require.Nil(t, p)
})
}
func TestEncodeX509KeyPair_InvalidRSAKey(t *testing.T) {
t.Run("Nil RSA private key", func(t *testing.T) {
cert := tls.Certificate{
Certificate: [][]byte{{0x30, 0x82}}, // minimal DER certificate bytes
PrivateKey: (*rsa.PrivateKey)(nil),
}
certPEM, keyPEM := EncodeX509KeyPair(cert)
assert.NotEmpty(t, certPEM)
assert.Empty(t, keyPEM)
})
t.Run("RSA private key that fails validation", func(t *testing.T) {
// Create an RSA key with invalid parameters that will fail Validate()
invalidKey := &rsa.PrivateKey{
PublicKey: rsa.PublicKey{
N: big.NewInt(1), // Too small modulus, will fail validation
E: 65537,
},
D: big.NewInt(1), // Invalid private exponent
}
cert := tls.Certificate{
Certificate: [][]byte{{0x30, 0x82}}, // minimal DER certificate bytes
PrivateKey: invalidKey,
}
certPEM, keyPEM := EncodeX509KeyPair(cert)
assert.NotEmpty(t, certPEM)
assert.Empty(t, keyPEM)
})
t.Run("RSA private key with inconsistent parameters", func(t *testing.T) {
invalidKey := &rsa.PrivateKey{
PublicKey: rsa.PublicKey{
N: big.NewInt(35),
E: 65537,
},
D: big.NewInt(99999),
}
cert := tls.Certificate{
Certificate: [][]byte{{0x30, 0x82}}, // minimal DER certificate bytes
PrivateKey: invalidKey,
}
certPEM, keyPEM := EncodeX509KeyPair(cert)
assert.NotEmpty(t, certPEM)
assert.Empty(t, keyPEM)
})
t.Run("Unsupported private key type", func(t *testing.T) {
// Use a type that's not *rsa.PrivateKey or *ecdsa.PrivateKey
cert := tls.Certificate{
Certificate: [][]byte{{0x30, 0x82}}, // minimal DER certificate bytes
PrivateKey: "not a private key", // Unsupported type
}
certPEM, keyPEM := EncodeX509KeyPair(cert)
assert.NotEmpty(t, certPEM)
assert.Empty(t, keyPEM)
})
t.Run("Valid RSA private key should work", func(t *testing.T) {
// Generate a valid RSA key for testing
opts := CertOptions{Hosts: []string{"localhost"}, Organization: "Test"}
validCert, err := GenerateX509KeyPair(opts)
require.NoError(t, err)
certPEM, keyPEM := EncodeX509KeyPair(*validCert)
assert.NotEmpty(t, certPEM)
assert.NotEmpty(t, keyPEM)
assert.Contains(t, string(keyPEM), "-----BEGIN RSA PRIVATE KEY-----")
assert.Contains(t, string(keyPEM), "-----END RSA PRIVATE KEY-----")
})
}