Re: [libvirt PATCH v6 28/36] qemu: implement password auth for ssh disks with nbdkit

21 Aug 2023

On Thu, Jul 20, 2023 at 17:19:55 -0500, Jonathon Jongsma wrote:
...
For ssh disks that are served by nbdkit, lookup the password from the
configured secret and securely pass it to the nbdkit process using fd
passing.
Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 src/qemu/qemu_nbdkit.c                        | 87 ++++++++++---------
 .../disk-network-ssh-password.args.disk0      |  8 ++
 ...k-network-ssh-password.args.disk0.pipe.778 |  1 +
 .../disk-network-ssh.args.disk1               |  8 ++
 .../disk-network-ssh.args.disk1.pipe.778      |  1 +
 tests/qemunbdkittest.c                        |  1 +
 ...sk-network-ssh-password.x86_64-latest.args | 35 ++++++++
 .../disk-network-ssh-password.xml             | 34 ++++++++
 tests/qemuxml2argvtest.c                      |  1 +
 9 files changed, 137 insertions(+), 39 deletions(-)
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh-password.args.disk0
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh-password.args.disk0.pipe.778
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk1
 create mode 100644 tests/qemunbdkitdata/disk-network-ssh.args.disk1.pipe.778
 create mode 100644 tests/qemuxml2argvdata/disk-network-ssh-password.x86_64-latest.args
 create mode 100644 tests/qemuxml2argvdata/disk-network-ssh-password.xml

diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 8bb91de994..9dbe3af1dd 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -936,6 +936,46 @@ qemuNbdkitCommandPassDataByPipe(virCommand *cmd,
 }
+static int
+qemuNbdkitProcessBuildCommandAuth(virStorageAuthDef *authdef,
+                                  virCommand *cmd)
+{
+    g_autoptr(virConnect) conn = NULL;
+    g_autofree uint8_t *secret = NULL;
+    size_t secretlen = 0;
+    int secrettype;
+
+    if (!authdef)
+        return 0;
+
+    if ((secrettype = virSecretUsageTypeFromString(authdef->secrettype)) < 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("invalid secret type %1$s"),
+                       authdef->secrettype);
+        return -1;
+    }
+
+    conn = virGetConnectSecret();
+    if (virSecretGetSecretString(conn,
+                                 &authdef->seclookupdef,
+                                 secrettype,
+                                 &secret,
+                                 &secretlen) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("failed to get auth secret for storage"));
This overwrites the error from virSecretGetSecretString with a much
worse error, e.g. not showing which secred we are looking for and in
addition to that with an VIR_ERR_INTERNAL_ERROR.
...
+        return -1;
+    }
+
+    virCommandAddArgPair(cmd, "user", authdef->username);
+
+    if (qemuNbdkitCommandPassDataByPipe(cmd, "password",
+                                        &secret, secretlen) < 0)
+        return -1;
+
+    return 0;
+}
+
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

    