[libvirt] [PATCH 0/3] last part of TLS encryption for char devices

Pavel Hrdina (3): qemu_hotplug: remove union for one member domain: Add optional 'tls' attribute for TCP chardev domain: fix migration to older libvirt docs/formatdomain.html.in | 28 +++++++++ docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 67 +++++++++++++++++----- src/conf/domain_conf.h | 6 +- src/qemu/qemu_command.c | 4 +- src/qemu/qemu_domain.c | 67 ++++++++++++++++++++++ src/qemu/qemu_domain.h | 8 +++ src/qemu/qemu_hotplug.c | 16 ++++-- src/qemu/qemu_process.c | 2 + ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 ++++++++++ ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 ++++++++++++++++ tests/qemuxml2argvtest.c | 3 + ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml | 1 + tests/qemuxml2xmltest.c | 1 + 14 files changed, 266 insertions(+), 22 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 -- 2.10.1

Currently the union has only one member so remove that union. If there is a need to add a new type of source for new bus in the future this will force the author to add a union and properly check bus type before any access to union member. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 23 +++++++++++------------ src/conf/domain_conf.h | 4 +--- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_hotplug.c | 4 ++-- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fd71e88..35cdbc3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2363,7 +2363,7 @@ void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def) if (!def) return; - virDomainChrSourceDefFree(def->source.chr); + virDomainChrSourceDefFree(def->source); virDomainDeviceInfoClear(&def->info); VIR_FREE(def); @@ -12986,7 +12986,7 @@ virDomainRedirdevDefParseXML(virDomainXMLOptionPtr xmlopt, if (VIR_ALLOC(def) < 0) return NULL; - if (!(def->source.chr = virDomainChrSourceDefNew(xmlopt))) + if (!(def->source = virDomainChrSourceDefNew(xmlopt))) goto error; bus = virXMLPropString(node, "bus"); @@ -13002,7 +13002,7 @@ virDomainRedirdevDefParseXML(virDomainXMLOptionPtr xmlopt, type = virXMLPropString(node, "type"); if (type) { - if ((def->source.chr->type = virDomainChrTypeFromString(type)) < 0) { + if ((def->source->type = virDomainChrTypeFromString(type)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown redirdev character device type '%s'"), type); goto error; @@ -13017,13 +13017,13 @@ virDomainRedirdevDefParseXML(virDomainXMLOptionPtr xmlopt, /* boot gets parsed in virDomainDeviceInfoParseXML * source gets parsed in virDomainChrSourceDefParseXML * we don't know any of the elements that might remain */ - remaining = virDomainChrSourceDefParseXML(def->source.chr, cur, flags, + remaining = virDomainChrSourceDefParseXML(def->source, cur, flags, NULL, NULL, NULL, 0); if (remaining < 0) goto error; - if (def->source.chr->type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) - def->source.chr->data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_USBREDIR; + if (def->source->type == VIR_DOMAIN_CHR_TYPE_SPICEVMC) + def->source->data.spicevmc = VIR_DOMAIN_CHR_SPICEVMC_USBREDIR; if (virDomainDeviceInfoParseXML(node, bootHash, &def->info, flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT) < 0) @@ -14853,8 +14853,7 @@ virDomainRedirdevDefFind(virDomainDefPtr def, if (redirdev->bus != tmp->bus) continue; - if (!virDomainChrSourceDefIsEqual(redirdev->source.chr, - tmp->source.chr)) + if (!virDomainChrSourceDefIsEqual(redirdev->source, tmp->source)) continue; if (redirdev->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && @@ -18670,12 +18669,12 @@ virDomainRedirdevDefCheckABIStability(virDomainRedirdevDefPtr src, switch ((virDomainRedirdevBus) src->bus) { case VIR_DOMAIN_REDIRDEV_BUS_USB: - if (src->source.chr->type != dst->source.chr->type) { + if (src->source->type != dst->source->type) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target redirected device source type %s does " "not match source device source type %s"), - virDomainChrTypeToString(dst->source.chr->type), - virDomainChrTypeToString(src->source.chr->type)); + virDomainChrTypeToString(dst->source->type), + virDomainChrTypeToString(src->source->type)); return false; } break; @@ -22748,7 +22747,7 @@ virDomainRedirdevDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<redirdev bus='%s'", bus); virBufferAdjustIndent(buf, 2); - if (virDomainChrSourceDefFormat(buf, NULL, def->source.chr, false, flags) < 0) + if (virDomainChrSourceDefFormat(buf, NULL, def->source, false, flags) < 0) return -1; if (virDomainDeviceInfoFormat(buf, &def->info, flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 36c2fa5..7fc1141 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1517,9 +1517,7 @@ typedef enum { struct _virDomainRedirdevDef { int bus; /* enum virDomainRedirdevBus */ - union { - virDomainChrSourceDefPtr chr; - } source; + virDomainChrSourceDefPtr source; virDomainDeviceInfo info; /* Guest address */ }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 167ab66..5713d18 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8833,7 +8833,7 @@ qemuBuildRedirdevCommandLine(virLogManagerPtr logManager, char *devstr; if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def, - redirdev->source.chr, + redirdev->source, redirdev->info.alias, qemuCaps, true))) { return -1; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 95b2f2a..706b736 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1529,7 +1529,7 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, if (VIR_REALLOC_N(def->redirdevs, def->nredirdevs+1) < 0) goto cleanup; - if (qemuDomainGetChardevTLSObjects(cfg, priv, redirdev->source.chr, + if (qemuDomainGetChardevTLSObjects(cfg, priv, redirdev->source, charAlias, &tlsProps, &tlsAlias) < 0) goto cleanup; @@ -1545,7 +1545,7 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, if (qemuMonitorAttachCharDev(priv->mon, charAlias, - redirdev->source.chr) < 0) + redirdev->source) < 0) goto exit_monitor; chardevAdded = true; -- 2.10.1

On 10/24/2016 08:41 AM, Pavel Hrdina wrote:
Currently the union has only one member so remove that union. If there is a need to add a new type of source for new bus in the future this will force the author to add a union and properly check bus type before any access to union member.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 23 +++++++++++------------ src/conf/domain_conf.h | 4 +--- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_hotplug.c | 4 ++-- 4 files changed, 15 insertions(+), 18 deletions(-)
ACK John

Add an optional "tls='yes|no'" attribute for a TCP chardev. For QEMU, this will allow for disabling the host config setting of the 'chardev_tls' for a domain chardev channel by setting the value to "no" or to attempt to use a host TLS environment when setting the value to "yes" when the host config 'chardev_tls' setting is disabled, but a TLS environment is configured via either the host config 'chardev_tls_x509_cert_dir' or 'default_tls_x509_cert_dir' Alter qemuDomainSupportTLSChardevTCP to augment the decision points for choosing whether to try to use TLS. Signed-off-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 28 +++++++++ docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 22 +++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 66 ++++++++++++++++++++++ src/qemu/qemu_domain.h | 8 +++ src/qemu/qemu_hotplug.c | 12 +++- src/qemu/qemu_process.c | 2 + ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 ++++++++++ ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 ++++++++++++++++ tests/qemuxml2argvtest.c | 3 + ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml | 1 + tests/qemuxml2xmltest.c | 1 + 14 files changed, 227 insertions(+), 4 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..0cd68a0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6204,6 +6204,34 @@ 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 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 a TLS environment can + be controlled on the host by the <code>chardev_tls</code> and + <code>chardev_tls_x509_cert_dir</code> or + <code>default_tls_x509_cert_dir</code> settings in the file + /etc/libvirt/qemu.conf. If <code>chardev_tls</code> is enabled, + then unless the <code>tls</code> attribute is set to "no", libvirt + will use the host configured TLS environment. + If <code>chardev_tls</code> is disabled, but the <code>tls</code> + attribute is set to "yes", then libvirt will attempt to use the + host TLS environment if either the <code>chardev_tls_x509_cert_dir</code> + or <code>default_tls_x509_cert_dir</code> TLS directory structure exists. + </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 35cdbc3..6e814b3 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: @@ -10039,6 +10041,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *master = NULL; char *slave = NULL; char *append = NULL; + char *haveTLS = NULL; int remaining = 0; while (cur != NULL) { @@ -10046,6 +10049,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: @@ -10222,6 +10227,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 = @@ -10306,6 +10320,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(append); VIR_FREE(logappend); VIR_FREE(logfile); + VIR_FREE(haveTLS); return remaining; @@ -21492,7 +21507,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 7fc1141..f1da9c3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1095,6 +1095,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 5713d18..6bf6510 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4935,7 +4935,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, if (dev->data.tcp.listen) virBufferAdd(&buf, nowait ? ",server,nowait" : ",server", -1); - if (cfg->chardevTLS) { + if (dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) { char *objalias = NULL; if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f11bc01..6cffff0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6186,6 +6186,72 @@ qemuDomainPrepareChannel(virDomainChrDefPtr channel, } +/* qemuProcessPrepareDomainChardevSourceTLS: + * @source: pointer to host interface data for char devices + * @cfg: driver configuration + * + * Updates host interface TLS encryption setting based on qemu.conf + * for char devices. This will be presented as "tls='yes|no'" in + * live XML of a guest. + */ +void +qemuDomainPrepareChardevSourceTLS(virDomainChrSourceDefPtr source, + virQEMUDriverConfigPtr cfg) +{ + if (source->type == VIR_DOMAIN_CHR_TYPE_TCP) { + if (source->data.tcp.haveTLS == VIR_TRISTATE_BOOL_ABSENT) { + if (cfg->chardevTLS) + source->data.tcp.haveTLS = VIR_TRISTATE_BOOL_YES; + else + source->data.tcp.haveTLS = VIR_TRISTATE_BOOL_NO; + } + } +} + + +/* qemuProcessPrepareDomainChardevSource: + * @def: live domain definition + * @driver: qemu driver + * + * Iterate through all devices that use virDomainChrSourceDefPtr as host + * interface part. + */ +void +qemuDomainPrepareChardevSource(virDomainDefPtr def, + virQEMUDriverPtr driver) +{ + size_t i; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + for (i = 0; i < def->nserials; i++) + qemuDomainPrepareChardevSourceTLS(def->serials[i]->source, cfg); + + for (i = 0; i < def->nparallels; i++) + qemuDomainPrepareChardevSourceTLS(def->parallels[i]->source, cfg); + + for (i = 0; i < def->nchannels; i++) + qemuDomainPrepareChardevSourceTLS(def->channels[i]->source, cfg); + + for (i = 0; i < def->nconsoles; i++) + qemuDomainPrepareChardevSourceTLS(def->consoles[i]->source, cfg); + + for (i = 0; i < def->nrngs; i++) + if (def->rngs[i]->backend == VIR_DOMAIN_RNG_BACKEND_EGD) + qemuDomainPrepareChardevSourceTLS(def->rngs[i]->source.chardev, cfg); + + for (i = 0; i < def->nsmartcards; i++) + if (def->smartcards[i]->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH) + qemuDomainPrepareChardevSourceTLS(def->smartcards[i]->data.passthru, + cfg); + + for (i = 0; i < def->nredirdevs; i++) + qemuDomainPrepareChardevSourceTLS(def->redirdevs[i]->source, cfg); + + virObjectUnref(cfg); +} + + + int qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 29125a2..4f9bf82 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -739,6 +739,14 @@ int qemuDomainPrepareChannel(virDomainChrDefPtr chr, const char *domainChannelTargetDir) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void qemuDomainPrepareChardevSourceTLS(virDomainChrSourceDefPtr source, + virQEMUDriverConfigPtr cfg) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +void qemuDomainPrepareChardevSource(virDomainDefPtr def, + virQEMUDriverPtr driver) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + int qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem) ATTRIBUTE_NONNULL(1); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 706b736..cf69945 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1482,7 +1482,8 @@ qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg, virJSONValuePtr *tlsProps, char **tlsAlias) { - if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP || !cfg->chardevTLS) + if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP || + dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES) return 0; if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir, @@ -1517,6 +1518,8 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, char *tlsAlias = NULL; virErrorPtr orig_err; + qemuDomainPrepareChardevSourceTLS(redirdev->source, cfg); + if (qemuAssignDeviceRedirdevAlias(def, redirdev, -1) < 0) goto cleanup; @@ -1771,6 +1774,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, qemuDomainPrepareChannel(chr, priv->channelTargetDir) < 0) goto cleanup; + qemuDomainPrepareChardevSourceTLS(dev, cfg); + if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0) goto cleanup; @@ -1901,6 +1906,9 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, goto cleanup; } + if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) + qemuDomainPrepareChardevSourceTLS(rng->source.chardev, cfg); + /* build required metadata */ if (!(devstr = qemuBuildRNGDevStr(vm->def, rng, priv->qemuCaps))) goto cleanup; @@ -4476,7 +4484,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, goto cleanup; if (tmpChr->source->type == VIR_DOMAIN_CHR_TYPE_TCP && - cfg->chardevTLS && + tmpChr->source->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES && !(objAlias = qemuAliasTLSObjFromChardevAlias(charAlias))) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a1e2896..33b78b1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5167,6 +5167,8 @@ qemuProcessPrepareDomain(virConnectPtr conn, goto cleanup; } + qemuDomainPrepareChardevSource(vm->def, driver); + if (VIR_ALLOC(priv->monConfig) < 0) goto cleanup; 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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3e9f825..52d85fa 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1167,6 +1167,9 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_OBJECT_TLS_CREDS_X509); driver.config->chardevTLSx509verify = 0; + 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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 95c0bf2..64da80a 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.10.1

On 10/24/2016 08:41 AM, Pavel Hrdina wrote:
Add an optional "tls='yes|no'" attribute for a TCP chardev.
For QEMU, this will allow for disabling the host config setting of the 'chardev_tls' for a domain chardev channel by setting the value to "no" or to attempt to use a host TLS environment when setting the value to "yes" when the host config 'chardev_tls' setting is disabled, but a TLS environment is configured via either the host config 'chardev_tls_x509_cert_dir' or 'default_tls_x509_cert_dir'
Alter qemuDomainSupportTLSChardevTCP to augment the decision points for choosing whether to try to use TLS.
The above sentence will no longer apply
Signed-off-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- docs/formatdomain.html.in | 28 +++++++++ docs/schemas/domaincommon.rng | 5 ++ src/conf/domain_conf.c | 22 +++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 2 +- src/qemu/qemu_domain.c | 66 ++++++++++++++++++++++ src/qemu/qemu_domain.h | 8 +++ src/qemu/qemu_hotplug.c | 12 +++- src/qemu/qemu_process.c | 2 + ...uxml2argv-serial-tcp-tlsx509-chardev-notls.args | 30 ++++++++++ ...muxml2argv-serial-tcp-tlsx509-chardev-notls.xml | 50 ++++++++++++++++ tests/qemuxml2argvtest.c | 3 + ...xml2xmlout-serial-tcp-tlsx509-chardev-notls.xml | 1 + tests/qemuxml2xmltest.c | 1 + 14 files changed, 227 insertions(+), 4 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..0cd68a0 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6204,6 +6204,34 @@ 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 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 a TLS environment can + be controlled on the host by the <code>chardev_tls</code> and + <code>chardev_tls_x509_cert_dir</code> or + <code>default_tls_x509_cert_dir</code> settings in the file + /etc/libvirt/qemu.conf. If <code>chardev_tls</code> is enabled, + then unless the <code>tls</code> attribute is set to "no", libvirt + will use the host configured TLS environment. + If <code>chardev_tls</code> is disabled, but the <code>tls</code> + attribute is set to "yes", then libvirt will attempt to use the + host TLS environment if either the <code>chardev_tls_x509_cert_dir</code> + or <code>default_tls_x509_cert_dir</code> TLS directory structure exists. + </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 35cdbc3..6e814b3 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: @@ -10039,6 +10041,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *master = NULL; char *slave = NULL; char *append = NULL; + char *haveTLS = NULL; int remaining = 0;
while (cur != NULL) { @@ -10046,6 +10049,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: @@ -10222,6 +10227,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 = @@ -10306,6 +10320,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(append); VIR_FREE(logappend); VIR_FREE(logfile); + VIR_FREE(haveTLS);
return remaining;
@@ -21492,7 +21507,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 7fc1141..f1da9c3 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1095,6 +1095,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 5713d18..6bf6510 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4935,7 +4935,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, if (dev->data.tcp.listen) virBufferAdd(&buf, nowait ? ",server,nowait" : ",server", -1);
- if (cfg->chardevTLS) { + if (dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) { char *objalias = NULL;
if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f11bc01..6cffff0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6186,6 +6186,72 @@ qemuDomainPrepareChannel(virDomainChrDefPtr channel, }
+/* qemuProcessPrepareDomainChardevSourceTLS: + * @source: pointer to host interface data for char devices + * @cfg: driver configuration + * + * Updates host interface TLS encryption setting based on qemu.conf + * for char devices. This will be presented as "tls='yes|no'" in + * live XML of a guest. + */ +void +qemuDomainPrepareChardevSourceTLS(virDomainChrSourceDefPtr source, + virQEMUDriverConfigPtr cfg) +{ + if (source->type == VIR_DOMAIN_CHR_TYPE_TCP) { + if (source->data.tcp.haveTLS == VIR_TRISTATE_BOOL_ABSENT) { + if (cfg->chardevTLS) + source->data.tcp.haveTLS = VIR_TRISTATE_BOOL_YES; + else + source->data.tcp.haveTLS = VIR_TRISTATE_BOOL_NO; + } + } +} + + +/* qemuProcessPrepareDomainChardevSource: + * @def: live domain definition + * @driver: qemu driver + * + * Iterate through all devices that use virDomainChrSourceDefPtr as host + * interface part. + */ +void +qemuDomainPrepareChardevSource(virDomainDefPtr def, + virQEMUDriverPtr driver) +{ + size_t i; + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + for (i = 0; i < def->nserials; i++) + qemuDomainPrepareChardevSourceTLS(def->serials[i]->source, cfg); + + for (i = 0; i < def->nparallels; i++) + qemuDomainPrepareChardevSourceTLS(def->parallels[i]->source, cfg); + + for (i = 0; i < def->nchannels; i++) + qemuDomainPrepareChardevSourceTLS(def->channels[i]->source, cfg); + + for (i = 0; i < def->nconsoles; i++) + qemuDomainPrepareChardevSourceTLS(def->consoles[i]->source, cfg);
I suppose these work, but in a way do open code the virDomainChrDefForeach... IDC either way ACK w/ the commit message adjusted
+ + for (i = 0; i < def->nrngs; i++) + if (def->rngs[i]->backend == VIR_DOMAIN_RNG_BACKEND_EGD) + qemuDomainPrepareChardevSourceTLS(def->rngs[i]->source.chardev, cfg); + + for (i = 0; i < def->nsmartcards; i++) + if (def->smartcards[i]->type == VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH) + qemuDomainPrepareChardevSourceTLS(def->smartcards[i]->data.passthru, + cfg); + + for (i = 0; i < def->nredirdevs; i++) + qemuDomainPrepareChardevSourceTLS(def->redirdevs[i]->source, cfg); + + virObjectUnref(cfg); +} + + + int qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem) { diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 29125a2..4f9bf82 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -739,6 +739,14 @@ int qemuDomainPrepareChannel(virDomainChrDefPtr chr, const char *domainChannelTargetDir) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+void qemuDomainPrepareChardevSourceTLS(virDomainChrSourceDefPtr source, + virQEMUDriverConfigPtr cfg) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +void qemuDomainPrepareChardevSource(virDomainDefPtr def, + virQEMUDriverPtr driver) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + int qemuDomainPrepareShmemChardev(virDomainShmemDefPtr shmem) ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 706b736..cf69945 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1482,7 +1482,8 @@ qemuDomainGetChardevTLSObjects(virQEMUDriverConfigPtr cfg, virJSONValuePtr *tlsProps, char **tlsAlias) { - if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP || !cfg->chardevTLS) + if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP || + dev->data.tcp.haveTLS != VIR_TRISTATE_BOOL_YES) return 0;
if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir, @@ -1517,6 +1518,8 @@ int qemuDomainAttachRedirdevDevice(virQEMUDriverPtr driver, char *tlsAlias = NULL; virErrorPtr orig_err;
+ qemuDomainPrepareChardevSourceTLS(redirdev->source, cfg); + if (qemuAssignDeviceRedirdevAlias(def, redirdev, -1) < 0) goto cleanup;
@@ -1771,6 +1774,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, qemuDomainPrepareChannel(chr, priv->channelTargetDir) < 0) goto cleanup;
+ qemuDomainPrepareChardevSourceTLS(dev, cfg); + if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0) goto cleanup;
@@ -1901,6 +1906,9 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, goto cleanup; }
+ if (rng->backend == VIR_DOMAIN_RNG_BACKEND_EGD) + qemuDomainPrepareChardevSourceTLS(rng->source.chardev, cfg); + /* build required metadata */ if (!(devstr = qemuBuildRNGDevStr(vm->def, rng, priv->qemuCaps))) goto cleanup; @@ -4476,7 +4484,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, goto cleanup;
if (tmpChr->source->type == VIR_DOMAIN_CHR_TYPE_TCP && - cfg->chardevTLS && + tmpChr->source->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES && !(objAlias = qemuAliasTLSObjFromChardevAlias(charAlias))) goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a1e2896..33b78b1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5167,6 +5167,8 @@ qemuProcessPrepareDomain(virConnectPtr conn, goto cleanup; }
+ qemuDomainPrepareChardevSource(vm->def, driver); + if (VIR_ALLOC(priv->monConfig) < 0) goto cleanup;
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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 3e9f825..52d85fa 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1167,6 +1167,9 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_OBJECT_TLS_CREDS_X509); driver.config->chardevTLSx509verify = 0; + 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/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 95c0bf2..64da80a 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);

Since TLS feature was introduced in libvirt 2.3.0 we have to modify migratable XML for specific case where 'tls' attribute is based on setting from qemu.conf. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 24 +++++++++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 1 + 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6e814b3..f556e4c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1999,6 +1999,7 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest, return -1; dest->data.tcp.haveTLS = src->data.tcp.haveTLS; + dest->data.tcp.tlsFromConfig = src->data.tcp.tlsFromConfig; break; case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -10042,6 +10043,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *slave = NULL; char *append = NULL; char *haveTLS = NULL; + char *tlsFromConfig = NULL; int remaining = 0; while (cur != NULL) { @@ -10051,6 +10053,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, mode = virXMLPropString(cur, "mode"); if (!haveTLS) haveTLS = virXMLPropString(cur, "tls"); + if (!tlsFromConfig) + tlsFromConfig = virXMLPropString(cur, "tlsFromConfig"); switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_FILE: @@ -10236,6 +10240,18 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, goto error; } + if (tlsFromConfig && + flags & VIR_DOMAIN_DEF_PARSE_STATUS) { + int tmp; + if (virStrToLong_i(tlsFromConfig, NULL, 10, &tmp) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid tlsFromConfig value: %s"), + tlsFromConfig); + goto error; + } + def->data.tcp.tlsFromConfig = !!tmp; + } + if (!protocol) def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW; else if ((def->data.tcp.protocol = @@ -10321,6 +10337,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(logappend); VIR_FREE(logfile); VIR_FREE(haveTLS); + VIR_FREE(tlsFromConfig); return remaining; @@ -21508,9 +21525,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf, def->data.tcp.listen ? "bind" : "connect"); virBufferEscapeString(buf, "host='%s' ", def->data.tcp.host); virBufferEscapeString(buf, "service='%s'", def->data.tcp.service); - if (def->data.tcp.haveTLS != VIR_TRISTATE_BOOL_ABSENT) + if (def->data.tcp.haveTLS != VIR_TRISTATE_BOOL_ABSENT && + !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && + def->data.tcp.tlsFromConfig)) virBufferAsprintf(buf, " tls='%s'", virTristateBoolTypeToString(def->data.tcp.haveTLS)); + if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) + virBufferAsprintf(buf, " tlsFromConfig='%d'", + def->data.tcp.tlsFromConfig); virBufferAddLit(buf, "/>\n"); virBufferAsprintf(buf, "<protocol type='%s'/>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1da9c3..dff28c0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1096,6 +1096,7 @@ struct _virDomainChrSourceDef { int protocol; bool tlscreds; int haveTLS; /* enum virTristateBool */ + bool tlsFromConfig; } tcp; struct { char *bindHost; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6cffff0..41ac52d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6204,6 +6204,7 @@ qemuDomainPrepareChardevSourceTLS(virDomainChrSourceDefPtr source, source->data.tcp.haveTLS = VIR_TRISTATE_BOOL_YES; else source->data.tcp.haveTLS = VIR_TRISTATE_BOOL_NO; + source->data.tcp.tlsFromConfig = true; } } } -- 2.10.1

On 10/24/2016 08:41 AM, Pavel Hrdina wrote:
Since TLS feature was introduced in libvirt 2.3.0 we have to modify
Since TLS was introduced hostwide for libvirt 2.3.0 and a domain configurable haveTLS was implemented for libvirt 2.4.0, we have to modify the
migratable XML for specific case where 'tls' attribute is based on
s/where/where the/
setting from qemu.conf.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 24 +++++++++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 1 + 3 files changed, 25 insertions(+), 1 deletion(-)
TBH this is ultimately very odd/confusing to read. The "tlsFromConfig" doesn't end up in the config XML right? Just the migratable. ACK since you've shown in the previous series that the permutations work. Perhaps an extra word or two in the commit message regarding the flag and how it's used will be helpful for the person who reads the code once we've both (shortly) forgotten about it John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6e814b3..f556e4c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1999,6 +1999,7 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest, return -1;
dest->data.tcp.haveTLS = src->data.tcp.haveTLS; + dest->data.tcp.tlsFromConfig = src->data.tcp.tlsFromConfig; break;
case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -10042,6 +10043,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *slave = NULL; char *append = NULL; char *haveTLS = NULL; + char *tlsFromConfig = NULL; int remaining = 0;
while (cur != NULL) { @@ -10051,6 +10053,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, mode = virXMLPropString(cur, "mode"); if (!haveTLS) haveTLS = virXMLPropString(cur, "tls"); + if (!tlsFromConfig) + tlsFromConfig = virXMLPropString(cur, "tlsFromConfig");
switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_FILE: @@ -10236,6 +10240,18 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, goto error; }
+ if (tlsFromConfig && + flags & VIR_DOMAIN_DEF_PARSE_STATUS) { + int tmp; + if (virStrToLong_i(tlsFromConfig, NULL, 10, &tmp) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid tlsFromConfig value: %s"), + tlsFromConfig); + goto error; + } + def->data.tcp.tlsFromConfig = !!tmp; + } + if (!protocol) def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW; else if ((def->data.tcp.protocol = @@ -10321,6 +10337,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(logappend); VIR_FREE(logfile); VIR_FREE(haveTLS); + VIR_FREE(tlsFromConfig);
return remaining;
@@ -21508,9 +21525,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf, def->data.tcp.listen ? "bind" : "connect"); virBufferEscapeString(buf, "host='%s' ", def->data.tcp.host); virBufferEscapeString(buf, "service='%s'", def->data.tcp.service); - if (def->data.tcp.haveTLS != VIR_TRISTATE_BOOL_ABSENT) + if (def->data.tcp.haveTLS != VIR_TRISTATE_BOOL_ABSENT && + !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && + def->data.tcp.tlsFromConfig)) virBufferAsprintf(buf, " tls='%s'", virTristateBoolTypeToString(def->data.tcp.haveTLS)); + if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) + virBufferAsprintf(buf, " tlsFromConfig='%d'", + def->data.tcp.tlsFromConfig); virBufferAddLit(buf, "/>\n");
virBufferAsprintf(buf, "<protocol type='%s'/>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1da9c3..dff28c0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1096,6 +1096,7 @@ struct _virDomainChrSourceDef { int protocol; bool tlscreds; int haveTLS; /* enum virTristateBool */ + bool tlsFromConfig; } tcp; struct { char *bindHost; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6cffff0..41ac52d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6204,6 +6204,7 @@ qemuDomainPrepareChardevSourceTLS(virDomainChrSourceDefPtr source, source->data.tcp.haveTLS = VIR_TRISTATE_BOOL_YES; else source->data.tcp.haveTLS = VIR_TRISTATE_BOOL_NO; + source->data.tcp.tlsFromConfig = true; } } }

On Mon, Oct 24, 2016 at 10:11:53AM -0400, John Ferlan wrote:
On 10/24/2016 08:41 AM, Pavel Hrdina wrote:
Since TLS feature was introduced in libvirt 2.3.0 we have to modify
Since TLS was introduced hostwide for libvirt 2.3.0 and a domain configurable haveTLS was implemented for libvirt 2.4.0, we have to modify the
migratable XML for specific case where 'tls' attribute is based on
s/where/where the/
setting from qemu.conf.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 24 +++++++++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 1 + 3 files changed, 25 insertions(+), 1 deletion(-)
TBH this is ultimately very odd/confusing to read. The "tlsFromConfig" doesn't end up in the config XML right? Just the migratable.
The "tlsFromConfig" is libvirt internal attribute and is stored only in status XML to ensure that when libvirtd is restarted this internal flag is not lost by the restart. That flag is used to decide whether we should put *tls* attribute to migratable XML or not. I'll add it to the commit message to make it clear. I'll wait with pushing for your reply. Pavel
ACK since you've shown in the previous series that the permutations work. Perhaps an extra word or two in the commit message regarding the flag and how it's used will be helpful for the person who reads the code once we've both (shortly) forgotten about it
John
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6e814b3..f556e4c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1999,6 +1999,7 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest, return -1;
dest->data.tcp.haveTLS = src->data.tcp.haveTLS; + dest->data.tcp.tlsFromConfig = src->data.tcp.tlsFromConfig; break;
case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -10042,6 +10043,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *slave = NULL; char *append = NULL; char *haveTLS = NULL; + char *tlsFromConfig = NULL; int remaining = 0;
while (cur != NULL) { @@ -10051,6 +10053,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, mode = virXMLPropString(cur, "mode"); if (!haveTLS) haveTLS = virXMLPropString(cur, "tls"); + if (!tlsFromConfig) + tlsFromConfig = virXMLPropString(cur, "tlsFromConfig");
switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_FILE: @@ -10236,6 +10240,18 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, goto error; }
+ if (tlsFromConfig && + flags & VIR_DOMAIN_DEF_PARSE_STATUS) { + int tmp; + if (virStrToLong_i(tlsFromConfig, NULL, 10, &tmp) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid tlsFromConfig value: %s"), + tlsFromConfig); + goto error; + } + def->data.tcp.tlsFromConfig = !!tmp; + } + if (!protocol) def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW; else if ((def->data.tcp.protocol = @@ -10321,6 +10337,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(logappend); VIR_FREE(logfile); VIR_FREE(haveTLS); + VIR_FREE(tlsFromConfig);
return remaining;
@@ -21508,9 +21525,14 @@ virDomainChrSourceDefFormat(virBufferPtr buf, def->data.tcp.listen ? "bind" : "connect"); virBufferEscapeString(buf, "host='%s' ", def->data.tcp.host); virBufferEscapeString(buf, "service='%s'", def->data.tcp.service); - if (def->data.tcp.haveTLS != VIR_TRISTATE_BOOL_ABSENT) + if (def->data.tcp.haveTLS != VIR_TRISTATE_BOOL_ABSENT && + !(flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE && + def->data.tcp.tlsFromConfig)) virBufferAsprintf(buf, " tls='%s'", virTristateBoolTypeToString(def->data.tcp.haveTLS)); + if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) + virBufferAsprintf(buf, " tlsFromConfig='%d'", + def->data.tcp.tlsFromConfig); virBufferAddLit(buf, "/>\n");
virBufferAsprintf(buf, "<protocol type='%s'/>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f1da9c3..dff28c0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1096,6 +1096,7 @@ struct _virDomainChrSourceDef { int protocol; bool tlscreds; int haveTLS; /* enum virTristateBool */ + bool tlsFromConfig; } tcp; struct { char *bindHost; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6cffff0..41ac52d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6204,6 +6204,7 @@ qemuDomainPrepareChardevSourceTLS(virDomainChrSourceDefPtr source, source->data.tcp.haveTLS = VIR_TRISTATE_BOOL_YES; else source->data.tcp.haveTLS = VIR_TRISTATE_BOOL_NO; + source->data.tcp.tlsFromConfig = true; } } }
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 10/24/2016 10:28 AM, Pavel Hrdina wrote:
On Mon, Oct 24, 2016 at 10:11:53AM -0400, John Ferlan wrote:
On 10/24/2016 08:41 AM, Pavel Hrdina wrote:
Since TLS feature was introduced in libvirt 2.3.0 we have to modify
Since TLS was introduced hostwide for libvirt 2.3.0 and a domain configurable haveTLS was implemented for libvirt 2.4.0, we have to modify the
migratable XML for specific case where 'tls' attribute is based on
s/where/where the/
setting from qemu.conf.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 24 +++++++++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 1 + 3 files changed, 25 insertions(+), 1 deletion(-)
TBH this is ultimately very odd/confusing to read. The "tlsFromConfig" doesn't end up in the config XML right? Just the migratable.
The "tlsFromConfig" is libvirt internal attribute and is stored only in status XML to ensure that when libvirtd is restarted this internal flag is not lost by the restart.
That flag is used to decide whether we should put *tls* attribute to migratable XML or not. I'll add it to the commit message to make it clear.
I'll wait with pushing for your reply.
That's fine - John
Pavel
ACK since you've shown in the previous series that the permutations work. Perhaps an extra word or two in the commit message regarding the flag and how it's used will be helpful for the person who reads the code once we've both (shortly) forgotten about it
John
[...]

On Mon, Oct 24, 2016 at 10:31:42AM -0400, John Ferlan wrote:
On 10/24/2016 10:28 AM, Pavel Hrdina wrote:
On Mon, Oct 24, 2016 at 10:11:53AM -0400, John Ferlan wrote:
On 10/24/2016 08:41 AM, Pavel Hrdina wrote:
Since TLS feature was introduced in libvirt 2.3.0 we have to modify
Since TLS was introduced hostwide for libvirt 2.3.0 and a domain configurable haveTLS was implemented for libvirt 2.4.0, we have to modify the
migratable XML for specific case where 'tls' attribute is based on
s/where/where the/
setting from qemu.conf.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/conf/domain_conf.c | 24 +++++++++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 1 + 3 files changed, 25 insertions(+), 1 deletion(-)
TBH this is ultimately very odd/confusing to read. The "tlsFromConfig" doesn't end up in the config XML right? Just the migratable.
The "tlsFromConfig" is libvirt internal attribute and is stored only in status XML to ensure that when libvirtd is restarted this internal flag is not lost by the restart.
That flag is used to decide whether we should put *tls* attribute to migratable XML or not. I'll add it to the commit message to make it clear.
I'll wait with pushing for your reply.
That's fine -
Thanks, pushed now. Pavel
participants (2)
-
John Ferlan
-
Pavel Hrdina