[libvirt PATCH v6 34/36] qemu: implement ssh-agent auth for ssh disks with nbdkit

[Peter Krempa]

On Thu, Jul 20, 2023 at 17:20:01 -0500, Jonathon Jongsma wrote:
> It's not possible to use password-protected ssh keys directly with
> libvirt because libvirt doesn't have any way to prompt a user for the
> password. To accomodate password-protected key files, an administrator
> can add these keys to an ssh agent and then configure the domain with
> the path to the ssh-agent socket.
> 
> Note that this requires an administrator or management app to
> configure the ssh-agent with an appropriate socket path and add the
> necessary keys to it. In addition, it does not currently work with
> selinux enabled. The ssh-agent socket would need a label that libvirt
> would be allowed to access rather than unconfined_t.
> 
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
>  src/conf/domain_conf.c                          | 11 ++++++++---
>  src/conf/storage_source_conf.c                  |  1 +
>  src/conf/storage_source_conf.h                  |  1 +
>  src/qemu/qemu_nbdkit.c                          | 10 ++++++++++
>  .../disk-network-ssh-key.args.disk0             |  6 +++---
>  .../disk-network-ssh-key.args.disk1             |  9 +++++++++
>  tests/qemuxml2argvdata/disk-network-ssh-key.xml | 17 ++++++++++++++---
>  7 files changed, 46 insertions(+), 9 deletions(-)
>  create mode 100644 tests/qemunbdkitdata/disk-network-ssh-key.args.disk1
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 08cf1be656..a70d7bf613 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7257,8 +7257,11 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
>              if (!(src->ssh_user = virXMLPropStringRequired(tmpnode, "username")))
>                  return -1;
>  
> -            if (!(src->ssh_keyfile = virXMLPropStringRequired(tmpnode, "keyfile")))
> -                return -1;
> +            /* optional path to an ssh key file */
> +            src->ssh_keyfile = virXMLPropString(tmpnode, "keyfile");
> +
> +            /* optional ssh-agent socket location */
> +            src->ssh_agent = virXMLPropString(tmpnode, "agentsock");

By doing this you'll lose validation that either of the two coices from
the schema is present. Thus the user can just provide a username ...

>          }
>      }
>  
> @@ -22175,13 +22178,15 @@ virDomainDiskSourceFormatNetwork(virBuffer *attrBuf,
>      if (src->protocol == VIR_STORAGE_NET_PROTOCOL_SSH) {
>          if (src->ssh_known_hosts_file)
>              virBufferEscapeString(childBuf, "<knownHosts path='%s'/>\n", src->ssh_known_hosts_file);
> -        if (src->ssh_keyfile) {
> +        if (src->ssh_keyfile || src->ssh_agent) {
>              virBufferAddLit(childBuf, "<identity");

... which will vanish from the XML.

>              if (src->ssh_user)
>                  virBufferEscapeString(childBuf, " username='%s'", src->ssh_user);
>              if (src->ssh_keyfile)
>                  virBufferEscapeString(childBuf, " keyfile='%s'", src->ssh_keyfile);
> +            if (src->ssh_agent)
> +                virBufferEscapeString(childBuf, " agentsock='%s'", src->ssh_agent);

virBufferEscapeString is NULL tolerant

>  
>              virBufferAddLit(childBuf, "/>\n");
>          }


[..]

> diff --git a/src/conf/storage_source_conf.h b/src/conf/storage_source_conf.h
> index 8c805664af..061faa66cb 100644
> --- a/src/conf/storage_source_conf.h
> +++ b/src/conf/storage_source_conf.h
> @@ -411,6 +411,7 @@ struct _virStorageSource {
>      bool ssh_host_key_check_disabled;
>      char *ssh_known_hosts_file;
>      char *ssh_keyfile;
> +    char *ssh_agent;

Missing impl in virStorageSourceCopy.

>  
>      /* nfs_user and nfs_group store the strings passed in by the user for NFS params.
>       * nfs_uid and nfs_gid represent the converted/looked up ID numbers which are used

Reviewed-by: Peter Krempa <pkrempa at redhat.com>