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 <jfer...@redhat.com>
> ---
>  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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to