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

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



On Thu, Jun 29, 2017 at 10:32:35AM -0400, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1458630

Introduce virQEMUDriverConfigSetCertDir which will handle reading the
qemu.conf config file specific setting for default, vnc, spice, chardev,
and migrate. If a setting is provided, then validate the existence of the
directory and overwrite the default set by virQEMUDriverConfigNew.

Update the qemu.conf description for default to describe the consequences
if the default directory path does not exist and as well as the descriptions
for each of the *_tls_x509_cert_dir entries.

Signed-off-by: John Ferlan <jferlan redhat com>
---

v1: https://www.redhat.com/archives/libvir-list/2017-June/msg01278.html

- Dropped the former 1/2 patch

- Alter the logic of virQEMUDriverConfigSetCertDir to fail instead of
 VIR_INFO if an uncommented entry for one of the *_tls_x509_cert_dir
 has a path that does not exist. This will cause a libvirtd startup
 failure as opposed to the previous logic which would have failed only
 when a domain using TLS was started.

- Alter the description for each of the values to more accurately describe
 what happens.

src/qemu/qemu.conf   | 29 ++++++++++++++++++++---------
src/qemu/qemu_conf.c | 38 +++++++++++++++++++++++++++++++++-----
2 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index e6c0832..b0ccffb 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -3,7 +3,7 @@
# defaults are used.

# Use of TLS requires that x509 certificates be issued. The default is
-# to keep them in /etc/pki/qemu. This directory must contain
+# to keep them in /etc/pki/qemu. This directory must exist and contain:

I suspect the POSIX specification mandates that the directory must exist
in order to contain files. :)

#
#  ca-cert.pem - the CA master certificate
#  server-cert.pem - the server certificate signed with ca-cert.pem
@@ -13,6 +13,12 @@
#
#  dh-params.pem - the DH params configuration file
#
+# If the directory does not exist or does not contain the necessary files,
+# QEMU domains will fail to start if they are configured to use TLS.
+#

Isn't this sufficiently covered by 'Use of TLS requires that x509
certificates be issued'?

+# In order to overwrite the default path alter the following. If the provided
+# path does not exist, then startup will fail.
+#

To apply the configuration, you need to restart the daemon. And since
daemon startup will fail, I think the user will be able to notice it.
We should error out on incorrect paths as soon as we can, without
mentioning it in the documentation.

#default_tls_x509_cert_dir = "/etc/pki/qemu"


@@ -79,8 +85,9 @@

# In order to override the default TLS certificate location for
# vnc certificates, supply a valid path to the certificate directory.
-# If the provided path does not exist then the default_tls_x509_cert_dir
-# path will be used.
+# If the default listed here does not exist, then the default /etc/pki/qemu
+# is used.

If I override default_tls_x509_cert_dir, without overriding all the
specific *_tls_x509_cert_dir values, I expect they will all use my
value, not the hardcoded default of /etc/pki/qemu.
So the behavior described by the original comment makes more sense.

If uncommented and the provided path does not exist, then startup
+# will fail.
#
#vnc_tls_x509_cert_dir = "/etc/pki/libvirt-vnc"

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 73c33d6..4eb6f0c 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -440,6 +440,34 @@ virQEMUDriverConfigHugeTLBFSInit(virHugeTLBFSPtr hugetlbfs,
}


+static int
+virQEMUDriverConfigSetCertDir(virConfPtr conf,
+                              const char *setting,
+                              char **value)
+{
+    char *tlsCertDir = NULL;
+
+    if (virConfGetValueString(conf, setting, &tlsCertDir) < 0)
+        return -1;
+
+    if (!tlsCertDir)
+        return 0;
+
+    if (!virFileExists(tlsCertDir)) {
+        virReportError(VIR_ERR_CONF_SYNTAX,

This is not a syntax error.
Validating these settings does not belong in virQEMUDriverConfigLoadFile;
qemuStateInitialize or a function called from it would be a better
place.

Jan

Attachment: signature.asc
Description: Digital signature


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