nbdkit: Use password=+filename to send passwords securely (kubevirt#3363

)

Passing passwords on the command line is not very secure since a user
on the same machine can see them using 'ps' etc.  nbdkit lets you pass
them via several secure mechanisms - for this I used the
password=+filename method.

Signed-off-by: Richard W.M. Jones <[email protected]>

pkg/image/nbdkit.go

@@ -84,7 +84,7 @@ func NewNbdkit(plugin NbdkitPlugin, nbdkitPidFile string) *Nbdkit {
 }
 
 // NewNbdkitCurl creates a new Nbdkit instance with the curl plugin
-func NewNbdkitCurl(nbdkitPidFile, user, password, certDir, socket string, extraHeaders, secretExtraHeaders []string) NbdkitOperation {
+func NewNbdkitCurl(nbdkitPidFile, user, password, certDir, socket string, extraHeaders, secretExtraHeaders []string) (NbdkitOperation, error) {
 	var pluginArgs []string
 	var redactArgs []string
 	args := []string{"-r"}
@@ -93,7 +93,11 @@ func NewNbdkitCurl(nbdkitPidFile, user, password, certDir, socket string, extraH
 		pluginArgs = append(pluginArgs, "user="+user)
 	}
 	if password != "" {
-		pluginArgs = append(pluginArgs, "password="+password)
+		passwordfile, err := writePasswordFile(password)
+		if err != nil {
+			return nil, err
+		}
+		pluginArgs = append(pluginArgs, "password=+"+passwordfile)
 	}
 	if certDir != "" {
 		pluginArgs = append(pluginArgs, fmt.Sprintf("cainfo=%s/%s", certDir, "tls.crt"))
@@ -119,7 +123,7 @@ func NewNbdkitCurl(nbdkitPidFile, user, password, certDir, socket string, extraH
 	n.AddFilter(NbdkitReadAheadFilter)
 	// Should be last filter
 	n.AddFilter(NbdkitRetryFilter)
-	return n
+	return n, nil
 }
 
 // NewNbdkitVddk creates a new Nbdkit instance with the vddk plugin
@@ -135,7 +139,11 @@ func NewNbdkitVddk(nbdkitPidFile, socket, server, username, password, thumbprint
 		pluginArgs = append(pluginArgs, "user="+username)
 	}
 	if password != "" {
-		pluginArgs = append(pluginArgs, "password="+password)
+		passwordfile, err := writePasswordFile(password)
+		if err != nil {
+			return nil, err
+		}
+		pluginArgs = append(pluginArgs, "password=+"+passwordfile)
 	}
 	if thumbprint != "" {
 		pluginArgs = append(pluginArgs, "thumbprint="+thumbprint)
@@ -163,6 +171,25 @@ func NewNbdkitVddk(nbdkitPidFile, socket, server, username, password, thumbprint
 	return n, nil
 }
 
+func writePasswordFile(password string) (string, error) {
+	f, err := os.CreateTemp("", "password")
+	if err != nil {
+		return "", err
+	}
+	defer f.Close()
+	// This would be nice but we don't want to delete it until
+	// program exit.
+	// defer os.Remove(f.Name())
+
+	if _, err := f.Write([]byte(password)); err != nil {
+		return "", err
+	}
+	if err := f.Close(); err != nil {
+		return "", err
+	}
+	return f.Name(), nil
+}
+
 // AddEnvVariable adds an environmental variable to the nbdkit command
 func (n *Nbdkit) AddEnvVariable(v string) {
 	env := os.Environ()
@@ -235,9 +262,7 @@ func (n *Nbdkit) StartNbdkit(source string) error {
 
 	quotedArgs := make([]string, len(argsNbdkit))
 	for index, value := range argsNbdkit {
-		if strings.HasPrefix(value, "password=") {
-			quotedArgs[index] = "'password=*****'"
-		} else if isRedacted(value) {
+		if isRedacted(value) {
 			if strings.HasPrefix(value, "header=") {
 				quotedArgs[index] = "'header=/secret redacted/'"
 			} else {
@@ -401,8 +426,8 @@ func (n *Nbdkit) validatePlugin() error {
 type mockNbdkit struct{}
 
 // NewMockNbdkitCurl creates a mock nbdkit curl plugin for testing
-func NewMockNbdkitCurl(nbdkitPidFile, user, password, certDir, socket string, extraHeaders, secretExtraHeaders []string) NbdkitOperation {
-	return &mockNbdkit{}
+func NewMockNbdkitCurl(nbdkitPidFile, user, password, certDir, socket string, extraHeaders, secretExtraHeaders []string) (NbdkitOperation, error) {
+	return &mockNbdkit{}, nil
 }
 
 func (m *mockNbdkit) StartNbdkit(source string) error {

pkg/importer/http-datasource.go

@@ -112,7 +112,11 @@ func NewHTTPDataSource(endpoint, accessKey, secKey, certDir string, contentType
 		brokenForQemuImg: brokenForQemuImg,
 		contentLength:    contentLength,
 	}
-	httpSource.n = createNbdkitCurl(nbdkitPid, accessKey, secKey, certDir, nbdkitSocket, extraHeaders, secretExtraHeaders)
+	httpSource.n, err = createNbdkitCurl(nbdkitPid, accessKey, secKey, certDir, nbdkitSocket, extraHeaders, secretExtraHeaders)
+	if err != nil {
+		cancel()
+		return nil, err
+	}
 	// We know this is a counting reader, so no need to check.
 	countingReader := httpReader.(*util.CountingReader)
 	go httpSource.pollProgress(countingReader, 10*time.Minute, time.Second)