[libvirt] [PATCH v10 2/4] domain: Add optional 'tls' attribute for TCP chardev
Pavel Hrdina
phrdina at redhat.com
Thu Oct 20 06:51:45 UTC 2016
On Wed, Oct 19, 2016 at 04:53:54PM -0400, John Ferlan 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.
>
> Signed-off-by: John Ferlan <jferlan at 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 | 20 +++++++--
> src/qemu/qemu_domain.h | 3 +-
> src/qemu/qemu_hotplug.c | 4 +-
> ...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 +
> 13 files changed, 162 insertions(+), 8 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..da6be67 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 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 a TLS envronment 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>
Nice, this is a good description how to use the *tls* attribute.
> +<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 89473db..e4fa9ad 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:
> @@ -10040,6 +10042,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> char *master = NULL;
> char *slave = NULL;
> char *append = NULL;
> + char *haveTLS = NULL;
> int remaining = 0;
>
> while (cur != NULL) {
> @@ -10047,6 +10050,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:
> @@ -10223,6 +10228,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 =
> @@ -10307,6 +10321,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> VIR_FREE(append);
> VIR_FREE(logappend);
> VIR_FREE(logfile);
> + VIR_FREE(haveTLS);
>
> return remaining;
>
> @@ -21466,7 +21481,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 04f2e40..fcadf6c 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 d45a7de..f00751a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4943,7 +4943,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> if (dev->data.tcp.listen)
> virBufferAdd(&buf, nowait ? ",server,nowait" : ",server", -1);
>
> - if (qemuDomainSupportTLSChardevTCP(cfg)) {
> + if (qemuDomainSupportTLSChardevTCP(cfg, dev)) {
> char *objalias = NULL;
>
> if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 746d94f..7b518c6 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -6302,15 +6302,29 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
>
> /* qemuDomainSupportTLSChardevTCP
> * @cfg: Pointer to driver cfg
> + * @dev: Pointer to chardev source
> *
> - * Let's check if this host supports using the TLS environment for chardev.
> + * Let's check if this host and/or domain supports or desires to use
> + * the TLS environment for the passed chardev TCP.
> + *
> + * If we have an environment and as long as the domain config doesn't have
> + * the "tls='no'" property, then we assume it's desired.
> + *
> + * If the host global isn't set, but the domain chardev config is requesting
> + * to use TLS and we find what appears to be some environment configured,
> + * then let's also try. This action could fail later in QEMU if the environment
> + * isn't set up to the exact specifications.
> *
> * Returns true if we want to use TLS, false otherwise.
> */
> bool
> -qemuDomainSupportTLSChardevTCP(virQEMUDriverConfigPtr cfg)
> +qemuDomainSupportTLSChardevTCP(virQEMUDriverConfigPtr cfg,
> + const virDomainChrSourceDef *dev)
> {
> - if (cfg->chardevTLS)
> + 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;
> }
So this function let's you decide whether we should try to set up *tls* for
chardev or not. It work's but I have few issues with it.
At first I don't like that libvirt would try to do something smart and don't
even tell user about the result. This will silently ignore the *tls*
attribute if no certificate is found. In case that *tls* attribute is set
to "yes" in XML and there is no certificate file to use we shouldn't start
that domain and print an error to user.
Secondly this way we don't reflect the current state for live domain in the
live XML. This was probably lost during the discussion, but in general if
there is an attribute that can affect running domain we should reflect the
current state using that attribute. I know, there are some cases where we
probably don't do that and they should be fixed.
I figure out that we cannot simply use haveTLS = cfg->chardevTLS but we
can set the haveTLS based on cfg->chardevTLS. The whole purpose of
qemuProcessPrepareDomain() is to prepare the domain definition so the
qemuBuildCommandLine() don't have to check other places to enable some
feature and not update the live definition. If the *tls* attribute is
properly set in live definition that it will be saved to status XML and
there is no need to do anything for qemuProcessReconnect.
Rest of the patch looks good.
Pavel
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 7ecafac..156e0fc 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -755,5 +755,6 @@ int qemuDomainCheckMonitor(virQEMUDriverPtr driver,
> bool qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
> virQEMUCapsPtr qemuCaps);
>
> -bool qemuDomainSupportTLSChardevTCP(virQEMUDriverConfigPtr cfg);
> +bool qemuDomainSupportTLSChardevTCP(virQEMUDriverConfigPtr cfg,
> + const virDomainChrSourceDef *dev);
> #endif /* __QEMU_DOMAIN_H__ */
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index c2b43b1..9643a68 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1730,7 +1730,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
> goto cleanup;
>
> if (dev->type == VIR_DOMAIN_CHR_TYPE_TCP &&
> - qemuDomainSupportTLSChardevTCP(cfg)) {
> + qemuDomainSupportTLSChardevTCP(cfg, dev)) {
> if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir,
> dev->data.tcp.listen,
> cfg->chardevTLSx509verify,
> @@ -4404,7 +4404,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver,
> goto cleanup;
>
> if (tmpChr->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
> - qemuDomainSupportTLSChardevTCP(cfg) &&
> + qemuDomainSupportTLSChardevTCP(cfg, &tmpChr->source) &&
> !(objAlias = qemuAliasTLSObjFromChardevAlias(charAlias)))
> 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.7.4
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161020/9d48a6fa/attachment-0001.sig>
More information about the libvir-list
mailing list