[libvirt] [PATCH v6 0/5] Add native TLS encrypted chardev TCP support
v5: http://www.redhat.com/archives/libvir-list/2016-August/msg00282.html Patches 1-5 from that series already pushed Patch 6 from that series is removed Patch 7 is untouched and is patch 3 in this series Patches 8-9 modified for new paradigm (patches 4-5 of this series) Patch 1 [NEW] From patch 6 review comment - provide a way to allow disabling the TLS for a specific guest and serial TCP chardev. A new test was added which shows that the xml2argv output does not add the TLS object within the same "frame" as the test that would add it. Patch 2 [NEW] Rather than use a <secret> object for the serial TCP chardev, create a qemu.conf option as a host wide setting. The patch implements a 'chardev_tls_x509_secret_uuid' value which will be used by patches 4-5 rather than the serial TCP specific setting. Patch 3 is unchanged Patches 4-5 were adjusted in order to use the chardevTCPtlsx509secretUUID rather than the chardev TCP specific 'chardev->source.data.tcp.seclookupdef' value sourced from a <secret>. John Ferlan (5): domain: Add optional 'disableTLS' 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 | 20 +++ 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 | 34 ++++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_conf.c | 22 ++++ src/qemu/qemu_conf.h | 3 + src/qemu/qemu_domain.c | 143 ++++++++++++++++++++- src/qemu/qemu_domain.h | 30 ++++- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hotplug.c | 63 ++++++++- 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 +- tests/qemuhotplugtest.c | 2 +- ...argv-serial-tcp-tlsx509-chardev-disableTLS.args | 30 +++++ ...2argv-serial-tcp-tlsx509-chardev-disableTLS.xml | 50 +++++++ ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 ++++++ ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++++++ tests/qemuxml2argvtest.c | 22 ++++ ...mlout-serial-tcp-tlsx509-chardev-disableTLS.xml | 1 + tests/qemuxml2xmltest.c | 1 + 30 files changed, 588 insertions(+), 34 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.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-disableTLS.xml -- 2.7.4
Add an optional "disableTLS='yes'" option for a TCP chardev for the express purpose to not enable setting up TLS for the chardev Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 20 +++++++++ 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_hotplug.c | 2 +- ...argv-serial-tcp-tlsx509-chardev-disableTLS.args | 30 +++++++++++++ ...2argv-serial-tcp-tlsx509-chardev-disableTLS.xml | 50 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 3 ++ ...mlout-serial-tcp-tlsx509-chardev-disableTLS.xml | 1 + tests/qemuxml2xmltest.c | 1 + 11 files changed, 134 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-disableTLS.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f48a4d8..82ccf0f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6192,6 +6192,26 @@ qemu-kvm -net nic,model=? /dev/null </devices> ...</pre> + <p> + <span class="since">Since 2.3.0,</span> some hypervisors support using + a TLS X.509 certificate environment in order to encrypt all serial TCP + connections via a hypervisor configuration option. In some instances + this may not be desired, so in order to disable TLS for a specific + chardev an optional attribute <code>disableTLS</code> is available. + By setting this to "yes" it will be possible for a hypervisor to not + use the TLS encryption for the particular character device. + </p> +<pre> + ... + <devices> + <serial type="tcp"> + <source mode='connect' host="127.0.0.1" service="5555" disableTLS="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 8aaa67e..26dd81c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3452,6 +3452,11 @@ <ref name="virOnOff"/> </attribute> </optional> + <optional> + <attribute name="disableTLS"> + <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 0828041..f16d417 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1960,6 +1960,8 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest, if (VIR_STRDUP(dest->data.tcp.service, src->data.tcp.service) < 0) return -1; + + dest->data.tcp.disableTLS = src->data.tcp.disableTLS; break; case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -9955,6 +9957,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *master = NULL; char *slave = NULL; char *append = NULL; + char *disableTLS = NULL; int remaining = 0; while (cur != NULL) { @@ -9962,6 +9965,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, if (xmlStrEqual(cur->name, BAD_CAST "source")) { if (!mode) mode = virXMLPropString(cur, "mode"); + if (!disableTLS) + disableTLS = virXMLPropString(cur, "disableTLS"); switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_FILE: @@ -10138,6 +10143,15 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, def->data.tcp.listen = true; } + if (disableTLS && + (def->data.tcp.disableTLS = + virTristateBoolTypeFromString(disableTLS)) <= 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown chardev disableTLS setting '%s'"), + disableTLS); + goto error; + } + if (!protocol) def->data.tcp.protocol = VIR_DOMAIN_CHR_TCP_PROTOCOL_RAW; else if ((def->data.tcp.protocol = @@ -10221,6 +10235,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(append); VIR_FREE(logappend); VIR_FREE(logfile); + VIR_FREE(disableTLS); return remaining; @@ -21340,7 +21355,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.disableTLS != VIR_TRISTATE_SWITCH_ABSENT) + virBufferAsprintf(buf, " disableTLS='%s'", + virTristateBoolTypeToString(def->data.tcp.disableTLS)); + 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 c14a39c..e94f6bb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1093,6 +1093,7 @@ struct _virDomainChrSourceDef { bool listen; int protocol; bool tlscreds; + int disableTLS; /* enum virTristateBool */ } tcp; struct { char *bindHost; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e0b0401..1fd57b0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5057,7 +5057,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, telnet ? ",telnet" : "", dev->data.tcp.listen ? ",server,nowait" : ""); - if (cfg->chardevTLS) { + if (cfg->chardevTLS && !dev->data.tcp.disableTLS) { char *objalias = NULL; if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index df374b1..4aa85e7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1678,7 +1678,7 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, if (qemuDomainChrPreInsert(vmdef, chr) < 0) goto cleanup; - if (cfg->chardevTLS) { + if (cfg->chardevTLS && !dev->data.tcp.disableTLS) { if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir, dev->data.tcp.listen, cfg->chardevTLSx509verify, diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.args new file mode 100644 index 0000000..cac0d85 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.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-disableTLS.xml b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.xml new file mode 100644 index 0000000..0b35fa7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.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' disableTLS='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 12fd732..ca48056 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1119,6 +1119,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-disableTLS", + 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-disableTLS.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-disableTLS.xml new file mode 120000 index 0000000..9c7559b --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-disableTLS.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-disableTLS.xml \ No newline at end of file diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 14c2b0c..f702f13 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-disableTLS", NONE); DO_TEST("serial-many", NONE); DO_TEST("serial-spiceport", NONE); DO_TEST("serial-spiceport-nospice", NONE); -- 2.7.4
On Fri, Sep 09, 2016 at 04:49:04PM -0400, John Ferlan wrote:
Add an optional "disableTLS='yes'" option for a TCP chardev for the express purpose to not enable setting up TLS for the chardev
Can we just call this "tls=yes|no" - negative attributes (disableFOO) are kind of ugly IMHO, as they imply that the default status is enabled, which is not really the case - the default is hypervisor specific and undefined. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 09/12/2016 04:26 AM, Daniel P. Berrange wrote:
On Fri, Sep 09, 2016 at 04:49:04PM -0400, John Ferlan wrote:
Add an optional "disableTLS='yes'" option for a TCP chardev for the express purpose to not enable setting up TLS for the chardev
Can we just call this "tls=yes|no" - negative attributes (disableFOO) are kind of ugly IMHO, as they imply that the default status is enabled, which is not really the case - the default is hypervisor specific and undefined.
That's fine - just changes subsequent patches slightly and alters the description slightly. Is a domain_conf.c name like haveTLS OK? I need to differentiate from 'tlscreds' which is used internally to add the 'tls-creds' and would true when "both" tls='yes' is set in XML and 'chardev_tls=1' in qemu.conf is set. John
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. The chardevTLSx509haveUUID bool will be used as the marker for whether the chardevTLSx509secretUUID was successfully read. In the future this is how it'd determined whether to add the secret object for a TLS object. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/libvirtd_qemu.aug | 2 ++ src/qemu/qemu.conf | 24 ++++++++++++++++++++++++ src/qemu/qemu_conf.c | 22 ++++++++++++++++++++++ src/qemu/qemu_conf.h | 3 +++ src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 5 files changed, 53 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..7114fa1 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -28,6 +28,20 @@ # #default_tls_x509_verify = 1 +# +# In order to provide a password to unlock the private key to be used +# in order to provide the TLS credentials, a libvirt secret will need +# to be created and then the UUID of that secret added as a configuration +# parameter. See the libvirt documentation for specific details regarding +# how to create a "tls" secret type. +# +# 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 e7b2d8d..c735357 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--; @@ -424,6 +426,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, int ret = -1; int rv; size_t i, j; + bool defaultTLSx509haveUUID; char *stdioHandler = NULL; char *user = NULL, *group = NULL; char **controllers = NULL; @@ -446,6 +449,12 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; if (virConfGetValueBool(conf, "default_tls_x509_verify", &cfg->defaultTLSx509verify) < 0) goto cleanup; + if ((rv = virConfGetValueString(conf, "default_tls_x509_secret_uuid", + &cfg->defaultTLSx509secretUUID)) < 0) + goto cleanup; + if (rv == 1) + defaultTLSx509haveUUID = true; + if (virConfGetValueBool(conf, "vnc_auto_unix_socket", &cfg->vncAutoUnixSocket) < 0) goto cleanup; if (virConfGetValueBool(conf, "vnc_tls", &cfg->vncTLS) < 0) @@ -513,6 +522,19 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto cleanup; if (rv == 0) cfg->chardevTLSx509verify = cfg->defaultTLSx509verify; + if ((rv = virConfGetValueString(conf, "chardev_tls_x509_secret_uuid", + &cfg->chardevTLSx509secretUUID)) < 0) + goto cleanup; + if (rv == 1) { + cfg->chardevTLSx509haveUUID = true; + } else { + if (defaultTLSx509haveUUID) { + if (VIR_STRDUP(cfg->chardevTLSx509secretUUID, + cfg->defaultTLSx509secretUUID) < 0) + goto cleanup; + cfg->chardevTLSx509haveUUID = true; + } + } 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 d8232cc..2887ba6 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,8 @@ struct _virQEMUDriverConfig { bool chardevTLS; char *chardevTLSx509certdir; bool chardevTLSx509verify; + bool chardevTLSx509haveUUID; + 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
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@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 +- 9 files changed, 88 insertions(+), 15 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f16d417..333e72c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2089,6 +2089,8 @@ void virDomainChrDefFree(virDomainChrDefPtr def) VIR_FREE(def->seclabels); } + virObjectUnref(def->privateData); + VIR_FREE(def); } @@ -10249,7 +10251,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, * default port. */ virDomainChrDefPtr -virDomainChrDefNew(void) +virDomainChrDefNew(virDomainXMLOptionPtr xmlopt) { virDomainChrDefPtr def = NULL; @@ -10257,6 +10259,11 @@ virDomainChrDefNew(void) return NULL; def->target.port = -1; + + if (xmlopt && xmlopt->privateData.chardevNew && + !(def->privateData = xmlopt->privateData.chardevNew())) + VIR_FREE(def); + return def; } @@ -10304,7 +10311,8 @@ virDomainChrDefNew(void) * */ static virDomainChrDefPtr -virDomainChrDefParseXML(xmlXPathContextPtr ctxt, +virDomainChrDefParseXML(virDomainXMLOptionPtr xmlopt, + xmlXPathContextPtr ctxt, xmlNodePtr node, virSecurityLabelDefPtr* vmSeclabels, int nvmSeclabels, @@ -10316,7 +10324,7 @@ virDomainChrDefParseXML(xmlXPathContextPtr ctxt, virDomainChrDefPtr def; bool seenTarget = false; - if (!(def = virDomainChrDefNew())) + if (!(def = virDomainChrDefNew(xmlopt))) return NULL; type = virXMLPropString(node, "type"); @@ -13493,7 +13501,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, @@ -17040,7 +17049,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, @@ -17067,7 +17077,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, @@ -17096,7 +17107,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, @@ -17115,7 +17127,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 e94f6bb..5e163e0 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1117,6 +1117,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 || @@ -2434,6 +2435,7 @@ struct _virDomainXMLPrivateDataCallbacks { virDomainXMLPrivateDataNewFunc diskNew; virDomainXMLPrivateDataNewFunc hostdevNew; virDomainXMLPrivateDataNewFunc vcpuNew; + virDomainXMLPrivateDataNewFunc chardevNew; virDomainXMLPrivateDataFormatFunc format; virDomainXMLPrivateDataParseFunc parse; }; @@ -2566,7 +2568,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 43f4a7f..ffdd21e 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -387,7 +387,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 94fb659..8d980a4 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 6c57ed2..cab1f24 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 13c0372..a4ead86 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -349,6 +349,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 82d1621..6540d60 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 40dc53c..e586e24 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; -- 2.7.4
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@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 | 19 +++++ 9 files changed, 253 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 1fd57b0..b9179f0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -683,6 +683,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 * @@ -694,6 +695,7 @@ int qemuBuildTLSx509BackendProps(const char *tlspath, bool listen, bool verifypeer, + const char *secalias, virQEMUCapsPtr qemuCaps, virJSONValuePtr *propsret) { @@ -719,6 +721,10 @@ qemuBuildTLSx509BackendProps(const char *tlspath, NULL) < 0) goto cleanup; + if (secalias && + virJSONValueObjectAdd(*propsret, "s:passwordid", secalias, NULL) < 0) + goto cleanup; + ret = 0; cleanup: @@ -733,6 +739,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 * @@ -745,6 +752,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, const char *tlspath, bool listen, bool verifypeer, + bool addpasswordid, const char *inalias, virQEMUCapsPtr qemuCaps) { @@ -752,11 +760,16 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, char *objalias = NULL; virJSONValuePtr props = NULL; char *tmp = NULL; + char *secalias = NULL; - if (qemuBuildTLSx509BackendProps(tlspath, listen, verifypeer, - qemuCaps, &props) < 0) + if (addpasswordid && + !(secalias = qemuDomainGetSecretAESAlias(inalias, false))) return -1; + if (qemuBuildTLSx509BackendProps(tlspath, listen, verifypeer, secalias, + qemuCaps, &props) < 0) + goto cleanup; + if (!(objalias = qemuAliasTLSObjFromChardevAlias(inalias))) goto cleanup; @@ -772,6 +785,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, virJSONValueFree(props); VIR_FREE(objalias); VIR_FREE(tmp); + VIR_FREE(secalias); return ret; } @@ -5063,6 +5077,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir, dev->data.tcp.listen, cfg->chardevTLSx509verify, + cfg->chardevTLSx509haveUUID, alias, qemuCaps) < 0) goto error; @@ -8708,6 +8723,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->chardevTLSx509haveUUID && + 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 b7c59bb..5d303ce 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 listen, bool verifypeer, + const char *secalias, virQEMUCapsPtr qemuCaps, virJSONValuePtr *propsret); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cab1f24..434bad1 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->chardevTLSx509haveUUID) { + 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 a4ead86..3e4e21e 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -723,11 +723,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 4aa85e7..eb5de7b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1682,6 +1682,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 7596579..9ca7f25 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4954,7 +4954,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))) @@ -5020,8 +5019,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++) { @@ -5046,7 +5045,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..832e2a2 --- /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'/> + <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 ca48056..fa190e9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1122,6 +1122,25 @@ mymain(void) DO_TEST("serial-tcp-tlsx509-chardev-disableTLS", 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; + driver.config->chardevTLSx509haveUUID = true; +# 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
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@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 d044a8c..e54e258 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7489,7 +7489,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 eb5de7b..3936a14 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1639,7 +1639,8 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, return ret; } -int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, +int qemuDomainAttachChrDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainChrDefPtr chr) { @@ -1653,8 +1654,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 && @@ -1678,11 +1682,29 @@ 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.disableTLS) { + 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; @@ -1693,6 +1715,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); @@ -1723,6 +1754,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); @@ -1730,6 +1763,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 */ @@ -4319,6 +4354,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))) { @@ -4332,9 +4368,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->chardevTLSx509haveUUID && + 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; @@ -4348,6 +4396,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 0a5f068..f84908f 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -117,7 +117,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
participants (2)
-
Daniel P. Berrange -
John Ferlan