Add some basic sanity checking of certificates before use

If the libvirt daemon or libvirt client is configured with bogus
certificates, it is very unhelpful to only find out about this
when a TLS connection is actually attempted. Not least because
the error messages you get back for failures are incredibly
obscure.

This adds some basic sanity checking of certificates at the
time the virNetTLSContext object is created. This is at libvirt
startup, or when creating a virNetClient instance.

This checks that the certificate expiry/start dates are valid
and that the certificate is actually signed by the CA that is
loaded.

* src/rpc/virnettlscontext.c: Add certificate sanity checks

diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
index 4fa6fbba6b6455fff395b6342dec11e48338cf3c..884e593bafeed50e024044440bfe05df8ee54c52 100644 (file)
--- a/src/rpc/virnettlscontext.c
+++ b/src/rpc/virnettlscontext.c
@@ -67,6 +67,7 @@ struct _virNetTLSSession {
 
     bool handshakeComplete;
 
+    bool isServer;
     char *hostname;
     gnutls_session_t session;
     virNetTLSSessionWriteFunc writeFunc;
@@ -95,6 +96,134 @@ static void virNetTLSLog(int level, const char *str) {
     VIR_DEBUG("%d %s", level, str);
 }
 
+
+static gnutls_x509_crt_t virNetTLSContextSanityCheckCert(bool isServer,
+                                                         const char *certFile)
+{
+    gnutls_datum_t data;
+    gnutls_x509_crt_t cert = NULL;
+    char *buf = NULL;
+    int ret = -1;
+    time_t now;
+
+    if ((now = time(NULL)) == ((time_t)-1)) {
+        virReportSystemError(errno, "%s",
+                             _("cannot get current time"));
+        goto cleanup;
+    }
+
+    if (gnutls_x509_crt_init(&cert) < 0) {
+        virNetError(VIR_ERR_SYSTEM_ERROR, "%s",
+                    _("Unable to initialize certificate"));
+        goto cleanup;
+    }
+
+    if (virFileReadAll(certFile, (1<<16), &buf) < 0)
+        goto cleanup;
+
+    data.data = (unsigned char *)buf;
+    data.size = strlen(buf);
+
+    if (gnutls_x509_crt_import(cert, &data, GNUTLS_X509_FMT_PEM) < 0) {
+        virNetError(VIR_ERR_SYSTEM_ERROR, isServer ?
+                    _("Unable to import server certificate %s") :
+                    _("Unable to import client certificate %s"),
+                    certFile);
+        goto cleanup;
+    }
+
+    if (gnutls_x509_crt_get_expiration_time(cert) < now) {
+        virNetError(VIR_ERR_SYSTEM_ERROR, isServer ?
+                    _("The server certificate %s has expired") :
+                    _("The client certificate %s has expired"),
+                    certFile);
+        goto cleanup;
+    }
+
+    if (gnutls_x509_crt_get_activation_time(cert) > now) {
+        virNetError(VIR_ERR_SYSTEM_ERROR, isServer ?
+                    _("The server certificate %s is not yet active") :
+                    _("The client certificate %s is not yet active"),
+                    certFile);
+        goto cleanup;
+    }
+
+    ret = 0;
+
+cleanup:
+    if (ret != 0) {
+        gnutls_x509_crt_deinit(cert);
+        cert = NULL;
+    }
+    VIR_FREE(buf);
+    return cert;
+}
+
+
+static int virNetTLSContextSanityCheckCredentials(bool isServer,
+                                                  const char *cacertFile,
+                                                  const char *certFile)
+{
+    gnutls_x509_crt_t cert = NULL;
+    gnutls_x509_crt_t cacert = NULL;
+    int ret = -1;
+    unsigned int status;
+
+    if (access(certFile, R_OK) == 0) {
+        if (!(cert = virNetTLSContextSanityCheckCert(isServer, certFile)))
+            goto cleanup;
+    }
+    if (access(cacertFile, R_OK) == 0) {
+        if (!(cacert = virNetTLSContextSanityCheckCert(isServer, cacertFile)))
+            goto cleanup;
+    }
+
+    if (cert && cacert) {
+        if (gnutls_x509_crt_list_verify(&cert, 1,
+                                        &cacert, 1,
+                                        NULL, 0,
+                                        0, &status) < 0) {
+            virNetError(VIR_ERR_SYSTEM_ERROR, "%s", isServer ?
+                        _("Unable to verify server certificate against CA certificate") :
+                        _("Unable to verify client certificate against CA certificate"));
+            goto cleanup;
+        }
+
+        if (status != 0) {
+            const char *reason = _("Invalid certificate");
+
+            if (status & GNUTLS_CERT_INVALID)
+                reason = _("The certificate is not trusted.");
+
+            if (status & GNUTLS_CERT_SIGNER_NOT_FOUND)
+                reason = _("The certificate hasn't got a known issuer.");
+
+            if (status & GNUTLS_CERT_REVOKED)
+                reason = _("The certificate has been revoked.");
+
+#ifndef GNUTLS_1_0_COMPAT
+            if (status & GNUTLS_CERT_INSECURE_ALGORITHM)
+                reason = _("The certificate uses an insecure algorithm");
+#endif
+
+            virNetError(VIR_ERR_SYSTEM_ERROR,
+                        _("Our own certificate %s failed validation against %s: %s"),
+                        certFile, cacertFile, reason);
+            goto cleanup;
+        }
+    }
+
+    ret = 0;
+
+cleanup:
+    if (cert)
+        gnutls_x509_crt_deinit(cert);
+    if (cacert)
+        gnutls_x509_crt_deinit(cacert);
+    return ret;
+}
+
+
 static int virNetTLSContextLoadCredentials(virNetTLSContextPtr ctxt,
                                            bool isServer,
                                            const char *cacert,
@@ -217,6 +346,10 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
         goto error;
     }
 
+    if (requireValidCert &&
+        virNetTLSContextSanityCheckCredentials(isServer, cacert, cert) < 0)
+        goto error;
+
     if (virNetTLSContextLoadCredentials(ctxt, isServer, cacert, cacrl, cert, key) < 0)
         goto error;
 
@@ -574,15 +707,21 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt,
         }
 
         if (gnutls_x509_crt_get_expiration_time(cert) < now) {
-            virNetError(VIR_ERR_SYSTEM_ERROR, "%s",
-                        _("The client certificate has expired"));
+            /* Warning is reversed from what you expect, since with
+             * this code it is the Server checking the client and
+             * vica-versa */
+            virNetError(VIR_ERR_SYSTEM_ERROR, "%s", sess->isServer ?
+                        _("The client certificate has expired") :
+                        _("The server certificate has expired"));
             gnutls_x509_crt_deinit(cert);
             goto authdeny;
         }
 
         if (gnutls_x509_crt_get_activation_time(cert) > now) {
-            virNetError(VIR_ERR_SYSTEM_ERROR, "%s",
-                        _("The client certificate is not yet active"));
+            /* client/server order reversed. see above */
+            virNetError(VIR_ERR_SYSTEM_ERROR, "%s", sess->isServer ?
+                        _("The client certificate is not yet active") :
+                        _("The server certificate is not yet active"));
             gnutls_x509_crt_deinit(cert);
             goto authdeny;
         }
@@ -756,6 +895,8 @@ virNetTLSSessionPtr virNetTLSSessionNew(virNetTLSContextPtr ctxt,
     gnutls_transport_set_pull_function(sess->session,
                                        virNetTLSSessionPull);
 
+    sess->isServer = ctxt->isServer;
+
     return sess;
 
 error: