[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH v3] qemu: Check for existence of provided *_tls_x509_cert_dir



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 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]