crypto: add sanity checking of TLS x509 credentials

If the administrator incorrectly sets up their x509 certificates,
the errors seen at runtime during connection attempts are very
obscure and difficult to diagnose. This has been a particular
problem for people using openssl to generate their certificates
instead of the gnutls certtool, because the openssl tools don't
turn on the various x509 extensions that gnutls expects to be
present by default.

This change thus adds support in the TLS credentials object to
sanity check the certificates when QEMU first loads them. This
gives the administrator immediate feedback for the majority of
common configuration mistakes, reducing the pain involved in
setting up TLS. The code is derived from equivalent code that
has been part of libvirt's TLS support and has been seen to be
valuable in assisting admins.

It is possible to disable the sanity checking, however, via
the new 'sanity-check' property on the tls-creds object type,
with a value of 'no'.

Unit tests are included in this change to verify the correctness
of the sanity checking code in all the key scenarios it is
intended to cope with. As part of the test suite, the pkix_asn1_tab.c
from gnutls is imported. This file is intentionally copied from the
(long since obsolete) gnutls 1.6.3 source tree, since that version
was still under GPLv2+, rather than the GPLv3+ of gnutls >= 2.0.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

1 files changed, 546 insertions, 0 deletions

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 6cc7b53435..dc46bc40f7 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -26,6 +26,516 @@
 
 #ifdef CONFIG_GNUTLS
 
+#include <gnutls/x509.h>
+
+
+static int
+qcrypto_tls_creds_check_cert_times(gnutls_x509_crt_t cert,
+                                   const char *certFile,
+                                   bool isServer,
+                                   bool isCA,
+                                   Error **errp)
+{
+    time_t now = time(NULL);
+
+    if (now == ((time_t)-1)) {
+        error_setg_errno(errp, errno, "cannot get current time");
+        return -1;
+    }
+
+    if (gnutls_x509_crt_get_expiration_time(cert) < now) {
+        error_setg(errp,
+                   (isCA ?
+                    "The CA certificate %s has expired" :
+                    (isServer ?
+                     "The server certificate %s has expired" :
+                     "The client certificate %s has expired")),
+                   certFile);
+        return -1;
+    }
+
+    if (gnutls_x509_crt_get_activation_time(cert) > now) {
+        error_setg(errp,
+                   (isCA ?
+                    "The CA certificate %s is not yet active" :
+                    (isServer ?
+                     "The server certificate %s is not yet active" :
+                     "The client certificate %s is not yet active")),
+                   certFile);
+        return -1;
+    }
+
+    return 0;
+}
+
+
+#if LIBGNUTLS_VERSION_NUMBER >= 2
+/*
+ * The gnutls_x509_crt_get_basic_constraints function isn't
+ * available in GNUTLS 1.0.x branches. This isn't critical
+ * though, since gnutls_certificate_verify_peers2 will do
+ * pretty much the same check at runtime, so we can just
+ * disable this code
+ */
+static int
+qcrypto_tls_creds_check_cert_basic_constraints(QCryptoTLSCredsX509 *creds,
+                                               gnutls_x509_crt_t cert,
+                                               const char *certFile,
+                                               bool isServer,
+                                               bool isCA,
+                                               Error **errp)
+{
+    int status;
+
+    status = gnutls_x509_crt_get_basic_constraints(cert, NULL, NULL, NULL);
+    trace_qcrypto_tls_creds_x509_check_basic_constraints(
+        creds, certFile, status);
+
+    if (status > 0) { /* It is a CA cert */
+        if (!isCA) {
+            error_setg(errp, isServer ?
+                       "The certificate %s basic constraints show a CA, "
+                       "but we need one for a server" :
+                       "The certificate %s basic constraints show a CA, "
+                       "but we need one for a client",
+                       certFile);
+            return -1;
+        }
+    } else if (status == 0) { /* It is not a CA cert */
+        if (isCA) {
+            error_setg(errp,
+                       "The certificate %s basic constraints do not "
+                       "show a CA",
+                       certFile);
+            return -1;
+        }
+    } else if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
+        /* Missing basicConstraints */
+        if (isCA) {
+            error_setg(errp,
+                       "The certificate %s is missing basic constraints "
+                       "for a CA",
+                       certFile);
+            return -1;
+        }
+    } else { /* General error */
+        error_setg(errp,
+                   "Unable to query certificate %s basic constraints: %s",
+                   certFile, gnutls_strerror(status));
+        return -1;
+    }
+
+    return 0;
+}
+#endif
+
+
+static int
+qcrypto_tls_creds_check_cert_key_usage(QCryptoTLSCredsX509 *creds,
+                                       gnutls_x509_crt_t cert,
+                                       const char *certFile,
+                                       bool isCA,
+                                       Error **errp)
+{
+    int status;
+    unsigned int usage = 0;
+    unsigned int critical = 0;
+
+    status = gnutls_x509_crt_get_key_usage(cert, &usage, &critical);
+    trace_qcrypto_tls_creds_x509_check_key_usage(
+        creds, certFile, status, usage, critical);
+
+    if (status < 0) {
+        if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
+            usage = isCA ? GNUTLS_KEY_KEY_CERT_SIGN :
+                GNUTLS_KEY_DIGITAL_SIGNATURE|GNUTLS_KEY_KEY_ENCIPHERMENT;
+        } else {
+            error_setg(errp,
+                       "Unable to query certificate %s key usage: %s",
+                       certFile, gnutls_strerror(status));
+            return -1;
+        }
+    }
+
+    if (isCA) {
+        if (!(usage & GNUTLS_KEY_KEY_CERT_SIGN)) {
+            if (critical) {
+                error_setg(errp,
+                           "Certificate %s usage does not permit "
+                           "certificate signing", certFile);
+                return -1;
+            }
+        }
+    } else {
+        if (!(usage & GNUTLS_KEY_DIGITAL_SIGNATURE)) {
+            if (critical) {
+                error_setg(errp,
+                           "Certificate %s usage does not permit digital "
+                           "signature", certFile);
+                return -1;
+            }
+        }
+        if (!(usage & GNUTLS_KEY_KEY_ENCIPHERMENT)) {
+            if (critical) {
+                error_setg(errp,
+                           "Certificate %s usage does not permit key "
+                           "encipherment", certFile);
+                return -1;
+            }
+        }
+    }
+
+    return 0;
+}
+
+
+static int
+qcrypto_tls_creds_check_cert_key_purpose(QCryptoTLSCredsX509 *creds,
+                                         gnutls_x509_crt_t cert,
+                                         const char *certFile,
+                                         bool isServer,
+                                         Error **errp)
+{
+    int status;
+    size_t i;
+    unsigned int purposeCritical;
+    unsigned int critical;
+    char *buffer = NULL;
+    size_t size;
+    bool allowClient = false, allowServer = false;
+
+    critical = 0;
+    for (i = 0; ; i++) {
+        size = 0;
+        status = gnutls_x509_crt_get_key_purpose_oid(cert, i, buffer,
+                                                     &size, NULL);
+
+        if (status == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
+
+            /* If there is no data at all, then we must allow
+               client/server to pass */
+            if (i == 0) {
+                allowServer = allowClient = true;
+            }
+            break;
+        }
+        if (status != GNUTLS_E_SHORT_MEMORY_BUFFER) {
+            error_setg(errp,
+                       "Unable to query certificate %s key purpose: %s",
+                       certFile, gnutls_strerror(status));
+            return -1;
+        }
+
+        buffer = g_new0(char, size);
+
+        status = gnutls_x509_crt_get_key_purpose_oid(cert, i, buffer,
+                                                     &size, &purposeCritical);
+
+        if (status < 0) {
+            trace_qcrypto_tls_creds_x509_check_key_purpose(
+                creds, certFile, status, "<none>", purposeCritical);
+            g_free(buffer);
+            error_setg(errp,
+                       "Unable to query certificate %s key purpose: %s",
+                       certFile, gnutls_strerror(status));
+            return -1;
+        }
+        trace_qcrypto_tls_creds_x509_check_key_purpose(
+            creds, certFile, status, buffer, purposeCritical);
+        if (purposeCritical) {
+            critical = true;
+        }
+
+        if (g_str_equal(buffer, GNUTLS_KP_TLS_WWW_SERVER)) {
+            allowServer = true;
+        } else if (g_str_equal(buffer, GNUTLS_KP_TLS_WWW_CLIENT)) {
+            allowClient = true;
+        } else if (g_str_equal(buffer, GNUTLS_KP_ANY)) {
+            allowServer = allowClient = true;
+        }
+
+        g_free(buffer);
+    }
+
+    if (isServer) {
+        if (!allowServer) {
+            if (critical) {
+                error_setg(errp,
+                           "Certificate %s purpose does not allow "
+                           "use with a TLS server", certFile);
+                return -1;
+            }
+        }
+    } else {
+        if (!allowClient) {
+            if (critical) {
+                error_setg(errp,
+                           "Certificate %s purpose does not allow use "
+                           "with a TLS client", certFile);
+                return -1;
+            }
+        }
+    }
+
+    return 0;
+}
+
+
+static int
+qcrypto_tls_creds_check_cert(QCryptoTLSCredsX509 *creds,
+                             gnutls_x509_crt_t cert,
+                             const char *certFile,
+                             bool isServer,
+                             bool isCA,
+                             Error **errp)
+{
+    if (qcrypto_tls_creds_check_cert_times(cert, certFile,
+                                           isServer, isCA,
+                                           errp) < 0) {
+        return -1;
+    }
+
+#if LIBGNUTLS_VERSION_NUMBER >= 2
+    if (qcrypto_tls_creds_check_cert_basic_constraints(creds,
+                                                       cert, certFile,
+                                                       isServer, isCA,
+                                                       errp) < 0) {
+        return -1;
+    }
+#endif
+
+    if (qcrypto_tls_creds_check_cert_key_usage(creds,
+                                               cert, certFile,
+                                               isCA, errp) < 0) {
+        return -1;
+    }
+
+    if (!isCA &&
+        qcrypto_tls_creds_check_cert_key_purpose(creds,
+                                                 cert, certFile,
+                                                 isServer, errp) < 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static int
+qcrypto_tls_creds_check_cert_pair(gnutls_x509_crt_t cert,
+                                  const char *certFile,
+                                  gnutls_x509_crt_t *cacerts,
+                                  size_t ncacerts,
+                                  const char *cacertFile,
+                                  bool isServer,
+                                  Error **errp)
+{
+    unsigned int status;
+
+    if (gnutls_x509_crt_list_verify(&cert, 1,
+                                    cacerts, ncacerts,
+                                    NULL, 0,
+                                    0, &status) < 0) {
+        error_setg(errp, isServer ?
+                   "Unable to verify server certificate %s against "
+                   "CA certificate %s" :
+                   "Unable to verify client certificate %s against "
+                   "CA certificate %s",
+                   certFile, cacertFile);
+        return -1;
+    }
+
+    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
+
+        error_setg(errp,
+                   "Our own certificate %s failed validation against %s: %s",
+                   certFile, cacertFile, reason);
+        return -1;
+    }
+
+    return 0;
+}
+
+
+static gnutls_x509_crt_t
+qcrypto_tls_creds_load_cert(QCryptoTLSCredsX509 *creds,
+                            const char *certFile,
+                            bool isServer,
+                            Error **errp)
+{
+    gnutls_datum_t data;
+    gnutls_x509_crt_t cert = NULL;
+    char *buf = NULL;
+    gsize buflen;
+    GError *gerr;
+    int ret = -1;
+
+    trace_qcrypto_tls_creds_x509_load_cert(creds, isServer, certFile);
+
+    if (gnutls_x509_crt_init(&cert) < 0) {
+        error_setg(errp, "Unable to initialize certificate");
+        goto cleanup;
+    }
+
+    if (!g_file_get_contents(certFile, &buf, &buflen, &gerr)) {
+        error_setg(errp, "Cannot load CA cert list %s: %s",
+                   certFile, gerr->message);
+        g_error_free(gerr);
+        goto cleanup;
+    }
+
+    data.data = (unsigned char *)buf;
+    data.size = strlen(buf);
+
+    if (gnutls_x509_crt_import(cert, &data, GNUTLS_X509_FMT_PEM) < 0) {
+        error_setg(errp, isServer ?
+                   "Unable to import server certificate %s" :
+                   "Unable to import client certificate %s",
+                   certFile);
+        goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    if (ret != 0) {
+        gnutls_x509_crt_deinit(cert);
+        cert = NULL;
+    }
+    g_free(buf);
+    return cert;
+}
+
+
+static int
+qcrypto_tls_creds_load_ca_cert_list(QCryptoTLSCredsX509 *creds,
+                                    const char *certFile,
+                                    gnutls_x509_crt_t *certs,
+                                    unsigned int certMax,
+                                    size_t *ncerts,
+                                    Error **errp)
+{
+    gnutls_datum_t data;
+    char *buf = NULL;
+    gsize buflen;
+    int ret = -1;
+    GError *gerr = NULL;
+
+    *ncerts = 0;
+    trace_qcrypto_tls_creds_x509_load_cert_list(creds, certFile);
+
+    if (!g_file_get_contents(certFile, &buf, &buflen, &gerr)) {
+        error_setg(errp, "Cannot load CA cert list %s: %s",
+                   certFile, gerr->message);
+        g_error_free(gerr);
+        goto cleanup;
+    }
+
+    data.data = (unsigned char *)buf;
+    data.size = strlen(buf);
+
+    if (gnutls_x509_crt_list_import(certs, &certMax, &data,
+                                    GNUTLS_X509_FMT_PEM, 0) < 0) {
+        error_setg(errp,
+                   "Unable to import CA certificate list %s",
+                   certFile);
+        goto cleanup;
+    }
+    *ncerts = certMax;
+
+    ret = 0;
+
+ cleanup:
+    g_free(buf);
+    return ret;
+}
+
+
+#define MAX_CERTS 16
+static int
+qcrypto_tls_creds_x509_sanity_check(QCryptoTLSCredsX509 *creds,
+                                    bool isServer,
+                                    const char *cacertFile,
+                                    const char *certFile,
+                                    Error **errp)
+{
+    gnutls_x509_crt_t cert = NULL;
+    gnutls_x509_crt_t cacerts[MAX_CERTS];
+    size_t ncacerts = 0;
+    size_t i;
+    int ret = -1;
+
+    memset(cacerts, 0, sizeof(cacerts));
+    if (access(certFile, R_OK) == 0) {
+        cert = qcrypto_tls_creds_load_cert(creds,
+                                           certFile, isServer,
+                                           errp);
+        if (!cert) {
+            goto cleanup;
+        }
+    }
+    if (access(cacertFile, R_OK) == 0) {
+        if (qcrypto_tls_creds_load_ca_cert_list(creds,
+                                                cacertFile, cacerts,
+                                                MAX_CERTS, &ncacerts,
+                                                errp) < 0) {
+            goto cleanup;
+        }
+    }
+
+    if (cert &&
+        qcrypto_tls_creds_check_cert(creds,
+                                     cert, certFile, isServer,
+                                     false, errp) < 0) {
+        goto cleanup;
+    }
+
+    for (i = 0; i < ncacerts; i++) {
+        if (qcrypto_tls_creds_check_cert(creds,
+                                         cacerts[i], cacertFile,
+                                         isServer, true, errp) < 0) {
+            goto cleanup;
+        }
+    }
+
+    if (cert && ncacerts &&
+        qcrypto_tls_creds_check_cert_pair(cert, certFile, cacerts,
+                                          ncacerts, cacertFile,
+                                          isServer, errp) < 0) {
+        goto cleanup;
+    }
+
+    ret = 0;
+
+ cleanup:
+    if (cert) {
+        gnutls_x509_crt_deinit(cert);
+    }
+    for (i = 0; i < ncacerts; i++) {
+        gnutls_x509_crt_deinit(cacerts[i]);
+    }
+    return ret;
+}
+
 
 static int
 qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
@@ -71,6 +581,13 @@ qcrypto_tls_creds_x509_load(QCryptoTLSCredsX509 *creds,
         }
     }
 
+    if (creds->sanityCheck &&
+        qcrypto_tls_creds_x509_sanity_check(creds,
+            creds->parent_obj.endpoint == QCRYPTO_TLS_CREDS_ENDPOINT_SERVER,
+            cacert, cert, errp) < 0) {
+        goto cleanup;
+    }
+
     ret = gnutls_certificate_allocate_credentials(&creds->data);
     if (ret < 0) {
         error_setg(errp, "Cannot allocate credentials: '%s'",
@@ -204,6 +721,27 @@ qcrypto_tls_creds_x509_prop_get_loaded(Object *obj G_GNUC_UNUSED,
 
 
 static void
+qcrypto_tls_creds_x509_prop_set_sanity(Object *obj,
+                                       bool value,
+                                       Error **errp G_GNUC_UNUSED)
+{
+    QCryptoTLSCredsX509 *creds = QCRYPTO_TLS_CREDS_X509(obj);
+
+    creds->sanityCheck = value;
+}
+
+
+static bool
+qcrypto_tls_creds_x509_prop_get_sanity(Object *obj,
+                                       Error **errp G_GNUC_UNUSED)
+{
+    QCryptoTLSCredsX509 *creds = QCRYPTO_TLS_CREDS_X509(obj);
+
+    return creds->sanityCheck;
+}
+
+
+static void
 qcrypto_tls_creds_x509_complete(UserCreatable *uc, Error **errp)
 {
     object_property_set_bool(OBJECT(uc), true, "loaded", errp);
@@ -213,10 +751,18 @@ qcrypto_tls_creds_x509_complete(UserCreatable *uc, Error **errp)
 static void
 qcrypto_tls_creds_x509_init(Object *obj)
 {
+    QCryptoTLSCredsX509 *creds = QCRYPTO_TLS_CREDS_X509(obj);
+
+    creds->sanityCheck = true;
+
     object_property_add_bool(obj, "loaded",
                              qcrypto_tls_creds_x509_prop_get_loaded,
                              qcrypto_tls_creds_x509_prop_set_loaded,
                              NULL);
+    object_property_add_bool(obj, "sanity-check",
+                             qcrypto_tls_creds_x509_prop_get_sanity,
+                             qcrypto_tls_creds_x509_prop_set_sanity,
+                             NULL);
 }