[libvirt] [PATCH v7 0/5] Add native TLS encrypted chardev TCP support

v6: http://www.redhat.com/archives/libvir-list/2016-September/msg00302.html Differences over v6 - rather than disable on specific chardev's, modify the code to be need to enable by setting tls='yes' for the chardev John Ferlan (5): domain: Add optional 'tls' attribute for TCP chardev conf: Introduce {default|chardev}_tls_x509_secret_uuid qemu: Introduce qemuDomainChardevPrivatePtr qemu: Add a secret object to/for a chardev tcp with secret qemu: Add the ability to hotplug a secret object for TCP chardev TLS docs/formatdomain.html.in | 21 +++ docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c | 51 ++++++-- src/conf/domain_conf.h | 5 +- src/libxl/libxl_domain.c | 2 +- src/lxc/lxc_native.c | 2 +- src/qemu/libvirtd_qemu.aug | 2 + src/qemu/qemu.conf | 24 ++++ src/qemu/qemu_command.c | 35 ++++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_conf.c | 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 | 64 ++++++++- src/qemu/qemu_hotplug.h | 3 +- src/qemu/qemu_parse_command.c | 4 +- src/qemu/qemu_process.c | 6 +- src/qemu/test_libvirtd_qemu.aug.in | 2 + src/vz/vz_sdk.c | 2 +- src/xenconfig/xen_sxpr.c | 2 +- tests/qemuhotplugtest.c | 2 +- ...emuxml2argv-serial-tcp-tlsx509-chardev-tls.args | 30 +++++ ...qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml | 50 +++++++ .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 2 +- ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 ++++++ ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++++++ tests/qemuxml2argvtest.c | 22 ++++ ...muxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml | 1 + .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 2 +- tests/qemuxml2xmltest.c | 1 + 32 files changed, 593 insertions(+), 36 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.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-tls.xml -- 2.7.4

Add an optional "tls='yes'" option for a TCP chardev for the express purpose to enable setting up TLS for the chardev. This will assume that the qemu.conf settings have been adjusted as well as the environment configured properly. Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 21 +++++++++ docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 22 +++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 3 +- src/qemu/qemu_hotplug.c | 3 +- ...emuxml2argv-serial-tcp-tlsx509-chardev-tls.args | 30 +++++++++++++ ...qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml | 50 ++++++++++++++++++++++ .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 2 +- tests/qemuxml2argvtest.c | 3 ++ ...muxml2xmlout-serial-tcp-tlsx509-chardev-tls.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-tls.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml create mode 120000 tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index f48a4d8..8f2b981 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6192,6 +6192,27 @@ 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 order to enable + TLS for the domain an optional attribute <code>tls</code> can be set to + "yes". Combined with the hypervisor's capability to utilize the TLS + environment allows for the character device to use the encrypted + communication. If the attribute is not present, then the default + setting is "no". + </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 751d914..da869b2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3452,6 +3452,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 b4a3e92..d2de15b 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.haveTLS = src->data.tcp.haveTLS; break; case VIR_DOMAIN_CHR_TYPE_UNIX: @@ -9955,6 +9957,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, char *master = NULL; char *slave = NULL; char *append = NULL; + char *haveTLS = 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 (!haveTLS) + haveTLS = virXMLPropString(cur, "tls"); switch ((virDomainChrType) def->type) { case VIR_DOMAIN_CHR_TYPE_FILE: @@ -10138,6 +10143,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 = @@ -10221,6 +10235,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def, VIR_FREE(append); VIR_FREE(logappend); VIR_FREE(logfile); + VIR_FREE(haveTLS); 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.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 c14a39c..25d0787 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 haveTLS; /* enum virTristateBool */ } tcp; struct { char *bindHost; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 051a0bc..9683c9e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5041,7 +5041,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, telnet ? ",telnet" : "", dev->data.tcp.listen ? ",server,nowait" : ""); - if (cfg->chardevTLS) { + if (cfg->chardevTLS && + dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) { char *objalias = NULL; if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 72dd93b..419296e 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1677,7 +1677,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_YES) { if (qemuBuildTLSx509BackendProps(cfg->chardevTLSx509certdir, dev->data.tcp.listen, cfg->chardevTLSx509verify, diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.args b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.args new file mode 100644 index 0000000..cac0d85 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.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-tls.xml b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml new file mode 100644 index 0000000..debc69b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml @@ -0,0 +1,50 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <serial type='udp'> + <source mode='bind' host='127.0.0.1' service='1111'/> + <source mode='connect' host='127.0.0.1' service='2222'/> + <target port='0'/> + </serial> + <serial type='tcp'> + <source mode='connect' host='127.0.0.1' service='5555' tls='no'/> + <protocol type='raw'/> + <target port='0'/> + </serial> + <console type='udp'> + <source mode='bind' host='127.0.0.1' service='1111'/> + <source mode='connect' host='127.0.0.1' service='2222'/> + <target type='serial' port='0'/> + </console> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml index 1618b02..1d7896d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev.xml @@ -27,7 +27,7 @@ <target port='0'/> </serial> <serial type='tcp'> - <source mode='connect' host='127.0.0.1' service='5555'/> + <source mode='connect' host='127.0.0.1' service='5555' tls='yes'/> <protocol type='raw'/> <target port='0'/> </serial> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index e854077..7df76c1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1125,6 +1125,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-tls", + 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-tls.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml new file mode 120000 index 0000000..3453497 --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml @@ -0,0 +1 @@ +../qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.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 fb05c85..a035ccf 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-tls", NONE); DO_TEST("serial-many", NONE); DO_TEST("serial-spiceport", NONE); DO_TEST("serial-spiceport-nospice", NONE); -- 2.7.4

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 635fa27..c650961 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 d32689a..a7b01c9 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

On Mon, Sep 19, 2016 at 10:39:21AM -0400, John Ferlan wrote:
Add a new qemu.conf variables to store the UUID for the secret that could be used to present credentials to access the TLS chardev. Since this will be a server level and it's possible to use some sort of default, introduce both the default and chardev logic at the same time making the setting of the chardev check for it's own value, then if not present checking whether the default value had been set.
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"
We could perhaps be a little more explicit about the fact that when this is commented out, the private key is required to be in non-encrypted PEM format. 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/19/2016 10:53 AM, Daniel P. Berrange wrote:
On Mon, Sep 19, 2016 at 10:39:21AM -0400, John Ferlan wrote:
Add a new qemu.conf variables to store the UUID for the secret that could be used to present credentials to access the TLS chardev. Since this will be a server level and it's possible to use some sort of default, introduce both the default and chardev logic at the same time making the setting of the chardev check for it's own value, then if not present checking whether the default value had been set.
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"
We could perhaps be a little more explicit about the fact that when this is commented out, the private key is required to be in non-encrypted PEM format.
Fair enough - a simple enough addition, so at the end of the first paragraph (and repeated again for chardev_tls_x509_secret_uuid), how about: " A libvirt secret requires usage of a non-encrypted PEM format certificate." Or is there some other wording that is preferable? Tks - John

On Tue, Sep 20, 2016 at 08:26:55AM -0400, John Ferlan wrote:
On 09/19/2016 10:53 AM, Daniel P. Berrange wrote:
On Mon, Sep 19, 2016 at 10:39:21AM -0400, John Ferlan wrote:
Add a new qemu.conf variables to store the UUID for the secret that could be used to present credentials to access the TLS chardev. Since this will be a server level and it's possible to use some sort of default, introduce both the default and chardev logic at the same time making the setting of the chardev check for it's own value, then if not present checking whether the default value had been set.
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"
We could perhaps be a little more explicit about the fact that when this is commented out, the private key is required to be in non-encrypted PEM format.
Fair enough - a simple enough addition, so at the end of the first paragraph (and repeated again for chardev_tls_x509_secret_uuid), how about:
" A libvirt secret requires usage of a non-encrypted PEM format certificate."
Or is there some other wording that is preferable?
Something like this: "Libvirt assumes the server-key.pem file is unencrypted by default. To use an encrypted server-key.pem file, the password to decrypt the PEM file is requird. This can be provided by creating a secret object in libvirt and then uncommenting this setting to set the UUID of the secret" 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/20/2016 08:52 AM, Daniel P. Berrange wrote:
On Tue, Sep 20, 2016 at 08:26:55AM -0400, John Ferlan wrote:
On 09/19/2016 10:53 AM, Daniel P. Berrange wrote:
On Mon, Sep 19, 2016 at 10:39:21AM -0400, John Ferlan wrote:
Add a new qemu.conf variables to store the UUID for the secret that could be used to present credentials to access the TLS chardev. Since this will be a server level and it's possible to use some sort of default, introduce both the default and chardev logic at the same time making the setting of the chardev check for it's own value, then if not present checking whether the default value had been set.
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"
We could perhaps be a little more explicit about the fact that when this is commented out, the private key is required to be in non-encrypted PEM format.
Fair enough - a simple enough addition, so at the end of the first paragraph (and repeated again for chardev_tls_x509_secret_uuid), how about:
" A libvirt secret requires usage of a non-encrypted PEM format certificate."
Or is there some other wording that is preferable?
Something like this:
"Libvirt assumes the server-key.pem file is unencrypted by default. To use an encrypted server-key.pem file, the password to decrypt the PEM file is requird. This can be provided by creating a secret object in libvirt and then uncommenting this setting to set the UUID of the secret"
OK - I'll adjust this patch. In terms of the others are they reasonable or is it more of the case that the available hours needed to review exceed capacity... Tks - John

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 d2de15b..31ec38d 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 25d0787..572cae0 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 3f16dbe..b79c292 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 a1404d0..9697274 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 9683c9e..15ecb4f 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 isListen, 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 isListen, 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, isListen, verifypeer, - qemuCaps, &props) < 0) + if (addpasswordid && + !(secalias = qemuDomainGetSecretAESAlias(inalias, false))) return -1; + if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer, secalias, + qemuCaps, &props) < 0) + goto cleanup; + if (!(objalias = qemuAliasTLSObjFromChardevAlias(inalias))) goto cleanup; @@ -772,6 +785,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, virJSONValueFree(props); VIR_FREE(objalias); VIR_FREE(tmp); + VIR_FREE(secalias); return ret; } @@ -5048,6 +5062,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir, dev->data.tcp.listen, cfg->chardevTLSx509verify, + cfg->chardevTLSx509haveUUID, alias, qemuCaps) < 0) goto error; @@ -8666,6 +8681,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 0c0b8f1..a5b6993 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -69,6 +69,7 @@ int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, int qemuBuildTLSx509BackendProps(const char *tlspath, bool isListen, bool verifypeer, + const char *secalias, virQEMUCapsPtr qemuCaps, virJSONValuePtr *propsret); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index b79c292..0b561cc 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 9697274..7f6ca2c 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 419296e..8e4057c 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 cecd321..df21276 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..23c244b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml @@ -0,0 +1,50 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='ide' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <serial type='udp'> + <source mode='bind' host='127.0.0.1' service='1111'/> + <source mode='connect' host='127.0.0.1' service='2222'/> + <target port='0'/> + </serial> + <serial type='tcp'> + <source mode='connect' host='127.0.0.1' service='5555' tls='yes'/> + <protocol type='raw'/> + <target port='0'/> + </serial> + <console type='udp'> + <source mode='bind' host='127.0.0.1' service='1111'/> + <source mode='connect' host='127.0.0.1' service='2222'/> + <target type='serial' port='0'/> + </console> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 7df76c1..983d5f5 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1128,6 +1128,25 @@ mymain(void) DO_TEST("serial-tcp-tlsx509-chardev-tls", 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 e29180d..eb6a82a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7497,7 +7497,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 8e4057c..31a822d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1638,7 +1638,8 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, return ret; } -int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, +int qemuDomainAttachChrDevice(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainChrDefPtr chr) { @@ -1652,8 +1653,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 && @@ -1677,12 +1681,30 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver, if (qemuDomainChrPreInsert(vmdef, chr) < 0) goto cleanup; + if (qemuDomainSecretChardevPrepare(conn, driver, priv, chr) < 0) + goto cleanup; + if (cfg->chardevTLS && dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) { + 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 6a9ed99..8333d2a 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

On 09/19/2016 10:39 AM, John Ferlan wrote:
v6: http://www.redhat.com/archives/libvir-list/2016-September/msg00302.html
Differences over v6 - rather than disable on specific chardev's, modify the code to be need to enable by setting tls='yes' for the chardev
ping? Any other comments for this or should I resend since it's continuing to diverge from top of tree? Tks - John
John Ferlan (5): domain: Add optional 'tls' attribute for TCP chardev conf: Introduce {default|chardev}_tls_x509_secret_uuid qemu: Introduce qemuDomainChardevPrivatePtr qemu: Add a secret object to/for a chardev tcp with secret qemu: Add the ability to hotplug a secret object for TCP chardev TLS
docs/formatdomain.html.in | 21 +++ docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c | 51 ++++++-- src/conf/domain_conf.h | 5 +- src/libxl/libxl_domain.c | 2 +- src/lxc/lxc_native.c | 2 +- src/qemu/libvirtd_qemu.aug | 2 + src/qemu/qemu.conf | 24 ++++ src/qemu/qemu_command.c | 35 ++++- src/qemu/qemu_command.h | 1 + src/qemu/qemu_conf.c | 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 | 64 ++++++++- src/qemu/qemu_hotplug.h | 3 +- src/qemu/qemu_parse_command.c | 4 +- src/qemu/qemu_process.c | 6 +- src/qemu/test_libvirtd_qemu.aug.in | 2 + src/vz/vz_sdk.c | 2 +- src/xenconfig/xen_sxpr.c | 2 +- tests/qemuhotplugtest.c | 2 +- ...emuxml2argv-serial-tcp-tlsx509-chardev-tls.args | 30 +++++ ...qemuxml2argv-serial-tcp-tlsx509-chardev-tls.xml | 50 +++++++ .../qemuxml2argv-serial-tcp-tlsx509-chardev.xml | 2 +- ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 ++++++ ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 +++++++ tests/qemuxml2argvtest.c | 22 ++++ ...muxml2xmlout-serial-tcp-tlsx509-chardev-tls.xml | 1 + .../qemuxml2xmlout-serial-tcp-tlsx509-chardev.xml | 2 +- tests/qemuxml2xmltest.c | 1 + 32 files changed, 593 insertions(+), 36 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-chardev-tls.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-tls.xml
participants (2)
-
Daniel P. Berrange
-
John Ferlan