Merge pull request #3220 from thaJeztah/remove_encrypted_tls_support

Remove support for encrypted TLS private keys

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