[libvirt] [PATCH v9 4/4] qemu: Add TLS support for Veritas HyperScale (VxHS)

[Peter Krempa]

On Tue, Sep 19, 2017 at 21:32:46 -0400, John Ferlan wrote:
> From: Ashish Mittal <Ashish.Mittal at veritas.com>
> 
> Alter qemu command line generation in order to possibly add TLS for
> a suitably configured domain.
> 
> Sample TLS args generated by libvirt -
> 
>     -object tls-creds-x509,id=objvirtio-disk0_tls0,dir=/etc/pki/qemu,\
>     endpoint=client,verify-peer=yes \
>     -drive file.driver=vxhs,file.tls-creds=objvirtio-disk0_tls0,\
>     file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
>     file.server.type=tcp,file.server.host=192.168.0.1,\
>     file.server.port=9999,format=raw,if=none,\
>     id=drive-virtio-disk0,cache=none \
>     -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,\
>     id=virtio-disk0
> 
> Update the qemuxml2argvtest with a couple of examples. One for a
> simple case and the other a bit more complex where multiple VxHS disks
> are added where at least one uses a VxHS that doesn't require TLS
> credentials and thus sets the domain disk source attribute "tls = 'no'".
> 
> Update the hotplug to be able to handle processing the tlsAlias whether
> it's to add the TLS object when hotplugging a disk or to remove the TLS
> object when hot unplugging a disk.  The hot plug/unplug code is largely
> generic, but the addition code does make the VXHS specific checks only
> because it needs to grab the correct config directory and generate the
> object as the command line would do.
> 
> Signed-off-by: Ashish Mittal <Ashish.Mittal at veritas.com>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_block.c                              |  8 +++
>  src/qemu/qemu_command.c                            | 33 +++++++++
>  src/qemu/qemu_hotplug.c                            | 79 ++++++++++++++++++++++
>  ...-disk-drive-network-tlsx509-multidisk-vxhs.args | 43 ++++++++++++
>  ...v-disk-drive-network-tlsx509-multidisk-vxhs.xml | 50 ++++++++++++++
>  ...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 30 ++++++++
>  tests/qemuxml2argvtest.c                           |  7 ++
>  7 files changed, 250 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
> 
> diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> index 3437302dd..77ffc6c51 100644
> --- a/src/qemu/qemu_block.c
> +++ b/src/qemu/qemu_block.c
> @@ -529,16 +529,24 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src)
>          return NULL;
>      }
>  
> +    if (src->haveTLS == VIR_TRISTATE_BOOL_YES && !src->tlsAlias) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("VxHS disk does not have TLS alias set"));
> +        return NULL;
> +    }

This is not the right place for validation. Also it's really only a
coding error since callers should make sure to properly configure the
object.

> +
>      if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, true)))
>          return NULL;
>  

[...]


> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 7dd6e5fd9..7751a608d 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -156,6 +156,52 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
>  
>  
>  static int
> +qemuDomainAddDiskSrcTLSObject(virQEMUDriverPtr driver,
> +                              virDomainObjPtr vm,
> +                              virStorageSourcePtr src,
> +                              const char *srcalias)
> +{
> +    int ret = -1;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    virJSONValuePtr tlsProps = NULL;
> +
> +    /* NB: Initial implementation doesn't require/use a secret to decrypt
> +     * a server certificate, so there's no need to manage a tlsSecAlias

client certificate

> +     * and tlsSecProps. See qemuDomainAddChardevTLSObjects for the
> +     * methodology required to add a secret object. */
> +
> +    /* Create the TLS object using the source tls* settings */
> +    if (qemuDomainGetTLSObjects(priv->qemuCaps, NULL,
> +                                src->tlsCertdir,
> +                                src->tlsListen,
> +                                src->tlsVerify,
> +                                srcalias, &tlsProps, &src->tlsAlias,
> +                                NULL, NULL) < 0)
> +        goto cleanup;
> +
> +    if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
> +                                NULL, NULL, src->tlsAlias, &tlsProps) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> +
> + cleanup:
> +    virJSONValueFree(tlsProps);
> +
> +    return ret;
> +}


[...]

> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index bf43beb10..21f057460 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -934,6 +934,13 @@ mymain(void)
>      DO_TEST("disk-drive-network-rbd-ipv6", NONE);
>      DO_TEST_FAILURE("disk-drive-network-rbd-no-colon", NONE);
>      DO_TEST("disk-drive-network-vxhs", QEMU_CAPS_VXHS);
> +    driver.config->vxhsTLS = 1;
> +    DO_TEST("disk-drive-network-tlsx509-vxhs", QEMU_CAPS_VXHS,
> +            QEMU_CAPS_OBJECT_TLS_CREDS_X509);
> +    DO_TEST("disk-drive-network-tlsx509-multidisk-vxhs", QEMU_CAPS_VXHS,
> +            QEMU_CAPS_OBJECT_TLS_CREDS_X509);

The second test case is a superset of the first, so the first one can be
dropped.

> +    driver.config->vxhsTLS = 0;
> +    VIR_FREE(driver.config->vxhsTLSx509certdir);
>      DO_TEST("disk-drive-no-boot",
>              QEMU_CAPS_BOOTINDEX);
>      DO_TEST_PARSE_ERROR("disk-device-lun-type-invalid",

ACK if you drop the validation and fix the certificate comment.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170927/ed089e7f/attachment-0001.sig>