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 <github@gone.nl>

cli/command/cli.go

@@ -33,9 +33,7 @@ import (
 	"github.com/moby/term"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
-	"github.com/theupdateframework/notary"
 	notaryclient "github.com/theupdateframework/notary/client"
-	"github.com/theupdateframework/notary/passphrase"
 )
 
 // Streams is an interface which exposes the standard input and output streams
@@ -252,14 +250,6 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize
 
 	if cli.client == nil {
 		cli.client, err = newAPIClientFromEndpoint(cli.dockerEndpoint, cli.configFile)
-		if tlsconfig.IsErrEncryptedKey(err) {
-			passRetriever := passphrase.PromptRetrieverWithInOut(cli.In(), cli.Out(), nil)
-			newClient := func(password string) (client.APIClient, error) {
-				cli.dockerEndpoint.TLSPassword = password //nolint: staticcheck // SA1019: cli.dockerEndpoint.TLSPassword is deprecated
-				return newAPIClientFromEndpoint(cli.dockerEndpoint, cli.configFile)
-			}
-			cli.client, err = getClientWithPassword(passRetriever, newClient)
-		}
 		if err != nil {
 			return err
 		}
@@ -377,20 +367,6 @@ func (cli *DockerCli) initializeFromClient() {
 	cli.client.NegotiateAPIVersionPing(ping)
 }
 
-func getClientWithPassword(passRetriever notary.PassRetriever, newClient func(password string) (client.APIClient, error)) (client.APIClient, error) {
-	for attempts := 0; ; attempts++ {
-		passwd, giveup, err := passRetriever("private", "encrypted TLS private", false, attempts)
-		if giveup || err != nil {
-			return nil, errors.Wrap(err, "private key is encrypted, but could not get passphrase")
-		}
-
-		apiclient, err := newClient(passwd)
-		if !tlsconfig.IsErrEncryptedKey(err) {
-			return apiclient, err
-		}
-	}
-}
-
 // NotaryClient provides a Notary Repository to interact with signed metadata for an image
 func (cli *DockerCli) NotaryClient(imgRefAndAuth trust.ImageRefAndAuth, actions []string) (notaryclient.Repository, error) {
 	return trust.GetNotaryRepository(cli.In(), cli.Out(), UserAgent(), imgRefAndAuth.RepoInfo(), imgRefAndAuth.AuthConfig(), actions...)

cli/command/cli_test.go

@@ -3,7 +3,6 @@ package command
 import (
 	"bytes"
 	"context"
-	"crypto/x509"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -209,73 +208,6 @@ func TestExperimentalCLI(t *testing.T) {
 	}
 }
 
-func TestGetClientWithPassword(t *testing.T) {
-	expected := "password"
-
-	var testcases = []struct {
-		doc             string
-		password        string
-		retrieverErr    error
-		retrieverGiveup bool
-		newClientErr    error
-		expectedErr     string
-	}{
-		{
-			doc:      "successful connect",
-			password: expected,
-		},
-		{
-			doc:             "password retriever exhausted",
-			retrieverGiveup: true,
-			retrieverErr:    errors.New("failed"),
-			expectedErr:     "private key is encrypted, but could not get passphrase",
-		},
-		{
-			doc:          "password retriever error",
-			retrieverErr: errors.New("failed"),
-			expectedErr:  "failed",
-		},
-		{
-			doc:          "newClient error",
-			newClientErr: errors.New("failed to connect"),
-			expectedErr:  "failed to connect",
-		},
-	}
-
-	for _, testcase := range testcases {
-		testcase := testcase
-		t.Run(testcase.doc, func(t *testing.T) {
-			passRetriever := func(_, _ string, _ bool, attempts int) (passphrase string, giveup bool, err error) {
-				// Always return an invalid pass first to test iteration
-				switch attempts {
-				case 0:
-					return "something else", false, nil
-				default:
-					return testcase.password, testcase.retrieverGiveup, testcase.retrieverErr
-				}
-			}
-
-			newClient := func(currentPassword string) (client.APIClient, error) {
-				if testcase.newClientErr != nil {
-					return nil, testcase.newClientErr
-				}
-				if currentPassword == expected {
-					return &client.Client{}, nil
-				}
-				return &client.Client{}, x509.IncorrectPasswordError
-			}
-
-			_, err := getClientWithPassword(passRetriever, newClient)
-			if testcase.expectedErr != "" {
-				assert.ErrorContains(t, err, testcase.expectedErr)
-				return
-			}
-
-			assert.NilError(t, err)
-		})
-	}
-}
-
 func TestNewDockerCliAndOperators(t *testing.T) {
 	// Test default operations and also overriding default ones
 	cli, err := NewDockerCli(

cli/context/docker/load.go

@@ -4,7 +4,6 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
-	"fmt"
 	"net"
 	"net/http"
 	"os"
@@ -67,17 +66,10 @@ func (c *Endpoint) tlsConfig() (*tls.Config, error) {
 		keyBytes := c.TLSData.Key
 		pemBlock, _ := pem.Decode(keyBytes)
 		if pemBlock == nil {
-			return nil, fmt.Errorf("no valid private key found")
+			return nil, errors.New("no valid private key found")
 		}
-
-		var err error
-		// TODO should we follow Golang, and deprecate RFC 1423 encryption, and produce a warning (or just error)? see https://github.com/docker/cli/issues/3212
 		if x509.IsEncryptedPEMBlock(pemBlock) { //nolint: staticcheck // SA1019: x509.IsEncryptedPEMBlock is deprecated, and insecure by design
-			keyBytes, err = x509.DecryptPEMBlock(pemBlock, []byte(c.TLSPassword)) //nolint: staticcheck // SA1019: x509.IsEncryptedPEMBlock is deprecated, and insecure by design
+			return nil, errors.New("private key is encrypted - support for encrypted private keys has been removed, see https://docs.docker.com/go/deprecated/")
-			if err != nil {
-				return nil, errors.Wrap(err, "private key is encrypted, but could not decrypt it")
-			}
-			keyBytes = pem.EncodeToMemory(&pem.Block{Type: pemBlock.Type, Bytes: keyBytes})
 		}
 
 		x509cert, err := tls.X509KeyPair(c.TLSData.Cert, keyBytes)

docs/deprecated.md

@@ -50,7 +50,7 @@ The table below provides an overview of the current status of deprecated feature
 
 Status     | Feature                                                                                                                            | Deprecated | Remove
 -----------|------------------------------------------------------------------------------------------------------------------------------------|------------|------------
-Deprecated | [Support for encrypted TLS private keys](#support-for-encrypted-tls-private-keys)                                                  | v20.10     | -
+Removed    | [Support for encrypted TLS private keys](#support-for-encrypted-tls-private-keys)                                                  | v20.10     | v21.xx
 Deprecated | [Kubernetes stack and context support](#kubernetes-stack-and-context-support)                                                      | v20.10     | -
 Deprecated | [Pulling images from non-compliant image registries](#pulling-images-from-non-compliant-image-registries)                          | v20.10     | -
 Removed    | [Linux containers on Windows (LCOW)](#linux-containers-on-windows-lcow-experimental)                                               | v20.10     | v21.xx
@@ -103,10 +103,17 @@ Removed    | [Three arguments form in `docker import`](#three-arguments-form-in-
 
 **Deprecated in Release: v20.10**
 
-Use of encrypted TLS private keys has been deprecated, and will be removed in a
+**Removed in Release: v21.xx**
-future release. Golang has deprecated support for legacy PEM encryption (as
+
-specified in [RFC 1423](https://datatracker.ietf.org/doc/html/rfc1423)), as it
+Use of encrypted TLS private keys has been deprecated, and has been removed.
-is insecure by design (see [https://go-review.googlesource.com/c/go/+/264159](https://go-review.googlesource.com/c/go/+/264159)).
+Golang has deprecated support for legacy PEM encryption (as specified in
+[RFC 1423](https://datatracker.ietf.org/doc/html/rfc1423)), as it is insecure by
+design (see [https://go-review.googlesource.com/c/go/+/264159](https://go-review.googlesource.com/c/go/+/264159)).
+
+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.
 
 ### Kubernetes stack and context support