[libvirt] [PATCH v9 0/4] Add support for Veritas HyperScale (VxHS) block device protocol
v8: https://www.redhat.com/archives/libvir-list/2017-September/msg00375.html Changes since v8 * Patches 1-6 patches, everything else deals with TLS. * Patches 7-8 - no change, now patch 1-2. Didn't push since they're related to these changes * Patch 9 - Old patch Was ACK'd, new patch is 3. Old patch11 review caused me to add more fields to _virStorageSource to pepare "tlsCertdir", "tlsListen", and "tlsVerify" using the cfg field for certdir, but direct sets for listen and verify. * Patch 10 - dropped * Patch 11 - now patch 4. Alter the qemu_command.c and qemu_hotplug.c code to use the virStorageSource fields rather than from disk or cfg. Code makes more use of whether the "src->haveTLS == VIR_TRISTATE_BOOL_YES" is true rather then src->tlsAlias. Added qemuDomainDelDiskSrcTLSObject. Renamed/reworked the qemuDomainAddDiskSrcTLSObject. Looks cleaner to me... Ashish will post a patch separately to fix an issue found in qemuDomainGetTLSObjects when secAlias == NULL and the deref of *secAlias. Ashish Mittal (3): conf: Introduce TLS options for VxHS block device clients util: Add TLS attributes to virStorageSource qemu: Add TLS support for Veritas HyperScale (VxHS) John Ferlan (1): qemu: Introduce qemuDomainPrepareDiskSource docs/formatdomain.html.in | 40 ++++++++--- docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 29 +++++++- src/qemu/libvirtd_qemu.aug | 4 ++ src/qemu/qemu.conf | 34 ++++++++++ src/qemu/qemu_block.c | 8 +++ src/qemu/qemu_command.c | 33 +++++++++ src/qemu/qemu_conf.c | 16 +++++ src/qemu/qemu_conf.h | 3 + src/qemu/qemu_domain.c | 73 ++++++++++++++++++++ src/qemu/qemu_domain.h | 11 +++ src/qemu/qemu_hotplug.c | 79 ++++++++++++++++++++++ src/qemu/qemu_process.c | 4 ++ src/qemu/test_libvirtd_qemu.aug.in | 2 + src/util/virstoragefile.c | 11 ++- src/util/virstoragefile.h | 15 ++++ ...-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 ++++++++ ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml | 32 +++++++++ tests/qemuxml2argvtest.c | 7 ++ ...uxml2xmlout-disk-drive-network-tlsx509-vxhs.xml | 34 ++++++++++ tests/qemuxml2xmltest.c | 1 + 23 files changed, 551 insertions(+), 13 deletions(-) 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 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml -- 2.13.5
From: Ashish Mittal <Ashish.Mittal@veritas.com> Add a new TLS X.509 certificate type - "vxhs". This will handle the creation of a TLS certificate capability for properly configured VxHS network block device clients. The following describes the behavior of TLS for VxHS block device: (1) Two new options have been added in /etc/libvirt/qemu.conf to control TLS behavior with VxHS block devices "vxhs_tls" and "vxhs_tls_x509_cert_dir". (2) Setting "vxhs_tls=1" in /etc/libvirt/qemu.conf will enable TLS for VxHS block devices. (3) "vxhs_tls_x509_cert_dir" can be set to the full path where the TLS CA certificate and the client certificate and keys are saved. If this value is missing, the "default_tls_x509_cert_dir" will be used instead. If the environment is not configured properly the authentication to the VxHS server will fail. Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/libvirtd_qemu.aug | 4 ++++ src/qemu/qemu.conf | 34 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.c | 16 ++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 5 files changed, 59 insertions(+) diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index e1983d1fd..c19bf3a43 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -115,6 +115,9 @@ module Libvirtd_qemu = let memory_entry = str_entry "memory_backing_dir" + let vxhs_entry = bool_entry "vxhs_tls" + | str_entry "vxhs_tls_x509_cert_dir" + (* Each entry in the config is one of the following ... *) let entry = default_tls_entry | vnc_entry @@ -133,6 +136,7 @@ module Libvirtd_qemu = | nvram_entry | gluster_debug_level_entry | memory_entry + | vxhs_entry let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index f977e3b71..2d20d790b 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -258,6 +258,40 @@ #chardev_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000" +# Enable use of TLS encryption for all VxHS network block devices that +# don't specifically disable. +# +# When the VxHS network block device server is set up appropriately, +# x509 certificates are required for authentication between the clients +# (qemu processes) and the remote VxHS server. +# +# It is necessary to setup CA and issue the client certificate before +# enabling this. +# +#vxhs_tls = 1 + + +# In order to override the default TLS certificate location for VxHS +# device TCP certificates, supply a valid path to the certificate directory. +# This is used to authenticate the VxHS block device clients to the VxHS +# server. +# +# If the provided path does not exist then the default_tls_x509_cert_dir +# path will be used. +# +# VxHS block device clients expect the client certificate and key to be +# present in the certificate directory along with the CA master certificate. +# If using the default environment, default_tls_x509_verify must be configured. +# The server key as well as secret UUID that would decrypt it is not used. +# Thus a VxHS directory must contain the following: +# +# ca-cert.pem - the CA master certificate +# client-cert.pem - the client certificate signed with the ca-cert.pem +# client-key.pem - the client private key +# +#vxhs_tls_x509_cert_dir = "/etc/pki/libvirt-vxhs" + + # In order to override the default TLS certificate location for migration # certificates, supply a valid path to the certificate directory. If the # provided path does not exist then the default_tls_x509_cert_dir path diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 68c00c1e8..ec61c9c52 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -283,6 +283,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) SET_TLS_X509_CERT_DEFAULT(spice); SET_TLS_X509_CERT_DEFAULT(chardev); SET_TLS_X509_CERT_DEFAULT(migrate); + SET_TLS_X509_CERT_DEFAULT(vxhs); #undef SET_TLS_X509_CERT_DEFAULT @@ -380,6 +381,8 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->chardevTLSx509certdir); VIR_FREE(cfg->chardevTLSx509secretUUID); + VIR_FREE(cfg->vxhsTLSx509certdir); + VIR_FREE(cfg->migrateTLSx509certdir); VIR_FREE(cfg->migrateTLSx509secretUUID); @@ -457,6 +460,7 @@ virQEMUDriverConfigTLSDirResetDefaults(virQEMUDriverConfigPtr cfg) CHECK_RESET_CERT_DIR_DEFAULT(spice); CHECK_RESET_CERT_DIR_DEFAULT(chardev); CHECK_RESET_CERT_DIR_DEFAULT(migrate); + CHECK_RESET_CERT_DIR_DEFAULT(vxhs); return 0; } @@ -556,6 +560,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; if (virConfGetValueBool(conf, "spice_auto_unix_socket", &cfg->spiceAutoUnixSocket) < 0) goto cleanup; + if (virConfGetValueBool(conf, "vxhs_tls", &cfg->vxhsTLS) < 0) + goto cleanup; + if (virConfGetValueString(conf, "vxhs_tls_x509_cert_dir", &cfg->vxhsTLSx509certdir) < 0) + goto cleanup; #define GET_CONFIG_TLS_CERTINFO(val) \ do { \ @@ -976,6 +984,14 @@ virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg) return -1; } + if (STRNEQ(cfg->vxhsTLSx509certdir, SYSCONFDIR "/pki/qemu") && + !virFileExists(cfg->vxhsTLSx509certdir)) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("vxhs_tls_x509_cert_dir directory '%s' does not exist"), + cfg->vxhsTLSx509certdir); + return -1; + } + return 0; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index d469b50bd..13b6f818a 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -203,6 +203,9 @@ struct _virQEMUDriverConfig { unsigned int glusterDebugLevel; char *memoryBackingDir; + + bool vxhsTLS; + char *vxhsTLSx509certdir; }; /* Main driver state */ diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 676d48cf5..688e5b9fd 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -25,6 +25,8 @@ module Test_libvirtd_qemu = { "chardev_tls_x509_cert_dir" = "/etc/pki/libvirt-chardev" } { "chardev_tls_x509_verify" = "1" } { "chardev_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } +{ "vxhs_tls" = "1" } +{ "vxhs_tls_x509_cert_dir" = "/etc/pki/libvirt-vxhs" } { "migrate_tls_x509_cert_dir" = "/etc/pki/libvirt-migrate" } { "migrate_tls_x509_verify" = "1" } { "migrate_tls_x509_secret_uuid" = "00000000-0000-0000-0000-000000000000" } -- 2.13.5
On Tue, Sep 19, 2017 at 21:32:43 -0400, John Ferlan wrote:
From: Ashish Mittal <Ashish.Mittal@veritas.com>
Add a new TLS X.509 certificate type - "vxhs". This will handle the creation of a TLS certificate capability for properly configured VxHS network block device clients.
The following describes the behavior of TLS for VxHS block device:
(1) Two new options have been added in /etc/libvirt/qemu.conf to control TLS behavior with VxHS block devices "vxhs_tls" and "vxhs_tls_x509_cert_dir". (2) Setting "vxhs_tls=1" in /etc/libvirt/qemu.conf will enable TLS for VxHS block devices. (3) "vxhs_tls_x509_cert_dir" can be set to the full path where the TLS CA certificate and the client certificate and keys are saved. If this value is missing, the "default_tls_x509_cert_dir" will be used instead. If the environment is not configured properly the authentication to the VxHS server will fail.
Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/libvirtd_qemu.aug | 4 ++++ src/qemu/qemu.conf | 34 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.c | 16 ++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 5 files changed, 59 insertions(+)
[...]
+# Enable use of TLS encryption for all VxHS network block devices that +# don't specifically disable. +# +# When the VxHS network block device server is set up appropriately, +# x509 certificates are required for authentication between the clients +# (qemu processes) and the remote VxHS server. +# +# It is necessary to setup CA and issue the client certificate before +# enabling this. +# +#vxhs_tls = 1 + + +# In order to override the default TLS certificate location for VxHS +# device TCP certificates, supply a valid path to the certificate directory.
The first part of the sentence does not make much sense. In order to override the default TLS certificate location for VxHS backed storage, supply a valid path to the certificate directory.
+# This is used to authenticate the VxHS block device clients to the VxHS +# server. +# +# If the provided path does not exist then the default_tls_x509_cert_dir +# path will be used. +# +# VxHS block device clients expect the client certificate and key to be +# present in the certificate directory along with the CA master certificate. +# If using the default environment, default_tls_x509_verify must be configured. +# The server key as well as secret UUID that would decrypt it is not used.
What do you mean by: "UUID that would decrypt it"? Rest looks okay, but I need clarification on the above statemet.
On 09/27/2017 08:43 AM, Peter Krempa wrote:
On Tue, Sep 19, 2017 at 21:32:43 -0400, John Ferlan wrote:
From: Ashish Mittal <Ashish.Mittal@veritas.com>
Add a new TLS X.509 certificate type - "vxhs". This will handle the creation of a TLS certificate capability for properly configured VxHS network block device clients.
The following describes the behavior of TLS for VxHS block device:
(1) Two new options have been added in /etc/libvirt/qemu.conf to control TLS behavior with VxHS block devices "vxhs_tls" and "vxhs_tls_x509_cert_dir". (2) Setting "vxhs_tls=1" in /etc/libvirt/qemu.conf will enable TLS for VxHS block devices. (3) "vxhs_tls_x509_cert_dir" can be set to the full path where the TLS CA certificate and the client certificate and keys are saved. If this value is missing, the "default_tls_x509_cert_dir" will be used instead. If the environment is not configured properly the authentication to the VxHS server will fail.
Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/libvirtd_qemu.aug | 4 ++++ src/qemu/qemu.conf | 34 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.c | 16 ++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 5 files changed, 59 insertions(+)
[...]
+# Enable use of TLS encryption for all VxHS network block devices that +# don't specifically disable. +# +# When the VxHS network block device server is set up appropriately, +# x509 certificates are required for authentication between the clients +# (qemu processes) and the remote VxHS server. +# +# It is necessary to setup CA and issue the client certificate before +# enabling this. +# +#vxhs_tls = 1 + + +# In order to override the default TLS certificate location for VxHS +# device TCP certificates, supply a valid path to the certificate directory.
The first part of the sentence does not make much sense.
In order to override the default TLS certificate location for VxHS backed storage, supply a valid path to the certificate directory.
<me> wondering where "device TCP certificates," slipped into the text... Looks like it was present back in v4 as well... I'll adjust to use your text
+# This is used to authenticate the VxHS block device clients to the VxHS +# server. +# +# If the provided path does not exist then the default_tls_x509_cert_dir +# path will be used. +# +# VxHS block device clients expect the client certificate and key to be +# present in the certificate directory along with the CA master certificate. +# If using the default environment, default_tls_x509_verify must be configured. +# The server key as well as secret UUID that would decrypt it is not used.
What do you mean by: "UUID that would decrypt it"?
Rest looks okay, but I need clarification on the above statemet.
The "server key" comes from the default description of "server-key.pem" and the rest from the description of default_tls_x509_secret_uuid which essentially states the UUID would be UUID of a secret that would decrypt the server-key.pem file. Basically, it's a way to point out that the VxHS TLS certificate environment wouldn't use a similar setup from a default (or Chardev or Migrate) to provide a secret UUID parameter since the configuration is client only. I can strike out that last sentence completely as it perhaps not that important and probably confusing. John
On Wed, Sep 27, 2017 at 09:12:06 -0400, John Ferlan wrote:
On 09/27/2017 08:43 AM, Peter Krempa wrote:
On Tue, Sep 19, 2017 at 21:32:43 -0400, John Ferlan wrote:
From: Ashish Mittal <Ashish.Mittal@veritas.com>
Add a new TLS X.509 certificate type - "vxhs". This will handle the creation of a TLS certificate capability for properly configured VxHS network block device clients.
The following describes the behavior of TLS for VxHS block device:
(1) Two new options have been added in /etc/libvirt/qemu.conf to control TLS behavior with VxHS block devices "vxhs_tls" and "vxhs_tls_x509_cert_dir". (2) Setting "vxhs_tls=1" in /etc/libvirt/qemu.conf will enable TLS for VxHS block devices. (3) "vxhs_tls_x509_cert_dir" can be set to the full path where the TLS CA certificate and the client certificate and keys are saved. If this value is missing, the "default_tls_x509_cert_dir" will be used instead. If the environment is not configured properly the authentication to the VxHS server will fail.
Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/libvirtd_qemu.aug | 4 ++++ src/qemu/qemu.conf | 34 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.c | 16 ++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 5 files changed, 59 insertions(+)
[...]
The "server key" comes from the default description of "server-key.pem" and the rest from the description of default_tls_x509_secret_uuid which essentially states the UUID would be UUID of a secret that would decrypt the server-key.pem file.
Ah, right.
Basically, it's a way to point out that the VxHS TLS certificate environment wouldn't use a similar setup from a default (or Chardev or Migrate) to provide a secret UUID parameter since the configuration is client only. I can strike out that last sentence completely as it perhaps not that important and probably confusing.
You can state that since this is only a client, the server key/cert is not needed in such configuration, which automatically implies that the secret for decrypting it is not necessary as well.
On Wed, Sep 27, 2017 at 15:27:15 +0200, Peter Krempa wrote:
On Wed, Sep 27, 2017 at 09:12:06 -0400, John Ferlan wrote:
On 09/27/2017 08:43 AM, Peter Krempa wrote:
On Tue, Sep 19, 2017 at 21:32:43 -0400, John Ferlan wrote:
From: Ashish Mittal <Ashish.Mittal@veritas.com>
Add a new TLS X.509 certificate type - "vxhs". This will handle the creation of a TLS certificate capability for properly configured VxHS network block device clients.
The following describes the behavior of TLS for VxHS block device:
(1) Two new options have been added in /etc/libvirt/qemu.conf to control TLS behavior with VxHS block devices "vxhs_tls" and "vxhs_tls_x509_cert_dir". (2) Setting "vxhs_tls=1" in /etc/libvirt/qemu.conf will enable TLS for VxHS block devices. (3) "vxhs_tls_x509_cert_dir" can be set to the full path where the TLS CA certificate and the client certificate and keys are saved. If this value is missing, the "default_tls_x509_cert_dir" will be used instead. If the environment is not configured properly the authentication to the VxHS server will fail.
Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/libvirtd_qemu.aug | 4 ++++ src/qemu/qemu.conf | 34 ++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.c | 16 ++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 5 files changed, 59 insertions(+)
[...]
The "server key" comes from the default description of "server-key.pem" and the rest from the description of default_tls_x509_secret_uuid which essentially states the UUID would be UUID of a secret that would decrypt the server-key.pem file.
Ah, right.
Basically, it's a way to point out that the VxHS TLS certificate environment wouldn't use a similar setup from a default (or Chardev or Migrate) to provide a secret UUID parameter since the configuration is client only. I can strike out that last sentence completely as it perhaps not that important and probably confusing.
You can state that since this is only a client, the server key/cert is not needed in such configuration, which automatically implies that the secret for decrypting it is not necessary as well.
ACK with the above modification.
From: Ashish Mittal <Ashish.Mittal@veritas.com> Add an optional virTristateBool haveTLS to virStorageSource to manage whether a storage source will be using TLS. Sample XML for a VxHS disk: <disk type='network' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251' tls='yes'> <host name='192.168.0.1' port='9999'/> </source> <target dev='vda' bus='virtio'/> </disk> Additionally add a tlsFromConfig boolean to control whether the TLS setting was due to domain configuration or qemu.conf global setting in order to decide whether to Format the haveTLS setting for either a live or saved domain configuration file. Update the qemuxml2xmltest in order to add a test to show the proper parsing. Also update the docs to describe the tls attribute plus clean up the description in the surrounding area to make the information a bit more readable rather than one winding paragraph. Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 40 ++++++++++++++++------ docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 29 ++++++++++++++-- src/util/virstoragefile.c | 2 ++ src/util/virstoragefile.h | 7 ++++ ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml | 32 +++++++++++++++++ ...uxml2xmlout-disk-drive-network-tlsx509-vxhs.xml | 34 ++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 138 insertions(+), 12 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9ce4620c6..3e10213b5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2514,19 +2514,39 @@ <dd> The <code>protocol</code> attribute specifies the protocol to access to the requested image. Possible values are "nbd", - "iscsi", "rbd", "sheepdog", "gluster" or "vxhs". If the - <code>protocol</code> attribute is "rbd", "sheepdog", "gluster" - or "vxhs", an additional attribute <code>name</code> is - mandatory to specify which volume/image will be used. For "nbd", - the <code>name</code> attribute is optional. For "iscsi" - (<span class="since">since 1.0.4</span>), the <code>name</code> - attribute may include a logical unit number, separated from the - target's name by a slash (e.g., + "iscsi", "rbd", "sheepdog", "gluster" or "vxhs". + + <p>If the <code>protocol</code> attribute is "rbd", "sheepdog", + "gluster", or "vxhs", an additional attribute <code>name</code> + is mandatory to specify which volume/image will be used. + </p> + + <p>For "nbd", the <code>name</code> attribute is optional. + </p> + + <p>For "iscsi" (<span class="since">since 1.0.4</span>), the + <code>name</code> attribute may include a logical unit number, + separated from the target's name by a slash (e.g., <code>iqn.2013-07.com.example:iscsi-pool/1</code>). If not specified, the default LUN is zero. - For "vxhs" (<span class="since">since 3.8.0</span>), the + </p> + + <p>For "vxhs" (<span class="since">since 3.8.0</span>), the <code>name</code> is the UUID of the volume, assigned by the - HyperScale server. + HyperScale server. Additionally, an optional attribute + <code>tls</code> (QEMU only) can be used to control whether a + VxHS block device would utilize a hypervisor configured TLS + X.509 certificate environment in order to encrypt the data + channel. For the QEMU hypervisor, usage of a TLS environment can + also be globally controlled on the host by the + <code>vxhs_tls</code> and <code>vxhs_tls_x509_cert_dir</code> or + <code>default_tls_x509_cert_dir</code> settings in the file + /etc/libvirt/qemu.conf. If <code>vxhs_tls</code> is enabled, + then unless the domain <code>tls</code> attribute is set to "no", + libvirt will use the host configured TLS environment. If the + <code>tls</code> attribute is set to "yes", then regardless of + the qemu.conf setting, TLS authentication will be attempted. + </p> <span class="since">Since 0.8.7</span> </dd> <dt><code>volume</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 76852abb3..bac371ea3 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1644,6 +1644,11 @@ </choice> </attribute> <attribute name="name"/> + <optional> + <attribute name="tls"> + <ref name="virYesNo"/> + </attribute> + </optional> <ref name="diskSourceNetworkHost"/> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cc5e79b70..a568d9140 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8114,6 +8114,7 @@ virDomainDiskSourceParse(xmlNodePtr node, int ret = -1; char *protocol = NULL; xmlNodePtr saveNode = ctxt->node; + char *haveTLS = NULL; ctxt->node = node; @@ -8147,6 +8148,19 @@ virDomainDiskSourceParse(xmlNodePtr node, goto cleanup; } + /* Check tls=yes|no domain setting for the block device + * At present only VxHS. Other block devices may be added later */ + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS && + (haveTLS = virXMLPropString(node, "tls"))) { + if ((src->haveTLS = + virTristateBoolTypeFromString(haveTLS)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk source 'tls' setting '%s'"), + haveTLS); + goto cleanup; + } + } + /* for historical reasons the volume name for gluster volume is stored * as a part of the path. This is hard to work with when dealing with * relative names. Split out the volume into a separate variable */ @@ -8202,6 +8216,7 @@ virDomainDiskSourceParse(xmlNodePtr node, cleanup: VIR_FREE(protocol); + VIR_FREE(haveTLS); ctxt->node = saveNode; return ret; } @@ -21673,7 +21688,8 @@ virDomainSourceDefFormatSeclabel(virBufferPtr buf, static int virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, virBufferPtr childBuf, - virStorageSourcePtr src) + virStorageSourcePtr src, + unsigned int flags) { size_t n; char *path = NULL; @@ -21690,6 +21706,14 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, VIR_FREE(path); + if (src->haveTLS != VIR_TRISTATE_BOOL_ABSENT && + !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && + src->tlsFromConfig)) + virBufferAsprintf(attrBuf, " tls='%s'", + virTristateBoolTypeToString(src->haveTLS)); + if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) + virBufferAsprintf(attrBuf, " tlsFromConfig='%d'", src->tlsFromConfig); + for (n = 0; n < src->nhosts; n++) { virBufferAddLit(childBuf, "<host"); virBufferEscapeString(childBuf, " name='%s'", src->hosts[n].name); @@ -21754,7 +21778,8 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf, break; case VIR_STORAGE_TYPE_NETWORK: - if (virDomainDiskSourceFormatNetwork(&attrBuf, &childBuf, src) < 0) + if (virDomainDiskSourceFormatNetwork(&attrBuf, &childBuf, + src, flags) < 0) goto error; break; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index ba2045369..35f468e35 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2039,6 +2039,8 @@ virStorageSourceCopy(const virStorageSource *src, ret->physical = src->physical; ret->readonly = src->readonly; ret->shared = src->shared; + ret->haveTLS = src->haveTLS; + ret->tlsFromConfig = src->tlsFromConfig; /* storage driver metadata are not copied */ ret->drv = NULL; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index f7e897f25..4817090fc 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -281,6 +281,13 @@ struct _virStorageSource { /* metadata that allows identifying given storage source */ char *nodeformat; /* name of the format handler object */ char *nodestorage; /* name of the storage object */ + + /* An optional setting to enable usage of TLS for the storage source */ + int haveTLS; /* enum virTristateBool */ + + /* Indication whether the haveTLS value was altered due to qemu.conf + * setting when haveTLS is missing from the domain config file */ + bool tlsFromConfig; }; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml new file mode 100644 index 000000000..61b5e2e79 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251' tls='yes'> + <host name='192.168.0.1' port='9999'/> + </source> + <target dev='vda' bus='virtio'/> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml new file mode 100644 index 000000000..16f0883e0 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml @@ -0,0 +1,34 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251' tls='yes'> + <host name='192.168.0.1' port='9999'/> + </source> + <target dev='vda' bus='virtio'/> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 80159386c..bf8eb46b1 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -475,6 +475,7 @@ mymain(void) DO_TEST("disk-drive-network-rbd-ceph-env", NONE); DO_TEST("disk-drive-network-sheepdog", NONE); DO_TEST("disk-drive-network-vxhs", NONE); + DO_TEST("disk-drive-network-tlsx509-vxhs", NONE); DO_TEST("disk-scsi-device", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_SCSI_LSI); DO_TEST("disk-scsi-vscsi", NONE); -- 2.13.5
On Tue, Sep 19, 2017 at 21:32:44 -0400, John Ferlan wrote:
From: Ashish Mittal <Ashish.Mittal@veritas.com>
Add an optional virTristateBool haveTLS to virStorageSource to manage whether a storage source will be using TLS.
Sample XML for a VxHS disk:
<disk type='network' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251' tls='yes'> <host name='192.168.0.1' port='9999'/> </source> <target dev='vda' bus='virtio'/> </disk>
Additionally add a tlsFromConfig boolean to control whether the TLS setting was due to domain configuration or qemu.conf global setting in order to decide whether to Format the haveTLS setting for either a live or saved domain configuration file.
Update the qemuxml2xmltest in order to add a test to show the proper parsing.
Also update the docs to describe the tls attribute plus clean up the description in the surrounding area to make the information a bit more readable rather than one winding paragraph.
Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 40 ++++++++++++++++------ docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 29 ++++++++++++++-- src/util/virstoragefile.c | 2 ++ src/util/virstoragefile.h | 7 ++++ ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml | 32 +++++++++++++++++ ...uxml2xmlout-disk-drive-network-tlsx509-vxhs.xml | 34 ++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 138 insertions(+), 12 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9ce4620c6..3e10213b5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2514,19 +2514,39 @@ <dd> The <code>protocol</code> attribute specifies the protocol to access to the requested image. Possible values are "nbd", - "iscsi", "rbd", "sheepdog", "gluster" or "vxhs". If the - <code>protocol</code> attribute is "rbd", "sheepdog", "gluster" - or "vxhs", an additional attribute <code>name</code> is - mandatory to specify which volume/image will be used. For "nbd", - the <code>name</code> attribute is optional. For "iscsi" - (<span class="since">since 1.0.4</span>), the <code>name</code> - attribute may include a logical unit number, separated from the - target's name by a slash (e.g., + "iscsi", "rbd", "sheepdog", "gluster" or "vxhs". + + <p>If the <code>protocol</code> attribute is "rbd", "sheepdog", + "gluster", or "vxhs", an additional attribute <code>name</code> + is mandatory to specify which volume/image will be used. + </p> + + <p>For "nbd", the <code>name</code> attribute is optional. + </p> + + <p>For "iscsi" (<span class="since">since 1.0.4</span>), the + <code>name</code> attribute may include a logical unit number, + separated from the target's name by a slash (e.g., <code>iqn.2013-07.com.example:iscsi-pool/1</code>). If not specified, the default LUN is zero. - For "vxhs" (<span class="since">since 3.8.0</span>), the + </p> + + <p>For "vxhs" (<span class="since">since 3.8.0</span>), the <code>name</code> is the UUID of the volume, assigned by the - HyperScale server.
Everything above is not really relevant to adding TLS to VxHS and thus deserves a separate patch.
+ HyperScale server. Additionally, an optional attribute + <code>tls</code> (QEMU only) can be used to control whether a + VxHS block device would utilize a hypervisor configured TLS + X.509 certificate environment in order to encrypt the data + channel. For the QEMU hypervisor, usage of a TLS environment can + also be globally controlled on the host by the + <code>vxhs_tls</code> and <code>vxhs_tls_x509_cert_dir</code> or + <code>default_tls_x509_cert_dir</code> settings in the file + /etc/libvirt/qemu.conf. If <code>vxhs_tls</code> is enabled, + then unless the domain <code>tls</code> attribute is set to "no", + libvirt will use the host configured TLS environment. If the + <code>tls</code> attribute is set to "yes", then regardless of + the qemu.conf setting, TLS authentication will be attempted. + </p> <span class="since">Since 0.8.7</span> </dd> <dt><code>volume</code></dt>
[...]
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cc5e79b70..a568d9140 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
[...]
@@ -8147,6 +8148,19 @@ virDomainDiskSourceParse(xmlNodePtr node, goto cleanup; }
+ /* Check tls=yes|no domain setting for the block device + * At present only VxHS. Other block devices may be added later */ + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS && + (haveTLS = virXMLPropString(node, "tls"))) { + if ((src->haveTLS = + virTristateBoolTypeFromString(haveTLS)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk source 'tls' setting '%s'"), + haveTLS); + goto cleanup; + } + } + /* for historical reasons the volume name for gluster volume is stored * as a part of the path. This is hard to work with when dealing with * relative names. Split out the volume into a separate variable */
[...]
@@ -21690,6 +21706,14 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
VIR_FREE(path);
+ if (src->haveTLS != VIR_TRISTATE_BOOL_ABSENT && + !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && + src->tlsFromConfig)) + virBufferAsprintf(attrBuf, " tls='%s'", + virTristateBoolTypeToString(src->haveTLS)); + if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) + virBufferAsprintf(attrBuf, " tlsFromConfig='%d'", src->tlsFromConfig);
Why is this attribute formatted as a number? Also you don't really seem to parse it.
On 09/27/2017 09:02 AM, Peter Krempa wrote:
On Tue, Sep 19, 2017 at 21:32:44 -0400, John Ferlan wrote:
From: Ashish Mittal <Ashish.Mittal@veritas.com>
Add an optional virTristateBool haveTLS to virStorageSource to manage whether a storage source will be using TLS.
Sample XML for a VxHS disk:
<disk type='network' device='disk'> <driver name='qemu' type='raw' cache='none'/> <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251' tls='yes'> <host name='192.168.0.1' port='9999'/> </source> <target dev='vda' bus='virtio'/> </disk>
Additionally add a tlsFromConfig boolean to control whether the TLS setting was due to domain configuration or qemu.conf global setting in order to decide whether to Format the haveTLS setting for either a live or saved domain configuration file.
Update the qemuxml2xmltest in order to add a test to show the proper parsing.
Also update the docs to describe the tls attribute plus clean up the description in the surrounding area to make the information a bit more readable rather than one winding paragraph.
Signed-off-by: Ashish Mittal <Ashish.Mittal@veritas.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 40 ++++++++++++++++------ docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 29 ++++++++++++++-- src/util/virstoragefile.c | 2 ++ src/util/virstoragefile.h | 7 ++++ ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml | 32 +++++++++++++++++ ...uxml2xmlout-disk-drive-network-tlsx509-vxhs.xml | 34 ++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 8 files changed, 138 insertions(+), 12 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 9ce4620c6..3e10213b5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2514,19 +2514,39 @@ <dd> The <code>protocol</code> attribute specifies the protocol to access to the requested image. Possible values are "nbd", - "iscsi", "rbd", "sheepdog", "gluster" or "vxhs". If the - <code>protocol</code> attribute is "rbd", "sheepdog", "gluster" - or "vxhs", an additional attribute <code>name</code> is - mandatory to specify which volume/image will be used. For "nbd", - the <code>name</code> attribute is optional. For "iscsi" - (<span class="since">since 1.0.4</span>), the <code>name</code> - attribute may include a logical unit number, separated from the - target's name by a slash (e.g., + "iscsi", "rbd", "sheepdog", "gluster" or "vxhs". + + <p>If the <code>protocol</code> attribute is "rbd", "sheepdog", + "gluster", or "vxhs", an additional attribute <code>name</code> + is mandatory to specify which volume/image will be used. + </p> + + <p>For "nbd", the <code>name</code> attribute is optional. + </p> + + <p>For "iscsi" (<span class="since">since 1.0.4</span>), the + <code>name</code> attribute may include a logical unit number, + separated from the target's name by a slash (e.g., <code>iqn.2013-07.com.example:iscsi-pool/1</code>). If not specified, the default LUN is zero. - For "vxhs" (<span class="since">since 3.8.0</span>), the + </p> + + <p>For "vxhs" (<span class="since">since 3.8.0</span>), the <code>name</code> is the UUID of the volume, assigned by the - HyperScale server.
Everything above is not really relevant to adding TLS to VxHS and thus deserves a separate patch.
I can separate - not a problem. The reason it was done the way it was is because as I was editing the section it became painfully obvious that the text was becoming impossible to read. I did note that in the commit message BTW.
+ HyperScale server. Additionally, an optional attribute + <code>tls</code> (QEMU only) can be used to control whether a + VxHS block device would utilize a hypervisor configured TLS + X.509 certificate environment in order to encrypt the data + channel. For the QEMU hypervisor, usage of a TLS environment can + also be globally controlled on the host by the + <code>vxhs_tls</code> and <code>vxhs_tls_x509_cert_dir</code> or + <code>default_tls_x509_cert_dir</code> settings in the file + /etc/libvirt/qemu.conf. If <code>vxhs_tls</code> is enabled, + then unless the domain <code>tls</code> attribute is set to "no", + libvirt will use the host configured TLS environment. If the + <code>tls</code> attribute is set to "yes", then regardless of + the qemu.conf setting, TLS authentication will be attempted. + </p> <span class="since">Since 0.8.7</span> </dd> <dt><code>volume</code></dt>
[...]
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cc5e79b70..a568d9140 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c
[...]
@@ -8147,6 +8148,19 @@ virDomainDiskSourceParse(xmlNodePtr node, goto cleanup; }
+ /* Check tls=yes|no domain setting for the block device + * At present only VxHS. Other block devices may be added later */ + if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS && + (haveTLS = virXMLPropString(node, "tls"))) { + if ((src->haveTLS = + virTristateBoolTypeFromString(haveTLS)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown disk source 'tls' setting '%s'"), + haveTLS); + goto cleanup; + } + } + /* for historical reasons the volume name for gluster volume is stored * as a part of the path. This is hard to work with when dealing with * relative names. Split out the volume into a separate variable */
[...]
@@ -21690,6 +21706,14 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf,
VIR_FREE(path);
+ if (src->haveTLS != VIR_TRISTATE_BOOL_ABSENT && + !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && + src->tlsFromConfig)) + virBufferAsprintf(attrBuf, " tls='%s'", + virTristateBoolTypeToString(src->haveTLS)); + if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) + virBufferAsprintf(attrBuf, " tlsFromConfig='%d'", src->tlsFromConfig);
Why is this attribute formatted as a number? Also you don't really seem to parse it.
It follows what was done for <chardev>... As for the parsing, strange it was there in my recollection, but it's not there in reality. I wonder where it went. Oh it gets really ugly to try and squash in something since virDomainDiskSourceParse will need @flags <sigh> Tks - John
Introduce a function to setup any TLS needs for a disk source. If there's a configuration or other error setting up the disk source for TLS, then cause the domain startup to fail. For VxHS, follow the chardevTLS model where if the src->haveTLS hasn't been configured, then take the system/global cfg->haveTLS setting for the storage source *and* mark that we've done so via the tlsFromConfig setting in storage source. Next, if we are using TLS, then generate an alias into a virStorageSource 'tlsAlias' field that will be used to create the TLS object and added to the disk object in order to link the two together for QEMU. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 11 +++++++ src/qemu/qemu_process.c | 4 +++ src/util/virstoragefile.c | 9 +++++- src/util/virstoragefile.h | 8 ++++++ 5 files changed, 104 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 50b536eec..8080b7fb1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7601,6 +7601,79 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def, } +/* qemuProcessPrepareDiskSourceTLS: + * @source: pointer to host interface data for disk device + * @diskAlias: alias use for the disk device + * @cfg: driver configuration + * + * Updates host interface TLS encryption setting based on qemu.conf + * for disk devices. This will be presented as "tls='yes|no'" in + * live XML of a guest. + * + * Returns 0 on success, -1 on bad config/failure + */ +int +qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, + const char *diskAlias, + virQEMUDriverConfigPtr cfg) +{ + + /* VxHS doesn't utilize a password protected server certificate, + * so no need to add a secinfo for a secret UUID. */ + if (src->type == VIR_STORAGE_TYPE_NETWORK && + src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) { + + if (src->haveTLS == VIR_TRISTATE_BOOL_ABSENT) { + if (cfg->vxhsTLS) + src->haveTLS = VIR_TRISTATE_BOOL_YES; + else + src->haveTLS = VIR_TRISTATE_BOOL_NO; + src->tlsFromConfig = true; + } + + if (src->haveTLS == VIR_TRISTATE_BOOL_YES) { + if (!diskAlias) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("disk does not have an alias")); + return -1; + } + + /* Grab the vxhsTLSx509certdir and set the verify/listen values. + * NB: tlsAlias filled in during qemuDomainGetTLSObjects. */ + if (VIR_STRDUP(src->tlsCertdir, cfg->vxhsTLSx509certdir) < 0) + return -1; + + src->tlsListen = false; + src->tlsVerify = true; + } + } + + return 0; +} + + +/* qemuProcessPrepareDiskSource: + * @def: live domain definition + * @driver: qemu driver + * + * Returns 0 on success, -1 on failure + */ +int +qemuDomainPrepareDiskSource(virDomainDefPtr def, + virQEMUDriverConfigPtr cfg) +{ + size_t i; + + for (i = 0; i < def->ndisks; i++) { + if (qemuDomainPrepareDiskSourceTLS(def->disks[i]->src, + def->disks[i]->info.alias, + cfg) < 0) + return -1; + } + + return 0; +} + int qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b291dc308..93db23c2b 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -864,6 +864,17 @@ void qemuDomainPrepareChardevSource(virDomainDefPtr def, virQEMUDriverConfigPtr cfg) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int +qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, + const char *diskAlias, + virQEMUDriverConfigPtr cfg) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + +int +qemuDomainPrepareDiskSource(virDomainDefPtr def, + virQEMUDriverConfigPtr cfg) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + int qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index e6cc41e13..c3a1db497 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5357,6 +5357,10 @@ qemuProcessPrepareDomain(virConnectPtr conn, if (qemuDomainMasterKeyCreate(vm) < 0) goto cleanup; + VIR_DEBUG("Prepare disk source backends for TLS"); + if (qemuDomainPrepareDiskSource(vm->def, cfg) < 0) + goto cleanup; + VIR_DEBUG("Prepare chardev source backends for TLS"); qemuDomainPrepareChardevSource(vm->def, cfg); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 35f468e35..95028e55b 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2041,6 +2041,8 @@ virStorageSourceCopy(const virStorageSource *src, ret->shared = src->shared; ret->haveTLS = src->haveTLS; ret->tlsFromConfig = src->tlsFromConfig; + ret->tlsListen = src->tlsListen; + ret->tlsVerify = src->tlsVerify; /* storage driver metadata are not copied */ ret->drv = NULL; @@ -2054,7 +2056,9 @@ virStorageSourceCopy(const virStorageSource *src, VIR_STRDUP(ret->configFile, src->configFile) < 0 || VIR_STRDUP(ret->nodeformat, src->nodeformat) < 0 || VIR_STRDUP(ret->nodestorage, src->nodestorage) < 0 || - VIR_STRDUP(ret->compat, src->compat) < 0) + VIR_STRDUP(ret->compat, src->compat) < 0 || + VIR_STRDUP(ret->tlsAlias, src->tlsAlias) < 0 || + VIR_STRDUP(ret->tlsCertdir, src->tlsCertdir) < 0) goto error; if (src->nhosts) { @@ -2279,6 +2283,9 @@ virStorageSourceClear(virStorageSourcePtr def) virStorageSourceBackingStoreClear(def); + VIR_FREE(def->tlsAlias); + VIR_FREE(def->tlsCertdir); + memset(def, 0, sizeof(*def)); } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 4817090fc..28cc718a4 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -288,6 +288,14 @@ struct _virStorageSource { /* Indication whether the haveTLS value was altered due to qemu.conf * setting when haveTLS is missing from the domain config file */ bool tlsFromConfig; + + /* If TLS is used, then mgmt of the TLS credentials occurs via an + * object that is generated using a specific alias for a specific + * certificate directory with listen and verify bools. */ + char *tlsAlias; + char *tlsCertdir; + bool tlsListen; + bool tlsVerify; }; -- 2.13.5
On Tue, Sep 19, 2017 at 21:32:45 -0400, John Ferlan wrote:
Introduce a function to setup any TLS needs for a disk source.
If there's a configuration or other error setting up the disk source for TLS, then cause the domain startup to fail.
For VxHS, follow the chardevTLS model where if the src->haveTLS hasn't been configured, then take the system/global cfg->haveTLS setting for the storage source *and* mark that we've done so via the tlsFromConfig setting in storage source.
Next, if we are using TLS, then generate an alias into a virStorageSource 'tlsAlias' field that will be used to create the TLS object and added to the disk object in order to link the two together for QEMU.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 11 +++++++ src/qemu/qemu_process.c | 4 +++ src/util/virstoragefile.c | 9 +++++- src/util/virstoragefile.h | 8 ++++++ 5 files changed, 104 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 50b536eec..8080b7fb1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7601,6 +7601,79 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def, }
+/* qemuProcessPrepareDiskSourceTLS: + * @source: pointer to host interface data for disk device + * @diskAlias: alias use for the disk device + * @cfg: driver configuration + * + * Updates host interface TLS encryption setting based on qemu.conf + * for disk devices. This will be presented as "tls='yes|no'" in + * live XML of a guest. + * + * Returns 0 on success, -1 on bad config/failure + */ +int +qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, + const char *diskAlias, + virQEMUDriverConfigPtr cfg) +{ + + /* VxHS doesn't utilize a password protected server certificate, + * so no need to add a secinfo for a secret UUID. */
It's a client certificate. Is there a technical limitation in the qemu VxHS driver for not supporting encrypted certificates? AFAIK it should use the standard x509 impl so this looks like a libvirt impl. limitation only. [...]
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 4817090fc..28cc718a4 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -288,6 +288,14 @@ struct _virStorageSource { /* Indication whether the haveTLS value was altered due to qemu.conf * setting when haveTLS is missing from the domain config file */ bool tlsFromConfig; + + /* If TLS is used, then mgmt of the TLS credentials occurs via an + * object that is generated using a specific alias for a specific + * certificate directory with listen and verify bools. */ + char *tlsAlias; + char *tlsCertdir; + bool tlsListen;
I think you inspired yourself too much in the chardev section. 'listen' does not make much sense with disks.
+ bool tlsVerify;
Rest looks okay, so ACK if you drop the 'listen' attribute.
On 09/27/2017 09:21 AM, Peter Krempa wrote:
On Tue, Sep 19, 2017 at 21:32:45 -0400, John Ferlan wrote:
Introduce a function to setup any TLS needs for a disk source.
If there's a configuration or other error setting up the disk source for TLS, then cause the domain startup to fail.
For VxHS, follow the chardevTLS model where if the src->haveTLS hasn't been configured, then take the system/global cfg->haveTLS setting for the storage source *and* mark that we've done so via the tlsFromConfig setting in storage source.
Next, if we are using TLS, then generate an alias into a virStorageSource 'tlsAlias' field that will be used to create the TLS object and added to the disk object in order to link the two together for QEMU.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_domain.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_domain.h | 11 +++++++ src/qemu/qemu_process.c | 4 +++ src/util/virstoragefile.c | 9 +++++- src/util/virstoragefile.h | 8 ++++++ 5 files changed, 104 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 50b536eec..8080b7fb1 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7601,6 +7601,79 @@ qemuDomainPrepareChardevSource(virDomainDefPtr def, }
+/* qemuProcessPrepareDiskSourceTLS: + * @source: pointer to host interface data for disk device + * @diskAlias: alias use for the disk device + * @cfg: driver configuration + * + * Updates host interface TLS encryption setting based on qemu.conf + * for disk devices. This will be presented as "tls='yes|no'" in + * live XML of a guest. + * + * Returns 0 on success, -1 on bad config/failure + */ +int +qemuDomainPrepareDiskSourceTLS(virStorageSourcePtr src, + const char *diskAlias, + virQEMUDriverConfigPtr cfg) +{ + + /* VxHS doesn't utilize a password protected server certificate, + * so no need to add a secinfo for a secret UUID. */
It's a client certificate. Is there a technical limitation in the qemu VxHS driver for not supporting encrypted certificates? AFAIK it should use the standard x509 impl so this looks like a libvirt impl. limitation only.
AFAIU - the server side is handled by libqnio which would have the server TLS certificate. The libqnio isn't part of QEMU - it some sort of separate entity. Truly, libvirt and qemu are both just clients in this environment.
[...]
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 4817090fc..28cc718a4 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -288,6 +288,14 @@ struct _virStorageSource { /* Indication whether the haveTLS value was altered due to qemu.conf * setting when haveTLS is missing from the domain config file */ bool tlsFromConfig; + + /* If TLS is used, then mgmt of the TLS credentials occurs via an + * object that is generated using a specific alias for a specific + * certificate directory with listen and verify bools. */ + char *tlsAlias; + char *tlsCertdir; + bool tlsListen;
I think you inspired yourself too much in the chardev section. 'listen' does not make much sense with disks.
OK - right... These would all be clients I suppose, so no use. Tks John
+ bool tlsVerify;
Rest looks okay, so ACK if you drop the 'listen' attribute.
From: Ashish Mittal <Ashish.Mittal@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@veritas.com> Signed-off-by: John Ferlan <jferlan@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; + } + 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:9999}} */ if (virJSONValueObjectCreate(&ret, "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) + return -1; + virCommandAddArg(cmd, "-drive"); if (!(optstr = qemuBuildDriveStr(disk, cfg, driveBoot, qemuCaps))) 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 + * 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; +} + + +static void +qemuDomainDelDiskSrcTLSObject(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virStorageSourcePtr src) +{ + qemuDomainDelTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE, NULL, src->tlsAlias); +} + + +static int qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, @@ -376,6 +422,14 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) goto error; + if (qemuDomainPrepareDiskSourceTLS(disk->src, disk->info.alias, cfg) < 0) + goto error; + + if (disk->src->haveTLS && + qemuDomainAddDiskSrcTLSObject(driver, vm, disk->src, + disk->info.alias) < 0) + goto error; + if (!(drivestr = qemuBuildDriveStr(disk, cfg, false, priv->qemuCaps))) goto error; @@ -453,6 +507,8 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virDomainAuditDisk(vm, NULL, disk->src, "attach", false); error: + qemuDomainDelDiskSrcTLSObject(driver, vm, disk->src); + if (releaseaddr) qemuDomainReleaseDeviceAddress(vm, &disk->info, src); @@ -667,6 +723,14 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; + if (qemuDomainPrepareDiskSourceTLS(disk->src, disk->info.alias, cfg) < 0) + goto error; + + if (disk->src->haveTLS && + qemuDomainAddDiskSrcTLSObject(driver, vm, disk->src, + disk->info.alias) < 0) + goto error; + if (!(drivestr = qemuBuildDriveStr(disk, cfg, false, priv->qemuCaps))) goto error; @@ -737,6 +801,8 @@ qemuDomainAttachSCSIDisk(virConnectPtr conn, virDomainAuditDisk(vm, NULL, disk->src, "attach", false); error: + qemuDomainDelDiskSrcTLSObject(driver, vm, disk->src); + ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true)); goto cleanup; } @@ -777,6 +843,14 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0) goto error; + if (qemuDomainPrepareDiskSourceTLS(disk->src, disk->info.alias, cfg) < 0) + goto error; + + if (disk->src->haveTLS && + qemuDomainAddDiskSrcTLSObject(driver, vm, disk->src, + disk->info.alias) < 0) + goto error; + if (!(drivestr = qemuBuildDriveStr(disk, cfg, false, priv->qemuCaps))) goto error; @@ -827,6 +901,8 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver, virDomainAuditDisk(vm, NULL, disk->src, "attach", false); error: + qemuDomainDelDiskSrcTLSObject(driver, vm, disk->src); + ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true)); goto cleanup; } @@ -3677,6 +3753,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, ignore_value(qemuMonitorDelObject(priv->mon, encAlias)); VIR_FREE(encAlias); + if (disk->src->haveTLS) + ignore_value(qemuMonitorDelObject(priv->mon, disk->src->tlsAlias)); + if (qemuDomainObjExitMonitor(driver, vm) < 0) return -1; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args new file mode 100644 index 000000000..572c9f36c --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args @@ -0,0 +1,43 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-cpu qemu32 \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-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 \ +-object tls-creds-x509,id=objvirtio-disk1_tls0,dir=/etc/pki/qemu,\ +endpoint=client,verify-peer=yes \ +-drive file.driver=vxhs,file.tls-creds=objvirtio-disk1_tls0,\ +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,file.server.type=tcp,\ +file.server.host=192.168.0.2,file.server.port=9999,format=raw,if=none,\ +id=drive-virtio-disk1,cache=none \ +-device virtio-blk-pci,bus=pci.0,addr=0x5,drive=drive-virtio-disk1,\ +id=virtio-disk1 \ +-drive file.driver=vxhs,file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc253,\ +file.server.type=tcp,file.server.host=192.168.0.3,file.server.port=9999,\ +format=raw,if=none,id=drive-virtio-disk2,cache=none \ +-device virtio-blk-pci,bus=pci.0,addr=0x6,drive=drive-virtio-disk2,\ +id=virtio-disk2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml new file mode 100644 index 000000000..a66e81f06 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml @@ -0,0 +1,50 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <disk type='network' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc251'> + <host name='192.168.0.1' port='9999'/> + </source> + <target dev='vda' bus='virtio'/> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc252'> + <host name='192.168.0.2' port='9999'/> + </source> + <target dev='vdb' bus='virtio'/> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </disk> + <disk type='network' device='disk'> + <driver name='qemu' type='raw' cache='none'/> + <source protocol='vxhs' name='eb90327c-8302-4725-9e1b-4e85ed4dc253' tls='no'> + <host name='192.168.0.3' port='9999'/> + </source> + <target dev='vdc' bus='virtio'/> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args new file mode 100644 index 000000000..aaf88635b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args @@ -0,0 +1,30 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name QEMUGuest1 \ +-S \ +-M pc \ +-cpu qemu32 \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-no-acpi \ +-boot c \ +-usb \ +-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 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); + 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", -- 2.13.5
On Tue, Sep 19, 2017 at 21:32:46 -0400, John Ferlan wrote:
From: Ashish Mittal <Ashish.Mittal@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@veritas.com> Signed-off-by: John Ferlan <jferlan@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.
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@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@veritas.com> Signed-off-by: John Ferlan <jferlan@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 (!(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.
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 <Ashish.Mittal@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@veritas.com> Signed-off-by: John Ferlan <jferlan@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_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.
[...]
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
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 <Ashish.Mittal@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@veritas.com> Signed-off-by: John Ferlan <jferlan@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_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 :|
ping - Would be good to close on this before freeze. I think design has been agreed upon and Ashish has tested. Only patches 3 and 4 are not re-reviewed. Tks - John On 09/19/2017 09:32 PM, John Ferlan wrote:
v8: https://www.redhat.com/archives/libvir-list/2017-September/msg00375.html
Changes since v8
* Patches 1-6 patches, everything else deals with TLS.
* Patches 7-8 - no change, now patch 1-2. Didn't push since they're related to these changes
* Patch 9 - Old patch Was ACK'd, new patch is 3. Old patch11 review caused me to add more fields to _virStorageSource to pepare "tlsCertdir", "tlsListen", and "tlsVerify" using the cfg field for certdir, but direct sets for listen and verify.
* Patch 10 - dropped
* Patch 11 - now patch 4. Alter the qemu_command.c and qemu_hotplug.c code to use the virStorageSource fields rather than from disk or cfg. Code makes more use of whether the "src->haveTLS == VIR_TRISTATE_BOOL_YES" is true rather then src->tlsAlias. Added qemuDomainDelDiskSrcTLSObject. Renamed/reworked the qemuDomainAddDiskSrcTLSObject. Looks cleaner to me...
Ashish will post a patch separately to fix an issue found in qemuDomainGetTLSObjects when secAlias == NULL and the deref of *secAlias.
Ashish Mittal (3): conf: Introduce TLS options for VxHS block device clients util: Add TLS attributes to virStorageSource qemu: Add TLS support for Veritas HyperScale (VxHS)
John Ferlan (1): qemu: Introduce qemuDomainPrepareDiskSource
docs/formatdomain.html.in | 40 ++++++++--- docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 29 +++++++- src/qemu/libvirtd_qemu.aug | 4 ++ src/qemu/qemu.conf | 34 ++++++++++ src/qemu/qemu_block.c | 8 +++ src/qemu/qemu_command.c | 33 +++++++++ src/qemu/qemu_conf.c | 16 +++++ src/qemu/qemu_conf.h | 3 + src/qemu/qemu_domain.c | 73 ++++++++++++++++++++ src/qemu/qemu_domain.h | 11 +++ src/qemu/qemu_hotplug.c | 79 ++++++++++++++++++++++ src/qemu/qemu_process.c | 4 ++ src/qemu/test_libvirtd_qemu.aug.in | 2 + src/util/virstoragefile.c | 11 ++- src/util/virstoragefile.h | 15 ++++ ...-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 ++++++++ ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml | 32 +++++++++ tests/qemuxml2argvtest.c | 7 ++ ...uxml2xmlout-disk-drive-network-tlsx509-vxhs.xml | 34 ++++++++++ tests/qemuxml2xmltest.c | 1 + 23 files changed, 551 insertions(+), 13 deletions(-) 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 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-drive-network-tlsx509-vxhs.xml
participants (3)
-
Daniel P. Berrange -
John Ferlan -
Peter Krempa