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@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) \
--
2.9.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@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) \
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07/21/2017 11: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@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(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.