From bc33254012b8b923aea5b1d1cfefde617e918ae4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Apr 2025 00:59:37 +0200 Subject: [PATCH] Remove support for encrypted TLS private keys > Legacy PEM encryption as specified in RFC 1423 is insecure by design. Since > it does not authenticate the ciphertext, it is vulnerable to padding oracle > attacks that can let an attacker recover the plaintext From https://go-review.googlesource.com/c/go/+/264159 > It's unfortunate that we don't implement PKCS#8 encryption so we can't > recommend an alternative but PEM encryption is so broken that it's worth > deprecating outright. This feature allowed using an encrypted private key with a supplied password, but did not provide additional security as the encryption is known to be broken, and the key is sitting next to the password in the filesystem. Users are recommended to decrypt the private key, and store it un-encrypted to continue using it. Signed-off-by: Sebastiaan van Stijn --- tlsconfig/config.go | 44 ++++++++++++++++------------------------ tlsconfig/config_test.go | 36 ++++++++------------------------ 2 files changed, 25 insertions(+), 55 deletions(-) diff --git a/tlsconfig/config.go b/tlsconfig/config.go index 606c98a3..e1f10ba9 100644 --- a/tlsconfig/config.go +++ b/tlsconfig/config.go @@ -34,14 +34,6 @@ type Options struct { // the system pool will be used. ExclusiveRootPools bool MinVersion uint16 - // If Passphrase is set, it will be used to decrypt a TLS private key - // if the key is encrypted. - // - // Deprecated: Use of encrypted TLS private keys has been deprecated, and - // will be removed in a future release. Golang has deprecated support for - // legacy PEM encryption (as specified in RFC 1423), as it is insecure by - // design (see https://go-review.googlesource.com/c/go/+/264159). - Passphrase string } // Extra (server-side) accepted CBC cipher suites - will phase out in the future @@ -144,34 +136,32 @@ func adjustMinVersion(options Options, config *tls.Config) error { return nil } -// IsErrEncryptedKey returns true if the 'err' is an error of incorrect -// password when trying to decrypt a TLS private key. +// errEncryptedKeyDeprecated is produced when we encounter an encrypted +// (password-protected) key. From https://go-review.googlesource.com/c/go/+/264159; // -// Deprecated: Use of encrypted TLS private keys has been deprecated, and -// will be removed in a future release. Golang has deprecated support for -// legacy PEM encryption (as specified in RFC 1423), as it is insecure by -// design (see https://go-review.googlesource.com/c/go/+/264159). -func IsErrEncryptedKey(err error) bool { - return errors.Is(err, x509.IncorrectPasswordError) -} +// > Legacy PEM encryption as specified in RFC 1423 is insecure by design. Since +// > it does not authenticate the ciphertext, it is vulnerable to padding oracle +// > attacks that can let an attacker recover the plaintext +// > +// > It's unfortunate that we don't implement PKCS#8 encryption so we can't +// > recommend an alternative but PEM encryption is so broken that it's worth +// > deprecating outright. +// +// Also see https://docs.docker.com/go/deprecated/ +var errEncryptedKeyDeprecated = errors.New("private key is encrypted; encrypted private keys are obsolete, and not supported") // getPrivateKey returns the private key in 'keyBytes', in PEM-encoded format. -// If the private key is encrypted, 'passphrase' is used to decrypted the -// private key. -func getPrivateKey(keyBytes []byte, passphrase string) ([]byte, error) { +// It returns an error if the file could not be decoded or was protected by +// a passphrase. +func getPrivateKey(keyBytes []byte) ([]byte, error) { // this section makes some small changes to code from notary/tuf/utils/x509.go pemBlock, _ := pem.Decode(keyBytes) if pemBlock == nil { return nil, fmt.Errorf("no valid private key found") } - var err error if x509.IsEncryptedPEMBlock(pemBlock) { //nolint:staticcheck // Ignore SA1019 (IsEncryptedPEMBlock is deprecated) - keyBytes, err = x509.DecryptPEMBlock(pemBlock, []byte(passphrase)) //nolint:staticcheck // Ignore SA1019 (DecryptPEMBlock is deprecated) - if err != nil { - return nil, fmt.Errorf("private key is encrypted, but could not decrypt it: %w", err) - } - keyBytes = pem.EncodeToMemory(&pem.Block{Type: pemBlock.Type, Bytes: keyBytes}) + return nil, errEncryptedKeyDeprecated } return keyBytes, nil @@ -195,7 +185,7 @@ func getCert(options Options) ([]tls.Certificate, error) { return nil, err } - prKeyBytes, err = getPrivateKey(prKeyBytes, options.Passphrase) + prKeyBytes, err = getPrivateKey(prKeyBytes) if err != nil { return nil, err } diff --git a/tlsconfig/config_test.go b/tlsconfig/config_test.go index 989dcd18..35ed02b9 100644 --- a/tlsconfig/config_test.go +++ b/tlsconfig/config_test.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "crypto/x509" "encoding/pem" + "errors" "os" "reflect" "runtime" @@ -523,39 +524,18 @@ func TestConfigClientTLSValidClientCertAndKey(t *testing.T) { } } -// The certificate is set if the client cert and encrypted client key are -// provided and valid and passphrase can decrypt the key -func TestConfigClientTLSValidClientCertAndEncryptedKey(t *testing.T) { +func TestConfigClientTLSEncryptedKey(t *testing.T) { key, cert := getCertAndEncryptedKey() tlsConfig, err := Client(Options{ - CertFile: cert, - KeyFile: key, - Passphrase: "FooBar123", + CertFile: cert, + KeyFile: key, }) - - if err != nil || tlsConfig == nil { - t.Fatal("Unable to configure client TLS", err) - } - - if len(tlsConfig.Certificates) != 1 { - t.Fatal("Unexpected client certificates") + if !errors.Is(err, errEncryptedKeyDeprecated) { + t.Errorf("Expected %v but got %v", errEncryptedKeyDeprecated, err) } -} - -// The certificate is not set if the provided passphrase cannot decrypt -// the encrypted key. -func TestConfigClientTLSNotSetWithInvalidPassphrase(t *testing.T) { - key, cert := getCertAndEncryptedKey() - - tlsConfig, err := Client(Options{ - CertFile: cert, - KeyFile: key, - Passphrase: "InvalidPassphrase", - }) - - if !IsErrEncryptedKey(err) || tlsConfig != nil { - t.Fatal("Expected failure due to incorrect passphrase.") + if tlsConfig != nil { + t.Errorf("Expected nil but got %v", tlsConfig) } }