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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 09/27/2017 10:25 AM, Peter Krempa wrote:
> On Tue, Sep 19, 2017 at 21:32:46 -0400, John Ferlan wrote:
>> From: Ashish Mittal <Ashish.Mittal@xxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxx>
>> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
>> ---
>>  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 (!(serv

OK dropped

er = 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
> 

No it's the server certificate (server-key.pem) that needs the secret in
order to be decrypted.

>> +     * 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.
> 

OK I'll remove the former

John


>> +    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.
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux