commit 7163389b550a36fa017f700713405fc3c89dc234 Author: Nick Mathewson nickm@torproject.org Date: Thu Aug 23 14:03:00 2018 -0400

Several unit tests to improve test coverage of x509*.c --- src/lib/crypt_ops/crypto_digest.c | 4 +- src/lib/crypt_ops/crypto_digest.h | 3 +- src/lib/tls/x509.c | 3 +- src/lib/tls/x509_nss.c | 4 +- src/lib/tls/x509_openssl.c | 8 +- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_x509.c | 204 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 221 insertions(+), 8 deletions(-)

diff --git a/src/lib/crypt_ops/crypto_digest.c b/src/lib/crypt_ops/crypto_digest.c index 2bc5f0834..77cf18dca 100644 --- a/src/lib/crypt_ops/crypto_digest.c +++ b/src/lib/crypt_ops/crypto_digest.c @@ -116,8 +116,8 @@ library_supports_digest(digest_algorithm_t alg) * <b>m</b>. Write the DIGEST_LEN byte result into <b>digest</b>. * Return 0 on success, -1 on failure. */ -int -crypto_digest(char *digest, const char *m, size_t len) +MOCK_IMPL(int, +crypto_digest,(char *digest, const char *m, size_t len)) { tor_assert(m); tor_assert(digest); diff --git a/src/lib/crypt_ops/crypto_digest.h b/src/lib/crypt_ops/crypto_digest.h index 59713d2b9..204f1aaff 100644 --- a/src/lib/crypt_ops/crypto_digest.h +++ b/src/lib/crypt_ops/crypto_digest.h @@ -16,6 +16,7 @@ #include "lib/cc/torint.h" #include "lib/defs/digest_sizes.h" #include "lib/malloc/malloc.h" +#include "lib/testsupport/testsupport.h"

/** Length of a sha1 message digest when encoded in base32 with trailing = * signs removed. */ @@ -75,7 +76,7 @@ typedef struct crypto_xof_t crypto_xof_t; struct smartlist_t;

/* SHA-1 and other digests */ -int crypto_digest(char *digest, const char *m, size_t len); +MOCK_DECL(int, crypto_digest,(char *digest, const char *m, size_t len)); int crypto_digest256(char *digest, const char *m, size_t len, digest_algorithm_t algorithm); int crypto_digest512(char *digest, const char *m, size_t len, diff --git a/src/lib/tls/x509.c b/src/lib/tls/x509.c index cff1c1302..c88298b6c 100644 --- a/src/lib/tls/x509.c +++ b/src/lib/tls/x509.c @@ -118,6 +118,7 @@ tor_x509_cert_new,(tor_x509_cert_impl_t *x509_cert)) crypto_pk_t *pk = tor_tls_cert_get_key(cert); if (pk) { if (crypto_pk_get_common_digests(pk, &cert->pkey_digests) < 0) { + log_warn(LD_CRYPTO, "unable to compute digests of certificate key"); crypto_pk_free(pk); goto err; } @@ -128,10 +129,8 @@ tor_x509_cert_new,(tor_x509_cert_impl_t *x509_cert))

return cert; err: - /* LCOV_EXCL_START for the same reason as the exclusion above */ tor_free(cert); log_err(LD_CRYPTO, "Couldn't wrap encoded X509 certificate."); tor_x509_cert_impl_free_(x509_cert); return NULL; - /* LCOV_EXCL_STOP */ } diff --git a/src/lib/tls/x509_nss.c b/src/lib/tls/x509_nss.c index 35b3d2542..68aebbb69 100644 --- a/src/lib/tls/x509_nss.c +++ b/src/lib/tls/x509_nss.c @@ -62,9 +62,11 @@ tor_tls_create_certificate_internal(crypto_pk_t *rsa,

validity = CERT_CreateValidity(((PRTime)start_time) * PRTIME_PER_SEC, ((PRTime)end_time) * PRTIME_PER_SEC); - if (! validity) { + if (BUG(! validity)) { + /* LCOV_EXCL_START */ crypto_nss_log_errors(LOG_WARN, "creating a validity object"); goto err; + /* LCOV_EXCL_STOP */ }

unsigned long serial_number; diff --git a/src/lib/tls/x509_openssl.c b/src/lib/tls/x509_openssl.c index 28a30b66e..f315b88f3 100644 --- a/src/lib/tls/x509_openssl.c +++ b/src/lib/tls/x509_openssl.c @@ -327,11 +327,15 @@ tor_tls_cert_is_valid(int severity, if (check_rsa_1024 && cert_key) { RSA *rsa = EVP_PKEY_get1_RSA(cert_key); #ifdef OPENSSL_1_1_API - if (rsa && RSA_bits(rsa) == 1024) + if (rsa && RSA_bits(rsa) == 1024) { #else - if (rsa && BN_num_bits(rsa->n) == 1024) + if (rsa && BN_num_bits(rsa->n) == 1024) { #endif key_ok = 1; + } else { + log_fn(severity, LD_CRYPTO, "Invalid certificate: Key is not RSA1024."); + } + if (rsa) RSA_free(rsa); } else if (cert_key) { diff --git a/src/test/include.am b/src/test/include.am index 05149b865..69680f969 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -178,6 +178,7 @@ src_test_test_SOURCES += \ src/test/test_util_format.c \ src/test/test_util_process.c \ src/test/test_voting_schedule.c \ + src/test/test_x509.c \ src/test/test_helpers.c \ src/test/test_dns.c \ src/test/testing_common.c \ diff --git a/src/test/test.c b/src/test/test.c index 962344305..df586fc76 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -924,6 +924,7 @@ struct testgroup_t testgroups[] = { #ifndef ENABLE_NSS { "tortls/openssl/", tortls_openssl_tests }, #endif + { "tortls/x509/", x509_tests }, { "util/", util_tests }, { "util/format/", util_format_tests }, { "util/logging/", logging_tests }, diff --git a/src/test/test.h b/src/test/test.h index 94aa9443d..a46fedf3e 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -265,6 +265,7 @@ extern struct testcase_t voting_schedule_tests[]; extern struct testcase_t dns_tests[]; extern struct testcase_t handle_tests[]; extern struct testcase_t sr_tests[]; +extern struct testcase_t x509_tests[];

extern struct testcase_t slow_crypto_tests[]; extern struct testcase_t slow_util_tests[]; diff --git a/src/test/test_x509.c b/src/test/test_x509.c new file mode 100644 index 000000000..9163977bd --- /dev/null +++ b/src/test/test_x509.c @@ -0,0 +1,204 @@ +/* Copyright (c) 2010-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#define TOR_X509_PRIVATE +#include "orconfig.h" + +#ifdef _WIN32 +#include <winsock2.h> +#endif +#include <math.h> +#include <stddef.h> + +#include "lib/cc/compat_compiler.h" + +#include "core/or/or.h" +#include "lib/log/log.h" +#include "app/config/config.h" +#include "lib/tls/x509.h" +#include "lib/tls/x509_internal.h" +#include "app/config/or_state_st.h" + +#include "test/test.h" +#include "test/log_test_helpers.h" + +#include "tinytest.h" + +/* A mock replacement for crypto_digest that always fails. */ +static int +mock_failing_digest(char *digest, const char *m, size_t len) +{ + (void)digest; + (void)m; + (void)len; + return -1; +} + +static void +test_x509_cert_new_failing_digest(void *arg) +{ + (void)arg; + crypto_pk_t *pk1=NULL, *pk2=NULL; + tor_x509_cert_impl_t *impl = NULL; + tor_x509_cert_t *cert = NULL; + pk1 = pk_generate(0); + pk2 = pk_generate(1); + + impl = tor_tls_create_certificate(pk1, pk2, "hello", "world", 86400*100); + tt_assert(impl); + MOCK(crypto_digest, mock_failing_digest); + + setup_full_capture_of_logs(LOG_WARN); + cert = tor_x509_cert_new(impl); + tt_assert(!cert); + expect_log_msg_containing("Couldn't wrap encoded X509 certificate"); + expect_log_msg_containing("unable to compute digests of certificate key"); + + done: + crypto_pk_free(pk1); + crypto_pk_free(pk2); + if (impl) + tor_x509_cert_impl_free_(impl); + UNMOCK(crypto_digest); + teardown_capture_of_logs(); +} + +static tor_x509_cert_t * +cert_from_der64(const char *der64) +{ + size_t der64len = strlen(der64); + unsigned char *der = tor_malloc_zero(der64len); + int derlen; + tor_x509_cert_t *cert = NULL; + + derlen = base64_decode((char*)der, der64len, + der64, der64len); + if (derlen >= 0) + cert = tor_x509_cert_decode(der, derlen); + tor_free(der); + return cert; +} + +static void +test_x509_consume_ec_cert(void *arg) +{ + (void)arg; + /* This is a small self-signed EC certificate. */ + const char certificate[] = + "MIIBEzCBugIJAIdl5svgOZ0OMAoGCCqGSM49BAMCMBIxEDAOBgNVBAMMB1Rlc3Rp\n" + "bmcwHhcNMTgwODIzMTcyMzI1WhcNMTkwODIzMTcyMzI1WjASMRAwDgYDVQQDDAdU\n" + "ZXN0aW5nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAExMDpnRc0Btic3tIyCKNE\n" + "iNY4j4gzcaYzS2sTYRoVK3RAukG29Qg6/c8e8XcnsSquU4fItYxDRbi/3nhYk4CP\n" + "GDAKBggqhkjOPQQDAgNIADBFAiA0h1q03C2xlONUgAOonJLrlV1SUtMeKDxNsxsU\n" + "+FSPvQIhAM7kY9Tlt0ELmyMnORPp1VJieXn/qhL5VoxGxSedTbny\n"; + const time_t now = 1535045321; /* when I'm writing this test. */ + tor_x509_cert_t *cert = cert_from_der64(certificate); + tt_assert(cert); + + tt_ptr_op(NULL, OP_EQ, tor_tls_cert_get_key(cert)); + + /* It's a self-signed cert -- make sure it signed itself. */ + tt_assert(tor_tls_cert_is_valid(LOG_ERR, cert, cert, now, 0)); + + /* Make sure we detect its key as non-RSA1024 */ + setup_capture_of_logs(LOG_INFO); + tt_assert(! tor_tls_cert_is_valid(LOG_INFO, cert, cert, now, 1)); + expect_log_msg_containing("Key is not RSA1024"); + + done: + tor_x509_cert_free(cert); + teardown_capture_of_logs(); +} + +static void +test_x509_reject_tiny_keys(void *arg) +{ + (void)arg; + const char *certificates[] = { + /* Self-signed RSA512 */ + "MIIBXDCCAQYCCQDKikjJYZI5uDANBgkqhkiG9w0BAQsFADA1MRUwEwYDVQQHDAxE\n" + "ZWZhdWx0IENpdHkxHDAaBgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQwHhcNMTgw\n" + "ODIzMTczNjQ4WhcNMTkwODIzMTczNjQ4WjA1MRUwEwYDVQQHDAxEZWZhdWx0IENp\n" + "dHkxHDAaBgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQwXDANBgkqhkiG9w0BAQEF\n" + "AANLADBIAkEAqOvVKzrSpmKOTNqDzBG/iZrUdhCrMRsymFXyIScJcdsyn7jB8RMy\n" + "fbHqG8EqB8HHLU/eqt/+zhh2w08Lx3+5QwIDAQABMA0GCSqGSIb3DQEBCwUAA0EA\n" + "RSCq0sNbD9uWfcBqF0U4MtfFjU5x+RQQCeBVtAzwC9bggSILKZfB9XUvtGh6vqig\n", + /* Self-signed secp112r2 */ + "MIIBLTCB+QIJAI0LtN9uWxy3MAoGCCqGSM49BAMCMEUxCzAJBgNVBAYTAkFVMRMw\n" + "EQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0\n" + "eSBMdGQwHhcNMTgwODIzMTc0MTQ4WhcNMTkwODIzMTc0MTQ4WjBFMQswCQYDVQQG\n" + "EwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50ZXJuZXQgV2lk\n" + "Z2l0cyBQdHkgTHRkMDIwEAYHKoZIzj0CAQYFK4EEAAcDHgAEf7dFHo7xhCtIcgyo\n" + "Px+IDcUUlntZCtar6V4O0zAKBggqhkjOPQQDAgMjADAgAg4yhBJMEmpkNbZU95Zf\n" + "uwIOJAan4J1ETxUII1RrGmw=\n" + }; + const time_t now = 1535046182; + tor_x509_cert_t *cert = NULL; + + unsigned i; + for (i = 0; i < ARRAY_LENGTH(certificates); ++i) { + cert = cert_from_der64(certificates[i]); + /* It might parse okay, depending on our version of NSS or OpenSSL. */ + if (cert == NULL) + continue; + /* But it should not validate. */ + tt_assert(! tor_tls_cert_is_valid(LOG_INFO, cert, cert, now, 0)); + tor_x509_cert_free(cert); + } + + done: + tor_x509_cert_free(cert); +} + +static void +test_x509_expiration(void *arg) +{ + (void)arg; + /* a 365-day RSA2048 cert, created between 0 and 60 minutes before "now" */ + const char certificate[] = + "MIICzjCCAbYCCQDxIONWIQ9OGDANBgkqhkiG9w0BAQsFADApMQswCQYDVQQGEwJV\n" + "UzEaMBgGA1UEAwwRSW50ZXJlc3RpbmcgdGltZXMwHhcNMTgwODIzMTc1NTE4WhcN\n" + "MTkwODIzMTc1NTE4WjApMQswCQYDVQQGEwJVUzEaMBgGA1UEAwwRSW50ZXJlc3Rp\n" + "bmcgdGltZXMwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQD0Blz1fBii\n" + "OffpFlzMrmfPah/vkPcNrwoyx5YiosbHErYUpqdCtfNb7rbBM5xcac1LmF9kjnOQ\n" + "uAw1jsCNE82QHwWMlXOqaZCEJsnttNo0Y7yaSR/ChbGJ54XCp+Lx2acyTeH9cBWU\n" + "de8/sKAQ4NqpbEP01pBH4+1mPu2MYWjVWVicUxmw0mJ3cfkJCWUzt0nC4ls8+Itk\n" + "7XliKb216Z9uQXu/zD/JGkxAljnFs1jXCX4NyWz46xnJFzXbYCeyQnBz0tUbAvgg\n" + "uRdryYtHzD46hd8LTXH6oK2gV64ILAhDnRb1aBjnCXxbex24XoW3hjSrKGTdNsXA\n" + "RMWU/8QZaoiBAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAFIYDBcbit2kOMrHECZK\n" + "ctem40A3s+0ZifzZ2KLhW8dTr/2Zb6DnlqVm2iUOV4cG/o1RAn/HzkQQuWEq+oBG\n" + "yOPVHudvCyGs+2ZQWudgAv9xq8N7KtZwJhnn42c2YSoreqRXDQgJqGFatyr+XdR7\n" + "gdQapLI4BFbZToeXp49Nl+q9330hKaSmIYmWEZ7R/33R64PU2el7X9/apYEcuZQT\n" + "+FjEqcO1lJ8/dTwM/2C1BJZqUeFTAu+ac1M+4//qyJRUUc6xSJLhiens8atWaxwL\n" + "eBCT8fCY8oPOwA1eImc/yWWmWXpv8bBWVe8OeLCMKM/OZoIdFqQpqSdcyGoh/kIW\n" + "Dws=\n"; + const time_t now = 1535046996; + + tor_x509_cert_t *cert = cert_from_der64(certificate); + tt_assert(cert); + + tt_assert(tor_tls_cert_is_valid(LOG_ERR, cert, cert, now, 0)); + + tt_assert(tor_tls_cert_is_valid(LOG_ERR, cert, cert, + now-TOR_X509_FUTURE_SLOP, 0)); + tt_assert(tor_tls_cert_is_valid(LOG_ERR, cert, cert, + now+365*86400+TOR_X509_PAST_SLOP - 3600, 0)); + + tt_assert(! tor_tls_cert_is_valid(LOG_INFO, cert, cert, + now-TOR_X509_FUTURE_SLOP - 3600, 0)); + tt_assert(! tor_tls_cert_is_valid(LOG_INFO, cert, cert, + now+365*86400+TOR_X509_FUTURE_SLOP, 0)); + + done: + tor_x509_cert_free(cert); +} + +#define TEST(name) { #name, test_x509_ ## name, TT_FORK, 0, NULL } + +struct testcase_t x509_tests[] = { + TEST(cert_new_failing_digest), + TEST(consume_ec_cert), + TEST(reject_tiny_keys), + TEST(expiration), + END_OF_TESTCASES +};