https://bugzilla.redhat.com/show_bug.cgi?id=1458630 Introduce virQEMUDriverConfigTLSDirValidateResetDefault to validate that if any of the *_tls_x509_cert_dir values were set properly and reset the default value if the default_tls_x509_cert_dir changed. Update the qemu.conf description for default to describe the consequences if the default directory path does not exist. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- v2: https://www.redhat.com/archives/libvir-list/2017-June/msg01341.html Follow-ups in July though. Changes since v2 - reduced verbosity in qemu.conf and adjusted the logic to create/call virQEMUDriverConfigTLSDirValidateResetDefault after all the values are read in order to validate the values and adjust the default if necessary. Tested by 1. Having everything commented out, w/ no /etc/pki/qemu: Works as expected 2. Uncomment the default_tls_x509_cert_dir, w/ no /etc/pki/qemu: Fails as expected 3. Uncomment each of the other *_tls_x509_cert_dir's when directory not exist: Fails as expected 4. Use a directory that exists for other _*tls_x509_cert_dirs: Works as expected 5. Change the default_tls_x509_cert_dir to an existing directory: Works as expected, each of the other uncommented *_tls_x509_cert_dirs will get the new value (tested via debug code) while one that used an existing default didn't change from it's default /etc/pki/libvirt-* src/qemu/qemu.conf | 8 +++++++ src/qemu/qemu_conf.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_conf.h | 4 ++++ src/qemu/qemu_driver.c | 3 +++ 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index e6c0832..9526aed 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -13,6 +13,14 @@ # # dh-params.pem - the DH params configuration file # +# If the directory does not exist or contain the necessary files, QEMU +# domains will fail to start if they are configured to use TLS. +# +# In order to overwrite the default path alter the following. This path +# definition will be used as the default path for other *_tls_x509_cert_dir +# configuration settings if their default path does not exist or is not +# specifically set. +# #default_tls_x509_cert_dir = "/etc/pki/qemu" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 6f44cbf..87d2c2d 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -451,8 +451,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, if (!(conf = virConfReadFile(filename, 0))) goto cleanup; - if (virConfGetValueString(conf, "default_tls_x509_cert_dir", &cfg->defaultTLSx509certdir) < 0) + if ((rv = virConfGetValueString(conf, "default_tls_x509_cert_dir", &cfg->defaultTLSx509certdir)) < 0) goto cleanup; + cfg->checkdefaultTLSx509certdir = (rv == 1); if (virConfGetValueBool(conf, "default_tls_x509_verify", &cfg->defaultTLSx509verify) < 0) goto cleanup; if (virConfGetValueString(conf, "default_tls_x509_secret_uuid", @@ -872,6 +873,68 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, return ret; } + +/** + * @cfg: Recently config values + * + * Validate the recently read *_tls_x509_cert_dir values and if necessary + * update the default value to match the default_tls_x509_cert_dir + * + * Returns 0 on success, -1 on failure + */ +int +virQEMUDriverConfigTLSDirValidateResetDefault(virQEMUDriverConfigPtr cfg) +{ + bool newDefault = false; + + /* If the default entry was uncommented, then validate existence */ + if (cfg->checkdefaultTLSx509certdir) { + if (!virFileExists(cfg->defaultTLSx509certdir)) { + virReportError(VIR_ERR_CONF_SYNTAX, + _("default_tls_x509_cert_dir directory '%s' " + "does not exist"), + cfg->defaultTLSx509certdir); + return -1; + } + if (STRNEQ(cfg->defaultTLSx509certdir, SYSCONFDIR "/pki/qemu")) + newDefault = true; + } + + /* We know virQEMUDriverConfigNew set the particular value to either + * it's default or default_tls_x509_cert_dir's default. So, if not the + * default default and the directory doesn't exist, then the entry was + * set in the config file to something that doesn't exist, so error. + * + * Also, if the defaultTLSx509certdir value was changed from the default, + * then we need to update the default for each setting as well to match + * the default_tls_x509_cert_dir. + */ +#define VALIDATE_TLS_X509_CERT_DIR(val) \ + do { \ + if (STRNEQ(cfg->val ## TLSx509certdir, SYSCONFDIR "/pki/qemu") && \ + !virFileExists(cfg->val ## TLSx509certdir)) { \ + virReportError(VIR_ERR_CONF_SYNTAX, \ + _(#val"_tls_x509_cert_dir directory '%s' " \ + "does not exist"), \ + cfg->val ## TLSx509certdir); \ + return -1; \ + } else if (newDefault) { \ + VIR_FREE(cfg->val ## TLSx509certdir); \ + if (VIR_STRDUP(cfg->val ## TLSx509certdir, \ + cfg->defaultTLSx509certdir) < 0) \ + return -1; \ + } \ + } while (0) + + VALIDATE_TLS_X509_CERT_DIR(vnc); + VALIDATE_TLS_X509_CERT_DIR(spice); + VALIDATE_TLS_X509_CERT_DIR(chardev); + VALIDATE_TLS_X509_CERT_DIR(migrate); + + return 0; +} + + virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver) { virQEMUDriverConfigPtr conf; diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 1407eef..fffa871 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -112,6 +112,7 @@ struct _virQEMUDriverConfig { char *nvramDir; char *defaultTLSx509certdir; + bool checkdefaultTLSx509certdir; bool defaultTLSx509verify; char *defaultTLSx509secretUUID; @@ -301,6 +302,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, const char *filename, bool privileged); +int +virQEMUDriverConfigTLSDirValidateResetDefault(virQEMUDriverConfigPtr cfg); + virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver); bool virQEMUDriverIsPrivileged(virQEMUDriverPtr driver); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6568def..2731f8e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -667,6 +667,9 @@ qemuStateInitialize(bool privileged, goto error; VIR_FREE(driverConf); + if (virQEMUDriverConfigTLSDirValidateResetDefault(cfg) < 0) + goto error; + if (virFileMakePath(cfg->stateDir) < 0) { virReportSystemError(errno, _("Failed to create state dir %s"), cfg->stateDir); -- 2.9.4