1:23 p.m.
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>
---
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