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

2017-09-27 Thread Daniel P. Berrange 
On Wed, Sep 27, 2017 at 05:08:51PM +0200, Peter Krempa wrote:
> On Wed, Sep 27, 2017 at 11:05:01 -0400, John Ferlan wrote:
> > 
> > 
> > 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 
> > >>
> > >> 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=,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 
> > >> Signed-off-by: John Ferlan 
> > >> ---
> > >>  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_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.
> 
> I think both can be encrypted. What I wanted to point out is that it
> does not make sense to refer to the server certificate in terms of disks
> since they are clients only.

I don't think so - AFAIK, the 'password' provided to gnutls in
gnutls_certificate_set_x509_key_file2 is only used for the 'key',
not the 'cert' data.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

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

2017-09-27 Thread John Ferlan 

[...]


  
  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.
> 
> I think both can be encrypted. What I wanted to point out is that it
> does not make sense to refer to the server certificate in terms of disks
> since they are clients only.
> 

True - I'll just the whole paragraph. It's one of those traces I leave
in code comments for later on...

John

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

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

2017-09-27 Thread Peter Krempa 
On Wed, Sep 27, 2017 at 11:05:01 -0400, John Ferlan wrote:
> 
> 
> 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 
> >>
> >> 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=,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 
> >> Signed-off-by: John Ferlan 
> >> ---
> >>  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_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.

I think both can be encrypted. What I wanted to point out is that it
does not make sense to refer to the server certificate in terms of disks
since they are clients only.



signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2017-09-27 Thread John Ferlan 


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 
>>
>> 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=,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 
>> Signed-off-by: John Ferlan 
>> ---
>>  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, , >tlsAlias,
>> +NULL, NULL) < 0)
>> +goto cleanup;
>> +
>> +if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
>> +NULL, NULL, src->tlsAlias, ) < 0)
>> +goto cleanup;
>> +
>> +ret = 0;
>> +
>> + cleanup:

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

2017-09-27 Thread Peter Krempa 
On Tue, Sep 19, 2017 at 21:32:46 -0400, John Ferlan wrote:
> From: Ashish Mittal 
> 
> 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=,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 
> Signed-off-by: John Ferlan 
> ---
>  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, , >tlsAlias,
> +NULL, NULL) < 0)
> +goto cleanup;
> +
> +if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE,
> +NULL, NULL, src->tlsAlias, ) < 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)
>

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

2017-09-27 Thread ashish mittal 
Hi Peter,

Do let me know if there's anything else I can help with on setting up the
VxHS devices for testing.

Thanks,
Ashish
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2017-09-21 Thread ashish mittal 
On Thu, Sep 21, 2017 at 12:58 AM, Peter Krempa  wrote:

> On Wed, Sep 20, 2017 at 16:32:45 -0700, ashish mittal wrote:
> > Hi,
> >
> > I have done TLS testing with this patch series and the tests passed fine
> > with the secAlias fix in place.
> >
> > (1) Applied all the v9 patches.
> > (2) make install. Reload and restart the libvirtd daemon.
> > (3) Make sure able to start guest with TLS enabled VxHS disk in the
> domain
> > XML.
> > (4) Try to hot-plug another TLS disk. libvirtd crashes.
> >
> > [root@audi libvirt] 2017-09-20 15:59:25# virsh attach-device myfc24
> > ../../hotplug_disk_1.xml
> > error: Disconnected from qemu:///system due to end of file
> > error: Failed to attach device from ../../hotplug_disk_1.xml
> > error: End of file while reading data: Input/output error
>
> So it's clearly ridiculous to write code without having a way how to
> test it. How can we (upstream) get a VxHS server so that we can properly
> test the code?
>

Fair ask :)
VxHS test server is a part of the libqnio GitHub repo. It can be downloaded
from https://github.com/VeritasHyperScale/libqnio.git

To run the test server with lots of debug messages -
make debug, make install (optional - just copies the library and test
server to a particular location)
./src/qnio_server -v -l /dev/stdout

For running the server in TLS mode, the following files are needed before
test server is started -
CACERT"/var/lib/libvxhs/cacert.pem"
Enable SSL mode by touching   "/var/lib/libvxhs/secure"
SERVER_KEY"/var/lib/libvxhs/server.key"
SERVER_CERT   "/var/lib/libvxhs/server.cert"

Regards,
Ashish
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2017-09-21 Thread ashish mittal 
On Thu, Sep 21, 2017 at 1:35 AM, ashish mittal  wrote:

>
>
> On Thu, Sep 21, 2017 at 12:58 AM, Peter Krempa  wrote:
>
>> On Wed, Sep 20, 2017 at 16:32:45 -0700, ashish mittal wrote:
>> > Hi,
>> >
>> > I have done TLS testing with this patch series and the tests passed fine
>> > with the secAlias fix in place.
>> >
>> > (1) Applied all the v9 patches.
>> > (2) make install. Reload and restart the libvirtd daemon.
>> > (3) Make sure able to start guest with TLS enabled VxHS disk in the
>> domain
>> > XML.
>> > (4) Try to hot-plug another TLS disk. libvirtd crashes.
>> >
>> > [root@audi libvirt] 2017-09-20 15:59:25# virsh attach-device myfc24
>> > ../../hotplug_disk_1.xml
>> > error: Disconnected from qemu:///system due to end of file
>> > error: Failed to attach device from ../../hotplug_disk_1.xml
>> > error: End of file while reading data: Input/output error
>>
>> So it's clearly ridiculous to write code without having a way how to
>> test it. How can we (upstream) get a VxHS server so that we can properly
>> test the code?
>>
>
> Fair ask :)
> VxHS test server is a part of the libqnio GitHub repo. It can be
> downloaded from https://github.com/VeritasHyperScale/libqnio.git
>
> To run the test server with lots of debug messages -
> make debug, make install (optional - just copies the library and test
> server to a particular location)
> ./src/qnio_server -v -l /dev/stdout
>
> For running the server in TLS mode, the following files are needed before
> test server is started -
> CACERT"/var/lib/libvxhs/cacert.pem"
> Enable SSL mode by touching   "/var/lib/libvxhs/secure"
> SERVER_KEY"/var/lib/libvxhs/server.key"
> SERVER_CERT   "/var/lib/libvxhs/server.cert"
>
> Regards,
> Ashish
>

To create the backing file that is added to the domain XML -
[amittal@localhost libqnio]$ ./src/test/create_vdisk.sh
Usage: create_vdisk  

I just use -
./src/test/create_vdisk.sh /tmp/vxhs_testfile 1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2017-09-21 Thread Peter Krempa 
On Wed, Sep 20, 2017 at 16:32:45 -0700, ashish mittal wrote:
> Hi,
> 
> I have done TLS testing with this patch series and the tests passed fine
> with the secAlias fix in place.
> 
> (1) Applied all the v9 patches.
> (2) make install. Reload and restart the libvirtd daemon.
> (3) Make sure able to start guest with TLS enabled VxHS disk in the domain
> XML.
> (4) Try to hot-plug another TLS disk. libvirtd crashes.
> 
> [root@audi libvirt] 2017-09-20 15:59:25# virsh attach-device myfc24
> ../../hotplug_disk_1.xml
> error: Disconnected from qemu:///system due to end of file
> error: Failed to attach device from ../../hotplug_disk_1.xml
> error: End of file while reading data: Input/output error

So it's clearly ridiculous to write code without having a way how to
test it. How can we (upstream) get a VxHS server so that we can properly
test the code?


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

2017-09-20 Thread ashish mittal 
Hi,

I have done TLS testing with this patch series and the tests passed fine
with the secAlias fix in place.

(1) Applied all the v9 patches.
(2) make install. Reload and restart the libvirtd daemon.
(3) Make sure able to start guest with TLS enabled VxHS disk in the domain
XML.
(4) Try to hot-plug another TLS disk. libvirtd crashes.

[root@audi libvirt] 2017-09-20 15:59:25# virsh attach-device myfc24
../../hotplug_disk_1.xml
error: Disconnected from qemu:///system due to end of file
error: Failed to attach device from ../../hotplug_disk_1.xml
error: End of file while reading data: Input/output error

(5) Now add the secAlias patch

[amittal2@audi libvirt] 2017-09-20 16:08:37$ git apply
~/20Sep2017_1/0001-Avoid-a-possible-NULL-pointer-dereference-in-qemuDom.patch

[amittal2@audi libvirt] 2017-09-20 16:09:07$ git diff
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7751a60..bd96272 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1719,7 +1719,8 @@ qemuDomainGetTLSObjects(virQEMUCapsPtr qemuCaps,
 }

 if (qemuBuildTLSx509BackendProps(tlsCertdir, tlsListen, tlsVerify,
- *secAlias, qemuCaps, tlsProps) < 0)
+ secAlias ? *secAlias : NULL, qemuCaps,
+ tlsProps) < 0)
 return -1;

 if (!(*tlsAlias = qemuAliasTLSObjFromSrcAlias(srcAlias)))
[amittal2@audi libvirt] 2017-09-20 16:09:15$

(6) Run the new libvirtd

[root@audi libvirt] 2017-09-20 16:13:04# make install
...
[root@audi libvirt] 2017-09-20 16:14:05# systemctl daemon-reload
[root@audi libvirt] 2017-09-20 16:14:11# systemctl restart libvirtd.service
[root@audi libvirt] 2017-09-20 16:14:13#

(7) Attached and detached two TLS enabled VxHS disks several times. All
were successful.

[root@audi libvirt] 2017-09-20 16:14:14# virsh attach-device myfc24
../../hotplug_disk_1.xml
Device attached successfully

[root@audi libvirt] 2017-09-20 16:14:24# virsh attach-device myfc24
../../hotplug_disk_2.xml
Device attached successfully

[root@audi libvirt] 2017-09-20 16:14:57# virsh detach-device myfc24
../../hotplug_disk_1.xml
Device detached successfully

[root@audi libvirt] 2017-09-20 16:15:11# virsh detach-device myfc24
../../hotplug_disk_2.xml
Device detached successfully

[root@audi libvirt] 2017-09-20 16:15:16# virsh attach-device myfc24
../../hotplug_disk_2.xml
Device attached successfully

[root@audi libvirt] 2017-09-20 16:15:19# virsh attach-device myfc24
../../hotplug_disk_1.xml
Device attached successfully

[root@audi libvirt] 2017-09-20 16:15:22# virsh attach-device myfc24
../../hotplug_disk_1.xml
error: Failed to attach device from ../../hotplug_disk_1.xml
error: XML error: target 'vdb' duplicated for disk sources
'/tmp/test_vxhs_disk_2' and '/tmp/test_vxhs_disk_2'

[root@audi libvirt] 2017-09-20 16:15:28# virsh detach-device myfc24
../../hotplug_disk_2.xml
Device detached successfully

[root@audi libvirt] 2017-09-20 16:15:51# virsh detach-device myfc24
../../hotplug_disk_1.xml
Device detached successfully

[root@audi libvirt] 2017-09-20 16:15:55#

[root@audi libvirt] 2017-09-20 16:28:23# cat ../../hotplug_disk_1.xml

  
  

  
  
  eb90327c-8302-4725-9e1b-4e85ed4dc252


[root@audi libvirt] 2017-09-20 16:28:36# cat ../../hotplug_disk_2.xml

  
  

  
  
  eb90327c-8302-4725-9e1b-4e85ed4dc253


IMHO, the patches are good to go :)

Thanks,
Ashish

On Tue, Sep 19, 2017 at 6:32 PM, John Ferlan  wrote:

> From: Ashish Mittal 
>
> 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=,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

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

2017-09-19 Thread John Ferlan 
From: Ashish Mittal 

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=,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 
Signed-off-by: John Ferlan 
---
 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;
+}
+
 if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(src->hosts, 
true)))
 return NULL;
 
 /* VxHS disk specification example:
  * { driver:"vxhs",
+ *   tls-creds:"objvirtio-disk0_tls0",
  *   vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
  *   server:{type:"tcp", host:"1.2.3.4", port:}}
  */
 if (virJSONValueObjectCreate(,
  "s:driver", protocol,
+ "S:tls-creds", src->tlsAlias,
  "s:vdisk-id", src->path,
  "a:server", server, NULL) < 0)
 virJSONValueFree(server);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9b3e3fc04..756bf3836 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -794,6 +794,35 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
 }
 
 
+/* qemuBuildDiskSrcTLSx509CommandLine:
+ *
+ * Add TLS object if the disk src uses a secure communication channel
+ *
+ * Returns 0 on success, -1 w/ error on some sort of failure.
+ */
+static int
+qemuBuildDiskSrcTLSx509CommandLine(virCommandPtr cmd,
+   virStorageSourcePtr src,
+   const char *srcalias,
+   virQEMUCapsPtr qemuCaps)
+{
+
+
+/* other protocols may be added later */
+if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS &&
+src->haveTLS == VIR_TRISTATE_BOOL_YES) {
+if (!(src->tlsAlias = qemuAliasTLSObjFromSrcAlias(srcalias)))
+return -1;
+
+return qemuBuildTLSx509CommandLine(cmd, src->tlsCertdir,
+   src->tlsListen, src->tlsVerify,
+   false, srcalias, qemuCaps);
+}
+
+return 0;
+}
+
+
 static char *
 qemuBuildNetworkDriveURI(virStorageSourcePtr src,
  qemuDomainSecretInfoPtr secinfo)
@@ -2221,6 +2250,10 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
 if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0)
 return -1;
 
+if (qemuBuildDiskSrcTLSx509CommandLine(cmd, disk->src, 
disk->info.alias,
+   qemuCaps) < 0)
+