10:13 a.m.
On 10/17/2016 10:11 AM, Pavel Hrdina wrote:
On Fri, Oct 14, 2016 at 04:23:07PM -0400, John Ferlan wrote:
> 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(a)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 | 18 ++++
> 9 files changed, 252 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 aaf7018..b2dfee0 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -695,6 +695,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
> *
> @@ -706,6 +707,7 @@ int
> qemuBuildTLSx509BackendProps(const char *tlspath,
> bool isListen,
> bool verifypeer,
> + const char *secalias,
> virQEMUCapsPtr qemuCaps,
> virJSONValuePtr *propsret)
> {
> @@ -731,6 +733,10 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
> NULL) < 0)
> goto cleanup;
>
> + if (secalias &&
> + virJSONValueObjectAdd(*propsret, "s:passwordid", secalias, NULL)
< 0)
> + goto cleanup;
> +
> ret = 0;
>
> cleanup:
> @@ -745,6 +751,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
> *
> @@ -757,6 +764,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
> const char *tlspath,
> bool isListen,
> bool verifypeer,
> + bool addpasswordid,
> const char *inalias,
> virQEMUCapsPtr qemuCaps)
> {
> @@ -764,11 +772,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;
>
> @@ -784,6 +797,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
> virJSONValueFree(props);
> VIR_FREE(objalias);
> VIR_FREE(tmp);
> + VIR_FREE(secalias);
> return ret;
> }
>
> @@ -4946,6 +4960,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
> dev->data.tcp.listen,
> cfg->chardevTLSx509verify,
> + !!cfg->chardevTLSx509secretUUID,
> alias, qemuCaps) < 0)
> goto error;
>
> @@ -8542,6 +8557,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->chardevTLSx509secretUUID
&&
> + serial->source.type == VIR_DOMAIN_CHR_TYPE_TCP &&
This needs to test also serial->source.data.tcp.haveTLS.
True, of course that depends on the order of the patches too. So if
these 4 get accepted, but 1/5 is not accepted, then this would have to
move to patch 1 and be handled with that.
> + 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 2f2a6ff..a793fb6 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 a07f981..02c67ae 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 ||
The check for !conn is redundant because the declaration uses
ATTRIBUTE_NONNULL for the first attribute.
OK... It's cut-n-paste of Disk and Hostdev Prepare which should also
change (separate patch). I actually have quite a few of these built up
from a Coverity build which actually checks; whereas, the normal build
doesn't. I'll adjust this one and send the other ones separately.
> + chardev->source.type != VIR_DOMAIN_CHR_TYPE_TCP)
> + return 0;
> +
> + cfg = virQEMUDriverGetConfig(driver);
> + if (cfg->chardevTLS && cfg->chardevTLSx509secretUUID) {
> + 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 707a995..cc063d1 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -726,11 +726,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 f4d7c17..aad7fa1 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1734,6 +1734,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 0f5a11b..643741d 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5092,7 +5092,6 @@ qemuProcessPrepareDomain(virConnectPtr conn,
> size_t i;
> char *nodeset = NULL;
> qemuDomainObjPrivatePtr priv = vm->privateData;
> - virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
The cfg cleanup could be moved to separate patch.
OK.
> virCapsPtr caps;
>
> if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> @@ -5158,8 +5157,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++) {
> @@ -5188,7 +5187,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 c65c769..1ca91d3 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1165,6 +1165,24 @@ mymain(void)
> DO_TEST("serial-tcp-tlsx509-chardev-notls",
> 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;
Setting the chardevTLSx509verify is not strictly required to test the generated
command line if secret is required. The verify feature should has its own test
case.
OK, right... Should be a separate patch, although it's almost a two-fer
;-)...
Making this change alters the .args file to have 'verify-peer=no'
instead of verify-peer=yes.
Tks -
John
Pavel
> + if (VIR_STRDUP_QUIET(driver.config->chardevTLSx509secretUUID,
> + "6fd3f62d-9fe7-4a4e-a869-7acd6376d8ea") < 0)
> + return EXIT_FAILURE;
> +# 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
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list