Skip to content

Commit 59d4519

Browse files
fix(tls): validate RSA keys before marshaling (cherry-pick #23295) (#23300)
Signed-off-by: Ville Vesilehto <[email protected]> Co-authored-by: Ville Vesilehto <[email protected]>
1 parent 0ef49b6 commit 59d4519

File tree

2 files changed

+88
-2
lines changed

2 files changed

+88
-2
lines changed

util/tls/tls.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,11 @@ func publicKey(priv any) any {
192192
func pemBlockForKey(priv any) *pem.Block {
193193
switch k := priv.(type) {
194194
case *rsa.PrivateKey:
195+
// In Go 1.24+, MarshalPKCS1PrivateKey calls Precompute() which can panic
196+
// if the key is invalid. Validate the key first.
197+
if k == nil || k.Validate() != nil {
198+
return nil
199+
}
195200
return &pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(k)}
196201
case *ecdsa.PrivateKey:
197202
b, err := x509.MarshalECPrivateKey(k)
@@ -297,7 +302,11 @@ func generatePEM(opts CertOptions) ([]byte, []byte, error) {
297302
return nil, nil, err
298303
}
299304
certpem := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certBytes})
300-
keypem := pem.EncodeToMemory(pemBlockForKey(privateKey))
305+
keyBlock := pemBlockForKey(privateKey)
306+
if keyBlock == nil {
307+
return nil, nil, errors.New("failed to encode private key")
308+
}
309+
keypem := pem.EncodeToMemory(keyBlock)
301310
return certpem, keypem, nil
302311
}
303312

@@ -320,7 +329,11 @@ func EncodeX509KeyPair(cert tls.Certificate) ([]byte, []byte) {
320329
for _, certtmp := range cert.Certificate {
321330
certpem = append(certpem, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certtmp})...)
322331
}
323-
keypem := pem.EncodeToMemory(pemBlockForKey(cert.PrivateKey))
332+
keyBlock := pemBlockForKey(cert.PrivateKey)
333+
if keyBlock == nil {
334+
return certpem, []byte{}
335+
}
336+
keypem := pem.EncodeToMemory(keyBlock)
324337
return certpem, keypem
325338
}
326339

util/tls/tls_test.go

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package tls
22

33
import (
4+
"crypto/rsa"
45
"crypto/tls"
56
"crypto/x509"
67
"encoding/pem"
78
"errors"
89
"fmt"
10+
"math/big"
911
"os"
1012
"strings"
1113
"testing"
@@ -458,3 +460,74 @@ func TestLoadX509CertPool(t *testing.T) {
458460
require.Nil(t, p)
459461
})
460462
}
463+
464+
func TestEncodeX509KeyPair_InvalidRSAKey(t *testing.T) {
465+
t.Run("Nil RSA private key", func(t *testing.T) {
466+
cert := tls.Certificate{
467+
Certificate: [][]byte{{0x30, 0x82}}, // minimal DER certificate bytes
468+
PrivateKey: (*rsa.PrivateKey)(nil),
469+
}
470+
certPEM, keyPEM := EncodeX509KeyPair(cert)
471+
assert.NotEmpty(t, certPEM)
472+
assert.Empty(t, keyPEM)
473+
})
474+
475+
t.Run("RSA private key that fails validation", func(t *testing.T) {
476+
// Create an RSA key with invalid parameters that will fail Validate()
477+
invalidKey := &rsa.PrivateKey{
478+
PublicKey: rsa.PublicKey{
479+
N: big.NewInt(1), // Too small modulus, will fail validation
480+
E: 65537,
481+
},
482+
D: big.NewInt(1), // Invalid private exponent
483+
}
484+
cert := tls.Certificate{
485+
Certificate: [][]byte{{0x30, 0x82}}, // minimal DER certificate bytes
486+
PrivateKey: invalidKey,
487+
}
488+
certPEM, keyPEM := EncodeX509KeyPair(cert)
489+
assert.NotEmpty(t, certPEM)
490+
assert.Empty(t, keyPEM)
491+
})
492+
493+
t.Run("RSA private key with inconsistent parameters", func(t *testing.T) {
494+
invalidKey := &rsa.PrivateKey{
495+
PublicKey: rsa.PublicKey{
496+
N: big.NewInt(35),
497+
E: 65537,
498+
},
499+
D: big.NewInt(99999),
500+
}
501+
cert := tls.Certificate{
502+
Certificate: [][]byte{{0x30, 0x82}}, // minimal DER certificate bytes
503+
PrivateKey: invalidKey,
504+
}
505+
certPEM, keyPEM := EncodeX509KeyPair(cert)
506+
assert.NotEmpty(t, certPEM)
507+
assert.Empty(t, keyPEM)
508+
})
509+
510+
t.Run("Unsupported private key type", func(t *testing.T) {
511+
// Use a type that's not *rsa.PrivateKey or *ecdsa.PrivateKey
512+
cert := tls.Certificate{
513+
Certificate: [][]byte{{0x30, 0x82}}, // minimal DER certificate bytes
514+
PrivateKey: "not a private key", // Unsupported type
515+
}
516+
certPEM, keyPEM := EncodeX509KeyPair(cert)
517+
assert.NotEmpty(t, certPEM)
518+
assert.Empty(t, keyPEM)
519+
})
520+
521+
t.Run("Valid RSA private key should work", func(t *testing.T) {
522+
// Generate a valid RSA key for testing
523+
opts := CertOptions{Hosts: []string{"localhost"}, Organization: "Test"}
524+
validCert, err := GenerateX509KeyPair(opts)
525+
require.NoError(t, err)
526+
527+
certPEM, keyPEM := EncodeX509KeyPair(*validCert)
528+
assert.NotEmpty(t, certPEM)
529+
assert.NotEmpty(t, keyPEM)
530+
assert.Contains(t, string(keyPEM), "-----BEGIN RSA PRIVATE KEY-----")
531+
assert.Contains(t, string(keyPEM), "-----END RSA PRIVATE KEY-----")
532+
})
533+
}

0 commit comments

Comments
 (0)