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@xxxxxxxxxx> > --- > 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@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list