From: Ashish Mittal <ashish.mittal@veritas.com>
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 certificates and keys are saved. If this value is missing,
the "default_tls_x509_cert_dir" will be used instead.
(4) If the value of "vxhs_tls" is set to 1, TLS creds will be added
automatically on the qemu command line for every VxHS
block device.
(5) With "vxhs_tls=1", TLS may selectively be disabled on individual
VxHS disks by specifying tls='no' in the device domain
specification.
(6) Valid values for domain TLS setting are tls='yes|no'.
(7) tls='yes' can only be specified if "vxhs_tls" is enabled.
Specifying tls='yes' when "vxhs_tls=0" results in an error.
(8) Test cases have been added to validate points (4), (5) and (7).
Test case also added to confirm that JSON arguments containing
tls attribute are parsed correctly.
QEMU changes for VxHS (including TLS support) are already upstream.
Sample TLS args generated by libvirt -
-object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
endpoint=client,verify-peer=yes \
-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
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
Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com>
---
docs/formatdomain.html.in | 18 +++-
docs/schemas/domaincommon.rng | 5 +
src/conf/domain_conf.c | 19 ++++
src/qemu/qemu_command.c | 107 ++++++++++++++++++---
src/util/virstoragefile.c | 13 +++
src/util/virstoragefile.h | 9 ++
...ml2argv-disk-drive-network-tlsx509-err-vxhs.xml | 34 +++++++
...-disk-drive-network-tlsx509-multidisk-vxhs.args | 41 ++++++++
...k-drive-network-tlsx509-multidisk-vxhs.args.new | 41 ++++++++
...v-disk-drive-network-tlsx509-multidisk-vxhs.xml | 56 +++++++++++
...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 28 ++++++
...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml | 34 +++++++
tests/qemuxml2argvtest.c | 9 ++
tests/virstoragetest.c | 11 +++
14 files changed, 413 insertions(+), 12 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-err-vxhs.xml
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.args.new
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
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 62d67f4..86808e5 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2511,7 +2511,7 @@
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.3.0</span>), the
+ For "vxhs" (<span class="since">since 3.3.1</span>), the
<code>name</code> is the UUID of the volume, assigned by the
HyperScale sever.
<span class="since">Since 0.8.7</span>
@@ -2630,6 +2630,22 @@
transport is "unix", the socket attribute specifies the path to an
AF_UNIX socket.
</p>
+ <p>
+ <span class="since">Since 3.3.1,</span> the optional attribute
+ <code>tls</code> (QEMU only) can be used to control whether a vxhs
+ network 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
+ be 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 <code>vxhs_tls</code> is disabled, but the <code>tls</code>
+ attribute is set to "yes" in the device domain specification,
+ then libvirt will throw an error.
+ </p>
</dd>
<dt><code>snapshot</code></dt>
<dd>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7525a2a..909af50 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1622,6 +1622,11 @@
</attribute>
<attribute name="name"/>
<ref name="diskSourceNetworkHost"/>
+ <optional>
+ <attribute name="tls">
+ <ref name="virYesNo"/>
+ </attribute>
+ </optional>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c3149f9..34d8451 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7745,6 +7745,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
int ret = -1;
char *protocol = NULL;
xmlNodePtr saveNode = ctxt->node;
+ char *haveTLS = NULL;
ctxt->node = node;
@@ -7778,6 +7779,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 ((haveTLS = virXMLPropString(node, "tls")) &&
+ src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) {
+ if ((src->haveTLS =
+ virTristateBoolTypeFromString(haveTLS)) <= 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unknown VxHS '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 */
@@ -7830,6 +7844,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
cleanup:
VIR_FREE(protocol);
+ VIR_FREE(haveTLS);
ctxt->node = saveNode;
return ret;
}
@@ -21266,6 +21281,10 @@ virDomainDiskSourceFormatNetwork(virBufferPtr buf,
VIR_FREE(path);
+ if (src->haveTLS != VIR_TRISTATE_BOOL_ABSENT)
+ virBufferAsprintf(buf, " tls='%s'",
+ virTristateBoolTypeToString(src->haveTLS));
+
if (src->nhosts == 0 && !src->snapshot && !src->configFile) {
virBufferAddLit(buf, "/>\n");
} else {
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8e00782..99bc94f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -931,6 +931,68 @@ qemuBuildGlusterDriveJSON(virStorageSourcePtr src)
return ret;
}
+/* qemuBuildDiskVxHSTLSinfoCommandLine:
+ * @cmd: Pointer to the command string
+ * @cfg: Pointer to the qemu driver config
+ * @disk: The disk we are processing
+ * @qemuCaps: qemu capabilities object
+ *
+ * Check if the VxHS disk meets all the criteria to enable TLS.
+ * If yes, add a new TLS object and mention it's ID on the disk
+ * command line.
+ *
+ * Returns 0 on success, -1 w/ error on some sort of failure.
+ */
+static int
+qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
+ virQEMUDriverConfigPtr cfg,
+ virDomainDiskDefPtr disk,
+ virQEMUCapsPtr qemuCaps)
+{
+ int ret = 0;
+
+ if (cfg->vxhsTLS == true && disk->src->haveTLS != VIR_TRISTATE_BOOL_NO) {
+ disk->src->addTLS = true;
+ ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
+ false,
+ true,
+ false,
+ "vxhs",
+ qemuCaps);
+ } else if (cfg->vxhsTLS == false &&
+ disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("Please enable VxHS specific TLS options in the qemu "
+ "conf file before using TLS in VxHS device domain "
+ "specification"));
+ ret = -1;
+ }
+
+ return ret;
+}
+
+
+/* qemuBuildDiskTLSinfoCommandLine:
+ *
+ * Add TLS object if the disk uses a secure communication channel
+ *
+ * Returns 0 on success, -1 w/ error on some sort of failure.
+ */
+static int
+qemuBuildDiskTLSinfoCommandLine(virCommandPtr cmd,
+ virQEMUDriverConfigPtr cfg,
+ virDomainDiskDefPtr disk,
+ virQEMUCapsPtr qemuCaps)
+{
+ virStorageSourcePtr src = disk->src;
+
+ /* other protocols may be added later */
+ if (src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS)
+ return qemuBuildDiskVxHSTLSinfoCommandLine(cmd, cfg, disk, qemuCaps);
+
+ return 0;
+}
+
#define QEMU_DEFAULT_VXHS_PORT "9999"
@@ -975,18 +1037,38 @@ qemuBuildVxHSDriveJSON(virStorageSourcePtr src)
if (!(server = qemuBuildVxHSDriveJSONHost(src)))
return NULL;
- /* VxHS disk specification example:
- * { driver:"vxhs",
- * vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
- * server.host:"1.2.3.4",
- * server.port:1234}
- */
- if (virJSONValueObjectCreate(&ret,
- "s:driver", protocol,
- "s:vdisk-id", src->path,
- "a:server", server, NULL) < 0)
- virJSONValueFree(server);
+ if (src->addTLS == true) {
+ char *objalias = NULL;
+ if (!(objalias = qemuAliasTLSObjFromSrcAlias("vxhs")))
+ goto cleanup;
+
+ if (virJSONValueObjectCreate(&ret,
+ "s:driver", protocol,
+ "s:tls-creds", objalias,
+ "s:vdisk-id", src->path,
+ "a:server", server, NULL) < 0) {
+ virJSONValueFree(server);
+ ret = NULL;
+ }
+ VIR_FREE(objalias);
+ } else {
+ /* VxHS disk specification example:
+ * { driver:"vxhs",
+ * vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
+ * server.host:"1.2.3.4",
+ * server.port:1234}
+ */
+ if (virJSONValueObjectCreate(&ret,
+ "s:driver", protocol,
+ "s:vdisk-id", src->path,
+ "a:server", server, NULL) < 0) {
+ virJSONValueFree(server);
+ ret = NULL;
+ }
+ }
+
+ cleanup:
return ret;
}
@@ -2438,6 +2520,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0)
return -1;
+ if (qemuBuildDiskTLSinfoCommandLine(cmd, cfg, disk, qemuCaps) < 0)
+ return -1;
+
virCommandAddArg(cmd, "-drive");
if (!(optstr = qemuBuildDriveStr(disk, cfg, driveBoot, qemuCaps)))
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index eb36694..449ace4 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2042,6 +2042,8 @@ virStorageSourceCopy(const virStorageSource *src,
ret->physical = src->physical;
ret->readonly = src->readonly;
ret->shared = src->shared;
+ ret->haveTLS = src->haveTLS;
+ ret->addTLS = src->addTLS;
/* storage driver metadata are not copied */
ret->drv = NULL;
@@ -3231,6 +3233,7 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src,
const char *uri = virJSONValueObjectGetString(json, "filename");
const char *vdisk_id = virJSONValueObjectGetString(json, "vdisk-id");
virJSONValuePtr server = virJSONValueObjectGetObject(json, "server");
+ const char *haveTLS = virJSONValueObjectGetString(json, "tls");
const char *hostname;
const char *port;
@@ -3258,6 +3261,16 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src,
return -1;
}
+ if (haveTLS) {
+ if ((src->haveTLS =
+ virTristateBoolTypeFromString(haveTLS)) <= 0) {
+ virReportError(VIR_ERR_INVALID_ARG,
+ _("unknown VxHS 'tls' setting '%s'"),
+ haveTLS);
+ return -1;
+ }
+ }
+
if (!port)
port = QEMU_DEFAULT_VXHS_PORT;
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 0b6e409..e586170 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -281,6 +281,15 @@ struct _virStorageSource {
/* metadata that allows identifying given storage source */
char *nodeformat; /* name of the format handler object */
char *nodebacking; /* name of the backing storage object */
+
+ /* This is the domain specific setting.
+ * It may be absent */
+ int haveTLS; /* enum virTristateBool */
+
+ /* This should be set to "true" only when TLS creds are to be added for
+ * the device. For e.g. this could be based on a combination of
+ * global conf setting + domain specific setting */
+ bool addTLS;
};
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-err-vxhs.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-err-vxhs.xml
new file mode 100644
index 0000000..951ad4e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-err-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>
+ <backingStore/>
+ <target dev='vda' bus='virtio'/>
+ <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
+ <alias name='virtio-disk0'/>
+ <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/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 0000000..960960d
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
@@ -0,0 +1,41 @@
+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 \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
+endpoint=client,verify-peer=yes \
+-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
+file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
+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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
+endpoint=client,verify-peer=yes \
+-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
+file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\
+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.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.args.new b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
new file mode 100644
index 0000000..960960d
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
@@ -0,0 +1,41 @@
+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 \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
+endpoint=client,verify-peer=yes \
+-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
+file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
+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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
+endpoint=client,verify-peer=yes \
+-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
+file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\
+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.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 0000000..3d28958
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
@@ -0,0 +1,56 @@
+<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>
+ <backingStore/>
+ <target dev='vda' bus='virtio'/>
+ <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
+ <alias name='virtio-disk0'/>
+ <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>
+ <backingStore/>
+ <target dev='vdb' bus='virtio'/>
+ <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
+ <alias name='virtio-disk0'/>
+ <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>
+ <backingStore/>
+ <target dev='vdc' bus='virtio'/>
+ <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
+ <alias name='virtio-disk0'/>
+ <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 0000000..e1ad36e
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
@@ -0,0 +1,28 @@
+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 \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
+endpoint=client,verify-peer=yes \
+-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
+file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
+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/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.xml
new file mode 100644
index 0000000..a488770
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-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'>
+ <host name='192.168.0.1' port='9999'/>
+ </source>
+ <backingStore/>
+ <target dev='vda' bus='virtio'/>
+ <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
+ <alias name='virtio-disk0'/>
+ <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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0a1ef01..7459522 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -904,6 +904,15 @@ 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", NONE);
+ DO_TEST_FAILURE("disk-drive-network-tlsx509-err-vxhs",
+ QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+ driver.config->vxhsTLS = 1;
+ DO_TEST("disk-drive-network-tlsx509-vxhs",
+ QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+ DO_TEST("disk-drive-network-tlsx509-multidisk-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",
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 3a4e03b..28747ff 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1594,6 +1594,17 @@ mymain(void)
TEST_BACKING_PARSE("json:{\"file.driver\":\"vxhs\","
"\"file.filename\":\"vxhs://192.168.0.1:9999/c6718f6b-0401-441d-a8c3-1f0064d75ee0\""
"}", NULL);
+ TEST_BACKING_PARSE("json:{\"file\":{\"driver\":\"vxhs\","
+ "\"vdisk-id\":\"c6718f6b-0401-441d-a8c3-1f0064d75ee0\","
+ "\"server\": { \"host\":\"example.com\","
+ "\"port\":\"1234\""
+ "},"
+ "\"tls\":\"yes\""
+ "}"
+ "}",
+ "<source protocol='vxhs' name='c6718f6b-0401-441d-a8c3-1f0064d75ee0' tls='yes'>\n"
+ " <host name='example.com' port='1234'/>\n"
+ "</source>\n");
#endif /* WITH_YAJL */
cleanup:
--
2.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Jun 29, 2017 at 19:02:41 -0700, Ashish Mittal wrote:
> From: Ashish Mittal <ashish.mittal@veritas.com>
>
> 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 certificates and keys are saved. If this value is missing,
> the "default_tls_x509_cert_dir" will be used instead.
> (4) If the value of "vxhs_tls" is set to 1, TLS creds will be added
> automatically on the qemu command line for every VxHS
> block device.
> (5) With "vxhs_tls=1", TLS may selectively be disabled on individual
> VxHS disks by specifying tls='no' in the device domain
> specification.
> (6) Valid values for domain TLS setting are tls='yes|no'.
> (7) tls='yes' can only be specified if "vxhs_tls" is enabled.
> Specifying tls='yes' when "vxhs_tls=0" results in an error.
> (8) Test cases have been added to validate points (4), (5) and (7).
> Test case also added to confirm that JSON arguments containing
> tls attribute are parsed correctly.
>
> QEMU changes for VxHS (including TLS support) are already upstream.
>
> Sample TLS args generated by libvirt -
> -object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
> endpoint=client,verify-peer=yes \
> -drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
> file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
> 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
>
> Signed-off-by: Ashish Mittal <ashish.mittal@veritas.com>
> ---
Too much stuff is going on in this patch. You need to split it.
> docs/formatdomain.html.in | 18 +++-
> docs/schemas/domaincommon.rng | 5 +
> src/conf/domain_conf.c | 19 ++++
> src/qemu/qemu_command.c | 107 ++++++++++++++++++---
The addition to the qemu driver should be separate too.
> src/util/virstoragefile.c | 13 +++
> src/util/virstoragefile.h | 9 ++
Changes to the backing store parser should be in the separate patch.
> ...ml2argv-disk-drive-network-tlsx509-err-vxhs.xml | 34 +++++++
> ...-disk-drive-network-tlsx509-multidisk-vxhs.args | 41 ++++++++
> ...k-drive-network-tlsx509-multidisk-vxhs.args.new | 41 ++++++++
> ...v-disk-drive-network-tlsx509-multidisk-vxhs.xml | 56 +++++++++++
> ...muxml2argv-disk-drive-network-tlsx509-vxhs.args | 28 ++++++
> ...emuxml2argv-disk-drive-network-tlsx509-vxhs.xml | 34 +++++++
> tests/qemuxml2argvtest.c | 9 ++
> tests/virstoragetest.c | 11 +++
Plus these belong to the respective patches.
> 14 files changed, 413 insertions(+), 12 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-err-vxhs.xml
> 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.args.new
> 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
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 62d67f4..86808e5 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2511,7 +2511,7 @@
> 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.3.0</span>), the
> + For "vxhs" (<span class="since">since 3.3.1</span>), the
There won't be any 3.3.1 release. The upcomming release is 3.5.0, and
your patches will be in 3.6.0 at best since we are already in freeze
state.
Also this hunk is misplaced from one of the earlier patches probably.
> <code>name</code> is the UUID of the volume, assigned by the
> HyperScale sever.
> <span class="since">Since 0.8.7</span>
> @@ -2630,6 +2630,22 @@
> transport is "unix", the socket attribute specifies the path to an
> AF_UNIX socket.
> </p>
> + <p>
> + <span class="since">Since 3.3.1,</span> the optional attribute
> + <code>tls</code> (QEMU only) can be used to control whether a vxhs
> + network 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
> + be 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 <code>vxhs_tls</code> is disabled, but the <code>tls</code>
> + attribute is set to "yes" in the device domain specification,
> + then libvirt will throw an error.
> + </p>
> </dd>
This seems misplaced too. You really need to split the addition of TLS
stuff to disks from the vxhs impl.
> <dt><code>snapshot</code></dt>
> <dd>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 7525a2a..909af50 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1622,6 +1622,11 @@
> </attribute>
> <attribute name="name"/>
> <ref name="diskSourceNetworkHost"/>
> + <optional>
> + <attribute name="tls">
> + <ref name="virYesNo"/>
> + </attribute>
Make this a definition for future reuse. Additionally I think that the
TLS part should be a separate element here. Something like
<disk>
<source>
> + </optional>
> </element>
> </define>
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c3149f9..34d8451 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7745,6 +7745,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
> int ret = -1;
> char *protocol = NULL;
> xmlNodePtr saveNode = ctxt->node;
> + char *haveTLS = NULL;
>
> ctxt->node = node;
>
> @@ -7778,6 +7779,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 */
[1]
> + if ((haveTLS = virXMLPropString(node, "tls")) &&
> + src->protocol == VIR_STORAGE_NET_PROTOCOL_VXHS) {
> + if ((src->haveTLS =
> + virTristateBoolTypeFromString(haveTLS)) <= 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("unknown VxHS 'tls' setting '%s'"),
While this currently is implemented only for vxhs, the TLS protocol can
be uin the future used with other storage, like NBD, thus please don't
hardcode the name here.
[1] You admit it yourself here.
> + haveTLS);
> + goto cleanup;
> + }
> + }
This also looks like it should belong to a separate patch.
> +
> /* 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 */
> @@ -7830,6 +7844,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
>
> cleanup:
> VIR_FREE(protocol);
> + VIR_FREE(haveTLS);
> ctxt->node = saveNode;
> return ret;
> }
[...]
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 8e00782..99bc94f 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -931,6 +931,68 @@ qemuBuildGlusterDriveJSON(virStorageSourcePtr src)
> return ret;
> }
>
> +/* qemuBuildDiskVxHSTLSinfoCommandLine:
> + * @cmd: Pointer to the command string
> + * @cfg: Pointer to the qemu driver config
> + * @disk: The disk we are processing
> + * @qemuCaps: qemu capabilities object
> + *
> + * Check if the VxHS disk meets all the criteria to enable TLS.
> + * If yes, add a new TLS object and mention it's ID on the disk
> + * command line.
> + *
> + * Returns 0 on success, -1 w/ error on some sort of failure.
> + */
> +static int
> +qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
> + virQEMUDriverConfigPtr cfg,
> + virDomainDiskDefPtr disk,
> + virQEMUCapsPtr qemuCaps)
> +{
> + int ret = 0;
> +
> + if (cfg->vxhsTLS == true && disk->src->haveTLS != VIR_TRISTATE_BOOL_NO) {
> + disk->src->addTLS = true;
> + ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
> + false,
> + true,
> + false,
> + "vxhs",
This argument can't be a constant. This is the alias for the certificate
object.
Otherwise you'd had to make sure that there's only one such object,
which obviously would make sense here, since (if you don't hotplug disks
after libvirtd restart) the TLS credentials are the same for this disk.
In case of hotplug though you need to make sure that the TLS object is
removed with the last disk and added if any other disk needing TLS is
added.
> + qemuCaps);
> + } else if (cfg->vxhsTLS == false &&
> + disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Please enable VxHS specific TLS options in the qemu "
> + "conf file before using TLS in VxHS device domain "
> + "specification"));
> + ret = -1;
> + }
> +
> + return ret;
> +}
[...]
> @@ -975,18 +1037,38 @@ qemuBuildVxHSDriveJSON(virStorageSourcePtr src)
> if (!(server = qemuBuildVxHSDriveJSONHost(src)))
> return NULL;
>
> - /* VxHS disk specification example:
> - * { driver:"vxhs",
> - * vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
> - * server.host:"1.2.3.4",
> - * server.port:1234}
> - */
> - if (virJSONValueObjectCreate(&ret,
> - "s:driver", protocol,
> - "s:vdisk-id", src->path,
> - "a:server", server, NULL) < 0)
> - virJSONValueFree(server);
> + if (src->addTLS == true) {
> + char *objalias = NULL;
>
> + if (!(objalias = qemuAliasTLSObjFromSrcAlias("vxhs")))
> + goto cleanup;
> +
> + if (virJSONValueObjectCreate(&ret,
> + "s:driver", protocol,
> + "s:tls-creds", objalias,
> + "s:vdisk-id", src->path,
> + "a:server", server, NULL) < 0) {
You can use virJSONValueObjectAdd with the same syntax instead of having
two paths adding most of the same stuff.
> + virJSONValueFree(server);
> + ret = NULL;
> + }
> + VIR_FREE(objalias);
> + } else {
> + /* VxHS disk specification example:
> + * { driver:"vxhs",
> + * vdisk-id:"eb90327c-8302-4725-4e85ed4dc251",
> + * server.host:"1.2.3.4",
> + * server.port:1234}
> + */
> + if (virJSONValueObjectCreate(&ret,
> + "s:driver", protocol,
> + "s:vdisk-id", src->path,
> + "a:server", server, NULL) < 0) {
> + virJSONValueFree(server);
> + ret = NULL;
> + }
> + }
> +
> + cleanup:
> return ret;
> }
>
> @@ -2438,6 +2520,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd,
> if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0)
> return -1;
>
> + if (qemuBuildDiskTLSinfoCommandLine(cmd, cfg, disk, qemuCaps) < 0)
> + return -1;
> +
> virCommandAddArg(cmd, "-drive");
>
> if (!(optstr = qemuBuildDriveStr(disk, cfg, driveBoot, qemuCaps)))
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index eb36694..449ace4 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
[...]
> @@ -3258,6 +3261,16 @@ virStorageSourceParseBackingJSONVxHS(virStorageSourcePtr src,
> return -1;
> }
>
> + if (haveTLS) {
> + if ((src->haveTLS =
> + virTristateBoolTypeFromString(haveTLS)) <= 0) {
This is more a question to the qemu implementation. Why is this a string
and not a boolean?! JSON has native support for booleans.
I suggest you fix the qemu implementation first and use a proper boolean
here.
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("unknown VxHS 'tls' setting '%s'"),
> + haveTLS);
> + return -1;
> + }
> + }
> +
> if (!port)
> port = QEMU_DEFAULT_VXHS_PORT;
>
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index 0b6e409..e586170 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -281,6 +281,15 @@ struct _virStorageSource {
> /* metadata that allows identifying given storage source */
> char *nodeformat; /* name of the format handler object */
> char *nodebacking; /* name of the backing storage object */
> +
> + /* This is the domain specific setting.
> + * It may be absent */
> + int haveTLS; /* enum virTristateBool */
> +
> + /* This should be set to "true" only when TLS creds are to be added for
> + * the device. For e.g. this could be based on a combination of
> + * global conf setting + domain specific setting */
> + bool addTLS;
I don't quite understand the point of the two flags above. Also any of
the TLS stuff should be in a separate patch.
> };
> 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 0000000..960960d
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
[this file has same mistake as the one below]
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
> new file mode 100644
> index 0000000..960960d
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
> @@ -0,0 +1,41 @@
> +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 \
> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
> +-no-acpi \
> +-boot c \
> +-usb \
> +-object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\a
This ...
> +endpoint=client,verify-peer=yes \
> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
> +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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
... and this looks wrong. You have two tls-creds-x509 with the same
alias. I doubt that qemu will be happy wit that.
> +endpoint=client,verify-peer=yes \
> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\
> +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.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 0000000..3d28958
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
> @@ -0,0 +1,56 @@
> +<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>
> + <backingStore/>
> + <target dev='vda' bus='virtio'/>
> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
> + <alias name='virtio-disk0'/>
This here ...
> + <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>
> + <backingStore/>
> + <target dev='vdb' bus='virtio'/>
> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
> + <alias name='virtio-disk0'/>
... and this have the same alias, which will not happen. Please add
proper examples/tests.
> + <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>
> + <backingStore/>
> + <target dev='vdc' bus='virtio'/>
> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
> + <alias name='virtio-disk0'/>
... here too.
> + <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>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jun 30, 2017 at 10:44:39 +0200, Peter Krempa wrote:
> On Thu, Jun 29, 2017 at 19:02:41 -0700, Ashish Mittal wrote:
> > From: Ashish Mittal <ashish.mittal@veritas.com>
[...]
> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > index 7525a2a..909af50 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -1622,6 +1622,11 @@
> > </attribute>
> > <attribute name="name"/>
> > <ref name="diskSourceNetworkHost"/>
> > + <optional>
> > + <attribute name="tls">
> > + <ref name="virYesNo"/>
> > + </attribute>
>
> Make this a definition for future reuse. Additionally I think that the
> TLS part should be a separate element here. Something like
>
> <disk>
> <source>
I forgot to finish my thought before sending. I think we want a separate
element with an attribute at this point. This allows adding other TLS
related stuff to it if such need arises.
<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'/>
<tls enabled='yes'/>
</source>
[...]
</disk>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 06/30/2017 04:56 AM, Peter Krempa wrote:
> On Fri, Jun 30, 2017 at 10:44:39 +0200, Peter Krempa wrote:
>> On Thu, Jun 29, 2017 at 19:02:41 -0700, Ashish Mittal wrote:
>>> From: Ashish Mittal <ashish.mittal@veritas.com>
>
> [...]
>
>>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>>> index 7525a2a..909af50 100644
>>> --- a/docs/schemas/domaincommon.rng
>>> +++ b/docs/schemas/domaincommon.rng
>>> @@ -1622,6 +1622,11 @@
>>> </attribute>
>>> <attribute name="name"/>
>>> <ref name="diskSourceNetworkHost"/>
>>> + <optional>
>>> + <attribute name="tls">
>>> + <ref name="virYesNo"/>
>>> + </attribute>
>>
>> Make this a definition for future reuse. Additionally I think that the
>> TLS part should be a separate element here. Something like
>>
>> <disk>
>> <source>
>
> I forgot to finish my thought before sending. I think we want a separate
> element with an attribute at this point. This allows adding other TLS
> related stuff to it if such need arises.
>
> <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'/>
> <tls enabled='yes'/>
> </source>
> [...]
> </disk>
>
I don't like a separate <tls ...> element. What do you mean by other TLS
related stuff such as 'verify' or 'secret'? Those would be qemu.conf
type settings - they wouldn't change on a disk by disk or domain by
domain basis.
Why not as a <source> or perhaps more precisely a <host> attribute? If
you compare with others it's related to the port as I would assume would
be the case for storage as well. If my understanding from the cover
letter is valid, then this is how QEMU is going to communicate with some
remote host/server in order to provide TLS credentials.
John
For comparison, other consumers of TLS and their XML:
VNC:
<devices>
...
<graphics type='vnc' port='5904' .../>
...
Configured only via qemu.conf AFAICT
Spice:
<devices>
...
<graphics type='spice' port='-1' tlsPort='-1' autoport='yes'>
...
Chardev:
...
<devices>
<serial type="tcp">
<source mode='connect' host="127.0.0.1" service="5555" tls="yes"/>
...
>
>
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[...]
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index 8e00782..99bc94f 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -931,6 +931,68 @@ qemuBuildGlusterDriveJSON(virStorageSourcePtr src)
>> return ret;
>> }
>>
>> +/* qemuBuildDiskVxHSTLSinfoCommandLine:
>> + * @cmd: Pointer to the command string
>> + * @cfg: Pointer to the qemu driver config
>> + * @disk: The disk we are processing
>> + * @qemuCaps: qemu capabilities object
>> + *
>> + * Check if the VxHS disk meets all the criteria to enable TLS.
>> + * If yes, add a new TLS object and mention it's ID on the disk
>> + * command line.
>> + *
>> + * Returns 0 on success, -1 w/ error on some sort of failure.
>> + */
>> +static int
>> +qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
>> + virQEMUDriverConfigPtr cfg,
>> + virDomainDiskDefPtr disk,
>> + virQEMUCapsPtr qemuCaps)
>> +{
>> + int ret = 0;
>> +
>> + if (cfg->vxhsTLS == true && disk->src->haveTLS != VIR_TRISTATE_BOOL_NO) {
>> + disk->src->addTLS = true;
>> + ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
>> + false,
>> + true,
>> + false,
>> + "vxhs",
>
> This argument can't be a constant. This is the alias for the certificate
> object.
>
> Otherwise you'd had to make sure that there's only one such object,
> which obviously would make sense here, since (if you don't hotplug disks
> after libvirtd restart) the TLS credentials are the same for this disk.
>
> In case of hotplug though you need to make sure that the TLS object is
> removed with the last disk and added if any other disk needing TLS is
> added.
>
So at least the conversation we had last week now makes a bit more sense
w/r/t the call to qemuBuildTLSx509CommandLine for chardevTLSx509certdir.
As I look at that code now quickly, although having multiple
tls-cert-x509 objects for each chardev isn't necessary, it does "work"
or start qemu because each would have a different alias (@charAlias is
uniquely generated via qemuAliasChardevFromDevAlias). Theoretically
speaking two objects wouldn't be required, except for the problem that
hotunplug would have to be made aware and we'd have to keep track of
when the last one was removed. So leaving with each having their own
object is the way the code will stay.
So given all that - your alias creation is going to have to contain that
uuid or you are going to have to figure out a way to just have one
object which each disk uses. You'll have to scan the disks looking to
determine if any of the vxhs ones have tls and if so, break to add the
object. Then add the 'tls-creds=$object_alias_id'.
BTW: if you haven't already, read Dan Berrange's blog on TLS:
https://www.berrange.com/posts/2016/04/01/improving-qemu-security-part-2-generic-tls-support/
that's a link to page 2, read/scan the remaining 5 blogs as well. Some
of the exact qemu syntax has changed since the blog was written, but the
description is really what helps the frame of reference.
>> + qemuCaps);
>> + } else if (cfg->vxhsTLS == false &&
>> + disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>> + _("Please enable VxHS specific TLS options in the qemu "
>> + "conf file before using TLS in VxHS device domain "
>> + "specification"));
>> + ret = -1;
>> + }
>> +
>> + return ret;
>> +}
[...]
>> 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 0000000..960960d
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
>
> [this file has same mistake as the one below]
>
>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
>> new file mode 100644
>> index 0000000..960960d
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
>> @@ -0,0 +1,41 @@
>> +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 \
>> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
>> +-no-acpi \
>> +-boot c \
>> +-usb \
>> +-object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\a
>
> This ...
>
>> +endpoint=client,verify-peer=yes \
>> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
>> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
>> +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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
>
> ... and this looks wrong. You have two tls-creds-x509 with the same
> alias. I doubt that qemu will be happy wit that.
>
Not only that, but dir=/usr/local/etc/pki/qemu causes qemuxml2argvtest
to fail for me since the default is supposed to be /etc/pki/libvirt-vxhs
John
>> +endpoint=client,verify-peer=yes \
>> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
>> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\
>> +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.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 0000000..3d28958
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
>> @@ -0,0 +1,56 @@
>> +<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>
>> + <backingStore/>
>> + <target dev='vda' bus='virtio'/>
>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
>> + <alias name='virtio-disk0'/>
>
> This here ...
>
>> + <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>
>> + <backingStore/>
>> + <target dev='vdb' bus='virtio'/>
>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
>> + <alias name='virtio-disk0'/>
>
> ... and this have the same alias, which will not happen. Please add
> proper examples/tests.
>
>> + <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>
>> + <backingStore/>
>> + <target dev='vdc' bus='virtio'/>
>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
>> + <alias name='virtio-disk0'/>
>
> ... here too.
>
>> + <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>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jun 30, 2017 at 2:21 PM, John Ferlan <jferlan@redhat.com> wrote:
> [...]
>
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index 8e00782..99bc94f 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -931,6 +931,68 @@ qemuBuildGlusterDriveJSON(virStorageSourcePtr src)
>>> return ret;
>>> }
>>>
>>> +/* qemuBuildDiskVxHSTLSinfoCommandLine:
>>> + * @cmd: Pointer to the command string
>>> + * @cfg: Pointer to the qemu driver config
>>> + * @disk: The disk we are processing
>>> + * @qemuCaps: qemu capabilities object
>>> + *
>>> + * Check if the VxHS disk meets all the criteria to enable TLS.
>>> + * If yes, add a new TLS object and mention it's ID on the disk
>>> + * command line.
>>> + *
>>> + * Returns 0 on success, -1 w/ error on some sort of failure.
>>> + */
>>> +static int
>>> +qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
>>> + virQEMUDriverConfigPtr cfg,
>>> + virDomainDiskDefPtr disk,
>>> + virQEMUCapsPtr qemuCaps)
>>> +{
>>> + int ret = 0;
>>> +
>>> + if (cfg->vxhsTLS == true && disk->src->haveTLS != VIR_TRISTATE_BOOL_NO) {
>>> + disk->src->addTLS = true;
>>> + ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
>>> + false,
>>> + true,
>>> + false,
>>> + "vxhs",
>>
>> This argument can't be a constant. This is the alias for the certificate
>> object.
>>
>> Otherwise you'd had to make sure that there's only one such object,
>> which obviously would make sense here, since (if you don't hotplug disks
>> after libvirtd restart) the TLS credentials are the same for this disk.
>>
>> In case of hotplug though you need to make sure that the TLS object is
>> removed with the last disk and added if any other disk needing TLS is
>> added.
>>
>
> So at least the conversation we had last week now makes a bit more sense
> w/r/t the call to qemuBuildTLSx509CommandLine for chardevTLSx509certdir.
> As I look at that code now quickly, although having multiple
> tls-cert-x509 objects for each chardev isn't necessary, it does "work"
> or start qemu because each would have a different alias (@charAlias is
> uniquely generated via qemuAliasChardevFromDevAlias). Theoretically
> speaking two objects wouldn't be required, except for the problem that
> hotunplug would have to be made aware and we'd have to keep track of
> when the last one was removed. So leaving with each having their own
> object is the way the code will stay.
>
> So given all that - your alias creation is going to have to contain that
> uuid or you are going to have to figure out a way to just have one
> object which each disk uses. You'll have to scan the disks looking to
> determine if any of the vxhs ones have tls and if so, break to add the
> object. Then add the 'tls-creds=$object_alias_id'.
>
Hi John, Peter,
The problem statement - Alias for the TLS certificate can't be a
constant. Two TLS objects cannot have the same ID/alias.
This was pointed out by both of you as something that needs to be
fixed. To that end, I have made some changes to use the block device
domain alias (e.g. virtio-disk0) as a unique identifier for each TLS
object. This is similar to how char devices create their TLS aliases.
I was hoping to get some feedback on whether this diff would be
acceptable to fix the said issue. I will reply to/fix all the other
comments. Just thought I'd tackle this one first as this appears to be
one of the bigger ones...
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 99bc94f..fc58236 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -952,13 +952,19 @@ qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
int ret = 0;
if (cfg->vxhsTLS == true && disk->src->haveTLS != VIR_TRISTATE_BOOL_NO) {
- disk->src->addTLS = true;
- ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
- false,
- true,
- false,
- "vxhs",
- qemuCaps);
+ if (virAsprintf(&disk->src->aliasTLS,
+ "vxhs_%s", disk->info.alias) < 0) {
+ ret = -1;
+ goto cleanup;
+ }
+
+ disk->src->addTLS = true;
+ ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
+ false,
+ true,
+ false,
+ disk->src->aliasTLS,
+ qemuCaps);
} else if (cfg->vxhsTLS == false &&
disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -968,6 +974,7 @@ qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
ret = -1;
}
+ cleanup:
return ret;
}
@@ -1040,7 +1047,7 @@ qemuBuildVxHSDriveJSON(virStorageSourcePtr src)
if (src->addTLS == true) {
char *objalias = NULL;
- if (!(objalias = qemuAliasTLSObjFromSrcAlias("vxhs")))
+ if (!(objalias = qemuAliasTLSObjFromSrcAlias(src->aliasTLS)))
goto cleanup;
if (virJSONValueObjectCreate(&ret,
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 449ace4..61cd54a 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2057,7 +2057,8 @@ virStorageSourceCopy(const virStorageSource *src,
VIR_STRDUP(ret->configFile, src->configFile) < 0 ||
VIR_STRDUP(ret->nodeformat, src->nodeformat) < 0 ||
VIR_STRDUP(ret->nodebacking, src->nodebacking) < 0 ||
- VIR_STRDUP(ret->compat, src->compat) < 0)
+ VIR_STRDUP(ret->compat, src->compat) < 0 ||
+ VIR_STRDUP(ret->aliasTLS, src->aliasTLS) < 0)
goto error;
if (src->nhosts) {
@@ -2273,6 +2274,7 @@ virStorageSourceClear(virStorageSourcePtr def)
virStorageSourceSeclabelsClear(def);
virStoragePermsFree(def->perms);
VIR_FREE(def->timestamps);
+ VIR_FREE(def->aliasTLS);
virStorageNetHostDefFree(def->nhosts, def->hosts);
virStorageAuthDefFree(def->auth);
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index e586170..c1d36bf 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -290,6 +290,9 @@ struct _virStorageSource {
* the device. For e.g. this could be based on a combination of
* global conf setting + domain specific setting */
bool addTLS;
+
+ /* The alias/ID of the TLS object */
+ char *aliasTLS;
};
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
index 4cacb61..b474ce3 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
@@ -18,17 +18,19 @@ QEMU_AUDIO_DRV=none \
-no-acpi \
-boot c \
-usb \
--object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
+-object tls-creds-x509,id=objvxhs_virtio-disk0_tls0,\
+dir=/usr/local/etc/pki/qemu,\
endpoint=client,verify-peer=yes \
--drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
+-drive file.driver=vxhs,file.tls-creds=objvxhs_virtio-disk0_tls0,\
file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
+-object tls-creds-x509,id=objvxhs_virtio-disk1_tls0,\
+dir=/usr/local/etc/pki/qemu,\
endpoint=client,verify-peer=yes \
--drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
+-drive file.driver=vxhs,file.tls-creds=objvxhs_virtio-disk1_tls0,\
file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\
file.server.host=192.168.0.2,file.server.port=9999,format=raw,\
if=none,id=drive-virtio-disk1,cache=none \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
index e1ad36e..ad78405 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
@@ -18,9 +18,10 @@ QEMU_AUDIO_DRV=none \
-no-acpi \
-boot c \
-usb \
--object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
+-object tls-creds-x509,id=objvxhs_virtio-disk0_tls0,\
+dir=/usr/local/etc/pki/qemu,\
endpoint=client,verify-peer=yes \
--drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
+-drive file.driver=vxhs,file.tls-creds=objvxhs_virtio-disk0_tls0,\
file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,\
id=drive-virtio-disk0,cache=none \
Thanks,
Ashish
> BTW: if you haven't already, read Dan Berrange's blog on TLS:
>
> https://www.berrange.com/posts/2016/04/01/improving-qemu-security-part-2-generic-tls-support/
>
> that's a link to page 2, read/scan the remaining 5 blogs as well. Some
> of the exact qemu syntax has changed since the blog was written, but the
> description is really what helps the frame of reference.
>
>>> + qemuCaps);
>>> + } else if (cfg->vxhsTLS == false &&
>>> + disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
>>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>> + _("Please enable VxHS specific TLS options in the qemu "
>>> + "conf file before using TLS in VxHS device domain "
>>> + "specification"));
>>> + ret = -1;
>>> + }
>>> +
>>> + return ret;
>>> +}
>
> [...]
>
>>> 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 0000000..960960d
>>> --- /dev/null
>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
>>
>> [this file has same mistake as the one below]
>>
>>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
>>> new file mode 100644
>>> index 0000000..960960d
>>> --- /dev/null
>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
>>> @@ -0,0 +1,41 @@
>>> +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 \
>>> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
>>> +-no-acpi \
>>> +-boot c \
>>> +-usb \
>>> +-object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\a
>>
>> This ...
>>
>>> +endpoint=client,verify-peer=yes \
>>> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
>>> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
>>> +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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
>>
>> ... and this looks wrong. You have two tls-creds-x509 with the same
>> alias. I doubt that qemu will be happy wit that.
>>
>
> Not only that, but dir=/usr/local/etc/pki/qemu causes qemuxml2argvtest
> to fail for me since the default is supposed to be /etc/pki/libvirt-vxhs
>
> John
>
>>> +endpoint=client,verify-peer=yes \
>>> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
>>> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\
>>> +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.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 0000000..3d28958
>>> --- /dev/null
>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
>>> @@ -0,0 +1,56 @@
>>> +<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>
>>> + <backingStore/>
>>> + <target dev='vda' bus='virtio'/>
>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
>>> + <alias name='virtio-disk0'/>
>>
>> This here ...
>>
>>> + <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>
>>> + <backingStore/>
>>> + <target dev='vdb' bus='virtio'/>
>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
>>> + <alias name='virtio-disk0'/>
>>
>> ... and this have the same alias, which will not happen. Please add
>> proper examples/tests.
>>
>>> + <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>
>>> + <backingStore/>
>>> + <target dev='vdc' bus='virtio'/>
>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
>>> + <alias name='virtio-disk0'/>
>>
>> ... here too.
>>
>>> + <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>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/11/2017 09:43 PM, ashish mittal wrote:
> On Fri, Jun 30, 2017 at 2:21 PM, John Ferlan <jferlan@redhat.com> wrote:
>> [...]
>>
>>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>>> index 8e00782..99bc94f 100644
>>>> --- a/src/qemu/qemu_command.c
>>>> +++ b/src/qemu/qemu_command.c
>>>> @@ -931,6 +931,68 @@ qemuBuildGlusterDriveJSON(virStorageSourcePtr src)
>>>> return ret;
>>>> }
>>>>
>>>> +/* qemuBuildDiskVxHSTLSinfoCommandLine:
>>>> + * @cmd: Pointer to the command string
>>>> + * @cfg: Pointer to the qemu driver config
>>>> + * @disk: The disk we are processing
>>>> + * @qemuCaps: qemu capabilities object
>>>> + *
>>>> + * Check if the VxHS disk meets all the criteria to enable TLS.
>>>> + * If yes, add a new TLS object and mention it's ID on the disk
>>>> + * command line.
>>>> + *
>>>> + * Returns 0 on success, -1 w/ error on some sort of failure.
>>>> + */
>>>> +static int
>>>> +qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
>>>> + virQEMUDriverConfigPtr cfg,
>>>> + virDomainDiskDefPtr disk,
>>>> + virQEMUCapsPtr qemuCaps)
>>>> +{
>>>> + int ret = 0;
>>>> +
>>>> + if (cfg->vxhsTLS == true && disk->src->haveTLS != VIR_TRISTATE_BOOL_NO) {
>>>> + disk->src->addTLS = true;
>>>> + ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
>>>> + false,
>>>> + true,
>>>> + false,
>>>> + "vxhs",
>>>
>>> This argument can't be a constant. This is the alias for the certificate
>>> object.
>>>
>>> Otherwise you'd had to make sure that there's only one such object,
>>> which obviously would make sense here, since (if you don't hotplug disks
>>> after libvirtd restart) the TLS credentials are the same for this disk.
>>>
>>> In case of hotplug though you need to make sure that the TLS object is
>>> removed with the last disk and added if any other disk needing TLS is
>>> added.
>>>
>>
>> So at least the conversation we had last week now makes a bit more sense
>> w/r/t the call to qemuBuildTLSx509CommandLine for chardevTLSx509certdir.
>> As I look at that code now quickly, although having multiple
>> tls-cert-x509 objects for each chardev isn't necessary, it does "work"
>> or start qemu because each would have a different alias (@charAlias is
>> uniquely generated via qemuAliasChardevFromDevAlias). Theoretically
>> speaking two objects wouldn't be required, except for the problem that
>> hotunplug would have to be made aware and we'd have to keep track of
>> when the last one was removed. So leaving with each having their own
>> object is the way the code will stay.
>>
>> So given all that - your alias creation is going to have to contain that
>> uuid or you are going to have to figure out a way to just have one
>> object which each disk uses. You'll have to scan the disks looking to
>> determine if any of the vxhs ones have tls and if so, break to add the
>> object. Then add the 'tls-creds=$object_alias_id'.
>>
>
> Hi John, Peter,
>
Both Peter and I have been wrapped up in something a bit more pressing
lately, but figured I'd take a look so you're not left wondering too long.
> The problem statement - Alias for the TLS certificate can't be a
> constant. Two TLS objects cannot have the same ID/alias.
>
> This was pointed out by both of you as something that needs to be
> fixed. To that end, I have made some changes to use the block device
> domain alias (e.g. virtio-disk0) as a unique identifier for each TLS
> object. This is similar to how char devices create their TLS aliases.
>
> I was hoping to get some feedback on whether this diff would be
> acceptable to fix the said issue. I will reply to/fix all the other
> comments. Just thought I'd tackle this one first as this appears to be
> one of the bigger ones...
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 99bc94f..fc58236 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -952,13 +952,19 @@ qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
> int ret = 0;
>
> if (cfg->vxhsTLS == true && disk->src->haveTLS != VIR_TRISTATE_BOOL_NO) {
> - disk->src->addTLS = true;
> - ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
> - false,
> - true,
> - false,
> - "vxhs",
> - qemuCaps);
> + if (virAsprintf(&disk->src->aliasTLS,
> + "vxhs_%s", disk->info.alias) < 0) {
> + ret = -1;
> + goto cleanup;
> + }
Typically something like this ^^^ would be turned into a helper so
there's no need to store @aliasTLS. My suggestion would be to use
qemuAliasChardevFromDevAlias as a guide. Create a qemu_alias.c
helper/API that allows passing 2 parameters - one the
"disk->src->protocol" and the other the "disk->info.alias". Then in the
function the protocol would be turned into a string via
virStorageNetProtocolTypeToString and the created alias can be "%s_%s"
(so you don't have to change your existing .args output).
This way if "iscsi" or "rbd" or whatever comes along some day, they'd
just pass src->protocol too and magically the string is created with teh
"vxhs" or "iscsi" or "rbd" (etc) prefixdisk.
BTW: A similar argument could be made for the qemuParseVxHSString change
you have. Similar to the @protocol in qemuBuildVxHSDriveJSON
Since you can generate the alias at any time, the aliasTLS would be
unnecessary.
John
> +
> + disk->src->addTLS = true;
> + ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
> + false,
> + true,
> + false,
> + disk->src->aliasTLS,
> + qemuCaps);
> } else if (cfg->vxhsTLS == false &&
> disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -968,6 +974,7 @@ qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
> ret = -1;
> }
>
> + cleanup:
> return ret;
> }
>
> @@ -1040,7 +1047,7 @@ qemuBuildVxHSDriveJSON(virStorageSourcePtr src)
> if (src->addTLS == true) {
> char *objalias = NULL;
>
> - if (!(objalias = qemuAliasTLSObjFromSrcAlias("vxhs")))
> + if (!(objalias = qemuAliasTLSObjFromSrcAlias(src->aliasTLS)))
> goto cleanup;
>
> if (virJSONValueObjectCreate(&ret,
> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 449ace4..61cd54a 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -2057,7 +2057,8 @@ virStorageSourceCopy(const virStorageSource *src,
> VIR_STRDUP(ret->configFile, src->configFile) < 0 ||
> VIR_STRDUP(ret->nodeformat, src->nodeformat) < 0 ||
> VIR_STRDUP(ret->nodebacking, src->nodebacking) < 0 ||
> - VIR_STRDUP(ret->compat, src->compat) < 0)
> + VIR_STRDUP(ret->compat, src->compat) < 0 ||
> + VIR_STRDUP(ret->aliasTLS, src->aliasTLS) < 0)
> goto error;
>
> if (src->nhosts) {
> @@ -2273,6 +2274,7 @@ virStorageSourceClear(virStorageSourcePtr def)
> virStorageSourceSeclabelsClear(def);
> virStoragePermsFree(def->perms);
> VIR_FREE(def->timestamps);
> + VIR_FREE(def->aliasTLS);
>
> virStorageNetHostDefFree(def->nhosts, def->hosts);
> virStorageAuthDefFree(def->auth);
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index e586170..c1d36bf 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -290,6 +290,9 @@ struct _virStorageSource {
> * the device. For e.g. this could be based on a combination of
> * global conf setting + domain specific setting */
> bool addTLS;
> +
> + /* The alias/ID of the TLS object */
> + char *aliasTLS;
> };
>
>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
> b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
> index 4cacb61..b474ce3 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
> @@ -18,17 +18,19 @@ QEMU_AUDIO_DRV=none \
> -no-acpi \
> -boot c \
> -usb \
> --object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
> +-object tls-creds-x509,id=objvxhs_virtio-disk0_tls0,\
> +dir=/usr/local/etc/pki/qemu,\
> endpoint=client,verify-peer=yes \
> --drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
> +-drive file.driver=vxhs,file.tls-creds=objvxhs_virtio-disk0_tls0,\
> file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
> 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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
> +-object tls-creds-x509,id=objvxhs_virtio-disk1_tls0,\
> +dir=/usr/local/etc/pki/qemu,\
> endpoint=client,verify-peer=yes \
> --drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
> +-drive file.driver=vxhs,file.tls-creds=objvxhs_virtio-disk1_tls0,\
> file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\
> file.server.host=192.168.0.2,file.server.port=9999,format=raw,\
> if=none,id=drive-virtio-disk1,cache=none \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
> b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
> index e1ad36e..ad78405 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-vxhs.args
> @@ -18,9 +18,10 @@ QEMU_AUDIO_DRV=none \
> -no-acpi \
> -boot c \
> -usb \
> --object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
> +-object tls-creds-x509,id=objvxhs_virtio-disk0_tls0,\
> +dir=/usr/local/etc/pki/qemu,\
> endpoint=client,verify-peer=yes \
> --drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
> +-drive file.driver=vxhs,file.tls-creds=objvxhs_virtio-disk0_tls0,\
> file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
> file.server.host=192.168.0.1,file.server.port=9999,format=raw,if=none,\
> id=drive-virtio-disk0,cache=none \
>
>
> Thanks,
> Ashish
>
>
>> BTW: if you haven't already, read Dan Berrange's blog on TLS:
>>
>> https://www.berrange.com/posts/2016/04/01/improving-qemu-security-part-2-generic-tls-support/
>>
>> that's a link to page 2, read/scan the remaining 5 blogs as well. Some
>> of the exact qemu syntax has changed since the blog was written, but the
>> description is really what helps the frame of reference.
>>
>>>> + qemuCaps);
>>>> + } else if (cfg->vxhsTLS == false &&
>>>> + disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
>>>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>>> + _("Please enable VxHS specific TLS options in the qemu "
>>>> + "conf file before using TLS in VxHS device domain "
>>>> + "specification"));
>>>> + ret = -1;
>>>> + }
>>>> +
>>>> + return ret;
>>>> +}
>>
>> [...]
>>
>>>> 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 0000000..960960d
>>>> --- /dev/null
>>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
>>>
>>> [this file has same mistake as the one below]
>>>
>>>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
>>>> new file mode 100644
>>>> index 0000000..960960d
>>>> --- /dev/null
>>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
>>>> @@ -0,0 +1,41 @@
>>>> +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 \
>>>> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
>>>> +-no-acpi \
>>>> +-boot c \
>>>> +-usb \
>>>> +-object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\a
>>>
>>> This ...
>>>
>>>> +endpoint=client,verify-peer=yes \
>>>> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
>>>> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
>>>> +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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
>>>
>>> ... and this looks wrong. You have two tls-creds-x509 with the same
>>> alias. I doubt that qemu will be happy wit that.
>>>
>>
>> Not only that, but dir=/usr/local/etc/pki/qemu causes qemuxml2argvtest
>> to fail for me since the default is supposed to be /etc/pki/libvirt-vxhs
>>
>> John
>>
>>>> +endpoint=client,verify-peer=yes \
>>>> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
>>>> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\
>>>> +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.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 0000000..3d28958
>>>> --- /dev/null
>>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
>>>> @@ -0,0 +1,56 @@
>>>> +<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>
>>>> + <backingStore/>
>>>> + <target dev='vda' bus='virtio'/>
>>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
>>>> + <alias name='virtio-disk0'/>
>>>
>>> This here ...
>>>
>>>> + <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>
>>>> + <backingStore/>
>>>> + <target dev='vdb' bus='virtio'/>
>>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
>>>> + <alias name='virtio-disk0'/>
>>>
>>> ... and this have the same alias, which will not happen. Please add
>>> proper examples/tests.
>>>
>>>> + <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>
>>>> + <backingStore/>
>>>> + <target dev='vdc' bus='virtio'/>
>>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
>>>> + <alias name='virtio-disk0'/>
>>>
>>> ... here too.
>>>
>>>> + <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>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jun 30, 2017 at 2:21 PM, John Ferlan <jferlan@redhat.com> wrote:
> [...]
>
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index 8e00782..99bc94f 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -931,6 +931,68 @@ qemuBuildGlusterDriveJSON(virStorageSourcePtr src)
>>> return ret;
>>> }
>>>
>>> +/* qemuBuildDiskVxHSTLSinfoCommandLine:
>>> + * @cmd: Pointer to the command string
>>> + * @cfg: Pointer to the qemu driver config
>>> + * @disk: The disk we are processing
>>> + * @qemuCaps: qemu capabilities object
>>> + *
>>> + * Check if the VxHS disk meets all the criteria to enable TLS.
>>> + * If yes, add a new TLS object and mention it's ID on the disk
>>> + * command line.
>>> + *
>>> + * Returns 0 on success, -1 w/ error on some sort of failure.
>>> + */
>>> +static int
>>> +qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
>>> + virQEMUDriverConfigPtr cfg,
>>> + virDomainDiskDefPtr disk,
>>> + virQEMUCapsPtr qemuCaps)
>>> +{
>>> + int ret = 0;
>>> +
>>> + if (cfg->vxhsTLS == true && disk->src->haveTLS != VIR_TRISTATE_BOOL_NO) {
>>> + disk->src->addTLS = true;
>>> + ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
>>> + false,
>>> + true,
>>> + false,
>>> + "vxhs",
>>
>> This argument can't be a constant. This is the alias for the certificate
>> object.
>>
>> Otherwise you'd had to make sure that there's only one such object,
>> which obviously would make sense here, since (if you don't hotplug disks
>> after libvirtd restart) the TLS credentials are the same for this disk.
>>
>> In case of hotplug though you need to make sure that the TLS object is
>> removed with the last disk and added if any other disk needing TLS is
>> added.
>>
>
> So at least the conversation we had last week now makes a bit more sense
> w/r/t the call to qemuBuildTLSx509CommandLine for chardevTLSx509certdir.
> As I look at that code now quickly, although having multiple
> tls-cert-x509 objects for each chardev isn't necessary, it does "work"
> or start qemu because each would have a different alias (@charAlias is
> uniquely generated via qemuAliasChardevFromDevAlias). Theoretically
> speaking two objects wouldn't be required, except for the problem that
> hotunplug would have to be made aware and we'd have to keep track of
> when the last one was removed. So leaving with each having their own
> object is the way the code will stay.
>
> So given all that - your alias creation is going to have to contain that
> uuid or you are going to have to figure out a way to just have one
> object which each disk uses. You'll have to scan the disks looking to
> determine if any of the vxhs ones have tls and if so, break to add the
> object. Then add the 'tls-creds=$object_alias_id'.
>
This makes sense. Will get back with the diff that could achieve this.
> BTW: if you haven't already, read Dan Berrange's blog on TLS:
>
> https://www.berrange.com/posts/2016/04/01/improving-qemu-security-part-2-generic-tls-support/
>
> that's a link to page 2, read/scan the remaining 5 blogs as well. Some
> of the exact qemu syntax has changed since the blog was written, but the
> description is really what helps the frame of reference.
>
Will do. Thanks!
>>> + qemuCaps);
>>> + } else if (cfg->vxhsTLS == false &&
>>> + disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
>>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>> + _("Please enable VxHS specific TLS options in the qemu "
>>> + "conf file before using TLS in VxHS device domain "
>>> + "specification"));
>>> + ret = -1;
>>> + }
>>> +
>>> + return ret;
>>> +}
>
> [...]
>
>>> 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 0000000..960960d
>>> --- /dev/null
>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
>>
>> [this file has same mistake as the one below]
>>
>>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
>>> new file mode 100644
>>> index 0000000..960960d
>>> --- /dev/null
>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
>>> @@ -0,0 +1,41 @@
>>> +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 \
>>> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
>>> +-no-acpi \
>>> +-boot c \
>>> +-usb \
>>> +-object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\a
>>
>> This ...
>>
>>> +endpoint=client,verify-peer=yes \
>>> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
>>> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
>>> +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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
>>
>> ... and this looks wrong. You have two tls-creds-x509 with the same
>> alias. I doubt that qemu will be happy wit that.
>>
>
> Not only that, but dir=/usr/local/etc/pki/qemu causes qemuxml2argvtest
> to fail for me since the default is supposed to be /etc/pki/libvirt-vxhs
>
Will check this.
> John
>
>>> +endpoint=client,verify-peer=yes \
>>> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
>>> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\
>>> +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.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 0000000..3d28958
>>> --- /dev/null
>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
>>> @@ -0,0 +1,56 @@
>>> +<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>
>>> + <backingStore/>
>>> + <target dev='vda' bus='virtio'/>
>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
>>> + <alias name='virtio-disk0'/>
>>
>> This here ...
>>
>>> + <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>
>>> + <backingStore/>
>>> + <target dev='vdb' bus='virtio'/>
>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
>>> + <alias name='virtio-disk0'/>
>>
>> ... and this have the same alias, which will not happen. Please add
>> proper examples/tests.
>>
>>> + <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>
>>> + <backingStore/>
>>> + <target dev='vdc' bus='virtio'/>
>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
>>> + <alias name='virtio-disk0'/>
>>
>> ... here too.
>>
>>> + <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>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Jun 30, 2017 at 2:58 PM, ashish mittal <ashmit602@gmail.com> wrote:
> On Fri, Jun 30, 2017 at 2:21 PM, John Ferlan <jferlan@redhat.com> wrote:
>> [...]
>>
>>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>>> index 8e00782..99bc94f 100644
>>>> --- a/src/qemu/qemu_command.c
>>>> +++ b/src/qemu/qemu_command.c
>>>> @@ -931,6 +931,68 @@ qemuBuildGlusterDriveJSON(virStorageSourcePtr src)
>>>> return ret;
>>>> }
>>>>
>>>> +/* qemuBuildDiskVxHSTLSinfoCommandLine:
>>>> + * @cmd: Pointer to the command string
>>>> + * @cfg: Pointer to the qemu driver config
>>>> + * @disk: The disk we are processing
>>>> + * @qemuCaps: qemu capabilities object
>>>> + *
>>>> + * Check if the VxHS disk meets all the criteria to enable TLS.
>>>> + * If yes, add a new TLS object and mention it's ID on the disk
>>>> + * command line.
>>>> + *
>>>> + * Returns 0 on success, -1 w/ error on some sort of failure.
>>>> + */
>>>> +static int
>>>> +qemuBuildDiskVxHSTLSinfoCommandLine(virCommandPtr cmd,
>>>> + virQEMUDriverConfigPtr cfg,
>>>> + virDomainDiskDefPtr disk,
>>>> + virQEMUCapsPtr qemuCaps)
>>>> +{
>>>> + int ret = 0;
>>>> +
>>>> + if (cfg->vxhsTLS == true && disk->src->haveTLS != VIR_TRISTATE_BOOL_NO) {
>>>> + disk->src->addTLS = true;
>>>> + ret = qemuBuildTLSx509CommandLine(cmd, cfg->vxhsTLSx509certdir,
>>>> + false,
>>>> + true,
>>>> + false,
>>>> + "vxhs",
>>>
>>> This argument can't be a constant. This is the alias for the certificate
>>> object.
>>>
>>> Otherwise you'd had to make sure that there's only one such object,
>>> which obviously would make sense here, since (if you don't hotplug disks
>>> after libvirtd restart) the TLS credentials are the same for this disk.
>>>
>>> In case of hotplug though you need to make sure that the TLS object is
>>> removed with the last disk and added if any other disk needing TLS is
>>> added.
>>>
>>
>> So at least the conversation we had last week now makes a bit more sense
>> w/r/t the call to qemuBuildTLSx509CommandLine for chardevTLSx509certdir.
>> As I look at that code now quickly, although having multiple
>> tls-cert-x509 objects for each chardev isn't necessary, it does "work"
>> or start qemu because each would have a different alias (@charAlias is
>> uniquely generated via qemuAliasChardevFromDevAlias). Theoretically
>> speaking two objects wouldn't be required, except for the problem that
>> hotunplug would have to be made aware and we'd have to keep track of
>> when the last one was removed. So leaving with each having their own
>> object is the way the code will stay.
>>
>> So given all that - your alias creation is going to have to contain that
>> uuid or you are going to have to figure out a way to just have one
>> object which each disk uses. You'll have to scan the disks looking to
>> determine if any of the vxhs ones have tls and if so, break to add the
>> object. Then add the 'tls-creds=$object_alias_id'.
>>
>
> This makes sense. Will get back with the diff that could achieve this.
>
>> BTW: if you haven't already, read Dan Berrange's blog on TLS:
>>
>> https://www.berrange.com/posts/2016/04/01/improving-qemu-security-part-2-generic-tls-support/
>>
>> that's a link to page 2, read/scan the remaining 5 blogs as well. Some
>> of the exact qemu syntax has changed since the blog was written, but the
>> description is really what helps the frame of reference.
>>
>
> Will do. Thanks!
>
>>>> + qemuCaps);
>>>> + } else if (cfg->vxhsTLS == false &&
>>>> + disk->src->haveTLS == VIR_TRISTATE_BOOL_YES) {
>>>> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>>> + _("Please enable VxHS specific TLS options in the qemu "
>>>> + "conf file before using TLS in VxHS device domain "
>>>> + "specification"));
>>>> + ret = -1;
>>>> + }
>>>> +
>>>> + return ret;
>>>> +}
>>
>> [...]
>>
>>>> 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 0000000..960960d
>>>> --- /dev/null
>>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args
>>>
>>> [this file has same mistake as the one below]
>>>
>>>> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
>>>> new file mode 100644
>>>> index 0000000..960960d
>>>> --- /dev/null
>>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.args.new
>>>> @@ -0,0 +1,41 @@
>>>> +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 \
>>>> +-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
>>>> +-no-acpi \
>>>> +-boot c \
>>>> +-usb \
>>>> +-object tls-creds-x509,id=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\a
>>>
>>> This ...
>>>
>>>> +endpoint=client,verify-peer=yes \
>>>> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
>>>> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc251,\
>>>> +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=objvxhs_tls0,dir=/usr/local/etc/pki/qemu,\
>>>
>>> ... and this looks wrong. You have two tls-creds-x509 with the same
>>> alias. I doubt that qemu will be happy wit that.
>>>
>>
>> Not only that, but dir=/usr/local/etc/pki/qemu causes qemuxml2argvtest
>> to fail for me since the default is supposed to be /etc/pki/libvirt-vxhs
>>
>
> Will check this.
>
>> John
>>
>>>> +endpoint=client,verify-peer=yes \
>>>> +-drive file.driver=vxhs,file.tls-creds=objvxhs_tls0,\
>>>> +file.vdisk-id=eb90327c-8302-4725-9e1b-4e85ed4dc252,\
>>>> +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.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 0000000..3d28958
>>>> --- /dev/null
>>>> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-tlsx509-multidisk-vxhs.xml
>>>> @@ -0,0 +1,56 @@
>>>> +<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>
>>>> + <backingStore/>
>>>> + <target dev='vda' bus='virtio'/>
>>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc251</serial>
>>>> + <alias name='virtio-disk0'/>
>>>
>>> This here ...
>>>
>>>> + <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>
>>>> + <backingStore/>
>>>> + <target dev='vdb' bus='virtio'/>
>>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
>>>> + <alias name='virtio-disk0'/>
>>>
>>> ... and this have the same alias, which will not happen. Please add
>>> proper examples/tests.
>>>
>>>> + <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>
>>>> + <backingStore/>
>>>> + <target dev='vdc' bus='virtio'/>
>>>> + <serial>eb90327c-8302-4725-9e1b-4e85ed4dc252</serial>
>>>> + <alias name='virtio-disk0'/>
>>>
>>> ... here too.
>>>
>>>> + <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>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.