cli/command/registry: preserve all whitespace in secrets

cli/command/registry.go

@@ -144,8 +144,8 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword
 		}
 	}
 
-	argPassword = strings.TrimSpace(argPassword)
-	if argPassword == "" {
+	isEmpty := strings.TrimSpace(argPassword) == ""
+	if isEmpty {
 		restoreInput, err := prompt.DisableInputEcho(cli.In())
 		if err != nil {
 			return registrytypes.AuthConfig{}, err

cli/command/registry/login.go

@@ -5,7 +5,6 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"strings"
 
 	"github.com/containerd/errdefs"
 	"github.com/docker/cli/cli"
@@ -88,6 +87,42 @@ func verifyLoginFlags(flags *pflag.FlagSet, opts loginOptions) error {
 	return nil
 }
 
+// readSecretFromStdin reads the secret from r and returns it as a string.
+// It trims terminal line-endings (LF, CRLF, or CR), which may be added when
+// inputting interactively or piping input. The value is otherwise treated as
+// opaque, preserving any other whitespace, including newlines, per [NIST SP 800-63B §5.1.1.2].
+// Note that trimming whitespace may still happen elsewhere (see [NIST SP 800-63B (revision 4) §3.1.1.2]);
+//
+// > Verifiers **MAY** make limited allowances for mistyping (e.g., removing
+// > leading and trailing whitespace characters before verification, allowing
+// > the verification of passwords with differing cases for the leading character)
+//
+// [NIST SP 800-63B §5.1.1.2]: https://pages.nist.gov/800-63-3/sp800-63b.html#memsecretver
+// [NIST SP 800-63B (revision 4) §3.1.1.2]: https://pages.nist.gov/800-63-4/sp800-63b.html#passwordver
+func readSecretFromStdin(r io.Reader) (string, error) {
+	b, err := io.ReadAll(r)
+	if err != nil {
+		return "", err
+	}
+	if len(b) == 0 {
+		return "", nil
+	}
+
+	n := len(b)
+	switch b[n-1] {
+	case '\n':
+		if n >= 2 && b[n-2] == '\r' {
+			b = b[:n-2]
+		} else {
+			b = b[:n-1]
+		}
+	case '\r':
+		b = b[:n-1]
+	}
+
+	return string(b), nil
+}
+
 func verifyLoginOptions(dockerCLI command.Streams, opts *loginOptions) error {
 	if opts.password != "" {
 		_, _ = fmt.Fprintln(dockerCLI.Err(), "WARNING! Using --password via the CLI is insecure. Use --password-stdin.")
@@ -97,14 +132,11 @@ func verifyLoginOptions(dockerCLI command.Streams, opts *loginOptions) error {
 		if opts.user == "" {
 			return errors.New("username is empty")
 		}
-
-		contents, err := io.ReadAll(dockerCLI.In())
+		p, err := readSecretFromStdin(dockerCLI.In())
 		if err != nil {
 			return err
 		}
-
-		opts.password = strings.TrimSuffix(string(contents), "\n")
-		opts.password = strings.TrimSuffix(opts.password, "\r")
+		opts.password = p
 	}
 	return nil
 }

cli/command/registry/login_test.go

@@ -1,6 +1,7 @@
 package registry
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -10,6 +11,7 @@ import (
 	"time"
 
 	"github.com/creack/pty"
+	"github.com/docker/cli/cli/config/configfile"
 	configtypes "github.com/docker/cli/cli/config/types"
 	"github.com/docker/cli/cli/streams"
 	"github.com/docker/cli/internal/prompt"
@@ -19,7 +21,6 @@ import (
 	"github.com/moby/moby/client"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
-	"gotest.tools/v3/fs"
 )
 
 const (
@@ -71,8 +72,9 @@ func TestLoginWithCredStoreCreds(t *testing.T) {
 		},
 	}
 	ctx := context.Background()
+	tmpDir := t.TempDir()
 	cli := test.NewFakeCli(&fakeClient{})
-	cli.ConfigFile().Filename = filepath.Join(t.TempDir(), "config.json")
+	cli.SetConfigFile(configfile.New(filepath.Join(tmpDir, "config.json")))
 	for _, tc := range testCases {
 		_, err := loginWithStoredCredentials(ctx, cli, tc.inputAuthConfig)
 		if tc.expectedErrMsg != "" {
@@ -91,6 +93,7 @@ func TestRunLogin(t *testing.T) {
 	testCases := []struct {
 		doc                 string
 		priorCredentials    map[string]configtypes.AuthConfig
+		stdIn               string
 		input               loginOptions
 		expectedCredentials map[string]configtypes.AuthConfig
 		expectedErr         string
@@ -286,20 +289,72 @@ func TestRunLogin(t *testing.T) {
 				},
 			},
 		},
+		{
+			doc:              "password with leading and trailing spaces",
+			priorCredentials: map[string]configtypes.AuthConfig{},
+			input: loginOptions{
+				serverAddress: "reg1",
+				user:          "my-username",
+				password:      "  my password with spaces  ",
+			},
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				"reg1": {
+					Username:      "my-username",
+					Password:      "  my password with spaces  ",
+					ServerAddress: "reg1",
+				},
+			},
+		},
+		{
+			doc:              "password stdin with line-endings",
+			priorCredentials: map[string]configtypes.AuthConfig{},
+			stdIn:            "  my password with spaces  \r\n",
+			input: loginOptions{
+				serverAddress: "reg1",
+				user:          "my-username",
+				passwordStdin: true,
+			},
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				"reg1": {
+					Username:      "my-username",
+					Password:      "  my password with spaces  ",
+					ServerAddress: "reg1",
+				},
+			},
+		},
+		{
+			doc:              "password stdin with multiple line-endings",
+			priorCredentials: map[string]configtypes.AuthConfig{},
+			stdIn:            "  my password\nwith spaces  \r\n\r\n",
+			input: loginOptions{
+				serverAddress: "reg1",
+				user:          "my-username",
+				passwordStdin: true,
+			},
+			expectedCredentials: map[string]configtypes.AuthConfig{
+				"reg1": {
+					Username:      "my-username",
+					Password:      "  my password\nwith spaces  \r\n",
+					ServerAddress: "reg1",
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.doc, func(t *testing.T) {
-			tmpFile := fs.NewFile(t, "test-run-login")
-			defer tmpFile.Remove()
+			tmpDir := t.TempDir()
+			cfg := configfile.New(filepath.Join(tmpDir, "config.json"))
 			cli := test.NewFakeCli(&fakeClient{})
-			configfile := cli.ConfigFile()
-			configfile.Filename = tmpFile.Path()
+			cli.SetConfigFile(cfg)
+			if tc.input.passwordStdin {
+				cli.SetIn(streams.NewIn(io.NopCloser(bytes.NewBufferString(tc.stdIn))))
+			}
 
 			for _, priorCred := range tc.priorCredentials {
-				assert.NilError(t, configfile.GetCredentialsStore(priorCred.ServerAddress).Store(priorCred))
+				assert.NilError(t, cfg.GetCredentialsStore(priorCred.ServerAddress).Store(priorCred))
 			}
-			storedCreds, err := configfile.GetAllCredentials()
+			storedCreds, err := cfg.GetAllCredentials()
 			assert.NilError(t, err)
 			assert.DeepEqual(t, storedCreds, tc.priorCredentials)
 
@@ -310,7 +365,7 @@ func TestRunLogin(t *testing.T) {
 			}
 			assert.NilError(t, loginErr)
 
-			outputCreds, err := configfile.GetAllCredentials()
+			outputCreds, err := cfg.GetAllCredentials()
 			assert.Check(t, err)
 			assert.DeepEqual(t, outputCreds, tc.expectedCredentials)
 		})
@@ -356,11 +411,10 @@ func TestLoginNonInteractive(t *testing.T) {
 		for _, registryAddr := range registries {
 			for _, tc := range testCases {
 				t.Run(tc.doc, func(t *testing.T) {
-					tmpFile := fs.NewFile(t, "test-run-login")
-					defer tmpFile.Remove()
+					tmpDir := t.TempDir()
+					cfg := configfile.New(filepath.Join(tmpDir, "config.json"))
 					cli := test.NewFakeCli(&fakeClient{})
-					cfg := cli.ConfigFile()
-					cfg.Filename = tmpFile.Path()
+					cli.SetConfigFile(cfg)
 					options := loginOptions{
 						serverAddress: registryAddr,
 					}
@@ -419,11 +473,10 @@ func TestLoginNonInteractive(t *testing.T) {
 		for _, registryAddr := range registries {
 			for _, tc := range testCases {
 				t.Run(tc.doc, func(t *testing.T) {
-					tmpFile := fs.NewFile(t, "test-run-login")
-					defer tmpFile.Remove()
+					tmpDir := t.TempDir()
+					cfg := configfile.New(filepath.Join(tmpDir, "config.json"))
 					cli := test.NewFakeCli(&fakeClient{})
-					cfg := cli.ConfigFile()
-					cfg.Filename = tmpFile.Path()
+					cli.SetConfigFile(cfg)
 					serverAddress := registryAddr
 					if serverAddress == "" {
 						serverAddress = "https://index.docker.io/v1/"
@@ -465,17 +518,15 @@ func TestLoginTermination(t *testing.T) {
 		_ = p.Close()
 	})
 
+	tmpDir := t.TempDir()
+	cfg := configfile.New(filepath.Join(tmpDir, "config.json"))
 	cli := test.NewFakeCli(&fakeClient{}, func(fc *test.FakeCli) {
 		fc.SetOut(streams.NewOut(tty))
 		fc.SetIn(streams.NewIn(tty))
 	})
-	tmpFile := fs.NewFile(t, "test-login-termination")
-	defer tmpFile.Remove()
-
-	configFile := cli.ConfigFile()
-	configFile.Filename = tmpFile.Path()
+	cli.SetConfigFile(cfg)
 
-	ctx, cancel := context.WithCancel(context.Background())
+	ctx, cancel := context.WithCancel(t.Context())
 	t.Cleanup(cancel)
 
 	runErr := make(chan error)