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

[Peter Krempa]

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 at 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 at redhat.com>