Skip to content

Commit 4fa9695

Browse files
thaJeztahthaJeztah
committed
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 <[email protected]>
1 parent b4dde3a commit 4fa9695

4 files changed

Lines changed: 14 additions & 107 deletions

File tree

‎cli/command/cli.go‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,7 @@ import (
3333
"github.com/moby/term"
3434
"github.com/pkg/errors"
3535
"github.com/spf13/cobra"
36-
"github.com/theupdateframework/notary"
3736
notaryclient "github.com/theupdateframework/notary/client"
38-
"github.com/theupdateframework/notary/passphrase"
3937
)
4038

4139
// 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
252250

253251
if cli.client == nil {
254252
cli.client, err = newAPIClientFromEndpoint(cli.dockerEndpoint, cli.configFile)
255-
if tlsconfig.IsErrEncryptedKey(err) {
256-
passRetriever := passphrase.PromptRetrieverWithInOut(cli.In(), cli.Out(), nil)
257-
newClient := func(password string) (client.APIClient, error) {
258-
cli.dockerEndpoint.TLSPassword = password //nolint: staticcheck // SA1019: cli.dockerEndpoint.TLSPassword is deprecated
259-
return newAPIClientFromEndpoint(cli.dockerEndpoint, cli.configFile)
260-
}
261-
cli.client, err = getClientWithPassword(passRetriever, newClient)
262-
}
263253
if err != nil {
264254
return err
265255
}
@@ -377,20 +367,6 @@ func (cli *DockerCli) initializeFromClient() {
377367
cli.client.NegotiateAPIVersionPing(ping)
378368
}
379369

380-
func getClientWithPassword(passRetriever notary.PassRetriever, newClient func(password string) (client.APIClient, error)) (client.APIClient, error) {
381-
for attempts := 0; ; attempts++ {
382-
passwd, giveup, err := passRetriever("private", "encrypted TLS private", false, attempts)
383-
if giveup || err != nil {
384-
return nil, errors.Wrap(err, "private key is encrypted, but could not get passphrase")
385-
}
386-
387-
apiclient, err := newClient(passwd)
388-
if !tlsconfig.IsErrEncryptedKey(err) {
389-
return apiclient, err
390-
}
391-
}
392-
}
393-
394370
// NotaryClient provides a Notary Repository to interact with signed metadata for an image
395371
func (cli *DockerCli) NotaryClient(imgRefAndAuth trust.ImageRefAndAuth, actions []string) (notaryclient.Repository, error) {
396372
return trust.GetNotaryRepository(cli.In(), cli.Out(), UserAgent(), imgRefAndAuth.RepoInfo(), imgRefAndAuth.AuthConfig(), actions...)

‎cli/command/cli_test.go‎

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package command
33
import (
44
"bytes"
55
"context"
6-
"crypto/x509"
76
"fmt"
87
"io/ioutil"
98
"net/http"
@@ -205,73 +204,6 @@ func TestExperimentalCLI(t *testing.T) {
205204
}
206205
}
207206

208-
func TestGetClientWithPassword(t *testing.T) {
209-
expected := "password"
210-
211-
var testcases = []struct {
212-
doc string
213-
password string
214-
retrieverErr error
215-
retrieverGiveup bool
216-
newClientErr error
217-
expectedErr string
218-
}{
219-
{
220-
doc: "successful connect",
221-
password: expected,
222-
},
223-
{
224-
doc: "password retriever exhausted",
225-
retrieverGiveup: true,
226-
retrieverErr: errors.New("failed"),
227-
expectedErr: "private key is encrypted, but could not get passphrase",
228-
},
229-
{
230-
doc: "password retriever error",
231-
retrieverErr: errors.New("failed"),
232-
expectedErr: "failed",
233-
},
234-
{
235-
doc: "newClient error",
236-
newClientErr: errors.New("failed to connect"),
237-
expectedErr: "failed to connect",
238-
},
239-
}
240-
241-
for _, testcase := range testcases {
242-
testcase := testcase
243-
t.Run(testcase.doc, func(t *testing.T) {
244-
passRetriever := func(_, _ string, _ bool, attempts int) (passphrase string, giveup bool, err error) {
245-
// Always return an invalid pass first to test iteration
246-
switch attempts {
247-
case 0:
248-
return "something else", false, nil
249-
default:
250-
return testcase.password, testcase.retrieverGiveup, testcase.retrieverErr
251-
}
252-
}
253-
254-
newClient := func(currentPassword string) (client.APIClient, error) {
255-
if testcase.newClientErr != nil {
256-
return nil, testcase.newClientErr
257-
}
258-
if currentPassword == expected {
259-
return &client.Client{}, nil
260-
}
261-
return &client.Client{}, x509.IncorrectPasswordError
262-
}
263-
264-
_, err := getClientWithPassword(passRetriever, newClient)
265-
if testcase.expectedErr != "" {
266-
assert.ErrorContains(t, err, testcase.expectedErr)
267-
return
268-
}
269-
270-
assert.NilError(t, err)
271-
})
272-
}
273-
}
274-
275207
func TestNewDockerCliAndOperators(t *testing.T) {
276208
// Test default operations and also overriding default ones
277209
cli, err := NewDockerCli(

‎cli/context/docker/load.go‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"crypto/tls"
55
"crypto/x509"
66
"encoding/pem"
7-
"fmt"
87
"net"
98
"net/http"
109
"os"
@@ -67,17 +66,10 @@ func (c *Endpoint) tlsConfig() (*tls.Config, error) {
6766
keyBytes := c.TLSData.Key
6867
pemBlock, _ := pem.Decode(keyBytes)
6968
if pemBlock == nil {
70-
return nil, fmt.Errorf("no valid private key found")
69+
return nil, errors.New("no valid private key found")
7170
}
72-
73-
var err error
74-
// 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
7571
if x509.IsEncryptedPEMBlock(pemBlock) { //nolint: staticcheck // SA1019: x509.IsEncryptedPEMBlock is deprecated, and insecure by design
76-
keyBytes, err = x509.DecryptPEMBlock(pemBlock, []byte(c.TLSPassword)) //nolint: staticcheck // SA1019: x509.IsEncryptedPEMBlock is deprecated, and insecure by design
77-
if err != nil {
78-
return nil, errors.Wrap(err, "private key is encrypted, but could not decrypt it")
79-
}
80-
keyBytes = pem.EncodeToMemory(&pem.Block{Type: pemBlock.Type, Bytes: keyBytes})
72+
return nil, errors.New("private key is encrypted - support for encrypted private keys has been removed, see https://docs.docker.com/go/deprecated/")
8173
}
8274

8375
x509cert, err := tls.X509KeyPair(c.TLSData.Cert, keyBytes)

‎docs/deprecated.md‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ The table below provides an overview of the current status of deprecated feature
5050

5151
Status | Feature | Deprecated | Remove
5252
-----------|------------------------------------------------------------------------------------------------------------------------------------|------------|------------
53-
Deprecated | [Support for encrypted TLS private keys](#support-for-encrypted-tls-private-keys) | v20.10 | -
53+
Removed | [Support for encrypted TLS private keys](#support-for-encrypted-tls-private-keys) | v20.10 | v21.xx
5454
Deprecated | [Kubernetes stack and context support](#kubernetes-stack-and-context-support) | v20.10 | -
5555
Deprecated | [Pulling images from non-compliant image registries](#pulling-images-from-non-compliant-image-registries) | v20.10 | -
5656
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-
103103

104104
**Deprecated in Release: v20.10**
105105

106-
Use of encrypted TLS private keys has been deprecated, and will be removed in a
107-
future release. Golang has deprecated support for legacy PEM encryption (as
108-
specified in [RFC 1423](https://datatracker.ietf.org/doc/html/rfc1423)), as it
109-
is insecure by design (see [https://go-review.googlesource.com/c/go/+/264159](https://go-review.googlesource.com/c/go/+/264159)).
106+
**Removed in Release: v21.xx**
107+
108+
Use of encrypted TLS private keys has been deprecated, and has been removed.
109+
Golang has deprecated support for legacy PEM encryption (as specified in
110+
[RFC 1423](https://datatracker.ietf.org/doc/html/rfc1423)), as it is insecure by
111+
design (see [https://go-review.googlesource.com/c/go/+/264159](https://go-review.googlesource.com/c/go/+/264159)).
112+
113+
This feature allowed using an encrypted private key with a supplied password,
114+
but did not provide additional security as the encryption is known to be broken,
115+
and the key is sitting next to the password in the filesystem. Users are recommended
116+
to decrypt the private key, and store it un-encrypted to continue using it.
110117

111118
### Kubernetes stack and context support
112119

0 commit comments

Comments
 (0)