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