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

John Ferlan posted 1 patch 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20170726210517.31341-1-jferlan@redhat.com
src/qemu/qemu.conf     |   8 ++++
src/qemu/qemu_conf.c   | 105 ++++++++++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_conf.h   |   4 ++
src/qemu/qemu_driver.c |   3 ++
4 files changed, 119 insertions(+), 1 deletion(-)
[libvirt] [PATCH v4] qemu: Check for existence of provided *_tls_x509_cert_dir
Posted by John Ferlan 6 years, 9 months ago
https://bugzilla.redhat.com/show_bug.cgi?id=1458630

Introduce virQEMUDriverConfigTLSDirResetDefaults in order to check
if the defaultTLSx509certdir was changed, then change the default
for any other *TLSx509certdir that was not set to the default default.

Introduce virQEMUDriverConfigValidate to validate the existence of
any of the *_tls_x509_cert_dir values that were uncommented/set,
incuding the default.

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>
---
 v3: https://www.redhat.com/archives/libvir-list/2017-July/msg00915.html

 Changes since v3 - rework even more based on code review.

 src/qemu/qemu.conf     |   8 ++++
 src/qemu/qemu_conf.c   | 105 ++++++++++++++++++++++++++++++++++++++++++++++++-
 src/qemu/qemu_conf.h   |   4 ++
 src/qemu/qemu_driver.c |   3 ++
 4 files changed, 119 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 1d81472..f977e3b 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 c4714ed..1a4a998 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -425,6 +425,43 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
 }
 
 
+/**
+ * @cfg: Just read config TLS values
+ *
+ * If the default_tls_x509_cert_dir was uncommented or changed from
+ * the default value assigned to the *_tls_x509_cert_dir values when
+ * virQEMUDriverConfigNew was executed, we need to check if we need
+ * to update the other defaults.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+static int
+virQEMUDriverConfigTLSDirResetDefaults(virQEMUDriverConfigPtr cfg)
+{
+    /* Not changed or set to the default default, nothing to do */
+    if (!cfg->checkdefaultTLSx509certdir ||
+        STREQ(cfg->defaultTLSx509certdir, SYSCONFDIR "/pki/qemu"))
+        return 0;
+
+#define CHECK_RESET_CERT_DIR_DEFAULT(val)                                 \
+    do {                                                                  \
+        if (STREQ(cfg->val ## TLSx509certdir, SYSCONFDIR "/pki/qemu")) {  \
+            VIR_FREE(cfg->val ## TLSx509certdir);                         \
+            if (VIR_STRDUP(cfg->val ## TLSx509certdir,                    \
+                           cfg->defaultTLSx509certdir) < 0)               \
+                return -1;                                                \
+        }                                                                 \
+    } while (0)
+
+    CHECK_RESET_CERT_DIR_DEFAULT(vnc);
+    CHECK_RESET_CERT_DIR_DEFAULT(spice);
+    CHECK_RESET_CERT_DIR_DEFAULT(chardev);
+    CHECK_RESET_CERT_DIR_DEFAULT(migrate);
+
+    return 0;
+}
+
+
 int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
                                 const char *filename,
                                 bool privileged)
@@ -452,8 +489,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",
@@ -549,6 +587,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 
 #undef GET_CONFIG_TLS_CERTINFO
 
+    if (virQEMUDriverConfigTLSDirResetDefaults(cfg) < 0)
+        goto cleanup;
+
     if (virConfGetValueUInt(conf, "remote_websocket_port_min", &cfg->webSocketPortMin) < 0)
         goto cleanup;
     if (cfg->webSocketPortMin < QEMU_WEBSOCKET_PORT_MIN) {
@@ -873,6 +914,68 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
     return ret;
 }
 
+
+/**
+ * @cfg: Recently read config values
+ *
+ * Validate the recently read configuration values.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virQEMUDriverConfigValidate(virQEMUDriverConfigPtr cfg)
+{
+    /* 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;
+        }
+    }
+
+    /* For each of the others - if the value is not to the default default
+     * then check if the directory exists (this may duplicate the check done
+     * during virQEMUDriverConfigNew).
+     */
+    if (STRNEQ(cfg->vncTLSx509certdir, SYSCONFDIR "/pki/qemu") &&
+        !virFileExists(cfg->vncTLSx509certdir)) {
+        virReportError(VIR_ERR_CONF_SYNTAX,
+                       _("vnc_tls_x509_cert_dir directory '%s' does not exist"),
+                       cfg->vncTLSx509certdir);
+        return -1;
+    }
+
+    if (STRNEQ(cfg->spiceTLSx509certdir, SYSCONFDIR "/pki/qemu") &&
+        !virFileExists(cfg->spiceTLSx509certdir)) {
+        virReportError(VIR_ERR_CONF_SYNTAX,
+                       _("spice_tls_x509_cert_dir directory '%s' does not exist"),
+                       cfg->spiceTLSx509certdir);
+        return -1;
+    }
+
+    if (STRNEQ(cfg->chardevTLSx509certdir, SYSCONFDIR "/pki/qemu") &&
+        !virFileExists(cfg->chardevTLSx509certdir)) {
+        virReportError(VIR_ERR_CONF_SYNTAX,
+                       _("chardev_tls_x509_cert_dir directory '%s' does not exist"),
+                       cfg->chardevTLSx509certdir);
+        return -1;
+    }
+
+    if (STRNEQ(cfg->migrateTLSx509certdir, SYSCONFDIR "/pki/qemu") &&
+        !virFileExists(cfg->migrateTLSx509certdir)) {
+        virReportError(VIR_ERR_CONF_SYNTAX,
+                       _("migrate_tls_x509_cert_dir directory '%s' does not exist"),
+                       cfg->migrateTLSx509certdir);
+        return -1;
+    }
+
+    return 0;
+}
+
+
 virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver)
 {
     virQEMUDriverConfigPtr conf;
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 3013f24..d469b50 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -113,6 +113,7 @@ struct _virQEMUDriverConfig {
     char *nvramDir;
 
     char *defaultTLSx509certdir;
+    bool checkdefaultTLSx509certdir;
     bool defaultTLSx509verify;
     char *defaultTLSx509secretUUID;
 
@@ -302,6 +303,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
                                 const char *filename,
                                 bool privileged);
 
+int
+virQEMUDriverConfigValidate(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 a423663..3ad71e6 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 (virQEMUDriverConfigValidate(cfg) < 0)
+        goto error;
+
     if (virFileMakePath(cfg->stateDir) < 0) {
         virReportSystemError(errno, _("Failed to create state dir %s"),
                              cfg->stateDir);
-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] qemu: Check for existence of provided *_tls_x509_cert_dir
Posted by Ján Tomko 6 years, 8 months ago
On Wed, Jul 26, 2017 at 05:05:17PM -0400, John Ferlan wrote:
>https://bugzilla.redhat.com/show_bug.cgi?id=1458630
>
>Introduce virQEMUDriverConfigTLSDirResetDefaults in order to check
>if the defaultTLSx509certdir was changed, then change the default
>for any other *TLSx509certdir that was not set to the default default.
>
>Introduce virQEMUDriverConfigValidate to validate the existence of
>any of the *_tls_x509_cert_dir values that were uncommented/set,
>incuding the default.
>
>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>
>---
> v3: https://www.redhat.com/archives/libvir-list/2017-July/msg00915.html
>
> Changes since v3 - rework even more based on code review.
>
> src/qemu/qemu.conf     |   8 ++++
> src/qemu/qemu_conf.c   | 105 ++++++++++++++++++++++++++++++++++++++++++++++++-
> src/qemu/qemu_conf.h   |   4 ++
> src/qemu/qemu_driver.c |   3 ++
> 4 files changed, 119 insertions(+), 1 deletion(-)
>

ACK

Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list