3:23 p.m.
v8: http://www.redhat.com/archives/libvir-list/2016-October/msg00306.html
Differences to v8
1. Alter patch 1 such that it's not choosing to add the tls object based
on both a config and domain xml setting, rather that the decision point
is to avoid adding the config setting if the domain is set to tls='no'
2. Alter patch 2 to remove the booleans - this impacts patch 4 and 5 insomuch
as they need to key off whether the chardevTLSx509secretUUID is set or not.
John Ferlan (5):
domain: Add optional 'tls' attribute for TCP chardev
conf: Introduce {default|chardev}_tls_x509_secret_uuid
qemu: Introduce qemuDomainChardevPrivatePtr
qemu: Add a secret object to/for a chardev tcp with secret
qemu: Add the ability to hotplug a secret object for TCP chardev TLS
docs/formatdomain.html.in | 21 +++
docs/schemas/domaincommon.rng | 5 +
src/conf/domain_conf.c | 51 ++++++--
src/conf/domain_conf.h | 5 +-
src/libxl/libxl_domain.c | 2 +-
src/lxc/lxc_native.c | 2 +-
src/qemu/libvirtd_qemu.aug | 2 +
src/qemu/qemu.conf | 24 ++++
src/qemu/qemu_command.c | 35 ++++-
src/qemu/qemu_command.h | 1 +
src/qemu/qemu_conf.c | 14 ++
src/qemu/qemu_conf.h | 2 +
src/qemu/qemu_domain.c | 143 ++++++++++++++++++++-
src/qemu/qemu_domain.h | 30 ++++-
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_hotplug.c | 64 ++++++++-
src/qemu/qemu_hotplug.h | 3 +-
src/qemu/qemu_parse_command.c | 4 +-
src/qemu/qemu_process.c | 6 +-
src/qemu/test_libvirtd_qemu.aug.in | 2 +
src/vz/vz_sdk.c | 2 +-
src/xenconfig/xen_sxpr.c | 2 +-
src/xenconfig/xen_xl.c | 2 +-
tests/qemuhotplugtest.c | 2 +-
...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +++++
...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 +++++++
.../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 2 +-
...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 ++++++
...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++++++
tests/qemuxml2argvtest.c | 21 +++
...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml | 1 +
.../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 2 +-
tests/qemuxml2xmltest.c | 1 +
33 files changed, 584 insertions(+), 37 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
create mode 120000
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
--
2.7.4
3:23 p.m.
New subject: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev
Add an optional "tls='yes|no'" attribute for a TCP chardev for the
express purpose to disable setting up TLS for the specific chardev in
the event the qemu.conf settings have enabled hypervisor wide TLS for
serial TCP chardevs.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatdomain.html.in | 21 +++++++++
docs/schemas/domaincommon.rng | 5 +++
src/conf/domain_conf.c | 22 +++++++++-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 3 +-
src/qemu/qemu_hotplug.c | 3 +-
...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +++++++++++++
...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 ++++++++++++++++++++++
.../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 2 +-
tests/qemuxml2argvtest.c | 3 ++
...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml | 1 +
.../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 2 +-
tests/qemuxml2xmltest.c | 1 +
13 files changed, 139 insertions(+), 5 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
create mode 120000
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9051178..6145d65 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6204,6 +6204,27 @@ qemu-kvm -net nic,model=? /dev/null
</devices>
...</pre>
+ <p>
+ <span class="since">Since 2.4.0,</span> the optional
attribute
+ <code>tls</code> can be used to control whether a serial chardev
+ TCP communication channel would utilize a hypervisor configured
+ TLS X.509 certificate environment in order to encrypt the data
+ channel. For the QEMU hypervisor usage of TLS is controlled by the
+ <code>chardev_tls</code> setting in file /etc/libvirt/qemu.conf. If
+ enabled, then by setting this attribute to "no" will disable usage
+ of the TLS environment for this particular serial TCP data channel.
+ </p>
+<pre>
+ ...
+ <devices>
+ <serial type="tcp">
+ <source mode='connect' host="127.0.0.1"
service="5555" tls="yes"/>
+ <protocol type="raw"/>
+ <target port="0"/>
+ </serial>
+ </devices>
+ ...</pre>
+
<h6><a name="elementsCharUDP">UDP network
console</a></h6>
<p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3106510..e6741bb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3453,6 +3453,11 @@
<ref name="virOnOff"/>
</attribute>
</optional>
+ <optional>
+ <attribute name="tls">
+ <ref name="virYesNo"/>
+ </attribute>
+ </optional>
<zeroOrMore>
<ref name='devSeclabel'/>
</zeroOrMore>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 93b34e0..9062544 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1997,6 +1997,8 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
if (VIR_STRDUP(dest->data.tcp.service, src->data.tcp.service) < 0)
return -1;
+
+ dest->data.tcp.haveTLS = src->data.tcp.haveTLS;
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
@@ -10038,6 +10040,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
char *master = NULL;
char *slave = NULL;
char *append = NULL;
+ char *haveTLS = NULL;
int remaining = 0;
while (cur != NULL) {
@@ -10045,6 +10048,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
if (xmlStrEqual(cur->name, BAD_CAST "source")) {
if (!mode)
mode = virXMLPropString(cur, "mode");
+ if (!haveTLS)
+ haveTLS = virXMLPropString(cur, "tls");
switch ((virDomainChrType) def->type) {
case VIR_DOMAIN_CHR_TYPE_FILE:
@@ -10221,6 +10226,15 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
def->data.tcp.listen = true;
}
+ if (haveTLS &&
+ (def->data.tcp.haveTLS =
+ virTristateBoolTypeFromString(haveTLS)) <= 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unknown chardev 'tls' setting
'%s'"),
+ haveTLS);
+ goto error;
+ }
+
if (!protocol)
def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW;
else if ((def->data.tcp.protocol =
@@ -10305,6 +10319,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
VIR_FREE(append);
VIR_FREE(logappend);
VIR_FREE(logfile);
+ VIR_FREE(haveTLS);
return remaining;
@@ -21453,7 +21468,12 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, "<source mode='%s' ",
def->data.tcp.listen ? "bind" :
"connect");
virBufferEscapeString(buf, "host='%s' ",
def->data.tcp.host);
- virBufferEscapeString(buf, "service='%s'/>\n",
def->data.tcp.service);
+ virBufferEscapeString(buf, "service='%s'",
def->data.tcp.service);
+ if (def->data.tcp.haveTLS != VIR_TRISTATE_BOOL_ABSENT)
+ virBufferAsprintf(buf, " tls='%s'",
+ virTristateBoolTypeToString(def->data.tcp.haveTLS));
+ virBufferAddLit(buf, "/>\n");
+
virBufferAsprintf(buf, "<protocol type='%s'/>\n",
virDomainChrTcpProtocolTypeToString(
def->data.tcp.protocol));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3e85ae4..c34e046 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1094,6 +1094,7 @@ struct _virDomainChrSourceDef {
bool listen;
int protocol;
bool tlscreds;
+ int haveTLS; /* enum virTristateBool */
} tcp;
struct {
char *bindHost;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 21fd85c..aaf7018 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4939,7 +4939,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
if (dev->data.tcp.listen)
virBufferAdd(&buf, nowait ? ",server,nowait" :
",server", -1);
- if (cfg->chardevTLS) {
+ if (cfg->chardevTLS &&
+ dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) {
char *objalias = NULL;
if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 14af4e1..f4d7c17 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1729,7 +1729,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
if (qemuDomainChrPreInsert(vmdef, chr) < 0)
goto cleanup;
- if (cfg->chardevTLS) {
+ if (cfg->chardevTLS &&
+ dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) {
if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
new file mode 100644
index 0000000..cac0d85
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
@@ -0,0 +1,30 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
+localport=1111 \
+-device isa-serial,chardev=charserial0,id=serial0 \
+-chardev socket,id=charserial1,host=127.0.0.1,port=5555 \
+-device isa-serial,chardev=charserial1,id=serial1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
new file mode 100644
index 0000000..debc69b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
@@ -0,0 +1,50 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <serial type='udp'>
+ <source mode='bind' host='127.0.0.1'
service='1111'/>
+ <source mode='connect' host='127.0.0.1'
service='2222'/>
+ <target port='0'/>
+ </serial>
+ <serial type='tcp'>
+ <source mode='connect' host='127.0.0.1' service='5555'
tls='no'/>
+ <protocol type='raw'/>
+ <target port='0'/>
+ </serial>
+ <console type='udp'>
+ <source mode='bind' host='127.0.0.1'
service='1111'/>
+ <source mode='connect' host='127.0.0.1'
service='2222'/>
+ <target type='serial' port='0'/>
+ </console>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
index 1618b02..1d7896d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
@@ -27,7 +27,7 @@
<target port='0'/>
</serial>
<serial type='tcp'>
- <source mode='connect' host='127.0.0.1'
service='5555'/>
+ <source mode='connect' host='127.0.0.1' service='5555'
tls='yes'/>
<protocol type='raw'/>
<target port='0'/>
</serial>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e8438b4..c65c769 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1162,6 +1162,9 @@ mymain(void)
DO_TEST("serial-tcp-tlsx509-chardev",
QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+ DO_TEST("serial-tcp-tlsx509-chardev-notls",
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_OBJECT_TLS_CREDS_X509);
driver.config->chardevTLS = 0;
VIR_FREE(driver.config->chardevTLSx509certdir);
DO_TEST("serial-many-chardev",
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
new file mode 120000
index 0000000..26484c9
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
index 832e2a2..23c244b 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
@@ -32,7 +32,7 @@
<target port='0'/>
</serial>
<serial type='tcp'>
- <source mode='connect' host='127.0.0.1'
service='5555'/>
+ <source mode='connect' host='127.0.0.1' service='5555'
tls='yes'/>
<protocol type='raw'/>
<target port='0'/>
</serial>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 62bff8c..3ab4c50 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -534,6 +534,7 @@ mymain(void)
DO_TEST("serial-udp", NONE);
DO_TEST("serial-tcp-telnet", NONE);
DO_TEST("serial-tcp-tlsx509-chardev", NONE);
+ DO_TEST("serial-tcp-tlsx509-chardev-notls", NONE);
DO_TEST("serial-many", NONE);
DO_TEST("serial-spiceport", NONE);
DO_TEST("serial-spiceport-nospice", NONE);
--
2.7.4
3:09 a.m.
New subject: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev
On Fri, Oct 14, 2016 at 04:23:04PM -0400, John Ferlan wrote:
Add an optional "tls='yes|no'" attribute for a TCP
chardev for the
express purpose to disable setting up TLS for the specific chardev in
the event the qemu.conf settings have enabled hypervisor wide TLS for
serial TCP chardevs.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
docs/formatdomain.html.in | 21 +++++++++
docs/schemas/domaincommon.rng | 5 +++
src/conf/domain_conf.c | 22 +++++++++-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 3 +-
src/qemu/qemu_hotplug.c | 3 +-
...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +++++++++++++
...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 ++++++++++++++++++++++
.../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 2 +-
tests/qemuxml2argvtest.c | 3 ++
...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml | 1 +
.../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 2 +-
tests/qemuxml2xmltest.c | 1 +
13 files changed, 139 insertions(+), 5 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
create mode 120000
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 9051178..6145d65 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6204,6 +6204,27 @@ qemu-kvm -net nic,model=? /dev/null
</devices>
...</pre>
+ <p>
+ <span class="since">Since 2.4.0,</span> the optional
attribute
+ <code>tls</code> can be used to control whether a serial chardev
+ TCP communication channel would utilize a hypervisor configured
+ TLS X.509 certificate environment in order to encrypt the data
+ channel. For the QEMU hypervisor usage of TLS is controlled by the
+ <code>chardev_tls</code> setting in file /etc/libvirt/qemu.conf. If
+ enabled, then by setting this attribute to "no" will disable usage
+ of the TLS environment for this particular serial TCP data channel.
+ </p>
Previous discussion:
http://www.redhat.com/archives/libvir-list/2016-October/msg00734.html
So now there is no functional issue if we agree that this design is what we
want. I personally thing that there could be also use-case where you want to
configure certificates in qemu.conf and use 'tls' attribute to explicitly
enable TLS encryption only for some VMs and only for some chardev and this is
not currently possible whit this design. Now user can enable the TLS encryption
for every chardev for every domain and explicitly disable for some chardevs.
This would require to add all the extra code from that discussion to handle
migration properly and that's why I've started the discussion.
+<pre>
+ ...
+ <devices>
+ <serial type="tcp">
+ <source mode='connect' host="127.0.0.1"
service="5555" tls="yes"/>
+ <protocol type="raw"/>
+ <target port="0"/>
+ </serial>
+ </devices>
+ ...</pre>
+
<h6><a name="elementsCharUDP">UDP network
console</a></h6>
<p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3106510..e6741bb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3453,6 +3453,11 @@
<ref name="virOnOff"/>
</attribute>
</optional>
+ <optional>
+ <attribute name="tls">
+ <ref name="virYesNo"/>
+ </attribute>
+ </optional>
<zeroOrMore>
<ref name='devSeclabel'/>
</zeroOrMore>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 93b34e0..9062544 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1997,6 +1997,8 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
if (VIR_STRDUP(dest->data.tcp.service, src->data.tcp.service) < 0)
return -1;
+
+ dest->data.tcp.haveTLS = src->data.tcp.haveTLS;
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
@@ -10038,6 +10040,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
char *master = NULL;
char *slave = NULL;
char *append = NULL;
+ char *haveTLS = NULL;
int remaining = 0;
while (cur != NULL) {
@@ -10045,6 +10048,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
if (xmlStrEqual(cur->name, BAD_CAST "source")) {
if (!mode)
mode = virXMLPropString(cur, "mode");
+ if (!haveTLS)
+ haveTLS = virXMLPropString(cur, "tls");
switch ((virDomainChrType) def->type) {
case VIR_DOMAIN_CHR_TYPE_FILE:
@@ -10221,6 +10226,15 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
def->data.tcp.listen = true;
}
+ if (haveTLS &&
+ (def->data.tcp.haveTLS =
+ virTristateBoolTypeFromString(haveTLS)) <= 0) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unknown chardev 'tls' setting
'%s'"),
+ haveTLS);
+ goto error;
+ }
+
if (!protocol)
def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW;
else if ((def->data.tcp.protocol =
@@ -10305,6 +10319,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
VIR_FREE(append);
VIR_FREE(logappend);
VIR_FREE(logfile);
+ VIR_FREE(haveTLS);
return remaining;
@@ -21453,7 +21468,12 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, "<source mode='%s' ",
def->data.tcp.listen ? "bind" :
"connect");
virBufferEscapeString(buf, "host='%s' ",
def->data.tcp.host);
- virBufferEscapeString(buf, "service='%s'/>\n",
def->data.tcp.service);
+ virBufferEscapeString(buf, "service='%s'",
def->data.tcp.service);
+ if (def->data.tcp.haveTLS != VIR_TRISTATE_BOOL_ABSENT)
+ virBufferAsprintf(buf, " tls='%s'",
+ virTristateBoolTypeToString(def->data.tcp.haveTLS));
+ virBufferAddLit(buf, "/>\n");
+
virBufferAsprintf(buf, "<protocol type='%s'/>\n",
virDomainChrTcpProtocolTypeToString(
def->data.tcp.protocol));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 3e85ae4..c34e046 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1094,6 +1094,7 @@ struct _virDomainChrSourceDef {
bool listen;
int protocol;
bool tlscreds;
+ int haveTLS; /* enum virTristateBool */
} tcp;
struct {
char *bindHost;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 21fd85c..aaf7018 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4939,7 +4939,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
if (dev->data.tcp.listen)
virBufferAdd(&buf, nowait ? ",server,nowait" :
",server", -1);
- if (cfg->chardevTLS) {
+ if (cfg->chardevTLS &&
+ dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) {
char *objalias = NULL;
if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 14af4e1..f4d7c17 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1729,7 +1729,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
if (qemuDomainChrPreInsert(vmdef, chr) < 0)
goto cleanup;
- if (cfg->chardevTLS) {
+ if (cfg->chardevTLS &&
+ dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) {
if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
new file mode 100644
index 0000000..cac0d85
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
@@ -0,0 +1,30 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
+localport=1111 \
+-device isa-serial,chardev=charserial0,id=serial0 \
+-chardev socket,id=charserial1,host=127.0.0.1,port=5555 \
+-device isa-serial,chardev=charserial1,id=serial1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
new file mode 100644
index 0000000..debc69b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
@@ -0,0 +1,50 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <serial type='udp'>
+ <source mode='bind' host='127.0.0.1'
service='1111'/>
+ <source mode='connect' host='127.0.0.1'
service='2222'/>
+ <target port='0'/>
+ </serial>
+ <serial type='tcp'>
+ <source mode='connect' host='127.0.0.1' service='5555'
tls='no'/>
+ <protocol type='raw'/>
+ <target port='0'/>
+ </serial>
+ <console type='udp'>
+ <source mode='bind' host='127.0.0.1'
service='1111'/>
+ <source mode='connect' host='127.0.0.1'
service='2222'/>
+ <target type='serial' port='0'/>
+ </console>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
index 1618b02..1d7896d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
@@ -27,7 +27,7 @@
<target port='0'/>
</serial>
<serial type='tcp'>
- <source mode='connect' host='127.0.0.1'
service='5555'/>
+ <source mode='connect' host='127.0.0.1' service='5555'
tls='yes'/>
This is not required and should work without the attribute.
Pavel
<protocol type='raw'/>
<target port='0'/>
</serial>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index e8438b4..c65c769 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1162,6 +1162,9 @@ mymain(void)
DO_TEST("serial-tcp-tlsx509-chardev",
QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+ DO_TEST("serial-tcp-tlsx509-chardev-notls",
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_OBJECT_TLS_CREDS_X509);
driver.config->chardevTLS = 0;
VIR_FREE(driver.config->chardevTLSx509certdir);
DO_TEST("serial-many-chardev",
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
new file mode 120000
index 0000000..26484c9
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
index 832e2a2..23c244b 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
@@ -32,7 +32,7 @@
<target port='0'/>
</serial>
<serial type='tcp'>
- <source mode='connect' host='127.0.0.1'
service='5555'/>
+ <source mode='connect' host='127.0.0.1' service='5555'
tls='yes'/>
<protocol type='raw'/>
<target port='0'/>
</serial>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 62bff8c..3ab4c50 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -534,6 +534,7 @@ mymain(void)
DO_TEST("serial-udp", NONE);
DO_TEST("serial-tcp-telnet", NONE);
DO_TEST("serial-tcp-tlsx509-chardev", NONE);
+ DO_TEST("serial-tcp-tlsx509-chardev-notls", NONE);
DO_TEST("serial-many", NONE);
DO_TEST("serial-spiceport", NONE);
DO_TEST("serial-spiceport-nospice", NONE);
--
2.7.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:54 a.m.
New subject: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev
On 10/17/2016 04:09 AM, Pavel Hrdina wrote:
On Fri, Oct 14, 2016 at 04:23:04PM -0400, John Ferlan wrote:
> Add an optional "tls='yes|no'" attribute for a TCP chardev for the
> express purpose to disable setting up TLS for the specific chardev in
> the event the qemu.conf settings have enabled hypervisor wide TLS for
> serial TCP chardevs.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> docs/formatdomain.html.in | 21 +++++++++
> docs/schemas/domaincommon.rng | 5 +++
> src/conf/domain_conf.c | 22 +++++++++-
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_command.c | 3 +-
> src/qemu/qemu_hotplug.c | 3 +-
> ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +++++++++++++
> ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 ++++++++++++++++++++++
> .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 2 +-
> tests/qemuxml2argvtest.c | 3 ++
> ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml | 1 +
> .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 2 +-
> tests/qemuxml2xmltest.c | 1 +
> 13 files changed, 139 insertions(+), 5 deletions(-)
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
> create mode 120000
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 9051178..6145d65 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -6204,6 +6204,27 @@ qemu-kvm -net nic,model=? /dev/null
> </devices>
> ...</pre>
>
> + <p>
> + <span class="since">Since 2.4.0,</span> the optional
attribute
> + <code>tls</code> can be used to control whether a serial chardev
> + TCP communication channel would utilize a hypervisor configured
> + TLS X.509 certificate environment in order to encrypt the data
> + channel. For the QEMU hypervisor usage of TLS is controlled by the
> + <code>chardev_tls</code> setting in file /etc/libvirt/qemu.conf.
If
> + enabled, then by setting this attribute to "no" will disable usage
> + of the TLS environment for this particular serial TCP data channel.
> + </p>
Previous discussion:
http://www.redhat.com/archives/libvir-list/2016-October/msg00734.html
So now there is no functional issue if we agree that this design is what we
want. I personally thing that there could be also use-case where you want to
configure certificates in qemu.conf and use 'tls' attribute to explicitly
enable TLS encryption only for some VMs and only for some chardev and this is
not currently possible whit this design. Now user can enable the TLS encryption
for every chardev for every domain and explicitly disable for some chardevs.
This would require to add all the extra code from that discussion to handle
migration properly and that's why I've started the discussion.
w/r/t: design - re-read what I pulled from Dan's v5 review:
http://www.redhat.com/archives/libvir-list/2016-October/msg00698.html
While forcing to set "tls='yes'" is certainly a mechanism that could be
used and adding extra code is possible, is that something that we want
to require of everyone that's using TLS chardev's? If you go through the
trouble of configuring for your host, then how would you feel about
having to update all your domains (for those that have more than 1 or 2)
to set the property in order to be able to use it? Again, I really don't
think it's a good idea to be mucking around with changing XML of running
domains, adding extra checks in the migration code, altering the
format/parse code to check flags, and/or anything else that may come up.
The less we do that the better - introduces less risk for making or
missing assumptions.
For comparison spice uses an attribute mechanism "tlsPort={-1|0|port}"
in order to use TLS, but it also checks cfg->spiceTLS. Alternatively,
vnc will just take the cfg->vncTLS to decide whether to use or not
(e.g., there's no way to avoid).
So one of the existing mechanisms forces you to define something to use
TLS configured for the host and the other doesn't. Spice has the
additional configuration option to determine specific channels by name
that would use the secure port.
This patch can still be considered "outside" of the subsequent 4 in this
series...
> +<pre>
> + ...
> + <devices>
> + <serial type="tcp">
> + <source mode='connect' host="127.0.0.1"
service="5555" tls="yes"/>
> + <protocol type="raw"/>
> + <target port="0"/>
> + </serial>
> + </devices>
> + ...</pre>
> +
> <h6><a name="elementsCharUDP">UDP network
console</a></h6>
>
> <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 3106510..e6741bb 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3453,6 +3453,11 @@
> <ref name="virOnOff"/>
> </attribute>
> </optional>
> + <optional>
> + <attribute name="tls">
> + <ref name="virYesNo"/>
> + </attribute>
> + </optional>
> <zeroOrMore>
> <ref name='devSeclabel'/>
> </zeroOrMore>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 93b34e0..9062544 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1997,6 +1997,8 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
>
> if (VIR_STRDUP(dest->data.tcp.service, src->data.tcp.service) < 0)
> return -1;
> +
> + dest->data.tcp.haveTLS = src->data.tcp.haveTLS;
> break;
>
> case VIR_DOMAIN_CHR_TYPE_UNIX:
> @@ -10038,6 +10040,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> char *master = NULL;
> char *slave = NULL;
> char *append = NULL;
> + char *haveTLS = NULL;
> int remaining = 0;
>
> while (cur != NULL) {
> @@ -10045,6 +10048,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> if (xmlStrEqual(cur->name, BAD_CAST "source")) {
> if (!mode)
> mode = virXMLPropString(cur, "mode");
> + if (!haveTLS)
> + haveTLS = virXMLPropString(cur, "tls");
>
> switch ((virDomainChrType) def->type) {
> case VIR_DOMAIN_CHR_TYPE_FILE:
> @@ -10221,6 +10226,15 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> def->data.tcp.listen = true;
> }
>
> + if (haveTLS &&
> + (def->data.tcp.haveTLS =
> + virTristateBoolTypeFromString(haveTLS)) <= 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("unknown chardev 'tls' setting
'%s'"),
> + haveTLS);
> + goto error;
> + }
> +
> if (!protocol)
> def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW;
> else if ((def->data.tcp.protocol =
> @@ -10305,6 +10319,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> VIR_FREE(append);
> VIR_FREE(logappend);
> VIR_FREE(logfile);
> + VIR_FREE(haveTLS);
>
> return remaining;
>
> @@ -21453,7 +21468,12 @@ virDomainChrSourceDefFormat(virBufferPtr buf,
> virBufferAsprintf(buf, "<source mode='%s' ",
> def->data.tcp.listen ? "bind" :
"connect");
> virBufferEscapeString(buf, "host='%s' ",
def->data.tcp.host);
> - virBufferEscapeString(buf, "service='%s'/>\n",
def->data.tcp.service);
> + virBufferEscapeString(buf, "service='%s'",
def->data.tcp.service);
> + if (def->data.tcp.haveTLS != VIR_TRISTATE_BOOL_ABSENT)
> + virBufferAsprintf(buf, " tls='%s'",
> + virTristateBoolTypeToString(def->data.tcp.haveTLS));
> + virBufferAddLit(buf, "/>\n");
> +
> virBufferAsprintf(buf, "<protocol type='%s'/>\n",
> virDomainChrTcpProtocolTypeToString(
> def->data.tcp.protocol));
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 3e85ae4..c34e046 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1094,6 +1094,7 @@ struct _virDomainChrSourceDef {
> bool listen;
> int protocol;
> bool tlscreds;
> + int haveTLS; /* enum virTristateBool */
> } tcp;
> struct {
> char *bindHost;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 21fd85c..aaf7018 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4939,7 +4939,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> if (dev->data.tcp.listen)
> virBufferAdd(&buf, nowait ? ",server,nowait" :
",server", -1);
>
> - if (cfg->chardevTLS) {
> + if (cfg->chardevTLS &&
> + dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) {
> char *objalias = NULL;
>
> if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 14af4e1..f4d7c17 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1729,7 +1729,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
> if (qemuDomainChrPreInsert(vmdef, chr) < 0)
> goto cleanup;
>
> - if (cfg->chardevTLS) {
> + if (cfg->chardevTLS &&
> + dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) {
> if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
> dev->data.tcp.listen,
> cfg->chardevTLSx509verify,
> diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
> new file mode 100644
> index 0000000..cac0d85
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
> @@ -0,0 +1,30 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu \
> +-name QEMUGuest1 \
> +-S \
> +-M pc \
> +-m 214 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-nographic \
> +-nodefconfig \
> +-nodefaults \
> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
> +server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=readline \
> +-no-acpi \
> +-boot c \
> +-usb \
> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
> +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> +-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
> +localport=1111 \
> +-device isa-serial,chardev=charserial0,id=serial0 \
> +-chardev socket,id=charserial1,host=127.0.0.1,port=5555 \
> +-device isa-serial,chardev=charserial1,id=serial1 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
> new file mode 100644
> index 0000000..debc69b
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
> @@ -0,0 +1,50 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu</emulator>
> + <disk type='block' device='disk'>
> + <source dev='/dev/HostVG/QEMUGuest1'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
> + </disk>
> + <controller type='usb' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
> + </controller>
> + <controller type='ide' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
> + </controller>
> + <controller type='pci' index='0'
model='pci-root'/>
> + <serial type='udp'>
> + <source mode='bind' host='127.0.0.1'
service='1111'/>
> + <source mode='connect' host='127.0.0.1'
service='2222'/>
> + <target port='0'/>
> + </serial>
> + <serial type='tcp'>
> + <source mode='connect' host='127.0.0.1'
service='5555' tls='no'/>
> + <protocol type='raw'/>
> + <target port='0'/>
> + </serial>
> + <console type='udp'>
> + <source mode='bind' host='127.0.0.1'
service='1111'/>
> + <source mode='connect' host='127.0.0.1'
service='2222'/>
> + <target type='serial' port='0'/>
> + </console>
> + <input type='mouse' bus='ps2'/>
> + <input type='keyboard' bus='ps2'/>
> + <memballoon model='virtio'>
> + <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
> + </memballoon>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
> index 1618b02..1d7896d 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml
> @@ -27,7 +27,7 @@
> <target port='0'/>
> </serial>
> <serial type='tcp'>
> - <source mode='connect' host='127.0.0.1'
service='5555'/>
> + <source mode='connect' host='127.0.0.1'
service='5555' tls='yes'/>
This is not required and should work without the attribute.
True - I can remove the " tls='yes'", the test though is still valid to
ensure we build the command line properly
John
Pavel
> <protocol type='raw'/>
> <target port='0'/>
> </serial>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index e8438b4..c65c769 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1162,6 +1162,9 @@ mymain(void)
> DO_TEST("serial-tcp-tlsx509-chardev",
> QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
> QEMU_CAPS_OBJECT_TLS_CREDS_X509);
> + DO_TEST("serial-tcp-tlsx509-chardev-notls",
> + QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
> + QEMU_CAPS_OBJECT_TLS_CREDS_X509);
> driver.config->chardevTLS = 0;
> VIR_FREE(driver.config->chardevTLSx509certdir);
> DO_TEST("serial-many-chardev",
> diff --git
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
> new file mode 120000
> index 0000000..26484c9
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
> @@ -0,0 +1 @@
> +../qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
> \ No newline at end of file
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
> index 832e2a2..23c244b 100644
> --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml
> @@ -32,7 +32,7 @@
> <target port='0'/>
> </serial>
> <serial type='tcp'>
> - <source mode='connect' host='127.0.0.1'
service='5555'/>
> + <source mode='connect' host='127.0.0.1'
service='5555' tls='yes'/>
> <protocol type='raw'/>
> <target port='0'/>
> </serial>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 62bff8c..3ab4c50 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -534,6 +534,7 @@ mymain(void)
> DO_TEST("serial-udp", NONE);
> DO_TEST("serial-tcp-telnet", NONE);
> DO_TEST("serial-tcp-tlsx509-chardev", NONE);
> + DO_TEST("serial-tcp-tlsx509-chardev-notls", NONE);
> DO_TEST("serial-many", NONE);
> DO_TEST("serial-spiceport", NONE);
> DO_TEST("serial-spiceport-nospice", NONE);
> --
> 2.7.4
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
9:37 a.m.
New subject: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev
On Mon, Oct 17, 2016 at 09:54:46AM -0400, John Ferlan wrote:
On 10/17/2016 04:09 AM, Pavel Hrdina wrote:
> On Fri, Oct 14, 2016 at 04:23:04PM -0400, John Ferlan wrote:
>> Add an optional "tls='yes|no'" attribute for a TCP chardev for
the
>> express purpose to disable setting up TLS for the specific chardev in
>> the event the qemu.conf settings have enabled hypervisor wide TLS for
>> serial TCP chardevs.
>>
>> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
>> ---
>> docs/formatdomain.html.in | 21 +++++++++
>> docs/schemas/domaincommon.rng | 5 +++
>> src/conf/domain_conf.c | 22 +++++++++-
>> src/conf/domain_conf.h | 1 +
>> src/qemu/qemu_command.c | 3 +-
>> src/qemu/qemu_hotplug.c | 3 +-
>> ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +++++++++++++
>> ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 ++++++++++++++++++++++
>> .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 2 +-
>> tests/qemuxml2argvtest.c | 3 ++
>> ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml | 1 +
>> .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 2 +-
>> tests/qemuxml2xmltest.c | 1 +
>> 13 files changed, 139 insertions(+), 5 deletions(-)
>> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
>> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
>> create mode 120000
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
>>
>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>> index 9051178..6145d65 100644
>> --- a/docs/formatdomain.html.in
>> +++ b/docs/formatdomain.html.in
>> @@ -6204,6 +6204,27 @@ qemu-kvm -net nic,model=? /dev/null
>> </devices>
>> ...</pre>
>>
>> + <p>
>> + <span class="since">Since 2.4.0,</span> the
optional attribute
>> + <code>tls</code> can be used to control whether a serial
chardev
>> + TCP communication channel would utilize a hypervisor configured
>> + TLS X.509 certificate environment in order to encrypt the data
>> + channel. For the QEMU hypervisor usage of TLS is controlled by the
>> + <code>chardev_tls</code> setting in file
/etc/libvirt/qemu.conf. If
>> + enabled, then by setting this attribute to "no" will disable
usage
>> + of the TLS environment for this particular serial TCP data channel.
>> + </p>
>
> Previous discussion:
>
> http://www.redhat.com/archives/libvir-list/2016-October/msg00734.html
>
> So now there is no functional issue if we agree that this design is what we
> want. I personally thing that there could be also use-case where you want to
> configure certificates in qemu.conf and use 'tls' attribute to explicitly
> enable TLS encryption only for some VMs and only for some chardev and this is
> not currently possible whit this design. Now user can enable the TLS encryption
> for every chardev for every domain and explicitly disable for some chardevs.
>
> This would require to add all the extra code from that discussion to handle
> migration properly and that's why I've started the discussion.
>
w/r/t: design - re-read what I pulled from Dan's v5 review:
http://www.redhat.com/archives/libvir-list/2016-October/msg00698.html
While forcing to set "tls='yes'" is certainly a mechanism that could
be
used and adding extra code is possible, is that something that we want
to require of everyone that's using TLS chardev's? If you go through the
trouble of configuring for your host, then how would you feel about
having to update all your domains (for those that have more than 1 or 2)
to set the property in order to be able to use it? Again, I really don't
I have a feeling that we are having trouble to understand each other. I'm not
saying that you will have to set "tls='yes'" for every domain and every
chardev.
Forcing to set "tls='no'" for every domain and every chardevs if you
want to use
TLS only for one domain is not a good mechanism.
What I would like to achieve is this:
1. Currently there is already existing "chardev_tls" config option that allows
you to enable/disable TLS for all domain and all chardevs
2. This patch improves the current functionality by adding an option to
explicitly disable TLS for some chardves by using "tls='no'" if
"chardev_tls"
is set to "1".
3. My additional proposal is to add another improvement to current functionality
by allowing to explicitly enable TLS for some chardevs by using
"tls='yes'" if
"chardev_tls" is set to "0". And I can see the use-case, you have a
shared host
between several people and only one of them would like to use TLS encryption for
his domain and not affect other users so he will configure certificates for TLS
encryption and use it explicitly only for his domain.
think it's a good idea to be mucking around with changing XML of
running
domains, adding extra checks in the migration code, altering the
format/parse code to check flags, and/or anything else that may come up.
The less we do that the better - introduces less risk for making or
missing assumptions.
This is not a question whether it's a good idea or not. We simply have to do
that in order to generate backward compatible XML. Yes, it adds extra code and
extra checks but it's a price we have to pay to maintain backward compatibility.
For comparison spice uses an attribute mechanism
"tlsPort={-1|0|port}"
in order to use TLS, but it also checks cfg->spiceTLS. Alternatively,
vnc will just take the cfg->vncTLS to decide whether to use or not
(e.g., there's no way to avoid).
Yes, SPICE uses tlsPort and spiceTLS config option, but the spice is different
because the tlsPort is separate and there is still non encrypted port. The VNC
is not a good example because it has only the vncTLS config option and there is
not attribute in XML to configure it (the current state for chardev before this
patch changes it).
So one of the existing mechanisms forces you to define something to
use
TLS configured for the host and the other doesn't. Spice has the
additional configuration option to determine specific channels by name
that would use the secure port.
This patch can still be considered "outside" of the subsequent 4 in this
series...
Yes it could be a separate patch outside of this series but I would rather see
it as part of this series.
Pavel
10:24 a.m.
New subject: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev
On 10/17/2016 10:37 AM, Pavel Hrdina wrote:
On Mon, Oct 17, 2016 at 09:54:46AM -0400, John Ferlan wrote:
>
>
> On 10/17/2016 04:09 AM, Pavel Hrdina wrote:
>> On Fri, Oct 14, 2016 at 04:23:04PM -0400, John Ferlan wrote:
>>> Add an optional "tls='yes|no'" attribute for a TCP chardev
for the
>>> express purpose to disable setting up TLS for the specific chardev in
>>> the event the qemu.conf settings have enabled hypervisor wide TLS for
>>> serial TCP chardevs.
>>>
>>> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
>>> ---
>>> docs/formatdomain.html.in | 21 +++++++++
>>> docs/schemas/domaincommon.rng | 5 +++
>>> src/conf/domain_conf.c | 22 +++++++++-
>>> src/conf/domain_conf.h | 1 +
>>> src/qemu/qemu_command.c | 3 +-
>>> src/qemu/qemu_hotplug.c | 3 +-
>>> ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +++++++++++++
>>> ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50
++++++++++++++++++++++
>>> .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 2 +-
>>> tests/qemuxml2argvtest.c | 3 ++
>>> ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml | 1 +
>>> .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 2 +-
>>> tests/qemuxml2xmltest.c | 1 +
>>> 13 files changed, 139 insertions(+), 5 deletions(-)
>>> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
>>> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
>>> create mode 120000
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
>>>
>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>> index 9051178..6145d65 100644
>>> --- a/docs/formatdomain.html.in
>>> +++ b/docs/formatdomain.html.in
>>> @@ -6204,6 +6204,27 @@ qemu-kvm -net nic,model=? /dev/null
>>> </devices>
>>> ...</pre>
>>>
>>> + <p>
>>> + <span class="since">Since 2.4.0,</span> the
optional attribute
>>> + <code>tls</code> can be used to control whether a serial
chardev
>>> + TCP communication channel would utilize a hypervisor configured
>>> + TLS X.509 certificate environment in order to encrypt the data
>>> + channel. For the QEMU hypervisor usage of TLS is controlled by the
>>> + <code>chardev_tls</code> setting in file
/etc/libvirt/qemu.conf. If
>>> + enabled, then by setting this attribute to "no" will disable
usage
>>> + of the TLS environment for this particular serial TCP data channel.
>>> + </p>
>>
>> Previous discussion:
>>
>> http://www.redhat.com/archives/libvir-list/2016-October/msg00734.html
>>
>> So now there is no functional issue if we agree that this design is what we
>> want. I personally thing that there could be also use-case where you want to
>> configure certificates in qemu.conf and use 'tls' attribute to
explicitly
>> enable TLS encryption only for some VMs and only for some chardev and this is
>> not currently possible whit this design. Now user can enable the TLS encryption
>> for every chardev for every domain and explicitly disable for some chardevs.
>>
>> This would require to add all the extra code from that discussion to handle
>> migration properly and that's why I've started the discussion.
>>
>
> w/r/t: design - re-read what I pulled from Dan's v5 review:
>
> http://www.redhat.com/archives/libvir-list/2016-October/msg00698.html
>
> While forcing to set "tls='yes'" is certainly a mechanism that
could be
> used and adding extra code is possible, is that something that we want
> to require of everyone that's using TLS chardev's? If you go through the
> trouble of configuring for your host, then how would you feel about
> having to update all your domains (for those that have more than 1 or 2)
> to set the property in order to be able to use it? Again, I really don't
I have a feeling that we are having trouble to understand each other. I'm not
saying that you will have to set "tls='yes'" for every domain and every
chardev.
Forcing to set "tls='no'" for every domain and every chardevs if you
want to use
TLS only for one domain is not a good mechanism.
Clearly ;-) Setting "tls='yes'" wasn't my implication either. The
way
this patch is written 'YES' or 'ABSENT' have the same meaning to take
the host default.
So "forcing" setting of "tls='no'"
What I would like to achieve is this:
1. Currently there is already existing "chardev_tls" config option that allows
you to enable/disable TLS for all domain and all chardevs
2. This patch improves the current functionality by adding an option to
explicitly disable TLS for some chardves by using "tls='no'" if
"chardev_tls"
is set to "1".
3. My additional proposal is to add another improvement to current functionality
by allowing to explicitly enable TLS for some chardevs by using
"tls='yes'" if
"chardev_tls" is set to "0". And I can see the use-case, you have a
shared host
between several people and only one of them would like to use TLS encryption for
his domain and not affect other users so he will configure certificates for TLS
encryption and use it explicitly only for his domain.
Feel free to write a competing patch... This patch as written I believe
more closely follows the request from Daniel from patch 5 review:
"As default behaviour I think it is desirable that we can turn TLS on
for every VM at once - I tend to view it as a host network integration
task, rather than a VM configuration task. Same rationale that we use
for TLS wth VNC/SPICE."
> think it's a good idea to be mucking around with changing XML
of running
> domains, adding extra checks in the migration code, altering the
> format/parse code to check flags, and/or anything else that may come up.
> The less we do that the better - introduces less risk for making or
> missing assumptions.
This is not a question whether it's a good idea or not. We simply have to do
that in order to generate backward compatible XML. Yes, it adds extra code and
extra checks but it's a price we have to pay to maintain backward compatibility.
But unnecessary when using "tls='no'"
> For comparison spice uses an attribute mechanism
"tlsPort={-1|0|port}"
> in order to use TLS, but it also checks cfg->spiceTLS. Alternatively,
> vnc will just take the cfg->vncTLS to decide whether to use or not
> (e.g., there's no way to avoid).
Yes, SPICE uses tlsPort and spiceTLS config option, but the spice is different
because the tlsPort is separate and there is still non encrypted port. The VNC
is not a good example because it has only the vncTLS config option and there is
not attribute in XML to configure it (the current state for chardev before this
patch changes it).
> So one of the existing mechanisms forces you to define something to use
> TLS configured for the host and the other doesn't. Spice has the
> additional configuration option to determine specific channels by name
> that would use the secure port.
>
> This patch can still be considered "outside" of the subsequent 4 in this
> series...
Yes it could be a separate patch outside of this series but I would rather see
it as part of this series.
Other than the pointed out spots that need a haveTLS in patch 4 and 5,
the other patches are different.
John
1:27 a.m.
New subject: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev
On Mon, Oct 17, 2016 at 11:24:58AM -0400, John Ferlan wrote:
On 10/17/2016 10:37 AM, Pavel Hrdina wrote:
> On Mon, Oct 17, 2016 at 09:54:46AM -0400, John Ferlan wrote:
>>
>>
>> On 10/17/2016 04:09 AM, Pavel Hrdina wrote:
>>> On Fri, Oct 14, 2016 at 04:23:04PM -0400, John Ferlan wrote:
>>>> Add an optional "tls='yes|no'" attribute for a TCP
chardev for the
>>>> express purpose to disable setting up TLS for the specific chardev in
>>>> the event the qemu.conf settings have enabled hypervisor wide TLS for
>>>> serial TCP chardevs.
>>>>
>>>> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
>>>> ---
>>>> docs/formatdomain.html.in | 21 +++++++++
>>>> docs/schemas/domaincommon.rng | 5 +++
>>>> src/conf/domain_conf.c | 22 +++++++++-
>>>> src/conf/domain_conf.h | 1 +
>>>> src/qemu/qemu_command.c | 3 +-
>>>> src/qemu/qemu_hotplug.c | 3 +-
>>>> ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 +++++++++++++
>>>> ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50
++++++++++++++++++++++
>>>> .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 2 +-
>>>> tests/qemuxml2argvtest.c | 3 ++
>>>> ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml | 1 +
>>>> .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 2 +-
>>>> tests/qemuxml2xmltest.c | 1 +
>>>> 13 files changed, 139 insertions(+), 5 deletions(-)
>>>> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.args
>>>> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-notls.xml
>>>> create mode 120000
tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-notls.xml
>>>>
>>>> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
>>>> index 9051178..6145d65 100644
>>>> --- a/docs/formatdomain.html.in
>>>> +++ b/docs/formatdomain.html.in
>>>> @@ -6204,6 +6204,27 @@ qemu-kvm -net nic,model=? /dev/null
>>>> </devices>
>>>> ...</pre>
>>>>
>>>> + <p>
>>>> + <span class="since">Since 2.4.0,</span> the
optional attribute
>>>> + <code>tls</code> can be used to control whether a
serial chardev
>>>> + TCP communication channel would utilize a hypervisor configured
>>>> + TLS X.509 certificate environment in order to encrypt the data
>>>> + channel. For the QEMU hypervisor usage of TLS is controlled by
the
>>>> + <code>chardev_tls</code> setting in file
/etc/libvirt/qemu.conf. If
>>>> + enabled, then by setting this attribute to "no" will
disable usage
>>>> + of the TLS environment for this particular serial TCP data
channel.
>>>> + </p>
>>>
>>> Previous discussion:
>>>
>>> http://www.redhat.com/archives/libvir-list/2016-October/msg00734.html
>>>
>>> So now there is no functional issue if we agree that this design is what we
>>> want. I personally thing that there could be also use-case where you want
to
>>> configure certificates in qemu.conf and use 'tls' attribute to
explicitly
>>> enable TLS encryption only for some VMs and only for some chardev and this
is
>>> not currently possible whit this design. Now user can enable the TLS
encryption
>>> for every chardev for every domain and explicitly disable for some
chardevs.
>>>
>>> This would require to add all the extra code from that discussion to handle
>>> migration properly and that's why I've started the discussion.
>>>
>>
>> w/r/t: design - re-read what I pulled from Dan's v5 review:
>>
>> http://www.redhat.com/archives/libvir-list/2016-October/msg00698.html
>>
>> While forcing to set "tls='yes'" is certainly a mechanism that
could be
>> used and adding extra code is possible, is that something that we want
>> to require of everyone that's using TLS chardev's? If you go through
the
>> trouble of configuring for your host, then how would you feel about
>> having to update all your domains (for those that have more than 1 or 2)
>> to set the property in order to be able to use it? Again, I really don't
>
> I have a feeling that we are having trouble to understand each other. I'm not
> saying that you will have to set "tls='yes'" for every domain and
every chardev.
> Forcing to set "tls='no'" for every domain and every chardevs if
you want to use
> TLS only for one domain is not a good mechanism.
Clearly ;-) Setting "tls='yes'" wasn't my implication either. The
way
this patch is written 'YES' or 'ABSENT' have the same meaning to take
the host default.
So "forcing" setting of "tls='no'"
>
> What I would like to achieve is this:
>
> 1. Currently there is already existing "chardev_tls" config option that
allows
> you to enable/disable TLS for all domain and all chardevs
>
> 2. This patch improves the current functionality by adding an option to
> explicitly disable TLS for some chardves by using "tls='no'" if
"chardev_tls"
> is set to "1".
>
> 3. My additional proposal is to add another improvement to current functionality
> by allowing to explicitly enable TLS for some chardevs by using
"tls='yes'" if
> "chardev_tls" is set to "0". And I can see the use-case, you
have a shared host
> between several people and only one of them would like to use TLS encryption for
> his domain and not affect other users so he will configure certificates for TLS
> encryption and use it explicitly only for his domain.
>
Feel free to write a competing patch... This patch as written I believe
more closely follows the request from Daniel from patch 5 review:
"As default behaviour I think it is desirable that we can turn TLS on
for every VM at once - I tend to view it as a host network integration
task, rather than a VM configuration task. Same rationale that we use
for TLS wth VNC/SPICE."
Don't forget this part of the same review:
"There's no reason we can't have a tri-state TLS flag against the chardev
in the XML too, to override the default behaviour of cfg->chardevTLS"
That also means to override chardev_tls = "0" by "tls='yes'".
Pavel
>> think it's a good idea to be mucking around with
changing XML of running
>> domains, adding extra checks in the migration code, altering the
>> format/parse code to check flags, and/or anything else that may come up.
>> The less we do that the better - introduces less risk for making or
>> missing assumptions.
>
> This is not a question whether it's a good idea or not. We simply have to do
> that in order to generate backward compatible XML. Yes, it adds extra code and
> extra checks but it's a price we have to pay to maintain backward
compatibility.
>
But unnecessary when using "tls='no'"
>> For comparison spice uses an attribute mechanism
"tlsPort={-1|0|port}"
>> in order to use TLS, but it also checks cfg->spiceTLS. Alternatively,
>> vnc will just take the cfg->vncTLS to decide whether to use or not
>> (e.g., there's no way to avoid).
>
> Yes, SPICE uses tlsPort and spiceTLS config option, but the spice is different
> because the tlsPort is separate and there is still non encrypted port. The VNC
> is not a good example because it has only the vncTLS config option and there is
> not attribute in XML to configure it (the current state for chardev before this
> patch changes it).
>
>> So one of the existing mechanisms forces you to define something to use
>> TLS configured for the host and the other doesn't. Spice has the
>> additional configuration option to determine specific channels by name
>> that would use the secure port.
>>
>> This patch can still be considered "outside" of the subsequent 4 in
this
>> series...
>
> Yes it could be a separate patch outside of this series but I would rather see
> it as part of this series.
>
Other than the pointed out spots that need a haveTLS in patch 4 and 5,
the other patches are different.
John
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
5:59 a.m.
New subject: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev
On 10/18/2016 02:27 AM, Pavel Hrdina wrote:
[...]
>
> "As default behaviour I think it is desirable that we can turn TLS on
> for every VM at once - I tend to view it as a host network integration
> task, rather than a VM configuration task. Same rationale that we use
> for TLS wth VNC/SPICE."
Don't forget this part of the same review:
"There's no reason we can't have a tri-state TLS flag against the chardev
in the XML too, to override the default behaviour of cfg->chardevTLS"
That also means to override chardev_tls = "0" by "tls='yes'".
If the default cfg behaviour is "1", then that tells us "someone" has
set up the TLS environment and thus the domain/chardev override would be
"no".
If the default cfg behaviour is "0", then that means we cannot guarantee
the environment necessary has been set up and we cannot allow the
domain/chardev setting to enable TLS.
John
6:16 a.m.
New subject: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev
On Tue, Oct 18, 2016 at 06:59:57AM -0400, John Ferlan wrote:
On 10/18/2016 02:27 AM, Pavel Hrdina wrote:
[...]
>>
>> "As default behaviour I think it is desirable that we can turn TLS on
>> for every VM at once - I tend to view it as a host network integration
>> task, rather than a VM configuration task. Same rationale that we use
>> for TLS wth VNC/SPICE."
>
> Don't forget this part of the same review:
>
> "There's no reason we can't have a tri-state TLS flag against the
chardev
> in the XML too, to override the default behaviour of cfg->chardevTLS"
>
> That also means to override chardev_tls = "0" by
"tls='yes'".
If the default cfg behaviour is "1", then that tells us "someone"
has
set up the TLS environment and thus the domain/chardev override would be
"no".
If the default cfg behaviour is "0", then that means we cannot guarantee
the environment necessary has been set up and we cannot allow the
domain/chardev setting to enable TLS.
Likewise someone can modify only qemu.conf without preparing certificates and
the domain would fail to start. Libvirt cannot guarantee that the environment
is configured in any case. So if someone doesn't setup the environment and
uses "tls='yes'" libvirt will print sane error message from qemu that
the
certificate files doesn't exist.
Pavel
6:21 a.m.
New subject: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev
On Tue, Oct 18, 2016 at 06:59:57AM -0400, John Ferlan wrote:
On 10/18/2016 02:27 AM, Pavel Hrdina wrote:
[...]
>>
>> "As default behaviour I think it is desirable that we can turn TLS on
>> for every VM at once - I tend to view it as a host network integration
>> task, rather than a VM configuration task. Same rationale that we use
>> for TLS wth VNC/SPICE."
>
> Don't forget this part of the same review:
>
> "There's no reason we can't have a tri-state TLS flag against the
chardev
> in the XML too, to override the default behaviour of cfg->chardevTLS"
>
> That also means to override chardev_tls = "0" by
"tls='yes'".
If the default cfg behaviour is "1", then that tells us "someone"
has
set up the TLS environment and thus the domain/chardev override would be
"no".
If the default cfg behaviour is "0", then that means we cannot guarantee
the environment necessary has been set up and we cannot allow the
domain/chardev setting to enable TLS.
We have two separate tasks at the host level
- Setup of TLS certificates (ie put the PEM files in the right places)
- Global default for use of TLS by chardevs
We only have a config option in qemu.conf for the latter. ie if
chardev_tls=1, this is implicitly saying that TLS certs are deployed
in right place. IIUC, you're saying that if chardev_tls=0, then we
should interpret that to meant TLS certs are *not* deployed.
Pavel is saying that if chardev_tls=0 in qemu.conf, and tls=1 in the
XML, then we should assume that TLS certs *are* deployed on the host
in the right place. I think this is not unreasonable - we can easily
check to see if the certs exist on disk in this case.
IOW, I agree that we should have a tri-state at the XML level
- no tls attribute in XML - honour chardev_tls from qemu.conf
- tls=1 in XML, then turn on TLS
- tls=0 in XML, then don't use TLS
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|
7:48 a.m.
New subject: [libvirt] [PATCH v9 1/5] domain: Add optional 'tls' attribute for TCP chardev
On 10/18/2016 07:21 AM, Daniel P. Berrange wrote:
On Tue, Oct 18, 2016 at 06:59:57AM -0400, John Ferlan wrote:
>
>
> On 10/18/2016 02:27 AM, Pavel Hrdina wrote:
> [...]
>
>>>
>>> "As default behaviour I think it is desirable that we can turn TLS on
>>> for every VM at once - I tend to view it as a host network integration
>>> task, rather than a VM configuration task. Same rationale that we use
>>> for TLS wth VNC/SPICE."
>>
>> Don't forget this part of the same review:
>>
>> "There's no reason we can't have a tri-state TLS flag against the
chardev
>> in the XML too, to override the default behaviour of cfg->chardevTLS"
>>
>> That also means to override chardev_tls = "0" by
"tls='yes'".
>
> If the default cfg behaviour is "1", then that tells us "someone"
has
> set up the TLS environment and thus the domain/chardev override would be
> "no".
>
> If the default cfg behaviour is "0", then that means we cannot guarantee
> the environment necessary has been set up and we cannot allow the
> domain/chardev setting to enable TLS.
We have two separate tasks at the host level
- Setup of TLS certificates (ie put the PEM files in the right places)
- Global default for use of TLS by chardevs
We only have a config option in qemu.conf for the latter. ie if
chardev_tls=1, this is implicitly saying that TLS certs are deployed
in right place. IIUC, you're saying that if chardev_tls=0, then we
should interpret that to meant TLS certs are *not* deployed.
If someone has gone through the trouble of setting up their environment
because they found in qemu.conf support was added, why would they set
chardev_tls=0 afterwards? IOW: Are we assuming something or are we over
engineering things?
Why would someone set up and define chardevTLSx509certdir and then have
chardevTLS=0? Maybe, they believe this will disable TLS for every
domain. After all, why would disabling a global setting allow some
localized setting override that.
Just because there's a "default" environment, why should we assume that
because they added "tls='yes'" into their domain that that is the
environment they want?
Maybe they do have a chardev specific environment, but also decided to
comment it out when they commented out chardev_tls=1? So we're assuming
that by finding that "tls='yes'" in the domain that the domain should
use that environment? How can we be so sure that's what's desired?
This is the problem with two triggers - which one do you pull and which
one has a bullet in the chamber? We're having a hard enough time
agreeing on the implementation. Now the customer has to figure out all
the permutations and possibilities.
Personally, if I disable a global variable/value, then I wouldn't expect
some other definition to override that. For example, if I set my SELinux
to be "permissive" or "disabled", then I'm probably not very happy
if
something ignores that and decides to be "enforcing".
Pavel is saying that if chardev_tls=0 in qemu.conf, and tls=1 in the
XML, then we should assume that TLS certs *are* deployed on the host
in the right place. I think this is not unreasonable - we can easily
check to see if the certs exist on disk in this case.
FWIW: Remember how qemu_conf.c sets things up...
During qemuStateInitialize, we call virQEMUDriverConfigNew which will
set a default directory path for "cfg->defaultTLSx509certdir" and then
will use that as a default value for vnc, spice, and chardev. There is
no check "if" that directory structure exists.
Once the defaults are set up, then if "chardev_tls_x509_cert_dir" is
defined, it will override the default (similar for others).
So chardevTLSx509certdir always has some default value, but is that the
certificate environment we want to use for chardev's? How do we know?
IOW, I agree that we should have a tri-state at the XML level
- no tls attribute in XML - honour chardev_tls from qemu.conf
- tls=1 in XML, then turn on TLS
- tls=0 in XML, then don't use TLS
So if we get this far, then a qemu_domain.c function, such as
bool
qemuDomainSupportTLSChardevTCP(virQEMUDriverConfigPtr cfg,
const virDomainChrSourceDef *dev)
{
if (cfg->chardevTLS && dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO)
return true;
if (!cfg->chardevTLS && dev->data.tcp.haveTLS ==
VIR_TRISTATE_BOOL_YES &&
virFileExists(cfg->chardevTLSx509certdir))
return true;
return false;
}
where I suppose we could also compare chardevTLSx509certdir and
defaultTLSx509certdir... Still how do we know that by merely having the
environment there and "tls='yes'" in the domain chardev config that
this
is what's desired. It might well be, but it may also be that the
clearing of the global chardev_tls was meant to disable regardless of
what the domain setting was.
John
3:23 p.m.
New subject: [libvirt] [PATCH v9 2/5] conf: Introduce {default|chardev}_tls_x509_secret_uuid
Add a new qemu.conf variables to store the UUID for the secret that could
be used to present credentials to access the TLS chardev. Since this will
be a server level and it's possible to use some sort of default, introduce
both the default and chardev logic at the same time making the setting of
the chardev check for it's own value, then if not present checking whether
the default value had been set.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/libvirtd_qemu.aug | 2 ++
src/qemu/qemu.conf | 24 ++++++++++++++++++++++++
src/qemu/qemu_conf.c | 14 ++++++++++++++
src/qemu/qemu_conf.h | 2 ++
src/qemu/test_libvirtd_qemu.aug.in | 2 ++
5 files changed, 44 insertions(+)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 988201e..73ebeda 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -29,6 +29,7 @@ module Libvirtd_qemu =
(* Config entry grouped by function - same order as example config *)
let default_tls_entry = str_entry "default_tls_x509_cert_dir"
| bool_entry "default_tls_x509_verify"
+ | str_entry "default_tls_x509_secret_uuid"
let vnc_entry = str_entry "vnc_listen"
| bool_entry "vnc_auto_unix_socket"
@@ -51,6 +52,7 @@ module Libvirtd_qemu =
let chardev_entry = bool_entry "chardev_tls"
| str_entry "chardev_tls_x509_cert_dir"
| bool_entry "chardev_tls_x509_verify"
+ | str_entry "chardev_tls_x509_secret_uuid"
let nogfx_entry = bool_entry "nographics_allow_host_audio"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index e4c2aae..493c171 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -28,6 +28,20 @@
#
#default_tls_x509_verify = 1
+#
+# Libvirt assumes the server-key.pem file is unencrypted by default.
+# To use an encrypted server-key.pem file, the password to decrypt the
+# the PEM file is required. This can be provided by creating a secret
+# object in libvirt and then to uncomment this setting to set the UUID
+# of the secret.
+#
+# NB This default all-zeros UUID will not work. Replace it with the
+# output from the UUID for the TLS secret from a 'virsh secret-list'
+# command and then uncomment the entry
+#
+#default_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000"
+
+
# VNC is configured to listen on 127.0.0.1 by default.
# To make it listen on all public interfaces, uncomment
# this next option.
@@ -214,6 +228,16 @@
#chardev_tls_x509_verify = 1
+# Uncomment and use the following option to override the default secret
+# uuid provided in the default_tls_x509_secret_uuid parameter.
+#
+# NB This default all-zeros UUID will not work. Replace it with the
+# output from the UUID for the TLS secret from a 'virsh secret-list'
+# command and then uncomment the entry
+#
+#chardev_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000"
+
+
# By default, if no graphical front end is configured, libvirt will disable
# QEMU audio output since directly talking to alsa/pulseaudio may not work
# with various security settings. If you know what you're doing, enable
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 635fa27..109668b 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -365,6 +365,7 @@ static void virQEMUDriverConfigDispose(void *obj)
VIR_FREE(cfg->nvramDir);
VIR_FREE(cfg->defaultTLSx509certdir);
+ VIR_FREE(cfg->defaultTLSx509secretUUID);
VIR_FREE(cfg->vncTLSx509certdir);
VIR_FREE(cfg->vncListen);
@@ -377,6 +378,7 @@ static void virQEMUDriverConfigDispose(void *obj)
VIR_FREE(cfg->spiceSASLdir);
VIR_FREE(cfg->chardevTLSx509certdir);
+ VIR_FREE(cfg->chardevTLSx509secretUUID);
while (cfg->nhugetlbfs) {
cfg->nhugetlbfs--;
@@ -446,6 +448,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (virConfGetValueBool(conf, "default_tls_x509_verify",
&cfg->defaultTLSx509verify) < 0)
goto cleanup;
+ if (virConfGetValueString(conf, "default_tls_x509_secret_uuid",
+ &cfg->defaultTLSx509secretUUID) < 0)
+ goto cleanup;
+
if (virConfGetValueBool(conf, "vnc_auto_unix_socket",
&cfg->vncAutoUnixSocket) < 0)
goto cleanup;
if (virConfGetValueBool(conf, "vnc_tls", &cfg->vncTLS) < 0)
@@ -513,6 +519,14 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (rv == 0)
cfg->chardevTLSx509verify = cfg->defaultTLSx509verify;
+ if (virConfGetValueString(conf, "chardev_tls_x509_secret_uuid",
+ &cfg->chardevTLSx509secretUUID) < 0)
+ goto cleanup;
+ if (!cfg->chardevTLSx509secretUUID && cfg->defaultTLSx509secretUUID) {
+ if (VIR_STRDUP(cfg->chardevTLSx509secretUUID,
+ cfg->defaultTLSx509secretUUID) < 0)
+ goto cleanup;
+ }
if (virConfGetValueUInt(conf, "remote_websocket_port_min",
&cfg->webSocketPortMin) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index d32689a..12b2661 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -111,6 +111,7 @@ struct _virQEMUDriverConfig {
char *defaultTLSx509certdir;
bool defaultTLSx509verify;
+ char *defaultTLSx509secretUUID;
bool vncAutoUnixSocket;
bool vncTLS;
@@ -132,6 +133,7 @@ struct _virQEMUDriverConfig {
bool chardevTLS;
char *chardevTLSx509certdir;
bool chardevTLSx509verify;
+ char *chardevTLSx509secretUUID;
unsigned int remotePortMin;
unsigned int remotePortMax;
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index cd162ae..805fa0e 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -4,6 +4,7 @@ module Test_libvirtd_qemu =
test Libvirtd_qemu.lns get conf =
{ "default_tls_x509_cert_dir" = "/etc/pki/qemu" }
{ "default_tls_x509_verify" = "1" }
+{ "default_tls_x509_secret_uuid" =
"00000000-0000-0000-0000-000000000000" }
{ "vnc_listen" = "0.0.0.0" }
{ "vnc_auto_unix_socket" = "1" }
{ "vnc_tls" = "1" }
@@ -23,6 +24,7 @@ module Test_libvirtd_qemu =
{ "chardev_tls" = "1" }
{ "chardev_tls_x509_cert_dir" = "/etc/pki/libvirt-chardev" }
{ "chardev_tls_x509_verify" = "1" }
+{ "chardev_tls_x509_secret_uuid" =
"00000000-0000-0000-0000-000000000000" }
{ "nographics_allow_host_audio" = "1" }
{ "remote_display_port_min" = "5900" }
{ "remote_display_port_max" = "65535" }
--
2.7.4
5:52 a.m.
New subject: [libvirt] [PATCH v9 2/5] conf: Introduce {default|chardev}_tls_x509_secret_uuid
On Fri, Oct 14, 2016 at 04:23:05PM -0400, John Ferlan wrote:
Add a new qemu.conf variables to store the UUID for the secret that
could
be used to present credentials to access the TLS chardev. Since this will
be a server level and it's possible to use some sort of default, introduce
both the default and chardev logic at the same time making the setting of
the chardev check for it's own value, then if not present checking whether
the default value had been set.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/libvirtd_qemu.aug | 2 ++
src/qemu/qemu.conf | 24 ++++++++++++++++++++++++
src/qemu/qemu_conf.c | 14 ++++++++++++++
src/qemu/qemu_conf.h | 2 ++
src/qemu/test_libvirtd_qemu.aug.in | 2 ++
5 files changed, 44 insertions(+)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 988201e..73ebeda 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -29,6 +29,7 @@ module Libvirtd_qemu =
(* Config entry grouped by function - same order as example config *)
let default_tls_entry = str_entry "default_tls_x509_cert_dir"
| bool_entry "default_tls_x509_verify"
+ | str_entry "default_tls_x509_secret_uuid"
let vnc_entry = str_entry "vnc_listen"
| bool_entry "vnc_auto_unix_socket"
@@ -51,6 +52,7 @@ module Libvirtd_qemu =
let chardev_entry = bool_entry "chardev_tls"
| str_entry "chardev_tls_x509_cert_dir"
| bool_entry "chardev_tls_x509_verify"
+ | str_entry "chardev_tls_x509_secret_uuid"
let nogfx_entry = bool_entry "nographics_allow_host_audio"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index e4c2aae..493c171 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -28,6 +28,20 @@
#
#default_tls_x509_verify = 1
+#
+# Libvirt assumes the server-key.pem file is unencrypted by default.
+# To use an encrypted server-key.pem file, the password to decrypt the
You've forgot to remove the extra "the".
+# the PEM file is required. This can be provided by creating a
secret
+# object in libvirt and then to uncomment this setting to set the UUID
+# of the secret.
+#
+# NB This default all-zeros UUID will not work. Replace it with the
+# output from the UUID for the TLS secret from a 'virsh secret-list'
+# command and then uncomment the entry
+#
+#default_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000"
+
+
# VNC is configured to listen on 127.0.0.1 by default.
# To make it listen on all public interfaces, uncomment
# this next option.
@@ -214,6 +228,16 @@
#chardev_tls_x509_verify = 1
+# Uncomment and use the following option to override the default secret
+# uuid provided in the default_tls_x509_secret_uuid parameter.
s/uuid/UUID/
ACK
+#
+# NB This default all-zeros UUID will not work. Replace it with the
+# output from the UUID for the TLS secret from a 'virsh secret-list'
+# command and then uncomment the entry
+#
+#chardev_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000"
+
+
# By default, if no graphical front end is configured, libvirt will disable
# QEMU audio output since directly talking to alsa/pulseaudio may not work
# with various security settings. If you know what you're doing, enable
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 635fa27..109668b 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -365,6 +365,7 @@ static void virQEMUDriverConfigDispose(void *obj)
VIR_FREE(cfg->nvramDir);
VIR_FREE(cfg->defaultTLSx509certdir);
+ VIR_FREE(cfg->defaultTLSx509secretUUID);
VIR_FREE(cfg->vncTLSx509certdir);
VIR_FREE(cfg->vncListen);
@@ -377,6 +378,7 @@ static void virQEMUDriverConfigDispose(void *obj)
VIR_FREE(cfg->spiceSASLdir);
VIR_FREE(cfg->chardevTLSx509certdir);
+ VIR_FREE(cfg->chardevTLSx509secretUUID);
while (cfg->nhugetlbfs) {
cfg->nhugetlbfs--;
@@ -446,6 +448,10 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (virConfGetValueBool(conf, "default_tls_x509_verify",
&cfg->defaultTLSx509verify) < 0)
goto cleanup;
+ if (virConfGetValueString(conf, "default_tls_x509_secret_uuid",
+ &cfg->defaultTLSx509secretUUID) < 0)
+ goto cleanup;
+
if (virConfGetValueBool(conf, "vnc_auto_unix_socket",
&cfg->vncAutoUnixSocket) < 0)
goto cleanup;
if (virConfGetValueBool(conf, "vnc_tls", &cfg->vncTLS) < 0)
@@ -513,6 +519,14 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
goto cleanup;
if (rv == 0)
cfg->chardevTLSx509verify = cfg->defaultTLSx509verify;
+ if (virConfGetValueString(conf, "chardev_tls_x509_secret_uuid",
+ &cfg->chardevTLSx509secretUUID) < 0)
+ goto cleanup;
+ if (!cfg->chardevTLSx509secretUUID && cfg->defaultTLSx509secretUUID)
{
+ if (VIR_STRDUP(cfg->chardevTLSx509secretUUID,
+ cfg->defaultTLSx509secretUUID) < 0)
+ goto cleanup;
+ }
if (virConfGetValueUInt(conf, "remote_websocket_port_min",
&cfg->webSocketPortMin) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index d32689a..12b2661 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -111,6 +111,7 @@ struct _virQEMUDriverConfig {
char *defaultTLSx509certdir;
bool defaultTLSx509verify;
+ char *defaultTLSx509secretUUID;
bool vncAutoUnixSocket;
bool vncTLS;
@@ -132,6 +133,7 @@ struct _virQEMUDriverConfig {
bool chardevTLS;
char *chardevTLSx509certdir;
bool chardevTLSx509verify;
+ char *chardevTLSx509secretUUID;
unsigned int remotePortMin;
unsigned int remotePortMax;
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index cd162ae..805fa0e 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -4,6 +4,7 @@ module Test_libvirtd_qemu =
test Libvirtd_qemu.lns get conf =
{ "default_tls_x509_cert_dir" = "/etc/pki/qemu" }
{ "default_tls_x509_verify" = "1" }
+{ "default_tls_x509_secret_uuid" =
"00000000-0000-0000-0000-000000000000" }
{ "vnc_listen" = "0.0.0.0" }
{ "vnc_auto_unix_socket" = "1" }
{ "vnc_tls" = "1" }
@@ -23,6 +24,7 @@ module Test_libvirtd_qemu =
{ "chardev_tls" = "1" }
{ "chardev_tls_x509_cert_dir" = "/etc/pki/libvirt-chardev" }
{ "chardev_tls_x509_verify" = "1" }
+{ "chardev_tls_x509_secret_uuid" =
"00000000-0000-0000-0000-000000000000" }
{ "nographics_allow_host_audio" = "1" }
{ "remote_display_port_min" = "5900" }
{ "remote_display_port_max" = "65535" }
--
2.7.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
8:56 a.m.
New subject: [libvirt] [PATCH v9 2/5] conf: Introduce {default|chardev}_tls_x509_secret_uuid
On 10/17/2016 06:52 AM, Pavel Hrdina wrote:
On Fri, Oct 14, 2016 at 04:23:05PM -0400, John Ferlan wrote:
> Add a new qemu.conf variables to store the UUID for the secret that could
> be used to present credentials to access the TLS chardev. Since this will
> be a server level and it's possible to use some sort of default, introduce
> both the default and chardev logic at the same time making the setting of
> the chardev check for it's own value, then if not present checking whether
> the default value had been set.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/libvirtd_qemu.aug | 2 ++
> src/qemu/qemu.conf | 24 ++++++++++++++++++++++++
> src/qemu/qemu_conf.c | 14 ++++++++++++++
> src/qemu/qemu_conf.h | 2 ++
> src/qemu/test_libvirtd_qemu.aug.in | 2 ++
> 5 files changed, 44 insertions(+)
>
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index 988201e..73ebeda 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -29,6 +29,7 @@ module Libvirtd_qemu =
> (* Config entry grouped by function - same order as example config *)
> let default_tls_entry = str_entry "default_tls_x509_cert_dir"
> | bool_entry "default_tls_x509_verify"
> + | str_entry "default_tls_x509_secret_uuid"
>
> let vnc_entry = str_entry "vnc_listen"
> | bool_entry "vnc_auto_unix_socket"
> @@ -51,6 +52,7 @@ module Libvirtd_qemu =
> let chardev_entry = bool_entry "chardev_tls"
> | str_entry "chardev_tls_x509_cert_dir"
> | bool_entry "chardev_tls_x509_verify"
> + | str_entry "chardev_tls_x509_secret_uuid"
>
> let nogfx_entry = bool_entry "nographics_allow_host_audio"
>
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index e4c2aae..493c171 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -28,6 +28,20 @@
> #
> #default_tls_x509_verify = 1
>
> +#
> +# Libvirt assumes the server-key.pem file is unencrypted by default.
> +# To use an encrypted server-key.pem file, the password to decrypt the
You've forgot to remove the extra "the".
Weird - I konw I made the change... where'd it go...
> +# the PEM file is required. This can be provided by creating a
secret
> +# object in libvirt and then to uncomment this setting to set the UUID
> +# of the secret.
> +#
> +# NB This default all-zeros UUID will not work. Replace it with the
> +# output from the UUID for the TLS secret from a 'virsh secret-list'
> +# command and then uncomment the entry
> +#
> +#default_tls_x509_secret_uuid = "00000000-0000-0000-0000-000000000000"
> +
> +
> # VNC is configured to listen on 127.0.0.1 by default.
> # To make it listen on all public interfaces, uncomment
> # this next option.
> @@ -214,6 +228,16 @@
> #chardev_tls_x509_verify = 1
>
>
> +# Uncomment and use the following option to override the default secret
> +# uuid provided in the default_tls_x509_secret_uuid parameter.
s/uuid/UUID/
ACK
change - thanks
John
[...]
3:23 p.m.
New subject: [libvirt] [PATCH v9 3/5] qemu: Introduce qemuDomainChardevPrivatePtr
Modeled after the qemuDomainHostdevPrivatePtr (commit id '27726d8c'),
create a privateData pointer in the _virDomainChardevDef to allow storage
of private data for a hypervisor in order to at least temporarily store
secret data for usage during qemuBuildCommandLine.
NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
expecting to restore the secret data, there's no need to add code
code to handle this new structure there.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/domain_conf.c | 29 ++++++++++++++++++++--------
src/conf/domain_conf.h | 4 +++-
src/libxl/libxl_domain.c | 2 +-
src/lxc/lxc_native.c | 2 +-
src/qemu/qemu_domain.c | 44 +++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 14 ++++++++++++++
src/qemu/qemu_parse_command.c | 4 ++--
src/vz/vz_sdk.c | 2 +-
src/xenconfig/xen_sxpr.c | 2 +-
src/xenconfig/xen_xl.c | 2 +-
10 files changed, 89 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9062544..e4fa9ad 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2127,6 +2127,8 @@ void virDomainChrDefFree(virDomainChrDefPtr def)
VIR_FREE(def->seclabels);
}
+ virObjectUnref(def->privateData);
+
VIR_FREE(def);
}
@@ -10333,7 +10335,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
* default port.
*/
virDomainChrDefPtr
-virDomainChrDefNew(void)
+virDomainChrDefNew(virDomainXMLOptionPtr xmlopt)
{
virDomainChrDefPtr def = NULL;
@@ -10341,6 +10343,11 @@ virDomainChrDefNew(void)
return NULL;
def->target.port = -1;
+
+ if (xmlopt && xmlopt->privateData.chardevNew &&
+ !(def->privateData = xmlopt->privateData.chardevNew()))
+ VIR_FREE(def);
+
return def;
}
@@ -10388,7 +10395,8 @@ virDomainChrDefNew(void)
*
*/
static virDomainChrDefPtr
-virDomainChrDefParseXML(xmlXPathContextPtr ctxt,
+virDomainChrDefParseXML(virDomainXMLOptionPtr xmlopt,
+ xmlXPathContextPtr ctxt,
xmlNodePtr node,
virSecurityLabelDefPtr* vmSeclabels,
int nvmSeclabels,
@@ -10400,7 +10408,7 @@ virDomainChrDefParseXML(xmlXPathContextPtr ctxt,
virDomainChrDefPtr def;
bool seenTarget = false;
- if (!(def = virDomainChrDefNew()))
+ if (!(def = virDomainChrDefNew(xmlopt)))
return NULL;
type = virXMLPropString(node, "type");
@@ -13578,7 +13586,8 @@ virDomainDeviceDefParse(const char *xmlStr,
goto error;
break;
case VIR_DOMAIN_DEVICE_CHR:
- if (!(dev->data.chr = virDomainChrDefParseXML(ctxt,
+ if (!(dev->data.chr = virDomainChrDefParseXML(xmlopt,
+ ctxt,
node,
def->seclabels,
def->nseclabels,
@@ -17197,7 +17206,8 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
for (i = 0; i < n; i++) {
- virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt,
+ virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
+ ctxt,
nodes[i],
def->seclabels,
def->nseclabels,
@@ -17224,7 +17234,8 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
for (i = 0; i < n; i++) {
- virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt,
+ virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
+ ctxt,
nodes[i],
def->seclabels,
def->nseclabels,
@@ -17253,7 +17264,8 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
for (i = 0; i < n; i++) {
- virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt,
+ virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
+ ctxt,
nodes[i],
def->seclabels,
def->nseclabels,
@@ -17272,7 +17284,8 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
for (i = 0; i < n; i++) {
- virDomainChrDefPtr chr = virDomainChrDefParseXML(ctxt,
+ virDomainChrDefPtr chr = virDomainChrDefParseXML(xmlopt,
+ ctxt,
nodes[i],
def->seclabels,
def->nseclabels,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index c34e046..fcadf6c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1118,6 +1118,7 @@ struct _virDomainChrSourceDef {
/* A complete character device, both host and domain views. */
struct _virDomainChrDef {
int deviceType; /* enum virDomainChrDeviceType */
+ virObjectPtr privateData;
bool targetTypeAttr;
int targetType; /* enum virDomainChrConsoleTargetType ||
@@ -2447,6 +2448,7 @@ struct _virDomainXMLPrivateDataCallbacks {
virDomainXMLPrivateDataNewFunc diskNew;
virDomainXMLPrivateDataNewFunc hostdevNew;
virDomainXMLPrivateDataNewFunc vcpuNew;
+ virDomainXMLPrivateDataNewFunc chardevNew;
virDomainXMLPrivateDataFormatFunc format;
virDomainXMLPrivateDataParseFunc parse;
};
@@ -2582,7 +2584,7 @@ bool virDomainDefHasDeviceAddress(virDomainDefPtr def,
void virDomainDefFree(virDomainDefPtr vm);
-virDomainChrDefPtr virDomainChrDefNew(void);
+virDomainChrDefPtr virDomainChrDefNew(virDomainXMLOptionPtr xmlopt);
virDomainDefPtr virDomainDefNew(void);
virDomainDefPtr virDomainDefNewFull(const char *name,
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 3df04cf..bdd8ae7 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -389,7 +389,7 @@ libxlDomainDefPostParse(virDomainDefPtr def,
if (def->os.type != VIR_DOMAIN_OSTYPE_HVM && def->nconsoles == 0) {
virDomainChrDefPtr chrdef;
- if (!(chrdef = virDomainChrDefNew()))
+ if (!(chrdef = virDomainChrDefNew(NULL)))
return -1;
chrdef->source.type = VIR_DOMAIN_CHR_TYPE_PTY;
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
index 76b7922..b6d26b2 100644
--- a/src/lxc/lxc_native.c
+++ b/src/lxc/lxc_native.c
@@ -703,7 +703,7 @@ lxcCreateConsoles(virDomainDefPtr def, virConfPtr properties)
def->nconsoles = nbttys;
for (i = 0; i < nbttys; i++) {
- if (!(console = virDomainChrDefNew()))
+ if (!(console = virDomainChrDefNew(NULL)))
goto error;
console->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0c9416f..a07f981 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -864,6 +864,49 @@ qemuDomainVcpuPrivateDispose(void *obj)
}
+static virClassPtr qemuDomainChardevPrivateClass;
+static void qemuDomainChardevPrivateDispose(void *obj);
+
+static int
+qemuDomainChardevPrivateOnceInit(void)
+{
+ qemuDomainChardevPrivateClass =
+ virClassNew(virClassForObject(),
+ "qemuDomainChardevPrivate",
+ sizeof(qemuDomainChardevPrivate),
+ qemuDomainChardevPrivateDispose);
+ if (!qemuDomainChardevPrivateClass)
+ return -1;
+ else
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(qemuDomainChardevPrivate)
+
+static virObjectPtr
+qemuDomainChardevPrivateNew(void)
+{
+ qemuDomainChardevPrivatePtr priv;
+
+ if (qemuDomainChardevPrivateInitialize() < 0)
+ return NULL;
+
+ if (!(priv = virObjectNew(qemuDomainChardevPrivateClass)))
+ return NULL;
+
+ return (virObjectPtr) priv;
+}
+
+
+static void
+qemuDomainChardevPrivateDispose(void *obj)
+{
+ qemuDomainChardevPrivatePtr priv = obj;
+
+ qemuDomainSecretInfoFree(&priv->secinfo);
+}
+
+
/* qemuDomainSecretPlainSetup:
* @conn: Pointer to connection
* @secinfo: Pointer to secret info
@@ -1767,6 +1810,7 @@ virDomainXMLPrivateDataCallbacks virQEMUDriverPrivateDataCallbacks =
{
.diskNew = qemuDomainDiskPrivateNew,
.vcpuNew = qemuDomainVcpuPrivateNew,
.hostdevNew = qemuDomainHostdevPrivateNew,
+ .chardevNew = qemuDomainChardevPrivateNew,
.parse = qemuDomainObjPrivateXMLParse,
.format = qemuDomainObjPrivateXMLFormat,
};
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index dee5d93..707a995 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -352,6 +352,20 @@ struct _qemuDomainHostdevPrivate {
qemuDomainSecretInfoPtr secinfo;
};
+# define QEMU_DOMAIN_CHARDEV_PRIVATE(chardev) \
+ ((qemuDomainChardevPrivatePtr) (chardev)->privateData)
+
+typedef struct _qemuDomainChardevPrivate qemuDomainChardevPrivate;
+typedef qemuDomainChardevPrivate *qemuDomainChardevPrivatePtr;
+struct _qemuDomainChardevPrivate {
+ virObject parent;
+
+ /* for char devices using secret
+ * NB: *not* to be written to qemu domain object XML */
+ qemuDomainSecretInfoPtr secinfo;
+};
+
+
typedef enum {
QEMU_PROCESS_EVENT_WATCHDOG = 0,
QEMU_PROCESS_EVENT_GUESTPANIC,
diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c
index 279f3da..331ab36 100644
--- a/src/qemu/qemu_parse_command.c
+++ b/src/qemu/qemu_parse_command.c
@@ -2189,7 +2189,7 @@ qemuParseCommandLine(virCapsPtr caps,
if (STRNEQ(val, "none")) {
virDomainChrDefPtr chr;
- if (!(chr = virDomainChrDefNew()))
+ if (!(chr = virDomainChrDefNew(NULL)))
goto error;
if (qemuParseCommandLineChr(&chr->source, val) < 0) {
@@ -2208,7 +2208,7 @@ qemuParseCommandLine(virCapsPtr caps,
if (STRNEQ(val, "none")) {
virDomainChrDefPtr chr;
- if (!(chr = virDomainChrDefNew()))
+ if (!(chr = virDomainChrDefNew(NULL)))
goto error;
if (qemuParseCommandLineChr(&chr->source, val) < 0) {
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index f2a5c96..b5b0197 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -1214,7 +1214,7 @@ prlsdkAddSerialInfo(PRL_HANDLE sdkdom,
ret = PrlVmCfg_GetSerialPort(sdkdom, i, &serialPort);
prlsdkCheckRetGoto(ret, cleanup);
- if (!(chr = virDomainChrDefNew()))
+ if (!(chr = virDomainChrDefNew(NULL)))
goto cleanup;
if (prlsdkGetSerialInfo(serialPort, chr))
diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c
index 0b04fc8..990c4ef 100644
--- a/src/xenconfig/xen_sxpr.c
+++ b/src/xenconfig/xen_sxpr.c
@@ -190,7 +190,7 @@ xenParseSxprChar(const char *value,
char *tmp;
virDomainChrDefPtr def;
- if (!(def = virDomainChrDefNew()))
+ if (!(def = virDomainChrDefNew(NULL)))
return NULL;
prefix = value;
diff --git a/src/xenconfig/xen_xl.c b/src/xenconfig/xen_xl.c
index a06983e..4316c25 100644
--- a/src/xenconfig/xen_xl.c
+++ b/src/xenconfig/xen_xl.c
@@ -737,7 +737,7 @@ xenParseXLChannel(virConfPtr conf, virDomainDefPtr def)
key = nextkey;
}
- if (!(channel = virDomainChrDefNew()))
+ if (!(channel = virDomainChrDefNew(NULL)))
goto cleanup;
if (STRPREFIX(type, "socket")) {
--
2.7.4
6:07 a.m.
New subject: [libvirt] [PATCH v9 3/5] qemu: Introduce qemuDomainChardevPrivatePtr
On Fri, Oct 14, 2016 at 04:23:06PM -0400, John Ferlan wrote:
Modeled after the qemuDomainHostdevPrivatePtr (commit id
'27726d8c'),
create a privateData pointer in the _virDomainChardevDef to allow storage
of private data for a hypervisor in order to at least temporarily store
secret data for usage during qemuBuildCommandLine.
NB: Since the qemu_parse_command (qemuParseCommandLine) code is not
expecting to restore the secret data, there's no need to add code
code to handle this new structure there.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
ACK
Pavel
3:23 p.m.
New subject: [libvirt] [PATCH v9 4/5] qemu: Add a secret object to/for a chardev tcp with secret
Add the secret object prior to the chardev tcp so the 'passwordid=' can
be added if the domain XML has a <secret> for the chardev TLS.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 32 ++++++-
src/qemu/qemu_command.h | 1 +
src/qemu/qemu_domain.c | 99 +++++++++++++++++++++-
src/qemu/qemu_domain.h | 16 +++-
src/qemu/qemu_hotplug.c | 1 +
src/qemu/qemu_process.c | 6 +-
...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 +++++++++
...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++++++++++
tests/qemuxml2argvtest.c | 18 ++++
9 files changed, 252 insertions(+), 9 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index aaf7018..b2dfee0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -695,6 +695,7 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
* @tlspath: path to the TLS credentials
* @listen: boolen listen for client or server setting
* @verifypeer: boolean to enable peer verification (form of authorization)
+ * @secalias: if one exists, the alias of the security object for passwordid
* @qemuCaps: capabilities
* @propsret: json properties to return
*
@@ -706,6 +707,7 @@ int
qemuBuildTLSx509BackendProps(const char *tlspath,
bool isListen,
bool verifypeer,
+ const char *secalias,
virQEMUCapsPtr qemuCaps,
virJSONValuePtr *propsret)
{
@@ -731,6 +733,10 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
NULL) < 0)
goto cleanup;
+ if (secalias &&
+ virJSONValueObjectAdd(*propsret, "s:passwordid", secalias, NULL) <
0)
+ goto cleanup;
+
ret = 0;
cleanup:
@@ -745,6 +751,7 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
* @tlspath: path to the TLS credentials
* @listen: boolen listen for client or server setting
* @verifypeer: boolean to enable peer verification (form of authorization)
+ * @addpasswordid: boolean to handle adding passwordid to object
* @inalias: Alias for the parent to generate object alias
* @qemuCaps: capabilities
*
@@ -757,6 +764,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
const char *tlspath,
bool isListen,
bool verifypeer,
+ bool addpasswordid,
const char *inalias,
virQEMUCapsPtr qemuCaps)
{
@@ -764,11 +772,16 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
char *objalias = NULL;
virJSONValuePtr props = NULL;
char *tmp = NULL;
+ char *secalias = NULL;
- if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer,
- qemuCaps, &props) < 0)
+ if (addpasswordid &&
+ !(secalias = qemuDomainGetSecretAESAlias(inalias, false)))
return -1;
+ if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer, secalias,
+ qemuCaps, &props) < 0)
+ goto cleanup;
+
if (!(objalias = qemuAliasTLSObjFromChardevAlias(inalias)))
goto cleanup;
@@ -784,6 +797,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
virJSONValueFree(props);
VIR_FREE(objalias);
VIR_FREE(tmp);
+ VIR_FREE(secalias);
return ret;
}
@@ -4946,6 +4960,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
+ !!cfg->chardevTLSx509secretUUID,
alias, qemuCaps) < 0)
goto error;
@@ -8542,6 +8557,19 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
/* Use -chardev with -device if they are available */
if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) {
+ qemuDomainChardevPrivatePtr chardevPriv =
+ QEMU_DOMAIN_CHARDEV_PRIVATE(serial);
+ qemuDomainSecretInfoPtr secinfo = chardevPriv->secinfo;
+
+ /* Add the secret object first if necessary. The
+ * secinfo is added only to a TCP serial device during
+ * qemuDomainSecretChardevPrepare. Subsequent called
+ * functions can just check the config fields */
+ if (cfg->chardevTLS && cfg->chardevTLSx509secretUUID
&&
+ serial->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
+ qemuBuildObjectSecretCommandLine(cmd, secinfo) < 0)
+ return -1;
+
if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
&serial->source,
serial->info.alias,
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 2f2a6ff..a793fb6 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -69,6 +69,7 @@ int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
int qemuBuildTLSx509BackendProps(const char *tlspath,
bool isListen,
bool verifypeer,
+ const char *secalias,
virQEMUCapsPtr qemuCaps,
virJSONValuePtr *propsret);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a07f981..02c67ae 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1042,7 +1042,8 @@ qemuDomainSecretSetup(virConnectPtr conn,
if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) &&
(secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH ||
- secretUsageType == VIR_SECRET_USAGE_TYPE_VOLUME)) {
+ secretUsageType == VIR_SECRET_USAGE_TYPE_VOLUME ||
+ secretUsageType == VIR_SECRET_USAGE_TYPE_TLS)) {
if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias,
secretUsageType, username,
seclookupdef, isLuks) < 0)
@@ -1223,6 +1224,91 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn,
}
+/* qemuDomainSecretChardevDestroy:
+ * @disk: Pointer to a chardev definition
+ *
+ * Clear and destroy memory associated with the secret
+ */
+void
+qemuDomainSecretChardevDestroy(virDomainChrDefPtr chardev)
+{
+ qemuDomainChardevPrivatePtr chardevPriv =
+ QEMU_DOMAIN_CHARDEV_PRIVATE(chardev);
+
+ if (!chardevPriv || !chardevPriv->secinfo)
+ return;
+
+ qemuDomainSecretInfoFree(&chardevPriv->secinfo);
+}
+
+
+/* qemuDomainSecretChardevPrepare:
+ * @conn: Pointer to connection
+ * @driver: Pointer to driver object
+ * @priv: pointer to domain private object
+ * @chardev: Pointer to a chardev definition
+ *
+ * For a serial TCP character device, generate a qemuDomainSecretInfo to be
+ * used by the command line code to generate the secret for the tls-creds
+ * to use.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+qemuDomainSecretChardevPrepare(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ qemuDomainObjPrivatePtr priv,
+ virDomainChrDefPtr chardev)
+{
+ virQEMUDriverConfigPtr cfg;
+ virSecretLookupTypeDef seclookupdef = {0};
+ qemuDomainSecretInfoPtr secinfo = NULL;
+
+ if (!conn || chardev->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL ||
+ chardev->source.type != VIR_DOMAIN_CHR_TYPE_TCP)
+ return 0;
+
+ cfg = virQEMUDriverGetConfig(driver);
+ if (cfg->chardevTLS && cfg->chardevTLSx509secretUUID) {
+ qemuDomainChardevPrivatePtr chardevPriv =
+ QEMU_DOMAIN_CHARDEV_PRIVATE(chardev);
+
+ if (virUUIDParse(cfg->chardevTLSx509secretUUID,
+ seclookupdef.u.uuid) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("malformed chardev TLS secret uuid in
qemu.conf"));
+ goto error;
+ }
+ seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
+
+ if (VIR_ALLOC(secinfo) < 0)
+ goto error;
+
+ if (qemuDomainSecretSetup(conn, priv, secinfo, chardev->info.alias,
+ VIR_SECRET_USAGE_TYPE_TLS, NULL,
+ &seclookupdef, false) < 0)
+ goto error;
+
+ if (secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("TLS X.509 requires encrypted secrets "
+ "to be supported"));
+ goto error;
+ }
+
+ chardevPriv->secinfo = secinfo;
+ }
+
+ virObjectUnref(cfg);
+ return 0;
+
+ error:
+ virObjectUnref(cfg);
+ qemuDomainSecretInfoFree(&secinfo);
+ return -1;
+}
+
+
/* qemuDomainSecretDestroy:
* @vm: Domain object
*
@@ -1239,11 +1325,15 @@ qemuDomainSecretDestroy(virDomainObjPtr vm)
for (i = 0; i < vm->def->nhostdevs; i++)
qemuDomainSecretHostdevDestroy(vm->def->hostdevs[i]);
+
+ for (i = 0; i < vm->def->nserials; i++)
+ qemuDomainSecretChardevDestroy(vm->def->serials[i]);
}
/* qemuDomainSecretPrepare:
* @conn: Pointer to connection
+ * @driver: Pointer to driver object
* @vm: Domain object
*
* For any objects that may require an auth/secret setup, create a
@@ -1256,6 +1346,7 @@ qemuDomainSecretDestroy(virDomainObjPtr vm)
*/
int
qemuDomainSecretPrepare(virConnectPtr conn,
+ virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -1272,6 +1363,12 @@ qemuDomainSecretPrepare(virConnectPtr conn,
return -1;
}
+ for (i = 0; i < vm->def->nserials; i++) {
+ if (qemuDomainSecretChardevPrepare(conn, driver, priv,
+ vm->def->serials[i]) < 0)
+ return -1;
+ }
+
return 0;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 707a995..cc063d1 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -726,11 +726,23 @@ int qemuDomainSecretHostdevPrepare(virConnectPtr conn,
virDomainHostdevDefPtr hostdev)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+void qemuDomainSecretChardevDestroy(virDomainChrDefPtr chardev)
+ ATTRIBUTE_NONNULL(1);
+
+int qemuDomainSecretChardevPrepare(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ qemuDomainObjPrivatePtr priv,
+ virDomainChrDefPtr chardev)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_NONNULL(4);
+
void qemuDomainSecretDestroy(virDomainObjPtr vm)
ATTRIBUTE_NONNULL(1);
-int qemuDomainSecretPrepare(virConnectPtr conn, virDomainObjPtr vm)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int qemuDomainSecretPrepare(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ virDomainObjPtr vm)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
int qemuDomainDefValidateDiskLunSource(const virStorageSource *src)
ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f4d7c17..aad7fa1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1734,6 +1734,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
+ NULL,
priv->qemuCaps,
&tlsProps) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0f5a11b..643741d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5092,7 +5092,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
size_t i;
char *nodeset = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virCapsPtr caps;
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
@@ -5158,8 +5157,8 @@ qemuProcessPrepareDomain(virConnectPtr conn,
if (qemuDomainMasterKeyCreate(vm) < 0)
goto cleanup;
- VIR_DEBUG("Add secrets to disks and hostdevs");
- if (qemuDomainSecretPrepare(conn, vm) < 0)
+ VIR_DEBUG("Add secrets to disks, hostdevs, and chardevs");
+ if (qemuDomainSecretPrepare(conn, driver, vm) < 0)
goto cleanup;
for (i = 0; i < vm->def->nchannels; i++) {
@@ -5188,7 +5187,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
cleanup:
VIR_FREE(nodeset);
virObjectUnref(caps);
- virObjectUnref(cfg);
return ret;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
new file mode 100644
index 0000000..5859c74
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
@@ -0,0 +1,38 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
+localport=1111 \
+-device isa-serial,chardev=charserial0,id=serial0 \
+-object secret,id=serial1-secret0,\
+data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
+keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
+-object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
+endpoint=client,verify-peer=yes,passwordid=serial1-secret0 \
+-chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
+tls-creds=objserial1_tls0 \
+-device isa-serial,chardev=charserial1,id=serial1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
new file mode 100644
index 0000000..23c244b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
@@ -0,0 +1,50 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <serial type='udp'>
+ <source mode='bind' host='127.0.0.1'
service='1111'/>
+ <source mode='connect' host='127.0.0.1'
service='2222'/>
+ <target port='0'/>
+ </serial>
+ <serial type='tcp'>
+ <source mode='connect' host='127.0.0.1' service='5555'
tls='yes'/>
+ <protocol type='raw'/>
+ <target port='0'/>
+ </serial>
+ <console type='udp'>
+ <source mode='bind' host='127.0.0.1'
service='1111'/>
+ <source mode='connect' host='127.0.0.1'
service='2222'/>
+ <target type='serial' port='0'/>
+ </console>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c65c769..1ca91d3 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1165,6 +1165,24 @@ mymain(void)
DO_TEST("serial-tcp-tlsx509-chardev-notls",
QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+ VIR_FREE(driver.config->chardevTLSx509certdir);
+ if (VIR_STRDUP_QUIET(driver.config->chardevTLSx509certdir,
"/etc/pki/libvirt-chardev") < 0)
+ return EXIT_FAILURE;
+ driver.config->chardevTLSx509verify = 1;
+ if (VIR_STRDUP_QUIET(driver.config->chardevTLSx509secretUUID,
+ "6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea") < 0)
+ return EXIT_FAILURE;
+# ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
+ DO_TEST("serial-tcp-tlsx509-secret-chardev",
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_OBJECT_SECRET,
+ QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+# else
+ DO_TEST_FAILURE("serial-tcp-tlsx509-secret-chardev",
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_OBJECT_SECRET,
+ QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+# endif
driver.config->chardevTLS = 0;
VIR_FREE(driver.config->chardevTLSx509certdir);
DO_TEST("serial-many-chardev",
--
2.7.4
9:11 a.m.
New subject: [libvirt] [PATCH v9 4/5] qemu: Add a secret object to/for a chardev tcp with secret
On Fri, Oct 14, 2016 at 04:23:07PM -0400, John Ferlan wrote:
Add the secret object prior to the chardev tcp so the
'passwordid=' can
be added if the domain XML has a <secret> for the chardev TLS.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 32 ++++++-
src/qemu/qemu_command.h | 1 +
src/qemu/qemu_domain.c | 99 +++++++++++++++++++++-
src/qemu/qemu_domain.h | 16 +++-
src/qemu/qemu_hotplug.c | 1 +
src/qemu/qemu_process.c | 6 +-
...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 +++++++++
...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++++++++++
tests/qemuxml2argvtest.c | 18 ++++
9 files changed, 252 insertions(+), 9 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index aaf7018..b2dfee0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -695,6 +695,7 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
* @tlspath: path to the TLS credentials
* @listen: boolen listen for client or server setting
* @verifypeer: boolean to enable peer verification (form of authorization)
+ * @secalias: if one exists, the alias of the security object for passwordid
* @qemuCaps: capabilities
* @propsret: json properties to return
*
@@ -706,6 +707,7 @@ int
qemuBuildTLSx509BackendProps(const char *tlspath,
bool isListen,
bool verifypeer,
+ const char *secalias,
virQEMUCapsPtr qemuCaps,
virJSONValuePtr *propsret)
{
@@ -731,6 +733,10 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
NULL) < 0)
goto cleanup;
+ if (secalias &&
+ virJSONValueObjectAdd(*propsret, "s:passwordid", secalias, NULL) <
0)
+ goto cleanup;
+
ret = 0;
cleanup:
@@ -745,6 +751,7 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
* @tlspath: path to the TLS credentials
* @listen: boolen listen for client or server setting
* @verifypeer: boolean to enable peer verification (form of authorization)
+ * @addpasswordid: boolean to handle adding passwordid to object
* @inalias: Alias for the parent to generate object alias
* @qemuCaps: capabilities
*
@@ -757,6 +764,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
const char *tlspath,
bool isListen,
bool verifypeer,
+ bool addpasswordid,
const char *inalias,
virQEMUCapsPtr qemuCaps)
{
@@ -764,11 +772,16 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
char *objalias = NULL;
virJSONValuePtr props = NULL;
char *tmp = NULL;
+ char *secalias = NULL;
- if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer,
- qemuCaps, &props) < 0)
+ if (addpasswordid &&
+ !(secalias = qemuDomainGetSecretAESAlias(inalias, false)))
return -1;
+ if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer, secalias,
+ qemuCaps, &props) < 0)
+ goto cleanup;
+
if (!(objalias = qemuAliasTLSObjFromChardevAlias(inalias)))
goto cleanup;
@@ -784,6 +797,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
virJSONValueFree(props);
VIR_FREE(objalias);
VIR_FREE(tmp);
+ VIR_FREE(secalias);
return ret;
}
@@ -4946,6 +4960,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
+ !!cfg->chardevTLSx509secretUUID,
alias, qemuCaps) < 0)
goto error;
@@ -8542,6 +8557,19 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
/* Use -chardev with -device if they are available */
if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) {
+ qemuDomainChardevPrivatePtr chardevPriv =
+ QEMU_DOMAIN_CHARDEV_PRIVATE(serial);
+ qemuDomainSecretInfoPtr secinfo = chardevPriv->secinfo;
+
+ /* Add the secret object first if necessary. The
+ * secinfo is added only to a TCP serial device during
+ * qemuDomainSecretChardevPrepare. Subsequent called
+ * functions can just check the config fields */
+ if (cfg->chardevTLS && cfg->chardevTLSx509secretUUID
&&
+ serial->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
This needs to test also serial->source.data.tcp.haveTLS.
+ qemuBuildObjectSecretCommandLine(cmd, secinfo) <
0)
+ return -1;
+
if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
&serial->source,
serial->info.alias,
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 2f2a6ff..a793fb6 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -69,6 +69,7 @@ int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
int qemuBuildTLSx509BackendProps(const char *tlspath,
bool isListen,
bool verifypeer,
+ const char *secalias,
virQEMUCapsPtr qemuCaps,
virJSONValuePtr *propsret);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index a07f981..02c67ae 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1042,7 +1042,8 @@ qemuDomainSecretSetup(virConnectPtr conn,
if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) &&
(secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH ||
- secretUsageType == VIR_SECRET_USAGE_TYPE_VOLUME)) {
+ secretUsageType == VIR_SECRET_USAGE_TYPE_VOLUME ||
+ secretUsageType == VIR_SECRET_USAGE_TYPE_TLS)) {
if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias,
secretUsageType, username,
seclookupdef, isLuks) < 0)
@@ -1223,6 +1224,91 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn,
}
+/* qemuDomainSecretChardevDestroy:
+ * @disk: Pointer to a chardev definition
+ *
+ * Clear and destroy memory associated with the secret
+ */
+void
+qemuDomainSecretChardevDestroy(virDomainChrDefPtr chardev)
+{
+ qemuDomainChardevPrivatePtr chardevPriv =
+ QEMU_DOMAIN_CHARDEV_PRIVATE(chardev);
+
+ if (!chardevPriv || !chardevPriv->secinfo)
+ return;
+
+ qemuDomainSecretInfoFree(&chardevPriv->secinfo);
+}
+
+
+/* qemuDomainSecretChardevPrepare:
+ * @conn: Pointer to connection
+ * @driver: Pointer to driver object
+ * @priv: pointer to domain private object
+ * @chardev: Pointer to a chardev definition
+ *
+ * For a serial TCP character device, generate a qemuDomainSecretInfo to be
+ * used by the command line code to generate the secret for the tls-creds
+ * to use.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+qemuDomainSecretChardevPrepare(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ qemuDomainObjPrivatePtr priv,
+ virDomainChrDefPtr chardev)
+{
+ virQEMUDriverConfigPtr cfg;
+ virSecretLookupTypeDef seclookupdef = {0};
+ qemuDomainSecretInfoPtr secinfo = NULL;
+
+ if (!conn || chardev->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL ||
The check for !conn is redundant because the declaration uses
ATTRIBUTE_NONNULL for the first attribute.
+ chardev->source.type != VIR_DOMAIN_CHR_TYPE_TCP)
+ return 0;
+
+ cfg = virQEMUDriverGetConfig(driver);
+ if (cfg->chardevTLS && cfg->chardevTLSx509secretUUID) {
+ qemuDomainChardevPrivatePtr chardevPriv =
+ QEMU_DOMAIN_CHARDEV_PRIVATE(chardev);
+
+ if (virUUIDParse(cfg->chardevTLSx509secretUUID,
+ seclookupdef.u.uuid) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("malformed chardev TLS secret uuid in
qemu.conf"));
+ goto error;
+ }
+ seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
+
+ if (VIR_ALLOC(secinfo) < 0)
+ goto error;
+
+ if (qemuDomainSecretSetup(conn, priv, secinfo, chardev->info.alias,
+ VIR_SECRET_USAGE_TYPE_TLS, NULL,
+ &seclookupdef, false) < 0)
+ goto error;
+
+ if (secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("TLS X.509 requires encrypted secrets "
+ "to be supported"));
+ goto error;
+ }
+
+ chardevPriv->secinfo = secinfo;
+ }
+
+ virObjectUnref(cfg);
+ return 0;
+
+ error:
+ virObjectUnref(cfg);
+ qemuDomainSecretInfoFree(&secinfo);
+ return -1;
+}
+
+
/* qemuDomainSecretDestroy:
* @vm: Domain object
*
@@ -1239,11 +1325,15 @@ qemuDomainSecretDestroy(virDomainObjPtr vm)
for (i = 0; i < vm->def->nhostdevs; i++)
qemuDomainSecretHostdevDestroy(vm->def->hostdevs[i]);
+
+ for (i = 0; i < vm->def->nserials; i++)
+ qemuDomainSecretChardevDestroy(vm->def->serials[i]);
}
/* qemuDomainSecretPrepare:
* @conn: Pointer to connection
+ * @driver: Pointer to driver object
* @vm: Domain object
*
* For any objects that may require an auth/secret setup, create a
@@ -1256,6 +1346,7 @@ qemuDomainSecretDestroy(virDomainObjPtr vm)
*/
int
qemuDomainSecretPrepare(virConnectPtr conn,
+ virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -1272,6 +1363,12 @@ qemuDomainSecretPrepare(virConnectPtr conn,
return -1;
}
+ for (i = 0; i < vm->def->nserials; i++) {
+ if (qemuDomainSecretChardevPrepare(conn, driver, priv,
+ vm->def->serials[i]) < 0)
+ return -1;
+ }
+
return 0;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 707a995..cc063d1 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -726,11 +726,23 @@ int qemuDomainSecretHostdevPrepare(virConnectPtr conn,
virDomainHostdevDefPtr hostdev)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+void qemuDomainSecretChardevDestroy(virDomainChrDefPtr chardev)
+ ATTRIBUTE_NONNULL(1);
+
+int qemuDomainSecretChardevPrepare(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ qemuDomainObjPrivatePtr priv,
+ virDomainChrDefPtr chardev)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ ATTRIBUTE_NONNULL(4);
+
void qemuDomainSecretDestroy(virDomainObjPtr vm)
ATTRIBUTE_NONNULL(1);
-int qemuDomainSecretPrepare(virConnectPtr conn, virDomainObjPtr vm)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int qemuDomainSecretPrepare(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ virDomainObjPtr vm)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
int qemuDomainDefValidateDiskLunSource(const virStorageSource *src)
ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index f4d7c17..aad7fa1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1734,6 +1734,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
+ NULL,
priv->qemuCaps,
&tlsProps) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0f5a11b..643741d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5092,7 +5092,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
size_t i;
char *nodeset = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
The cfg cleanup could be moved to separate patch.
virCapsPtr caps;
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
@@ -5158,8 +5157,8 @@ qemuProcessPrepareDomain(virConnectPtr conn,
if (qemuDomainMasterKeyCreate(vm) < 0)
goto cleanup;
- VIR_DEBUG("Add secrets to disks and hostdevs");
- if (qemuDomainSecretPrepare(conn, vm) < 0)
+ VIR_DEBUG("Add secrets to disks, hostdevs, and chardevs");
+ if (qemuDomainSecretPrepare(conn, driver, vm) < 0)
goto cleanup;
for (i = 0; i < vm->def->nchannels; i++) {
@@ -5188,7 +5187,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
cleanup:
VIR_FREE(nodeset);
virObjectUnref(caps);
- virObjectUnref(cfg);
return ret;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
new file mode 100644
index 0000000..5859c74
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
@@ -0,0 +1,38 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefconfig \
+-nodefaults \
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=readline \
+-no-acpi \
+-boot c \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
+localport=1111 \
+-device isa-serial,chardev=charserial0,id=serial0 \
+-object secret,id=serial1-secret0,\
+data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
+keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
+-object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
+endpoint=client,verify-peer=yes,passwordid=serial1-secret0 \
+-chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
+tls-creds=objserial1_tls0 \
+-device isa-serial,chardev=charserial1,id=serial1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
new file mode 100644
index 0000000..23c244b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
@@ -0,0 +1,50 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
+ </controller>
+ <controller type='ide' index='0'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+ </controller>
+ <controller type='pci' index='0' model='pci-root'/>
+ <serial type='udp'>
+ <source mode='bind' host='127.0.0.1'
service='1111'/>
+ <source mode='connect' host='127.0.0.1'
service='2222'/>
+ <target port='0'/>
+ </serial>
+ <serial type='tcp'>
+ <source mode='connect' host='127.0.0.1' service='5555'
tls='yes'/>
+ <protocol type='raw'/>
+ <target port='0'/>
+ </serial>
+ <console type='udp'>
+ <source mode='bind' host='127.0.0.1'
service='1111'/>
+ <source mode='connect' host='127.0.0.1'
service='2222'/>
+ <target type='serial' port='0'/>
+ </console>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='virtio'>
+ <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c65c769..1ca91d3 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1165,6 +1165,24 @@ mymain(void)
DO_TEST("serial-tcp-tlsx509-chardev-notls",
QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+ VIR_FREE(driver.config->chardevTLSx509certdir);
+ if (VIR_STRDUP_QUIET(driver.config->chardevTLSx509certdir,
"/etc/pki/libvirt-chardev") < 0)
+ return EXIT_FAILURE;
+ driver.config->chardevTLSx509verify = 1;
Setting the chardevTLSx509verify is not strictly required to test the generated
command line if secret is required. The verify feature should has its own test
case.
Pavel
+ if
(VIR_STRDUP_QUIET(driver.config->chardevTLSx509secretUUID,
+ "6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea") < 0)
+ return EXIT_FAILURE;
+# ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
+ DO_TEST("serial-tcp-tlsx509-secret-chardev",
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_OBJECT_SECRET,
+ QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+# else
+ DO_TEST_FAILURE("serial-tcp-tlsx509-secret-chardev",
+ QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_OBJECT_SECRET,
+ QEMU_CAPS_OBJECT_TLS_CREDS_X509);
+# endif
driver.config->chardevTLS = 0;
VIR_FREE(driver.config->chardevTLSx509certdir);
DO_TEST("serial-many-chardev",
--
2.7.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
10:13 a.m.
New subject: [libvirt] [PATCH v9 4/5] qemu: Add a secret object to/for a chardev tcp with secret
On 10/17/2016 10:11 AM, Pavel Hrdina wrote:
On Fri, Oct 14, 2016 at 04:23:07PM -0400, John Ferlan wrote:
> Add the secret object prior to the chardev tcp so the 'passwordid=' can
> be added if the domain XML has a <secret> for the chardev TLS.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 32 ++++++-
> src/qemu/qemu_command.h | 1 +
> src/qemu/qemu_domain.c | 99 +++++++++++++++++++++-
> src/qemu/qemu_domain.h | 16 +++-
> src/qemu/qemu_hotplug.c | 1 +
> src/qemu/qemu_process.c | 6 +-
> ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 +++++++++
> ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++++++++++
> tests/qemuxml2argvtest.c | 18 ++++
> 9 files changed, 252 insertions(+), 9 deletions(-)
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index aaf7018..b2dfee0 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -695,6 +695,7 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
> * @tlspath: path to the TLS credentials
> * @listen: boolen listen for client or server setting
> * @verifypeer: boolean to enable peer verification (form of authorization)
> + * @secalias: if one exists, the alias of the security object for passwordid
> * @qemuCaps: capabilities
> * @propsret: json properties to return
> *
> @@ -706,6 +707,7 @@ int
> qemuBuildTLSx509BackendProps(const char *tlspath,
> bool isListen,
> bool verifypeer,
> + const char *secalias,
> virQEMUCapsPtr qemuCaps,
> virJSONValuePtr *propsret)
> {
> @@ -731,6 +733,10 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
> NULL) < 0)
> goto cleanup;
>
> + if (secalias &&
> + virJSONValueObjectAdd(*propsret, "s:passwordid", secalias, NULL)
< 0)
> + goto cleanup;
> +
> ret = 0;
>
> cleanup:
> @@ -745,6 +751,7 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
> * @tlspath: path to the TLS credentials
> * @listen: boolen listen for client or server setting
> * @verifypeer: boolean to enable peer verification (form of authorization)
> + * @addpasswordid: boolean to handle adding passwordid to object
> * @inalias: Alias for the parent to generate object alias
> * @qemuCaps: capabilities
> *
> @@ -757,6 +764,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
> const char *tlspath,
> bool isListen,
> bool verifypeer,
> + bool addpasswordid,
> const char *inalias,
> virQEMUCapsPtr qemuCaps)
> {
> @@ -764,11 +772,16 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
> char *objalias = NULL;
> virJSONValuePtr props = NULL;
> char *tmp = NULL;
> + char *secalias = NULL;
>
> - if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer,
> - qemuCaps, &props) < 0)
> + if (addpasswordid &&
> + !(secalias = qemuDomainGetSecretAESAlias(inalias, false)))
> return -1;
>
> + if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer, secalias,
> + qemuCaps, &props) < 0)
> + goto cleanup;
> +
> if (!(objalias = qemuAliasTLSObjFromChardevAlias(inalias)))
> goto cleanup;
>
> @@ -784,6 +797,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
> virJSONValueFree(props);
> VIR_FREE(objalias);
> VIR_FREE(tmp);
> + VIR_FREE(secalias);
> return ret;
> }
>
> @@ -4946,6 +4960,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
> dev->data.tcp.listen,
> cfg->chardevTLSx509verify,
> + !!cfg->chardevTLSx509secretUUID,
> alias, qemuCaps) < 0)
> goto error;
>
> @@ -8542,6 +8557,19 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
>
> /* Use -chardev with -device if they are available */
> if (virQEMUCapsSupportsChardev(def, qemuCaps, serial)) {
> + qemuDomainChardevPrivatePtr chardevPriv =
> + QEMU_DOMAIN_CHARDEV_PRIVATE(serial);
> + qemuDomainSecretInfoPtr secinfo = chardevPriv->secinfo;
> +
> + /* Add the secret object first if necessary. The
> + * secinfo is added only to a TCP serial device during
> + * qemuDomainSecretChardevPrepare. Subsequent called
> + * functions can just check the config fields */
> + if (cfg->chardevTLS && cfg->chardevTLSx509secretUUID
&&
> + serial->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
This needs to test also serial->source.data.tcp.haveTLS.
True, of course that depends on the order of the patches too. So if
these 4 get accepted, but 1/5 is not accepted, then this would have to
move to patch 1 and be handled with that.
> + qemuBuildObjectSecretCommandLine(cmd, secinfo)
< 0)
> + return -1;
> +
> if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
> &serial->source,
> serial->info.alias,
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 2f2a6ff..a793fb6 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -69,6 +69,7 @@ int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo,
> int qemuBuildTLSx509BackendProps(const char *tlspath,
> bool isListen,
> bool verifypeer,
> + const char *secalias,
> virQEMUCapsPtr qemuCaps,
> virJSONValuePtr *propsret);
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index a07f981..02c67ae 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1042,7 +1042,8 @@ qemuDomainSecretSetup(virConnectPtr conn,
> if (virCryptoHaveCipher(VIR_CRYPTO_CIPHER_AES256CBC) &&
> virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_OBJECT_SECRET) &&
> (secretUsageType == VIR_SECRET_USAGE_TYPE_CEPH ||
> - secretUsageType == VIR_SECRET_USAGE_TYPE_VOLUME)) {
> + secretUsageType == VIR_SECRET_USAGE_TYPE_VOLUME ||
> + secretUsageType == VIR_SECRET_USAGE_TYPE_TLS)) {
> if (qemuDomainSecretAESSetup(conn, priv, secinfo, srcalias,
> secretUsageType, username,
> seclookupdef, isLuks) < 0)
> @@ -1223,6 +1224,91 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn,
> }
>
>
> +/* qemuDomainSecretChardevDestroy:
> + * @disk: Pointer to a chardev definition
> + *
> + * Clear and destroy memory associated with the secret
> + */
> +void
> +qemuDomainSecretChardevDestroy(virDomainChrDefPtr chardev)
> +{
> + qemuDomainChardevPrivatePtr chardevPriv =
> + QEMU_DOMAIN_CHARDEV_PRIVATE(chardev);
> +
> + if (!chardevPriv || !chardevPriv->secinfo)
> + return;
> +
> + qemuDomainSecretInfoFree(&chardevPriv->secinfo);
> +}
> +
> +
> +/* qemuDomainSecretChardevPrepare:
> + * @conn: Pointer to connection
> + * @driver: Pointer to driver object
> + * @priv: pointer to domain private object
> + * @chardev: Pointer to a chardev definition
> + *
> + * For a serial TCP character device, generate a qemuDomainSecretInfo to be
> + * used by the command line code to generate the secret for the tls-creds
> + * to use.
> + *
> + * Returns 0 on success, -1 on failure
> + */
> +int
> +qemuDomainSecretChardevPrepare(virConnectPtr conn,
> + virQEMUDriverPtr driver,
> + qemuDomainObjPrivatePtr priv,
> + virDomainChrDefPtr chardev)
> +{
> + virQEMUDriverConfigPtr cfg;
> + virSecretLookupTypeDef seclookupdef = {0};
> + qemuDomainSecretInfoPtr secinfo = NULL;
> +
> + if (!conn || chardev->deviceType != VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL ||
The check for !conn is redundant because the declaration uses
ATTRIBUTE_NONNULL for the first attribute.
OK... It's cut-n-paste of Disk and Hostdev Prepare which should also
change (separate patch). I actually have quite a few of these built up
from a Coverity build which actually checks; whereas, the normal build
doesn't. I'll adjust this one and send the other ones separately.
> + chardev->source.type != VIR_DOMAIN_CHR_TYPE_TCP)
> + return 0;
> +
> + cfg = virQEMUDriverGetConfig(driver);
> + if (cfg->chardevTLS && cfg->chardevTLSx509secretUUID) {
> + qemuDomainChardevPrivatePtr chardevPriv =
> + QEMU_DOMAIN_CHARDEV_PRIVATE(chardev);
> +
> + if (virUUIDParse(cfg->chardevTLSx509secretUUID,
> + seclookupdef.u.uuid) < 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("malformed chardev TLS secret uuid in
qemu.conf"));
> + goto error;
> + }
> + seclookupdef.type = VIR_SECRET_LOOKUP_TYPE_UUID;
> +
> + if (VIR_ALLOC(secinfo) < 0)
> + goto error;
> +
> + if (qemuDomainSecretSetup(conn, priv, secinfo, chardev->info.alias,
> + VIR_SECRET_USAGE_TYPE_TLS, NULL,
> + &seclookupdef, false) < 0)
> + goto error;
> +
> + if (secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("TLS X.509 requires encrypted secrets "
> + "to be supported"));
> + goto error;
> + }
> +
> + chardevPriv->secinfo = secinfo;
> + }
> +
> + virObjectUnref(cfg);
> + return 0;
> +
> + error:
> + virObjectUnref(cfg);
> + qemuDomainSecretInfoFree(&secinfo);
> + return -1;
> +}
> +
> +
> /* qemuDomainSecretDestroy:
> * @vm: Domain object
> *
> @@ -1239,11 +1325,15 @@ qemuDomainSecretDestroy(virDomainObjPtr vm)
>
> for (i = 0; i < vm->def->nhostdevs; i++)
> qemuDomainSecretHostdevDestroy(vm->def->hostdevs[i]);
> +
> + for (i = 0; i < vm->def->nserials; i++)
> + qemuDomainSecretChardevDestroy(vm->def->serials[i]);
> }
>
>
> /* qemuDomainSecretPrepare:
> * @conn: Pointer to connection
> + * @driver: Pointer to driver object
> * @vm: Domain object
> *
> * For any objects that may require an auth/secret setup, create a
> @@ -1256,6 +1346,7 @@ qemuDomainSecretDestroy(virDomainObjPtr vm)
> */
> int
> qemuDomainSecretPrepare(virConnectPtr conn,
> + virQEMUDriverPtr driver,
> virDomainObjPtr vm)
> {
> qemuDomainObjPrivatePtr priv = vm->privateData;
> @@ -1272,6 +1363,12 @@ qemuDomainSecretPrepare(virConnectPtr conn,
> return -1;
> }
>
> + for (i = 0; i < vm->def->nserials; i++) {
> + if (qemuDomainSecretChardevPrepare(conn, driver, priv,
> + vm->def->serials[i]) < 0)
> + return -1;
> + }
> +
> return 0;
> }
>
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 707a995..cc063d1 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -726,11 +726,23 @@ int qemuDomainSecretHostdevPrepare(virConnectPtr conn,
> virDomainHostdevDefPtr hostdev)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>
> +void qemuDomainSecretChardevDestroy(virDomainChrDefPtr chardev)
> + ATTRIBUTE_NONNULL(1);
> +
> +int qemuDomainSecretChardevPrepare(virConnectPtr conn,
> + virQEMUDriverPtr driver,
> + qemuDomainObjPrivatePtr priv,
> + virDomainChrDefPtr chardev)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
> + ATTRIBUTE_NONNULL(4);
> +
> void qemuDomainSecretDestroy(virDomainObjPtr vm)
> ATTRIBUTE_NONNULL(1);
>
> -int qemuDomainSecretPrepare(virConnectPtr conn, virDomainObjPtr vm)
> - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +int qemuDomainSecretPrepare(virConnectPtr conn,
> + virQEMUDriverPtr driver,
> + virDomainObjPtr vm)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>
> int qemuDomainDefValidateDiskLunSource(const virStorageSource *src)
> ATTRIBUTE_NONNULL(1);
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index f4d7c17..aad7fa1 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1734,6 +1734,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
> if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
> dev->data.tcp.listen,
> cfg->chardevTLSx509verify,
> + NULL,
> priv->qemuCaps,
> &tlsProps) < 0)
> goto cleanup;
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 0f5a11b..643741d 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5092,7 +5092,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
> size_t i;
> char *nodeset = NULL;
> qemuDomainObjPrivatePtr priv = vm->privateData;
> - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
The cfg cleanup could be moved to separate patch.
OK.
> virCapsPtr caps;
>
> if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> @@ -5158,8 +5157,8 @@ qemuProcessPrepareDomain(virConnectPtr conn,
> if (qemuDomainMasterKeyCreate(vm) < 0)
> goto cleanup;
>
> - VIR_DEBUG("Add secrets to disks and hostdevs");
> - if (qemuDomainSecretPrepare(conn, vm) < 0)
> + VIR_DEBUG("Add secrets to disks, hostdevs, and chardevs");
> + if (qemuDomainSecretPrepare(conn, driver, vm) < 0)
> goto cleanup;
>
> for (i = 0; i < vm->def->nchannels; i++) {
> @@ -5188,7 +5187,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
> cleanup:
> VIR_FREE(nodeset);
> virObjectUnref(caps);
> - virObjectUnref(cfg);
> return ret;
> }
>
> diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
> new file mode 100644
> index 0000000..5859c74
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
> @@ -0,0 +1,38 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/home/test \
> +USER=test \
> +LOGNAME=test \
> +QEMU_AUDIO_DRV=none \
> +/usr/bin/qemu \
> +-name QEMUGuest1 \
> +-S \
> +-object secret,id=masterKey0,format=raw,\
> +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
> +-M pc \
> +-m 214 \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-nographic \
> +-nodefconfig \
> +-nodefaults \
> +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
> +server,nowait \
> +-mon chardev=charmonitor,id=monitor,mode=readline \
> +-no-acpi \
> +-boot c \
> +-usb \
> +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
> +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
> +-chardev udp,id=charserial0,host=127.0.0.1,port=2222,localaddr=127.0.0.1,\
> +localport=1111 \
> +-device isa-serial,chardev=charserial0,id=serial0 \
> +-object secret,id=serial1-secret0,\
> +data=9eao5F8qtkGt+seB1HYivWIxbtwUu6MQtg1zpj/oDtUsPr1q8wBYM91uEHCn6j/1,\
> +keyid=masterKey0,iv=AAECAwQFBgcICQoLDA0ODw==,format=base64 \
> +-object tls-creds-x509,id=objserial1_tls0,dir=/etc/pki/libvirt-chardev,\
> +endpoint=client,verify-peer=yes,passwordid=serial1-secret0 \
> +-chardev socket,id=charserial1,host=127.0.0.1,port=5555,\
> +tls-creds=objserial1_tls0 \
> +-device isa-serial,chardev=charserial1,id=serial1 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
> diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
> new file mode 100644
> index 0000000..23c244b
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
> @@ -0,0 +1,50 @@
> +<domain type='qemu'>
> + <name>QEMUGuest1</name>
> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> + <memory unit='KiB'>219136</memory>
> + <currentMemory unit='KiB'>219136</currentMemory>
> + <vcpu placement='static'>1</vcpu>
> + <os>
> + <type arch='i686' machine='pc'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <clock offset='utc'/>
> + <on_poweroff>destroy</on_poweroff>
> + <on_reboot>restart</on_reboot>
> + <on_crash>destroy</on_crash>
> + <devices>
> + <emulator>/usr/bin/qemu</emulator>
> + <disk type='block' device='disk'>
> + <source dev='/dev/HostVG/QEMUGuest1'/>
> + <target dev='hda' bus='ide'/>
> + <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
> + </disk>
> + <controller type='usb' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x2'/>
> + </controller>
> + <controller type='ide' index='0'>
> + <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
> + </controller>
> + <controller type='pci' index='0'
model='pci-root'/>
> + <serial type='udp'>
> + <source mode='bind' host='127.0.0.1'
service='1111'/>
> + <source mode='connect' host='127.0.0.1'
service='2222'/>
> + <target port='0'/>
> + </serial>
> + <serial type='tcp'>
> + <source mode='connect' host='127.0.0.1'
service='5555' tls='yes'/>
> + <protocol type='raw'/>
> + <target port='0'/>
> + </serial>
> + <console type='udp'>
> + <source mode='bind' host='127.0.0.1'
service='1111'/>
> + <source mode='connect' host='127.0.0.1'
service='2222'/>
> + <target type='serial' port='0'/>
> + </console>
> + <input type='mouse' bus='ps2'/>
> + <input type='keyboard' bus='ps2'/>
> + <memballoon model='virtio'>
> + <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
> + </memballoon>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index c65c769..1ca91d3 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1165,6 +1165,24 @@ mymain(void)
> DO_TEST("serial-tcp-tlsx509-chardev-notls",
> QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
> QEMU_CAPS_OBJECT_TLS_CREDS_X509);
> + VIR_FREE(driver.config->chardevTLSx509certdir);
> + if (VIR_STRDUP_QUIET(driver.config->chardevTLSx509certdir,
"/etc/pki/libvirt-chardev") < 0)
> + return EXIT_FAILURE;
> + driver.config->chardevTLSx509verify = 1;
Setting the chardevTLSx509verify is not strictly required to test the generated
command line if secret is required. The verify feature should has its own test
case.
OK, right... Should be a separate patch, although it's almost a two-fer
;-)...
Making this change alters the .args file to have 'verify-peer=no'
instead of verify-peer=yes.
Tks -
John
Pavel
> + if (VIR_STRDUP_QUIET(driver.config->chardevTLSx509secretUUID,
> + "6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea") < 0)
> + return EXIT_FAILURE;
> +# ifdef HAVE_GNUTLS_CIPHER_ENCRYPT
> + DO_TEST("serial-tcp-tlsx509-secret-chardev",
> + QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
> + QEMU_CAPS_OBJECT_SECRET,
> + QEMU_CAPS_OBJECT_TLS_CREDS_X509);
> +# else
> + DO_TEST_FAILURE("serial-tcp-tlsx509-secret-chardev",
> + QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG,
> + QEMU_CAPS_OBJECT_SECRET,
> + QEMU_CAPS_OBJECT_TLS_CREDS_X509);
> +# endif
> driver.config->chardevTLS = 0;
> VIR_FREE(driver.config->chardevTLSx509certdir);
> DO_TEST("serial-many-chardev",
> --
> 2.7.4
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
3:23 p.m.
New subject: [libvirt] [PATCH v9 5/5] qemu: Add the ability to hotplug a secret object for TCP chardev TLS
https://bugzilla.redhat.com/show_bug.cgi?id=1300776
Complete the implementation of support for TLS encryption on
chardev TCP transports by adding the hotplug ability of a secret
to generate the passwordid for the TLS object
Likewise, add the ability to hot unplug that secret object as well
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_hotplug.c | 62 +++++++++++++++++++++++++++++++++++++++++++++----
src/qemu/qemu_hotplug.h | 3 ++-
tests/qemuhotplugtest.c | 2 +-
4 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8789c9d..5a1cf7b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7567,7 +7567,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
break;
case VIR_DOMAIN_DEVICE_CHR:
- ret = qemuDomainAttachChrDevice(driver, vm,
+ ret = qemuDomainAttachChrDevice(conn, driver, vm,
dev->data.chr);
if (!ret) {
alias = dev->data.chr->info.alias;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index aad7fa1..69d562f 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1690,7 +1690,8 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def,
return ret;
}
-int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
+int qemuDomainAttachChrDevice(virConnectPtr conn,
+ virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainChrDefPtr chr)
{
@@ -1704,8 +1705,11 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
char *charAlias = NULL;
bool chardevAttached = false;
bool tlsobjAdded = false;
+ bool secobjAdded = false;
virJSONValuePtr tlsProps = NULL;
char *tlsAlias = NULL;
+ virJSONValuePtr secProps = NULL;
+ char *secAlias = NULL;
bool need_release = false;
if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
@@ -1729,12 +1733,30 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
if (qemuDomainChrPreInsert(vmdef, chr) < 0)
goto cleanup;
+ if (qemuDomainSecretChardevPrepare(conn, driver, priv, chr) < 0)
+ goto cleanup;
+
if (cfg->chardevTLS &&
dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) {
+ qemuDomainChardevPrivatePtr chardevPriv =
+ QEMU_DOMAIN_CHARDEV_PRIVATE(chr);
+ qemuDomainSecretInfoPtr secinfo = chardevPriv->secinfo;
+
+ /* Add a secret object in order to access the TLS environment.
+ * The secinfo will only be created for serial TCP device. */
+ if (secinfo) {
+ if (qemuBuildSecretInfoProps(secinfo, &secProps) < 0)
+ goto cleanup;
+
+ if (!(secAlias = qemuDomainGetSecretAESAlias(chr->info.alias,
+ false)))
+ goto cleanup;
+ }
+
if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
- NULL,
+ secAlias,
priv->qemuCaps,
&tlsProps) < 0)
goto cleanup;
@@ -1745,6 +1767,15 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
}
qemuDomainObjEnterMonitor(driver, vm);
+ if (secAlias) {
+ rc = qemuMonitorAddObject(priv->mon, "secret",
+ secAlias, secProps);
+ secProps = NULL;
+ if (rc < 0)
+ goto exit_monitor;
+ secobjAdded = true;
+ }
+
if (tlsAlias) {
rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
tlsAlias, tlsProps);
@@ -1775,6 +1806,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL);
VIR_FREE(tlsAlias);
virJSONValueFree(tlsProps);
+ VIR_FREE(secAlias);
+ virJSONValueFree(secProps);
VIR_FREE(charAlias);
VIR_FREE(devstr);
virObjectUnref(cfg);
@@ -1782,6 +1815,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
exit_monitor:
orig_err = virSaveLastError();
+ if (secobjAdded)
+ ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
if (tlsobjAdded)
ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
/* detach associated chardev on error */
@@ -4387,6 +4422,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
virDomainDefPtr vmdef = vm->def;
virDomainChrDefPtr tmpChr;
char *objAlias = NULL;
+ char *secAlias = NULL;
char *devstr = NULL;
if (!(tmpChr = virDomainChrFind(vmdef, chr))) {
@@ -4400,9 +4436,21 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
sa_assert(tmpChr->info.alias);
- if (cfg->chardevTLS &&
- !(objAlias = qemuAliasTLSObjFromChardevAlias(tmpChr->info.alias)))
- goto cleanup;
+ if (cfg->chardevTLS) {
+ if (!(objAlias = qemuAliasTLSObjFromChardevAlias(tmpChr->info.alias)))
+ goto cleanup;
+
+ /* Best shot at this as the secinfo is destroyed after process launch
+ * and this path does not recreate it. Thus, if the config has the
+ * secret UUID and we have a serial TCP chardev, then formulate a
+ * secAlias which we'll attempt to destroy. */
+ if (cfg->chardevTLSx509secretUUID &&
+ tmpChr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+ tmpChr->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
+ !(secAlias = qemuDomainGetSecretAESAlias(tmpChr->info.alias,
+ false)))
+ goto cleanup;
+ }
if (qemuBuildChrDeviceStr(&devstr, vmdef, chr, priv->qemuCaps) < 0)
goto cleanup;
@@ -4416,6 +4464,10 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
if (objAlias && qemuMonitorDelObject(priv->mon, objAlias) < 0)
goto exit_monitor;
+ /* If it fails, then so be it - it was a best shot */
+ if (secAlias)
+ ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
+
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index b048cf4..8f23176 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -92,7 +92,8 @@ int qemuDomainAttachLease(virQEMUDriverPtr driver,
int qemuDomainDetachLease(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainLeaseDefPtr lease);
-int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
+int qemuDomainAttachChrDevice(virConnectPtr conn,
+ virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainChrDefPtr chr);
int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 43eb1cf..14561a8 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -118,7 +118,7 @@ testQemuHotplugAttach(virDomainObjPtr vm,
ret = qemuDomainAttachDeviceDiskLive(NULL, &driver, vm, dev);
break;
case VIR_DOMAIN_DEVICE_CHR:
- ret = qemuDomainAttachChrDevice(&driver, vm, dev->data.chr);
+ ret = qemuDomainAttachChrDevice(NULL, &driver, vm, dev->data.chr);
break;
default:
VIR_TEST_VERBOSE("device type '%s' cannot be attached\n",
--
2.7.4
10:13 a.m.
New subject: [libvirt] [PATCH v9 5/5] qemu: Add the ability to hotplug a secret object for TCP chardev TLS
On Fri, Oct 14, 2016 at 04:23:08PM -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1300776
Complete the implementation of support for TLS encryption on
chardev TCP transports by adding the hotplug ability of a secret
to generate the passwordid for the TLS object
Likewise, add the ability to hot unplug that secret object as well
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_hotplug.c | 62 +++++++++++++++++++++++++++++++++++++++++++++----
src/qemu/qemu_hotplug.h | 3 ++-
tests/qemuhotplugtest.c | 2 +-
4 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8789c9d..5a1cf7b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7567,7 +7567,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
break;
case VIR_DOMAIN_DEVICE_CHR:
- ret = qemuDomainAttachChrDevice(driver, vm,
+ ret = qemuDomainAttachChrDevice(conn, driver, vm,
dev->data.chr);
if (!ret) {
alias = dev->data.chr->info.alias;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index aad7fa1..69d562f 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1690,7 +1690,8 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def,
return ret;
}
-int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
+int qemuDomainAttachChrDevice(virConnectPtr conn,
+ virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainChrDefPtr chr)
{
@@ -1704,8 +1705,11 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
char *charAlias = NULL;
bool chardevAttached = false;
bool tlsobjAdded = false;
+ bool secobjAdded = false;
virJSONValuePtr tlsProps = NULL;
char *tlsAlias = NULL;
+ virJSONValuePtr secProps = NULL;
+ char *secAlias = NULL;
bool need_release = false;
if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
@@ -1729,12 +1733,30 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
if (qemuDomainChrPreInsert(vmdef, chr) < 0)
goto cleanup;
+ if (qemuDomainSecretChardevPrepare(conn, driver, priv, chr) < 0)
+ goto cleanup;
+
if (cfg->chardevTLS &&
dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_NO) {
+ qemuDomainChardevPrivatePtr chardevPriv =
+ QEMU_DOMAIN_CHARDEV_PRIVATE(chr);
+ qemuDomainSecretInfoPtr secinfo = chardevPriv->secinfo;
+
+ /* Add a secret object in order to access the TLS environment.
+ * The secinfo will only be created for serial TCP device. */
+ if (secinfo) {
+ if (qemuBuildSecretInfoProps(secinfo, &secProps) < 0)
+ goto cleanup;
+
+ if (!(secAlias = qemuDomainGetSecretAESAlias(chr->info.alias,
+ false)))
+ goto cleanup;
+ }
+
if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
dev->data.tcp.listen,
cfg->chardevTLSx509verify,
- NULL,
+ secAlias,
priv->qemuCaps,
&tlsProps) < 0)
goto cleanup;
@@ -1745,6 +1767,15 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
}
qemuDomainObjEnterMonitor(driver, vm);
+ if (secAlias) {
+ rc = qemuMonitorAddObject(priv->mon, "secret",
+ secAlias, secProps);
+ secProps = NULL;
+ if (rc < 0)
+ goto exit_monitor;
+ secobjAdded = true;
+ }
+
if (tlsAlias) {
rc = qemuMonitorAddObject(priv->mon, "tls-creds-x509",
tlsAlias, tlsProps);
@@ -1775,6 +1806,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL);
VIR_FREE(tlsAlias);
virJSONValueFree(tlsProps);
+ VIR_FREE(secAlias);
+ virJSONValueFree(secProps);
VIR_FREE(charAlias);
VIR_FREE(devstr);
virObjectUnref(cfg);
@@ -1782,6 +1815,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
exit_monitor:
orig_err = virSaveLastError();
+ if (secobjAdded)
+ ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
if (tlsobjAdded)
ignore_value(qemuMonitorDelObject(priv->mon, tlsAlias));
/* detach associated chardev on error */
@@ -4387,6 +4422,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
virDomainDefPtr vmdef = vm->def;
virDomainChrDefPtr tmpChr;
char *objAlias = NULL;
+ char *secAlias = NULL;
char *devstr = NULL;
if (!(tmpChr = virDomainChrFind(vmdef, chr))) {
@@ -4400,9 +4436,21 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
sa_assert(tmpChr->info.alias);
- if (cfg->chardevTLS &&
- !(objAlias = qemuAliasTLSObjFromChardevAlias(tmpChr->info.alias)))
- goto cleanup;
+ if (cfg->chardevTLS) {
This needs to test serial->source.data.tcp.haveTLS.
Pavel
+ if (!(objAlias =
qemuAliasTLSObjFromChardevAlias(tmpChr->info.alias)))
+ goto cleanup;
+
+ /* Best shot at this as the secinfo is destroyed after process launch
+ * and this path does not recreate it. Thus, if the config has the
+ * secret UUID and we have a serial TCP chardev, then formulate a
+ * secAlias which we'll attempt to destroy. */
+ if (cfg->chardevTLSx509secretUUID &&
+ tmpChr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+ tmpChr->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
+ !(secAlias = qemuDomainGetSecretAESAlias(tmpChr->info.alias,
+ false)))
+ goto cleanup;
+ }
if (qemuBuildChrDeviceStr(&devstr, vmdef, chr, priv->qemuCaps) < 0)
goto cleanup;
@@ -4416,6 +4464,10 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
if (objAlias && qemuMonitorDelObject(priv->mon, objAlias) < 0)
goto exit_monitor;
+ /* If it fails, then so be it - it was a best shot */
+ if (secAlias)
+ ignore_value(qemuMonitorDelObject(priv->mon, secAlias));
+
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index b048cf4..8f23176 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -92,7 +92,8 @@ int qemuDomainAttachLease(virQEMUDriverPtr driver,
int qemuDomainDetachLease(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainLeaseDefPtr lease);
-int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
+int qemuDomainAttachChrDevice(virConnectPtr conn,
+ virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainChrDefPtr chr);
int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 43eb1cf..14561a8 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -118,7 +118,7 @@ testQemuHotplugAttach(virDomainObjPtr vm,
ret = qemuDomainAttachDeviceDiskLive(NULL, &driver, vm, dev);
break;
case VIR_DOMAIN_DEVICE_CHR:
- ret = qemuDomainAttachChrDevice(&driver, vm, dev->data.chr);
+ ret = qemuDomainAttachChrDevice(NULL, &driver, vm, dev->data.chr);
break;
default:
VIR_TEST_VERBOSE("device type '%s' cannot be attached\n",
--
2.7.4
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2957
days inactive
2961
days old
20 comments
3 participants
participants (3)
-
Daniel P. Berrange -
John Ferlan -
Pavel Hrdina