New subject: [libvirt] [PATCH 2/2] Add a test case for certificate validation

22 Jul 2011

From: "Daniel P. Berrange" <berrange@redhat.com>

When libvirtd starts it it will sanity check its own certs,
and before libvirt clients connect to a remote server they
will sanity check their own certs. This patch allows such
sanity checking to be skipped. There is no strong reason to
need to do this, other than to bypass possible libvirt bugs
in sanity checking, or for testing purposes.

libvirt.conf gains tls_no_sanity_certificate parameter to
go along with tls_no_verify_certificate. The remote driver
client URIs gain a no_sanity URI parameter

* daemon/test_libvirtd.aug, daemon/libvirtd.conf,
  daemon/libvirtd.c, daemon/libvirtd.aug: Add parameter to
  allow cert sanity checks to be skipped
* src/remote/remote_driver.c: Add no_sanity parameter to
  skip cert checks
* src/rpc/virnettlscontext.c, src/rpc/virnettlscontext.h:
  Add new parameter for skipping sanity checks independantly
  of skipping session cert validation checks
---
 daemon/libvirtd.aug        |    1 +
 daemon/libvirtd.c          |    4 ++++
 daemon/libvirtd.conf       |    9 +++++++++
 daemon/test_libvirtd.aug   |    2 ++
 src/remote/remote_driver.c |   15 +++++++++------
 src/rpc/virnettlscontext.c |   36 +++++++++++++++++++++++-------------
 src/rpc/virnettlscontext.h |    4 ++++
 7 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/daemon/libvirtd.aug b/daemon/libvirtd.aug
index 0e06142..3f47ebb 100644
--- a/daemon/libvirtd.aug
+++ b/daemon/libvirtd.aug
@@ -48,6 +48,7 @@ module Libvirtd =
                          | str_entry "crl_file"
 
    let authorization_entry = bool_entry "tls_no_verify_certificate"
+                           | bool_entry "tls_no_sanity_certificate"
                            | str_array_entry "tls_allowed_dn_list"
                            | str_array_entry "sasl_allowed_username_list"
 
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index f2f3a4e..9e044e2 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -120,6 +120,7 @@ struct daemonConfig {
     char *mdns_name;
 
     int tls_no_verify_certificate;
+    int tls_no_sanity_certificate;
     char **tls_allowed_dn_list;
     char **sasl_allowed_username_list;
 
@@ -535,12 +536,14 @@ static int daemonSetupNetworking(virNetServerPtr srv,
                                                        config->cert_file,
                                                        config->key_file,
                                                        (const char *const*)config->tls_allowed_dn_list,
+                                                       config->tls_no_sanity_certificate ? false : true,
                                                        config->tls_no_verify_certificate ? false : true)))
                     goto error;
             } else {
                 if (!(ctxt = virNetTLSContextNewServerPath(NULL,
                                                            !privileged,
                                                            (const char *const*)config->tls_allowed_dn_list,
+                                                           config->tls_no_sanity_certificate ? false : true,
                                                            config->tls_no_verify_certificate ? false : true)))
                     goto error;
             }
@@ -1054,6 +1057,7 @@ daemonConfigLoad(struct daemonConfig *data,
     GET_CONF_INT (conf, filename, mdns_adv);
     GET_CONF_STR (conf, filename, mdns_name);
 
+    GET_CONF_INT (conf, filename, tls_no_sanity_certificate);
     GET_CONF_INT (conf, filename, tls_no_verify_certificate);
 
     GET_CONF_STR (conf, filename, key_file);
diff --git a/daemon/libvirtd.conf b/daemon/libvirtd.conf
index 3a071b0..217f2f4 100644
--- a/daemon/libvirtd.conf
+++ b/daemon/libvirtd.conf
@@ -187,6 +187,15 @@
 #
 
 
+# Flag to disable verification of our own server certificates
+#
+# When libvirtd starts it performs some sanity checks against
+# its own certificates.
+#
+# Default is to always sanity. Uncommenting this will disable
+# sanity checks which is not a good idea
+#tls_no_sanity_certificate = 1
+
 # Flag to disable verification of client certificates
 #
 # Client certificate verification is the primary authentication mechanism.
diff --git a/daemon/test_libvirtd.aug b/daemon/test_libvirtd.aug
index 5f8b644..58b7170 100644
--- a/daemon/test_libvirtd.aug
+++ b/daemon/test_libvirtd.aug
@@ -193,6 +193,7 @@ crl_file = \"/etc/pki/CA/crl.pem\"
 # Default is to always verify. Uncommenting this will disable
 # verification - make sure an IP whitelist is set
 tls_no_verify_certificate = 1
+tls_no_sanity_certificate = 1
 
 
 # A whitelist of allowed x509  Distinguished Names
@@ -468,6 +469,7 @@ audit_level = 2
         { "#comment" = "Default is to always verify. Uncommenting this will disable" }
         { "#comment" = "verification - make sure an IP whitelist is set" }
         { "tls_no_verify_certificate" = "1" }
+        { "tls_no_sanity_certificate" = "1" }
         { "#empty" }
         { "#empty" }
         { "#comment" = "A whitelist of allowed x509  Distinguished Names" }
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index ec4133b..7993d50 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -351,7 +351,7 @@ doRemoteOpen (virConnectPtr conn,
      */
     char *name = NULL, *command = NULL, *sockname = NULL, *netcat = NULL;
     char *port = NULL, *authtype = NULL, *username = NULL;
-    int no_verify = 0, no_tty = 0;
+    bool sanity = true, verify = true, tty = true;
     char *pkipath = NULL;
 
     /* Return code from this function, and the private data. */
@@ -425,11 +425,14 @@ doRemoteOpen (virConnectPtr conn,
                 netcat = strdup (var->value);
                 if (!netcat) goto out_of_memory;
                 var->ignore = 1;
+            } else if (STRCASEEQ (var->name, "no_sanity")) {
+                sanity = atoi(var->value) == 0;
+                var->ignore = 1;
             } else if (STRCASEEQ (var->name, "no_verify")) {
-                no_verify = atoi (var->value);
+                verify = atoi (var->value) == 0;
                 var->ignore = 1;
             } else if (STRCASEEQ (var->name, "no_tty")) {
-                no_tty = atoi (var->value);
+                tty = atoi (var->value) == 0;
                 var->ignore = 1;
             } else if (STRCASEEQ(var->name, "pkipath")) {
                 VIR_FREE(pkipath);
@@ -509,7 +512,7 @@ doRemoteOpen (virConnectPtr conn,
     case trans_tls:
         priv->tls = virNetTLSContextNewClientPath(pkipath,
                                                   geteuid() != 0 ? true : false,
-                                                  no_verify ? false : true);
+                                                  sanity, verify);
         if (!priv->tls)
             goto failed;
         priv->is_secure = 1;
@@ -579,8 +582,8 @@ doRemoteOpen (virConnectPtr conn,
                                                 port,
                                                 command,
                                                 username,
-                                                no_tty,
-                                                no_verify,
+                                                !tty,
+                                                !verify,
                                                 netcat ? netcat : "nc",
                                                 sockname)))
             goto failed;
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 5c94df6..bde4e7a 100644
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -382,7 +382,7 @@ virNetTLSContextCheckCertDN(gnutls_x509_crt_t cert,
                     certFile, gnutls_strerror(ret));
         return -1;
     }
-
+    VIR_DEBUG("Peer DN is %s", name);
     if (whitelist &&
         virNetTLSContextCheckCertDNWhitelist(name, whitelist) <= 0)
         return -1;
@@ -637,6 +637,7 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
                                                const char *cert,
                                                const char *key,
                                                const char *const*x509dnWhitelist,
+                                               bool sanityCheckCert,
                                                bool requireValidCert,
                                                bool isServer)
 {
@@ -644,8 +645,8 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
     char *gnutlsdebug;
     int err;
 
-    VIR_DEBUG("cacert=%s cacrl=%s cert=%s key=%s requireValid=%d isServer=%d",
-              cacert, NULLSTR(cacrl), cert, key, requireValidCert, isServer);
+    VIR_DEBUG("cacert=%s cacrl=%s cert=%s key=%s sanityCheckCert=%d requireValid=%d isServer=%d",
+              cacert, NULLSTR(cacrl), cert, key, sanityCheckCert, requireValidCert, isServer);
 
     if (VIR_ALLOC(ctxt) < 0) {
         virReportOOMError();
@@ -675,7 +676,7 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
         goto error;
     }
 
-    if (requireValidCert &&
+    if (sanityCheckCert &&
         virNetTLSContextSanityCheckCredentials(isServer, cacert, cert) < 0)
         goto error;
 
@@ -851,6 +852,7 @@ out_of_memory:
 static virNetTLSContextPtr virNetTLSContextNewPath(const char *pkipath,
                                                    bool tryUserPkiPath,
                                                    const char *const*x509dnWhitelist,
+                                                   bool sanityCheckCert,
                                                    bool requireValidCert,
                                                    bool isServer)
 {
@@ -862,7 +864,8 @@ static virNetTLSContextPtr virNetTLSContextNewPath(const char *pkipath,
         return NULL;
 
     ctxt = virNetTLSContextNew(cacert, cacrl, cert, key,
-                               x509dnWhitelist, requireValidCert, isServer);
+                               x509dnWhitelist, sanityCheckCert,
+                               requireValidCert, isServer);
 
     VIR_FREE(cacert);
     VIR_FREE(cacrl);
@@ -875,18 +878,20 @@ static virNetTLSContextPtr virNetTLSContextNewPath(const char *pkipath,
 virNetTLSContextPtr virNetTLSContextNewServerPath(const char *pkipath,
                                                   bool tryUserPkiPath,
                                                   const char *const*x509dnWhitelist,
+                                                  bool sanityCheckCert,
                                                   bool requireValidCert)
 {
-    return virNetTLSContextNewPath(pkipath, tryUserPkiPath,
-                                   x509dnWhitelist, requireValidCert, true);
+    return virNetTLSContextNewPath(pkipath, tryUserPkiPath, x509dnWhitelist,
+                                   sanityCheckCert, requireValidCert, true);
 }
 
 virNetTLSContextPtr virNetTLSContextNewClientPath(const char *pkipath,
                                                   bool tryUserPkiPath,
+                                                  bool sanityCheckCert,
                                                   bool requireValidCert)
 {
-    return virNetTLSContextNewPath(pkipath, tryUserPkiPath,
-                                   NULL, requireValidCert, false);
+    return virNetTLSContextNewPath(pkipath, tryUserPkiPath, NULL,
+                                   sanityCheckCert, requireValidCert, false);
 }
 
 
@@ -895,10 +900,11 @@ virNetTLSContextPtr virNetTLSContextNewServer(const char *cacert,
                                               const char *cert,
                                               const char *key,
                                               const char *const*x509dnWhitelist,
+                                              bool sanityCheckCert,
                                               bool requireValidCert)
 {
-    return virNetTLSContextNew(cacert, cacrl, cert, key,
-                               x509dnWhitelist, requireValidCert, true);
+    return virNetTLSContextNew(cacert, cacrl, cert, key, x509dnWhitelist,
+                               sanityCheckCert, requireValidCert, true);
 }
 
 
@@ -906,10 +912,11 @@ virNetTLSContextPtr virNetTLSContextNewClient(const char *cacert,
                                               const char *cacrl,
                                               const char *cert,
                                               const char *key,
+                                              bool sanityCheckCert,
                                               bool requireValidCert)
 {
-    return virNetTLSContextNew(cacert, cacrl, key, cert,
-                               NULL, requireValidCert, false);
+    return virNetTLSContextNew(cacert, cacrl, cert, key, NULL,
+                               sanityCheckCert, requireValidCert, false);
 }
 
 
@@ -1047,11 +1054,14 @@ int virNetTLSContextCheckCertificate(virNetTLSContextPtr ctxt,
                                      virNetTLSSessionPtr sess)
 {
     if (virNetTLSContextValidCertificate(ctxt, sess) < 0) {
+        virErrorPtr err = virGetLastError();
+        VIR_WARN("Certificate check failed %s", err && err->message ? err->message : "<unknown>");
         if (ctxt->requireValidCert) {
             virNetError(VIR_ERR_AUTH_FAILED, "%s",
                         _("Failed to verify peer's certificate"));
             return -1;
         }
+        virResetLastError();
         VIR_INFO("Ignoring bad certificate at user request");
     }
     return 0;
diff --git a/src/rpc/virnettlscontext.h b/src/rpc/virnettlscontext.h
index f23667f..641d67e 100644
--- a/src/rpc/virnettlscontext.h
+++ b/src/rpc/virnettlscontext.h
@@ -33,10 +33,12 @@ typedef virNetTLSSession *virNetTLSSessionPtr;
 virNetTLSContextPtr virNetTLSContextNewServerPath(const char *pkipath,
                                                   bool tryUserPkiPath,
                                                   const char *const*x509dnWhitelist,
+                                                  bool sanityCheckCert,
                                                   bool requireValidCert);
 
 virNetTLSContextPtr virNetTLSContextNewClientPath(const char *pkipath,
                                                   bool tryUserPkiPath,
+                                                  bool sanityCheckCert,
                                                   bool requireValidCert);
 
 virNetTLSContextPtr virNetTLSContextNewServer(const char *cacert,
@@ -44,12 +46,14 @@ virNetTLSContextPtr virNetTLSContextNewServer(const char *cacert,
                                               const char *cert,
                                               const char *key,
                                               const char *const*x509dnWhitelist,
+                                              bool sanityCheckCert,
                                               bool requireValidCert);
 
 virNetTLSContextPtr virNetTLSContextNewClient(const char *cacert,
                                               const char *cacrl,
                                               const char *cert,
                                               const char *key,
+                                              bool sanityCheckCert,
                                               bool requireValidCert);
 
 void virNetTLSContextRef(virNetTLSContextPtr ctxt);
-- 
1.7.6

    

[libvirt] [PATCH 1/2] Allow certificate sanity checking to be disabled

Daniel P. Berrange

Daniel P. Berrange

Eric Blake

Eric Blake

Eric Blake

Daniel P. Berrange

Eric Blake

Daniel P. Berrange

Eric Blake

tags

participants (2)