On Mon, Oct 24, 2016 at 06:46:20PM -0400, John Ferlan wrote:
> Add the secret object so the 'passwordid=' can be added if the command line
> if there's a secret defined in/on the host for TCP chardev TLS objects.
>
> Preparation for the secret involves adding the secinfo to the char source
> device prior to command line processing. There are multiple possibilities
> for TCP chardev source backend usage.
>
> Add test for at least a serial chardev as an example.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/qemu/qemu_command.c | 30 +++-
> src/qemu/qemu_command.h | 1 +
> src/qemu/qemu_domain.c | 173 ++++++++++++++++++++-
> src/qemu/qemu_domain.h | 17 +-
> src/qemu/qemu_hotplug.c | 1 +
> src/qemu/qemu_process.c | 9 +-
> ...xml2argv-serial-tcp-tlsx509-secret-chardev.args | 38 +++++
> ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 50 ++++++
> tests/qemuxml2argvtest.c | 17 ++
> 9 files changed, 327 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 6bf6510..1c07959 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;
> }
>
> @@ -4936,11 +4950,23 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> virBufferAdd(&buf, nowait ? ",server,nowait" : ",server", -1);
>
> if (dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) {
> + qemuDomainChrSourcePrivatePtr chrSourcePriv =
> + QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev);
> char *objalias = NULL;
>
> + /* Add the secret object first if necessary. The
> + * secinfo is added only to a TCP serial device during
> + * qemuDomainSecretChardevPrepare. Subsequently called
> + * functions can just check the config fields */
> + if (chrSourcePriv && chrSourcePriv->secinfo &&
> + qemuBuildObjectSecretCommandLine(cmd,
> + chrSourcePriv->secinfo) < 0)
> + goto error;
> +
> if (qemuBuildTLSx509CommandLine(cmd, cfg->chardevTLSx509certdir,
> dev->data.tcp.listen,
> cfg->chardevTLSx509verify,
> + !!cfg->chardevTLSx509secretUUID,
> charAlias, qemuCaps) < 0)
> goto error;
>
> 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 41ac52d..38c0f18 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)
> @@ -1220,6 +1221,97 @@ qemuDomainSecretHostdevPrepare(virConnectPtr conn,
> }
>
>
> +/* qemuDomainSecretChardevDestroy:
> + * @disk: Pointer to a chardev definition
> + *
> + * Clear and destroy memory associated with the secret
> + */
> +void
> +qemuDomainSecretChardevDestroy(virDomainChrSourceDefPtr dev)
> +{
> + qemuDomainChrSourcePrivatePtr chrSourcePriv =
> + QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev);
> +
> + if (!chrSourcePriv || !chrSourcePriv->secinfo)
> + return;
> +
> + qemuDomainSecretInfoFree(&chrSourcePriv->secinfo);
> +}
> +
> +
> +/* qemuDomainSecretChardevPrepare:
> + * @conn: Pointer to connection
> + * @driver: Pointer to driver object
> + * @priv: pointer to domain private object
> + * @chrAlias: Alias of the chr device
> + * @dev: Pointer to a char source definition
> + *
> + * For a 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,
> + const char *chrAlias,
> + virDomainChrSourceDefPtr dev)
> +{
> + virQEMUDriverConfigPtr cfg;
> + virSecretLookupTypeDef seclookupdef = {0};
> + qemuDomainSecretInfoPtr secinfo = NULL;
> + char *charAlias = NULL;
> +
> + if (dev->type != VIR_DOMAIN_CHR_TYPE_TCP)
> + return 0;
> +
> + cfg = virQEMUDriverGetConfig(driver);
The *driver* is used only to get the *cfg* and the whole function is used
in several loops so it would be better to pass *cfg* directly to this function
in order to save a lot of locks & unlocks and refs & unrefs.
> + if (dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES &&
> + cfg->chardevTLSx509secretUUID) {
> + qemuDomainChrSourcePrivatePtr chrSourcePriv =
> + QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev);
> +
> + 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 (!(charAlias = qemuAliasChardevFromDevAlias(chrAlias)))
> + goto error;
> +
> + if (qemuDomainSecretSetup(conn, priv, secinfo, charAlias,
> + 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;
> + }
> +
> + chrSourcePriv->secinfo = secinfo;
> + }
> +
> + VIR_FREE(charAlias);
> + virObjectUnref(cfg);
> + return 0;
> +
> + error:
> + virObjectUnref(cfg);
> + qemuDomainSecretInfoFree(&secinfo);
> + return -1;
> +}
> +
> +
> /* qemuDomainSecretDestroy:
> * @vm: Domain object
> *
> @@ -1236,11 +1328,38 @@ 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]->source);
> +
> + for (i = 0; i < vm->def->nparallels; i++)
> + qemuDomainSecretChardevDestroy(vm->def->parallels[i]->source);
> +
> + for (i = 0; i < vm->def->nchannels; i++)
> + qemuDomainSecretChardevDestroy(vm->def->channels[i]->source);
> +
> + for (i = 0; i < vm->def->nconsoles; i++)
> + qemuDomainSecretChardevDestroy(vm->def->consoles[i]->source);
> +
> + for (i = 0; i < vm->def->nsmartcards; i++) {
> + if (vm->def->smartcards[i]->type ==
> + VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH)
> + qemuDomainSecretChardevDestroy(vm->def->smartcards[i]->data.passthru);
> + }
> +
> + for (i = 0; i < vm->def->nrngs; i++) {
> + if (vm->def->rngs[i]->backend == VIR_DOMAIN_RNG_BACKEND_EGD)
> + qemuDomainSecretChardevDestroy(vm->def->rngs[i]->source.chardev);
> + }
> +
> + for (i = 0; i < vm->def->nredirdevs; i++)
> + qemuDomainSecretChardevDestroy(vm->def->redirdevs[i]->source);
> }
>
>
> /* 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
> @@ -1253,6 +1372,7 @@ qemuDomainSecretDestroy(virDomainObjPtr vm)
> */
> int
> qemuDomainSecretPrepare(virConnectPtr conn,
> + virQEMUDriverPtr driver,
> virDomainObjPtr vm)
> {
> qemuDomainObjPrivatePtr priv = vm->privateData;
> @@ -1269,6 +1389,57 @@ qemuDomainSecretPrepare(virConnectPtr conn,
> return -1;
> }
>
> + for (i = 0; i < vm->def->nserials; i++) {
> + if (qemuDomainSecretChardevPrepare(conn, driver, priv,
> + vm->def->serials[i]->info.alias,
> + vm->def->serials[i]->source) < 0)
> + return -1;
> + }
> +
> + for (i = 0; i < vm->def->nparallels; i++) {
> + if (qemuDomainSecretChardevPrepare(conn, driver, priv,
> + vm->def->parallels[i]->info.alias,
> + vm->def->parallels[i]->source) < 0)
> + return -1;
> + }
> +
> + for (i = 0; i < vm->def->nchannels; i++) {
> + if (qemuDomainSecretChardevPrepare(conn, driver, priv,
> + vm->def->channels[i]->info.alias,
> + vm->def->channels[i]->source) < 0)
> + return -1;
> + }
> +
> + for (i = 0; i < vm->def->nconsoles; i++) {
> + if (qemuDomainSecretChardevPrepare(conn, driver, priv,
> + vm->def->consoles[i]->info.alias,
> + vm->def->consoles[i]->source) < 0)
> + return -1;
> + }
> +
> + for (i = 0; i < vm->def->nsmartcards; i++)
> + if (vm->def->smartcards[i]->type ==
> + VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH &&
> + qemuDomainSecretChardevPrepare(conn, driver, priv,
> + vm->def->smartcards[i]->info.alias,
> + vm->def->smartcards[i]->data.passthru) < 0)
> + return -1;
> +
> + for (i = 0; i < vm->def->nrngs; i++) {
> + if (vm->def->rngs[i]->backend == VIR_DOMAIN_RNG_BACKEND_EGD &&
> + qemuDomainSecretChardevPrepare(conn, driver, priv,
> + vm->def->rngs[i]->info.alias,
> + vm->def->rngs[i]->source.chardev) < 0)
> + return -1;
> + }
> +
> + for (i = 0; i < vm->def->nredirdevs; i++) {
> + if (qemuDomainSecretChardevPrepare(conn, driver, priv,
> + vm->def->redirdevs[i]->info.alias,
> + vm->def->redirdevs[i]->source) < 0)
> + return -1;
> + }
> +
> return 0;
> }
>
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 4f9bf82..0820afd 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -726,11 +726,24 @@ int qemuDomainSecretHostdevPrepare(virConnectPtr conn,
> virDomainHostdevDefPtr hostdev)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>
> +void qemuDomainSecretChardevDestroy(virDomainChrSourceDefPtr dev)
> + ATTRIBUTE_NONNULL(1);
> +
> +int qemuDomainSecretChardevPrepare(virConnectPtr conn,
> + virQEMUDriverPtr driver,
> + qemuDomainObjPrivatePtr priv,
> + const char *chrAlias,
> + virDomainChrSourceDefPtr dev)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
> + ATTRIBUTE_NONNULL(4);
And ATTRIBUTE_NONNULL(5)
ACK with the issues fixed.
Pavel
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list