Re: [libvirt PATCH v4 19/31] qemu: pass sensitive data to nbdkit via pipe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/7/23 10:51 AM, Peter Krempa wrote:
On Fri, Jan 20, 2023 at 16:03:13 -0600, Jonathon Jongsma wrote:

...

@@ -759,6 +765,29 @@ qemuNbdkitInitStorageSource(qemuNbdkitCaps *caps,
  }
+static int
+qemuNbdkitCommandPassDataByPipe(virCommand *cmd,
+                                const char *argName,
+                                unsigned char **buf,
+                                size_t buflen)
+{
+    g_autofree char *fdfmt = NULL;
+    int fd = virCommandSetSendBuffer(cmd, buf, buflen);
+
+    if (fd < 0)
+        return -1;
+
+    /* some nbdkit arguments accept a variation where nbdkit will read the data
+     * from a file descriptor, e.g. password=-FD */
+    fdfmt = g_strdup_printf("-%i", fd);
+    virCommandAddArgPair(cmd, argName, fdfmt);
+
+    virCommandDoAsyncIO(cmd);
+
+    return 0;
+}
+
+
  static int
  qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc,
                                    virCommand *cmd)
@@ -808,22 +836,19 @@ qemuNbdkitProcessBuildCommandCurl(qemuNbdkitProcess *proc,
              return -1;
          }
- /* ensure that the secret is a NULL-terminated string */
-        password = g_strndup((char*)secret, secretlen);
-
-        /* for now, just report an error rather than passing the password in
-         * cleartext on the commandline */
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Password not yet supported for nbdkit sources"));
-        return -1;
+        if (qemuNbdkitCommandPassDataByPipe(cmd, "password",
+                                            &secret, secretlen) < 0)
+            return -1a

Don't forget to destroy 'secret' using virSecureErase.

hmm. 'secret' is passed to virCommandSetSendBuffer() (see above), which takes ownership of the buffer, so I can't actually free it. It looks like all users of virCommandSetSendBuffer() are doing so in order to pass sensitive data to a child process securely (For example, qemuTPMSetupEncryption() has this same issue). I can add a new commit changing virCommand to free all send buffers with virSecureErase().

Jonathon




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux