go-dockerclient: Error message may leak credentials used for mounting

It would be possible to filter all errors/methods, but you will never know in which places users will have credentials, so that will become tricky. In this particular instance you (@fsouza) already pinned down the error handling in moby, so maybe it’s good to patch it right there? They already resolve addr= options and replace them for the error messages:

https://github.com/moby/moby/blob/7c69b6dc08c7ce128c3015e08076641c2c5c40e5/volume/local/local_unix.go#L137-L143

We could adapt that to mask the password, maybe something along these lines:

From e7f5e23da0382ef7091b7473ea87d1ef5b8e91a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sebastian=20H=C3=B6ffner?=
 <sebastian.hoeffner@mevis.fraunhofer.de>
Date: Thu, 12 May 2022 16:53:02 +0200
Subject: [PATCH] volume: mask password in cifs mount error messages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Sebastian Höffner <sebastian.hoeffner@mevis.fraunhofer.de>
---
 volume/local/local.go      | 12 ++++++++++++
 volume/local/local_test.go | 19 +++++++++++++++++++
 volume/local/local_unix.go |  3 +++
 3 files changed, 34 insertions(+)

diff --git a/volume/local/local.go b/volume/local/local.go
index 29e3cc9a54..ec03327a4e 100644
--- a/volume/local/local.go
+++ b/volume/local/local.go
@@ -366,3 +366,15 @@ func getAddress(opts string) string {
 	}
 	return ""
 }
+
+// getPassword finds out a password from options
+func getPassword(opts string) string {
+	optsList := strings.Split(opts, ",")
+	for i := 0; i < len(optsList); i++ {
+		if strings.HasPrefix(optsList[i], "password=") {
+			passwd := strings.SplitN(optsList[i], "=", 2)[1]
+			return passwd
+		}
+	}
+	return ""
+}
diff --git a/volume/local/local_test.go b/volume/local/local_test.go
index 180ad09380..8aea918941 100644
--- a/volume/local/local_test.go
+++ b/volume/local/local_test.go
@@ -29,6 +29,25 @@ func TestGetAddress(t *testing.T) {
 
 }
 
+func TestGetPassword(t *testing.T) {
+	cases := map[string]string{
+		"password=secret":                       "secret",
+		" ":                                     "",
+		"password=":                             "",
+		"password=Tr0ub4dor&3":                  "Tr0ub4dor&3",
+		"password=correcthorsebatterystaple":    "correcthorsebatterystaple",
+		"username=moby,password=secret":         "secret",
+		"username=moby,password=secret,addr=11": "secret",
+		"username=moby,addr=11":                 "",
+	}
+	for optsstring, success := range cases {
+		v := getPassword(optsstring)
+		if v != success {
+			t.Errorf("Test case failed for %s actual: %s expected : %s", optsstring, v, success)
+		}
+	}
+}
+
 func TestRemove(t *testing.T) {
 	skip.If(t, runtime.GOOS == "windows", "FIXME: investigate why this test fails on CI")
 	rootDir, err := os.MkdirTemp("", "local-volume-test")
diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go
index 4fdd182544..7cbb74dca4 100644
--- a/volume/local/local_unix.go
+++ b/volume/local/local_unix.go
@@ -141,6 +141,9 @@ func (v *localVolume) mount() error {
 			}
 			mountOpts = strings.Replace(mountOpts, "addr="+addrValue, "addr="+ipAddr.String(), 1)
 		}
+		if password := getPassword(v.opts.MountOpts); password != "" {
+			mountOpts = strings.Replace(mountOpts, "password="+password, "password=********", 1)
+		}
 	}
 	err := mount.Mount(v.opts.MountDevice, v.path, v.opts.MountType, mountOpts)
 	return errors.Wrap(err, "failed to mount local volume")
-- 
2.36.1

I will try to setup some local test to see if the error output is suppressed this way; if so, I will create an issue/PR with moby and see what they say about it. Or do you have other ideas?

Yeah confirmed it’s a matter of upgrading Docker itself and there’s no action needed on the client.

@hmeine thank you very much for pushing this through!