Commit 9a2fd434 authored by Daniel P. Berrangé's avatar Daniel P. Berrangé
Browse files

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: default avatarDaniel P. Berrange <berrange@redhat.com>
parent 85bcbc78

configure

+22 −0
Original line number Diff line number Diff line
@@ -416,6 +416,9 @@ if test "$debug_info" = "yes"; then 
    LDFLAGS="-g $LDFLAGS"

fi



test_cflags=""

test_libs=""



# make source path absolute

source_path=`cd "$source_path"; pwd`
@@ -2249,6 +2252,19 @@ if test "$gnutls_nettle" != "no"; then 
    fi

fi



##########################################

# libtasn1 - only for the TLS creds/session test suite



tasn1=yes

if $pkg_config --exists "libtasn1"; then

    tasn1_cflags=`$pkg_config --cflags libtasn1`

    tasn1_libs=`$pkg_config --libs libtasn1`

    test_cflags="$test_cflags $tasn1_cflags"

    test_libs="$test_libs $tasn1_libs"

else

    tasn1=no

fi





##########################################

# VTE probe
@@ -4574,6 +4590,7 @@ echo "GNUTLS support $gnutls" 
echo "GNUTLS hash       $gnutls_hash"

echo "GNUTLS gcrypt     $gnutls_gcrypt"

echo "GNUTLS nettle     $gnutls_nettle ${gnutls_nettle+($nettle_version)}"

echo "libtasn1          $tasn1"

echo "VTE support       $vte"

echo "curses support    $curses"

echo "curl support      $curl"
@@ -4945,6 +4962,9 @@ if test "$gnutls_nettle" = "yes" ; then 
  echo "CONFIG_GNUTLS_NETTLE=y" >> $config_host_mak

  echo "CONFIG_NETTLE_VERSION_MAJOR=${nettle_version%%.*}" >> $config_host_mak

fi

if test "$tasn1" = "yes" ; then

  echo "CONFIG_TASN1=y" >> $config_host_mak

fi

if test "$vte" = "yes" ; then

  echo "CONFIG_VTE=y" >> $config_host_mak

  echo "VTE_CFLAGS=$vte_cflags" >> $config_host_mak
@@ -5268,6 +5288,8 @@ echo "EXESUF=$EXESUF" >> $config_host_mak 
echo "DSOSUF=$DSOSUF" >> $config_host_mak

echo "LDFLAGS_SHARED=$LDFLAGS_SHARED" >> $config_host_mak

echo "LIBS_QGA+=$libs_qga" >> $config_host_mak

echo "TEST_LIBS=$test_libs" >> $config_host_mak

echo "TEST_CFLAGS=$test_cflags" >> $config_host_mak

echo "POD2MAN=$POD2MAN" >> $config_host_mak

echo "TRANSLATE_OPT_CFLAGS=$TRANSLATE_OPT_CFLAGS" >> $config_host_mak

if test "$gcov" = "yes" ; then

crypto/tlscredsx509.c

+546 −0
Original line number Diff line number Diff line
@@ -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'",
@@ -203,6 +720,27 @@ qcrypto_tls_creds_x509_prop_get_loaded(Object *obj G_GNUC_UNUSED, 
#endif /* ! CONFIG_GNUTLS */





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)

{
@@ -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);

}

include/crypto/tlscredsx509.h

+1 −0
Original line number Diff line number Diff line
@@ -100,6 +100,7 @@ struct QCryptoTLSCredsX509 { 
#ifdef CONFIG_GNUTLS

    gnutls_certificate_credentials_t data;

#endif

    bool sanityCheck;

};

tests/.gitignore

+3 −0
Original line number Diff line number Diff line
@@ -12,6 +12,9 @@ test-bitops 
test-coroutine

test-crypto-cipher

test-crypto-hash

test-crypto-tlscredsx509

test-crypto-tlscredsx509-work/

test-crypto-tlscredsx509-certs/

test-cutils

test-hbitmap

test-int128

tests/Makefile

+5 −0
Original line number Diff line number Diff line
@@ -78,6 +78,7 @@ check-unit-y += tests/test-write-threshold$(EXESUF) 
gcov-files-test-write-threshold-y = block/write-threshold.c

check-unit-$(CONFIG_GNUTLS_HASH) += tests/test-crypto-hash$(EXESUF)

check-unit-y += tests/test-crypto-cipher$(EXESUF)

check-unit-$(CONFIG_GNUTLS) += tests/test-crypto-tlscredsx509$(EXESUF)



check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
@@ -358,6 +359,8 @@ tests/test-mul64$(EXESUF): tests/test-mul64.o $(test-util-obj-y) 
tests/test-bitops$(EXESUF): tests/test-bitops.o $(test-util-obj-y)

tests/test-crypto-hash$(EXESUF): tests/test-crypto-hash.o $(test-crypto-obj-y)

tests/test-crypto-cipher$(EXESUF): tests/test-crypto-cipher.o $(test-crypto-obj-y)

tests/test-crypto-tlscredsx509$(EXESUF): tests/test-crypto-tlscredsx509.o \

	tests/crypto-tls-x509-helpers.o tests/pkix_asn1_tab.o $(test-crypto-obj-y)



libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o

libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
@@ -426,6 +429,8 @@ tests/test-write-threshold$(EXESUF): tests/test-write-threshold.o $(test-block-o 
ifeq ($(CONFIG_POSIX),y)

LIBS += -lutil

endif

LIBS += $(TEST_LIBS)

CFLAGS += $(TEST_CFLAGS)



# QTest rules