[libvirt] [PATCH] qemu: Fix bug assuming usage of default UUID for certificate passphrase

[John Ferlan]

ping?

Tks,

John

On 07/21/2017 05:47 PM, John Ferlan wrote:
> If an environment specific _tls_x509_cert_dir is provided, then
> do not VIR_STRDUP the defaultTLSx509secretUUID as that would be
> for the "default" environment and not the vnc, spice, chardev, or
> migrate environments. If the environment needs a secret to decode
> it's certificate, then it must provide the secret. If the secrets
> happen to be the same, then configuration would use the same UUID
> as the default (but we cannot assume that nor can we assume that
> the secret would be necessary).
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
> 
>  While responding to a different patch today regarding Veritas and
>  usage of a default environment w/ or w/o secrets I realized that
>  the existing logic has a flaw in "assuming" that someone would want
>  to use the default secret. What if they defined their own environment
>  without a secret?  Then the code would create a secret object to pass
>  to QEMU which would think it needs to use it to decode the server
>  certificate (but it doesn't), so it would seemingly fail the start.
>  I assume based on the lack of complaints about this that everyone just
>  uses the default environment!
> 
>  src/qemu/qemu_conf.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index c4714ed..a7a2aaa 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -526,14 +526,18 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
>              goto cleanup;                                                   \
>          if (rv == 0)                                                        \
>              cfg->val## TLSx509verify = cfg->defaultTLSx509verify;           \
> -        if (virConfGetValueString(conf, #val "_tls_x509_cert_dir",          \
> -                                  &cfg->val## TLSx509certdir) < 0)          \
> +        if ((rv = virConfGetValueString(conf, #val "_tls_x509_cert_dir",    \
> +                                  &cfg->val## TLSx509certdir)) < 0)         \
>              goto cleanup;                                                   \
>          if (virConfGetValueString(conf,                                     \
>                                    #val "_tls_x509_secret_uuid",             \
>                                    &cfg->val## TLSx509secretUUID) < 0)       \
>              goto cleanup;                                                   \
> -        if (!cfg->val## TLSx509secretUUID &&                                \
> +        /* Only if a *tls_x509_cert_dir wasn't found (e.g. rv == 0), should \
> +         * we copy the defaultTLSx509secretUUID. If this environment needs  \
> +         * a passphrase to decode the certificate, then it should provide   \
> +         * it's own secretUUID for that. */                                 \
> +        if (rv == 0 && !cfg->val## TLSx509secretUUID &&                     \
>              cfg->defaultTLSx509secretUUID) {                                \
>              if (VIR_STRDUP(cfg->val## TLSx509secretUUID,                    \
>                             cfg->defaultTLSx509secretUUID) < 0)              \
>