September 2018 - tor-commits - lists.torproject.org

[tor/master] Rename crypto_pk_check_key(), use it more reasonably, add tests
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 600e046ed32f9826bcbdf223d99ed8af23a67504 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 31 16:15:20 2018 -0400 Rename crypto_pk_check_key(), use it more reasonably, add tests This function was a wrapper around RSA_check_key() in openssl, which checks for invalid RSA private keys (like those where p or q are composite, or where d is not the inverse of e, or where n != p*q). We don't need a function like this in NSS, since unlike OpenSSL, NSS won't let you import a bogus private key. I've renamed the function and changed its return type to make it more reasonable, and added a unit test for trying to read a key where n != p*q. --- src/feature/relay/router.c | 2 +- src/feature/rend/rendservice.c | 2 +- src/lib/crypt_ops/crypto_rsa.h | 2 +- src/lib/crypt_ops/crypto_rsa_nss.c | 15 +++++++--- src/lib/crypt_ops/crypto_rsa_openssl.c | 18 +++++++---- src/test/test_crypto.c | 55 ++++++++++++++++++++++++++++++++++ 6 files changed, 82 insertions(+), 12 deletions(-) diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 7685760ac..55e8f1403 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -590,7 +590,7 @@ init_key_from_file(const char *fname, int generate, int severity, tor_log(severity, LD_GENERAL,"Error generating onion key"); goto error; } - if (crypto_pk_check_key(prkey) <= 0) { + if (! crypto_pk_is_valid_private_key(prkey)) { tor_log(severity, LD_GENERAL,"Generated key seems invalid"); goto error; } diff --git a/src/feature/rend/rendservice.c b/src/feature/rend/rendservice.c index 1a99bd56e..77775602b 100644 --- a/src/feature/rend/rendservice.c +++ b/src/feature/rend/rendservice.c @@ -1629,7 +1629,7 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) crypto_pk_free(prkey); goto err; } - if (crypto_pk_check_key(prkey) <= 0) { + if (! crypto_pk_is_valid_private_key(prkey)) { log_warn(LD_BUG,"Generated client key seems invalid"); crypto_pk_free(prkey); goto err; diff --git a/src/lib/crypt_ops/crypto_rsa.h b/src/lib/crypt_ops/crypto_rsa.h index aaf32ec1b..dd25bc31b 100644 --- a/src/lib/crypt_ops/crypto_rsa.h +++ b/src/lib/crypt_ops/crypto_rsa.h @@ -64,7 +64,7 @@ int crypto_pk_read_private_key_from_string(crypto_pk_t *env, int crypto_pk_write_private_key_to_filename(crypto_pk_t *env, const char *fname); -int crypto_pk_check_key(crypto_pk_t *env); +int crypto_pk_is_valid_private_key(crypto_pk_t *env); int crypto_pk_cmp_keys(const crypto_pk_t *a, const crypto_pk_t *b); int crypto_pk_eq_keys(const crypto_pk_t *a, const crypto_pk_t *b); size_t crypto_pk_keysize(const crypto_pk_t *env); diff --git a/src/lib/crypt_ops/crypto_rsa_nss.c b/src/lib/crypt_ops/crypto_rsa_nss.c index b6d8bb647..90e9c34d3 100644 --- a/src/lib/crypt_ops/crypto_rsa_nss.c +++ b/src/lib/crypt_ops/crypto_rsa_nss.c @@ -244,12 +244,14 @@ crypto_pk_generate_key_with_bits,(crypto_pk_t *key, int bits)) return result; } -/** Return true iff env has a valid key. +/** Return true iff env is a valid private key. */ int -crypto_pk_check_key(crypto_pk_t *key) +crypto_pk_is_valid_private_key(crypto_pk_t *key) { - return key && key->pubkey; + /* We don't need to do validation here, since unlike OpenSSL, NSS won't let + * us load private keys without validating them. */ + return key && key->seckey; } /** Return true iff env contains a public key whose public exponent @@ -435,7 +437,7 @@ crypto_pk_public_encrypt(crypto_pk_t *env, char *to, size_t tolen, tor_assert(tolen < INT_MAX); tor_assert(fromlen < INT_MAX); - if (BUG(!crypto_pk_check_key(env))) + if (BUG(! env->pubkey)) return -1; unsigned int result_len = 0; @@ -724,6 +726,11 @@ crypto_pk_asn1_decode_private(const char *str, size_t len) crypto_nss_log_errors(LOG_WARN, "decoding an RSA private key"); } + if (! crypto_pk_is_valid_private_key(output)) { + crypto_pk_free(output); + output = NULL; + } + if (slot) PK11_FreeSlot(slot); diff --git a/src/lib/crypt_ops/crypto_rsa_openssl.c b/src/lib/crypt_ops/crypto_rsa_openssl.c index 7ff71f6dd..f203a0683 100644 --- a/src/lib/crypt_ops/crypto_rsa_openssl.c +++ b/src/lib/crypt_ops/crypto_rsa_openssl.c @@ -183,18 +183,21 @@ crypto_pk_generate_key_with_bits,(crypto_pk_t *env, int bits)) return 0; } -/** Return true iff env has a valid key. +/** Return true if env has a valid key; false otherwise. */ int -crypto_pk_check_key(crypto_pk_t *env) +crypto_pk_is_valid_private_key(crypto_pk_t *env) { int r; tor_assert(env); r = RSA_check_key(env->key); - if (r <= 0) + if (r <= 0) { crypto_openssl_log_errors(LOG_WARN,"checking RSA key"); - return r; + return 0; + } else { + return 1; + } } /** Return true iff env contains a public key whose public exponent @@ -578,5 +581,10 @@ crypto_pk_asn1_decode_private(const char *str, size_t len) crypto_openssl_log_errors(LOG_WARN,"decoding public key"); return NULL; } - return crypto_new_pk_from_openssl_rsa_(rsa); + crypto_pk_t *result = crypto_new_pk_from_openssl_rsa_(rsa); + if (! crypto_pk_is_valid_private_key(result)) { + crypto_pk_free(result); + return NULL; + } + return result; } diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 04077b42f..b08c5cbc2 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -18,6 +18,7 @@ #include "lib/crypt_ops/crypto_rand.h" #include "lib/crypt_ops/crypto_init.h" #include "ed25519_vectors.inc" +#include "test/log_test_helpers.h" #ifdef HAVE_SYS_STAT_H #include <sys/stat.h> @@ -1488,6 +1489,58 @@ test_crypto_pk_pem_encrypted(void *arg) done: crypto_pk_free(pk); } + +static void +test_crypto_pk_invalid_private_key(void *arg) +{ + (void)arg; + /* Here is a simple invalid private key: it was produced by making + * a regular private key, and then adding 2 to the modulus. */ + const char pem[] = + "-----BEGIN RSA PRIVATE KEY-----\n" + "MIIEpQIBAAKCAQEAskRyZrs+YAukvBmZlgo6/rCxyKF2xyUk073ap+2CgRUnSfGG\n" + "mflHlzqVq7tpH50DafpS+fFAbaEaNV/ac20QG0rUZi38HTB4qURWOu6n0Bws6E4l\n" + "UX/AkvDlWnuYH0pHHi2c3DGNFjwoJpjKuUTk+cRffVR8X3Kjr62SUDUaBNW0Kecz\n" + "3SYLbmgmZI16dFZ+g9sNM3znXZbhvb33WwPqpZSSPs37cPgF7eS6mAw/gUMx6zfE\n" + "HRmUnOQSzUdS05rvc/hsiCLhiIZ8hgfkD07XnTT1Ds8DwE55k7BUWY2wvwWCNLsH\n" + "qtqAxTr615XdkMxVkYgImpqPybarpfNYhFqkOwIDAQABAoIBACPC3VxEdbfYvhxJ\n" + "2mih9sG++nswAN7kUaX0cRe86rAwaShJPmJHApiQ1ROVTfpciiHJaLnhLraPWe2Z\n" + "I/6Bw3hmI4O399p3Lc1u+wlpdNqnvE6B1rSptx0DHE9xecvVH70rE0uM2Su7t6Y+\n" + "gnR2IKUGQs2mlCilm7aTUEWs0WJkkl4CG1dyxItuOSdNBjOEzXimJyiB10jEBFsp\n" + "SZeCF2FZ7AJbck5CVC42+oTsiDbZrHTHOn7v26rFGdONeHD1wOI1v7JwHFpCB923\n" + "aEHBzsPbMeq7DWG1rjzCYpcXHhTDBDBWSia4SEhyr2Nl7m7qxWWWwR+x4dqAj3rD\n" + "HeTmos0CgYEA6uf1CLpjPpOs5IaW1DQI8dJA/xFEAC/6GVgq4nFOGHZrm8G3L5o+\n" + "qvtQNMpDs2naWuZpqROFqv24o01DykHygR72GlPIY6uvmmf5tvJLoGnbFUay33L4\n" + "7b9dkNhuEIBNPzVDie0pgS77WgaPbYkVv5fnDwgPuVnkqfakEt7Pz2MCgYEAwkZ5\n" + "R1wLuTQEA2Poo6Gf4L8Bg6yNYI46LHDqDIs818iYLjtcnEEvbPfaoKNpOn7s7s4O\n" + "Pc+4HnT1aIQs0IKVLRTp+5a/9wfOkPZnobWOUHZk9UzBL3Hc1uy/qhp93iE3tSzx\n" + "v0O1pvR+hr3guTCZx8wZnDvaMgG3hlyPnVlHdrMCgYEAzQQxGbMC1ySv6quEjCP2\n" + "AogMbhE1lixJTUFj/EoDbNo9xKznIkauly/Lqqc1OysRhfA/G2+MY9YZBX1zwtyX\n" + "uBW7mPKynDrFgi9pBECnvJNmwET57Ic9ttIj6Tzbos83nAjyrzgr1zGX8dRz7ZeN\n" + "QbBj2vygLJbGOYinXkjUeh0CgYEAhN5aF9n2EqZmkEMGWtMxWy6HRJ0A3Cap1rcq\n" + "+4VHCXWhzwy+XAeg/e/N0MuyLlWcif7XcqLcE8h+BwtO8xQ8HmcNWApUJAls12wO\n" + "mGRpftJaXgIupdpD5aJpu1b++qrRRNIGTH9sf1D8L/8w8LcylZkbcuTkaAsQj45C\n" + "kqT64U0CgYEAq47IKS6xc3CDc17BqExR6t+1yRe+4ml+z1zcVbfUKony4pGvl1yo\n" + "rk0IYDN5Vd8h5xtXrkPdX9h+ywmohnelDKsayEuE+opyqEpSU4/96Bb22RZUoucb\n" + "LWkV5gZx5hFnDFtEd4vadMIiY4jVv/3JqiZDKwMVBJKlHRXJEEmIEBk=\n" + "-----END RSA PRIVATE KEY-----\n"; + + crypto_pk_t *pk = NULL; + + pk = crypto_pk_new(); + setup_capture_of_logs(LOG_WARN); + tt_int_op(-1, OP_EQ, + crypto_pk_read_private_key_from_string(pk, pem, strlen(pem))); +#ifdef ENABLE_NSS + expect_single_log_msg_containing("received bad data"); +#else + expect_single_log_msg_containing("while checking RSA key"); +#endif + done: + teardown_capture_of_logs(); + crypto_pk_free(pk); +} + #ifdef HAVE_TRUNCATE #define do_truncate truncate #else @@ -3109,6 +3162,8 @@ struct testcase_t crypto_tests[] = { { "pk_fingerprints", test_crypto_pk_fingerprints, TT_FORK, NULL, NULL }, { "pk_base64", test_crypto_pk_base64, TT_FORK, NULL, NULL }, { "pk_pem_encrypted", test_crypto_pk_pem_encrypted, TT_FORK, NULL, NULL }, + { "pk_invalid_private_key", test_crypto_pk_invalid_private_key, 0, + NULL, NULL }, CRYPTO_LEGACY(digests), { "digest_names", test_crypto_digest_names, 0, NULL, NULL }, { "sha3", test_crypto_sha3, TT_FORK, NULL, NULL},

1 0

[tor/master] Fix documentation of ENABLE_NSS
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 3b10108a2abec89a9f1f8ba57a99a8b10a67515c Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 4 09:51:39 2018 -0400 Fix documentation of ENABLE_NSS --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index f99697a44..de53b1ab2 100644 --- a/configure.ac +++ b/configure.ac @@ -71,7 +71,7 @@ AM_CONDITIONAL(USE_OPENSSL, test "x$enable_nss" != "xyes") if test "x$enable_nss" = "xyes"; then AC_DEFINE(ENABLE_NSS, 1, - [Defined if we're building with NSS in addition to OpenSSL.]) + [Defined if we're building with NSS.]) else AC_DEFINE(ENABLE_OPENSSL, 1, [Defined if we're building with OpenSSL or LibreSSL])

1 0

[tor/master] Test a few more tortls.c functions
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 52ac539b9938af6c47355b67ce1def1236f4ee39 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Aug 23 18:57:26 2018 -0400 Test a few more tortls.c functions --- src/lib/tls/tortls_nss.c | 17 ++++++---- src/test/test_tortls.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 6 deletions(-) diff --git a/src/lib/tls/tortls_nss.c b/src/lib/tls/tortls_nss.c index 40a98dd87..df430765a 100644 --- a/src/lib/tls/tortls_nss.c +++ b/src/lib/tls/tortls_nss.c @@ -331,6 +331,7 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, (void)tls; PRErrorCode code = PORT_GetError(); + const char *addr = tls ? tls->address : NULL; const char *string = PORT_ErrorToString(code); const char *name = PORT_ErrorToName(code); char buf[16]; @@ -341,10 +342,14 @@ tls_log_errors(tor_tls_t *tls, int severity, int domain, name = buf; } + const char *with = addr ? " with " : ""; + addr = addr ? addr : ""; if (doing) { - log_fn(severity, domain, "TLS error %s while %s: %s", name, doing, string); + log_fn(severity, domain, "TLS error %s while %s%s%s: %s", + name, doing, with, addr, string); } else { - log_fn(severity, domain, "TLS error %s: %s", name, string); + log_fn(severity, domain, "TLS error %s%s%s: %s", name, string, + with, addr); } } @@ -381,7 +386,7 @@ tor_tls_new(tor_socket_t sock, int is_server) } SECStatus s = SSL_ResetHandshake(ssl, is_server ? PR_TRUE : PR_FALSE); if (s != SECSuccess) { - crypto_nss_log_errors(LOG_WARN, "resetting handshake state"); + tls_log_errors(tls, LOG_WARN, LD_CRYPTO, "resetting handshake state"); } return tls; @@ -457,7 +462,7 @@ tor_tls_read, (tor_tls_t *tls, char *cp, size_t len)) if (err == PR_WOULD_BLOCK_ERROR) { return TOR_TLS_WANTREAD; // XXXX ???? } else { - crypto_nss_log_errors(LOG_NOTICE, "reading"); // XXXX + tls_log_errors(tls, LOG_NOTICE, LD_CRYPTO, "reading"); // XXXX return TOR_TLS_ERROR_MISC; // ???? } } @@ -482,7 +487,7 @@ tor_tls_write(tor_tls_t *tls, const char *cp, size_t n) if (err == PR_WOULD_BLOCK_ERROR) { return TOR_TLS_WANTWRITE; // XXXX ???? } else { - crypto_nss_log_errors(LOG_NOTICE, "writing"); // XXXX + tls_log_errors(tls, LOG_NOTICE, LD_CRYPTO, "writing"); // XXXX return TOR_TLS_ERROR_MISC; // ???? } } @@ -541,7 +546,7 @@ tor_tls_get_pending_bytes(tor_tls_t *tls) tor_assert(tls); int n = SSL_DataPending(tls->ssl); if (n < 0) { - crypto_nss_log_errors(LOG_WARN, "Looking up pending bytes"); + tls_log_errors(tls, LOG_WARN, LD_CRYPTO, "looking up pending bytes"); return 0; } return (int)n; diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c index 0e4b5afaf..b16a88058 100644 --- a/src/test/test_tortls.c +++ b/src/test/test_tortls.c @@ -247,6 +247,85 @@ test_tortls_evaluate_ecgroup_for_tls(void *ignored) (void)0; } +static void +test_tortls_double_init(void *arg) +{ + (void) arg; + /* If we call tor_tls_context_init() a second time, nothing should go + * wrong. + */ + crypto_pk_t *pk1 = NULL, *pk2 = NULL; + pk1 = pk_generate(2); + pk2 = pk_generate(0); + + int r = tor_tls_context_init(TOR_TLS_CTX_IS_PUBLIC_SERVER, + pk1, pk2, 86400); + tt_int_op(r, OP_EQ, 0); + + r = tor_tls_context_init(TOR_TLS_CTX_IS_PUBLIC_SERVER, + pk2, pk1, 86400); + tt_int_op(r, OP_EQ, 0); + + done: + crypto_pk_free(pk1); + crypto_pk_free(pk2); +} + +static void +test_tortls_address(void *arg) +{ + (void)arg; + tor_tls_t *tls = NULL; + crypto_pk_t *pk1=NULL, *pk2=NULL; + pk1 = pk_generate(2); + pk2 = pk_generate(0); + + int r = tor_tls_context_init(TOR_TLS_CTX_IS_PUBLIC_SERVER, + pk1, pk2, 86400); + tt_int_op(r, OP_EQ, 0); + + tls = tor_tls_new(-1, 0); + tls->state = TOR_TLS_ST_OPEN; + tor_tls_set_logged_address(tls, "zombo.com"); + + /* This write should fail, since the fd is -1. */ + setup_capture_of_logs(LOG_INFO); + int n = tor_tls_write(tls, "welcome", 7); + tt_int_op(n, OP_LT, 0); + expect_log_msg_containing("with zombo.com"); + + done: + teardown_capture_of_logs(); + tor_tls_free(tls); + crypto_pk_free(pk1); + crypto_pk_free(pk2); +} + +static void +test_tortls_is_server(void *arg) +{ + (void)arg; + crypto_pk_t *pk1=NULL, *pk2=NULL; + tor_tls_t *tls1=NULL, *tls2=NULL; + pk1 = pk_generate(2); + pk2 = pk_generate(0); + + int r = tor_tls_context_init(TOR_TLS_CTX_IS_PUBLIC_SERVER, + pk1, pk2, 86400); + tt_int_op(r, OP_EQ, 0); + tls1 = tor_tls_new(-1, 0); + tls2 = tor_tls_new(-1, 1); + + tt_assert(! tor_tls_is_server(tls1)); + tt_assert(tor_tls_is_server(tls2)); + + done: + tor_tls_free(tls1); + tor_tls_free(tls2); + crypto_pk_free(pk1); + crypto_pk_free(pk2); +} + #define LOCAL_TEST_CASE(name, flags) \ { #name, test_tortls_##name, (flags|TT_FORK), NULL, NULL } @@ -262,5 +341,8 @@ struct testcase_t tortls_tests[] = { LOCAL_TEST_CASE(server_got_renegotiate, 0), #endif LOCAL_TEST_CASE(evaluate_ecgroup_for_tls, 0), + LOCAL_TEST_CASE(double_init, TT_FORK), + LOCAL_TEST_CASE(address, TT_FORK), + LOCAL_TEST_CASE(is_server, 0), END_OF_TESTCASES };

1 0

[tor/master] Update prefork and postfork NSS code for unit tests.
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 36f3bdac032523c6d98022c84059d8ebd69dfdfe Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Aug 29 08:39:19 2018 -0400 Update prefork and postfork NSS code for unit tests. --- src/ext/tinytest.c | 5 ++++- src/lib/crypt_ops/crypto_init.c | 12 +++++++++++- src/lib/crypt_ops/crypto_init.h | 1 + src/lib/crypt_ops/crypto_nss_mgt.c | 19 ++++++++++++++----- src/lib/crypt_ops/crypto_nss_mgt.h | 3 ++- src/test/testing_common.c | 8 +++++++- 6 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/ext/tinytest.c b/src/ext/tinytest.c index 2a475bd91..052fb6483 100644 --- a/src/ext/tinytest.c +++ b/src/ext/tinytest.c @@ -120,8 +120,10 @@ testcase_run_bare_(const struct testcase_t *testcase) #ifndef NO_FORKING #ifdef TINYTEST_POSTFORK +void tinytest_prefork(void); void tinytest_postfork(void); #else +static void tinytest_prefork(void) { } static void tinytest_postfork(void) { } #endif @@ -185,16 +187,17 @@ testcase_run_forked_(const struct testgroup_t *group, if (opt_verbosity>0) printf("[forking] "); + tinytest_prefork(); pid = fork(); #ifdef FORK_BREAKS_GCOV vproc_transaction_begin(0); #endif + tinytest_postfork(); if (!pid) { /* child. */ int test_r, write_r; char b[1]; close(outcome_pipe[0]); - tinytest_postfork(); test_r = testcase_run_bare_(testcase); assert(0<=(int)test_r && (int)test_r<=2); b[0] = "NYS"[test_r]; diff --git a/src/lib/crypt_ops/crypto_init.c b/src/lib/crypt_ops/crypto_init.c index f9b077e9e..6fc8e5d94 100644 --- a/src/lib/crypt_ops/crypto_init.c +++ b/src/lib/crypt_ops/crypto_init.c @@ -58,7 +58,7 @@ crypto_early_init(void) crypto_openssl_early_init(); #endif #ifdef ENABLE_NSS - crypto_nss_early_init(); + crypto_nss_early_init(0); #endif if (crypto_seed_rng() < 0) @@ -137,6 +137,16 @@ crypto_global_cleanup(void) /** Run operations that the crypto library requires to be happy again * after forking. */ void +crypto_prefork(void) +{ +#ifdef ENABLE_NSS + crypto_nss_prefork(); +#endif +} + +/** Run operations that the crypto library requires to be happy again + * after forking. */ +void crypto_postfork(void) { #ifdef ENABLE_NSS diff --git a/src/lib/crypt_ops/crypto_init.h b/src/lib/crypt_ops/crypto_init.h index 05b281720..5b6d65d48 100644 --- a/src/lib/crypt_ops/crypto_init.h +++ b/src/lib/crypt_ops/crypto_init.h @@ -24,6 +24,7 @@ int crypto_global_init(int hardwareAccel, void crypto_thread_cleanup(void); int crypto_global_cleanup(void); +void crypto_prefork(void); void crypto_postfork(void); const char *crypto_get_library_name(void); diff --git a/src/lib/crypt_ops/crypto_nss_mgt.c b/src/lib/crypt_ops/crypto_nss_mgt.c index 187f556bd..a1da74aff 100644 --- a/src/lib/crypt_ops/crypto_nss_mgt.c +++ b/src/lib/crypt_ops/crypto_nss_mgt.c @@ -50,10 +50,12 @@ nss_password_func_always_fail(PK11SlotInfo *slot, } void -crypto_nss_early_init(void) +crypto_nss_early_init(int nss_only) { - PR_Init(PR_USER_THREAD, PR_PRIORITY_NORMAL, 0); - PK11_SetPasswordFunc(nss_password_func_always_fail); + if (! nss_only) { + PR_Init(PR_USER_THREAD, PR_PRIORITY_NORMAL, 0); + PK11_SetPasswordFunc(nss_password_func_always_fail); + } /* Eventually we should use NSS_Init() instead -- but that wants a directory. The documentation says that we can't use this if we want @@ -113,11 +115,18 @@ void crypto_nss_global_cleanup(void) { NSS_Shutdown(); + PL_ArenaFinish(); + PR_Cleanup(); +} + +void +crypto_nss_prefork(void) +{ + NSS_Shutdown(); } void crypto_nss_postfork(void) { - crypto_nss_global_cleanup(); - crypto_nss_early_init(); + crypto_nss_early_init(1); } diff --git a/src/lib/crypt_ops/crypto_nss_mgt.h b/src/lib/crypt_ops/crypto_nss_mgt.h index c4c94f4d8..27793dcc4 100644 --- a/src/lib/crypt_ops/crypto_nss_mgt.h +++ b/src/lib/crypt_ops/crypto_nss_mgt.h @@ -22,11 +22,12 @@ const char *crypto_nss_get_header_version_str(void); void crypto_nss_log_errors(int severity, const char *doing); -void crypto_nss_early_init(void); +void crypto_nss_early_init(int nss_only); int crypto_nss_late_init(void); void crypto_nss_global_cleanup(void); +void crypto_nss_prefork(void); void crypto_nss_postfork(void); #endif diff --git a/src/test/testing_common.c b/src/test/testing_common.c index 17a075680..42b5190ca 100644 --- a/src/test/testing_common.c +++ b/src/test/testing_common.c @@ -223,11 +223,17 @@ an_assertion_failed(void) tinytest_set_test_failed_(); } +void tinytest_prefork(void); void tinytest_postfork(void); void -tinytest_postfork(void) +tinytest_prefork(void) { free_pregenerated_keys(); + crypto_prefork(); +} +void +tinytest_postfork(void) +{ crypto_postfork(); init_pregenerated_keys(); }

1 0

[tor/master] Fix documentation of initialized fields in crypto_init.c
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 70c27b7e39b9437bf18c0342b15a5c6a3022e204 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 4 10:51:26 2018 -0400 Fix documentation of initialized fields in crypto_init.c --- src/lib/crypt_ops/crypto_init.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/crypt_ops/crypto_init.c b/src/lib/crypt_ops/crypto_init.c index 6fc8e5d94..c731662d4 100644 --- a/src/lib/crypt_ops/crypto_init.c +++ b/src/lib/crypt_ops/crypto_init.c @@ -23,10 +23,10 @@ #include "siphash.h" -/** Boolean: has OpenSSL's crypto been initialized? */ +/** Boolean: has our crypto library been initialized? (early phase) */ static int crypto_early_initialized_ = 0; -/** Boolean: has OpenSSL's crypto been initialized? */ +/** Boolean: has our crypto library been initialized? (late phase) */ static int crypto_global_initialized_ = 0; static int have_seeded_siphash = 0;

1 0

[tor/master] Document winsock includes better
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 7acb8c8d1835f919fee09585c966dee837f2e9f6 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 4 10:55:15 2018 -0400 Document winsock includes better --- src/lib/tls/tortls_nss.c | 2 +- src/lib/tls/tortls_openssl.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib/tls/tortls_nss.c b/src/lib/tls/tortls_nss.c index df430765a..266455049 100644 --- a/src/lib/tls/tortls_nss.c +++ b/src/lib/tls/tortls_nss.c @@ -14,7 +14,7 @@ #define TORTLS_PRIVATE #define TOR_X509_PRIVATE -#ifdef _WIN32 /*wrkard for dtls1.h >= 0.9.8m of "#include <winsock.h>"*/ +#ifdef _WIN32 #include <winsock2.h> #include <ws2tcpip.h> #endif diff --git a/src/lib/tls/tortls_openssl.c b/src/lib/tls/tortls_openssl.c index d10e50a97..6ff03ee81 100644 --- a/src/lib/tls/tortls_openssl.c +++ b/src/lib/tls/tortls_openssl.c @@ -20,7 +20,9 @@ #define TORTLS_OPENSSL_PRIVATE #define TOR_X509_PRIVATE -#ifdef _WIN32 /*wrkard for dtls1.h >= 0.9.8m of "#include <winsock.h>"*/ +#ifdef _WIN32 + /* We need to include these here, or else the dtls1.h header will include + * <winsock.h> and mess things up, in at least some openssl versions. */ #include <winsock2.h> #include <ws2tcpip.h> #endif

1 0

[tor/master] Unify functions for reading/writing PEM keys, to avoid duplication.
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 3b5d6ef15bec26c3bda22057ba09e1301613ee71 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Aug 29 16:43:54 2018 -0400 Unify functions for reading/writing PEM keys, to avoid duplication. --- src/lib/crypt_ops/crypto_rsa.c | 145 ++++++++++++++++++++--------------------- 1 file changed, 71 insertions(+), 74 deletions(-) diff --git a/src/lib/crypt_ops/crypto_rsa.c b/src/lib/crypt_ops/crypto_rsa.c index 31497e650..6a9e2948f 100644 --- a/src/lib/crypt_ops/crypto_rsa.c +++ b/src/lib/crypt_ops/crypto_rsa.c @@ -398,29 +398,43 @@ crypto_pk_get_common_digests(crypto_pk_t *pk, common_digests_t *digests_out) static const char RSA_PUBLIC_TAG[] = "RSA PUBLIC KEY"; static const char RSA_PRIVATE_TAG[] = "RSA PRIVATE KEY"; -/** PEM-encode the public key portion of env and write it to a - * newly allocated string. On success, set *dest to the new - * string, *len to the string's length, and return 0. On - * failure, return -1. +/* These are overestimates for how many extra bytes we might need to encode + * a key in DER */ +#define PRIVATE_ASN_MAX_OVERHEAD_FACTOR 16 +#define PUBLIC_ASN_MAX_OVERHEAD_FACTOR 3 + +/** Helper: PEM-encode env and write it to a newly allocated string. + * If private_key, write the private part of env; otherwise + * write only the public portion. On success, set *dest to the new + * string, *len to the string's length, and return 0. On failure, + * return -1. */ -int -crypto_pk_write_public_key_to_string(crypto_pk_t *env, - char **dest, size_t *len) +static int +crypto_pk_write_to_string_generic(crypto_pk_t *env, + char **dest, size_t *len, + bool private_key) { - size_t buflen = crypto_pk_keysize(env) * 3; + const int factor = + private_key ? PRIVATE_ASN_MAX_OVERHEAD_FACTOR + : PUBLIC_ASN_MAX_OVERHEAD_FACTOR; + size_t buflen = crypto_pk_keysize(env) * factor; + const char *tag = + private_key ? RSA_PRIVATE_TAG : RSA_PUBLIC_TAG; char *buf = tor_malloc(buflen); char *result = NULL; size_t resultlen = 0; int rv = -1; - int n = crypto_pk_asn1_encode(env, buf, buflen); + int n = private_key + ? crypto_pk_asn1_encode_private(env, buf, buflen) + : crypto_pk_asn1_encode(env, buf, buflen); if (n < 0) goto done; - resultlen = pem_encoded_size(n, RSA_PUBLIC_TAG); + resultlen = pem_encoded_size(n, tag); result = tor_malloc(resultlen); if (pem_encode(result, resultlen, - (const unsigned char *)buf, n, RSA_PUBLIC_TAG) < 0) { + (const unsigned char *)buf, n, tag) < 0) { goto done; } @@ -438,6 +452,18 @@ crypto_pk_write_public_key_to_string(crypto_pk_t *env, return rv; } +/** PEM-encode the public key portion of env and write it to a + * newly allocated string. On success, set *dest to the new + * string, *len to the string's length, and return 0. On + * failure, return -1. + */ +int +crypto_pk_write_public_key_to_string(crypto_pk_t *env, + char **dest, size_t *len) +{ + return crypto_pk_write_to_string_generic(env, dest, len, false); +} + /** PEM-encode the private key portion of env and write it to a * newly allocated string. On success, set *dest to the new * string, *len to the string's length, and return 0. On @@ -447,59 +473,42 @@ int crypto_pk_write_private_key_to_string(crypto_pk_t *env, char **dest, size_t *len) { - size_t buflen = crypto_pk_keysize(env) * 16; - char *buf = tor_malloc(buflen); - char *result = NULL; - size_t resultlen = 0; - int rv = -1; - - int n = crypto_pk_asn1_encode_private(env, buf, buflen); - if (n < 0) - goto done; - - resultlen = pem_encoded_size(n, RSA_PRIVATE_TAG); - result = tor_malloc(resultlen); - if (pem_encode(result, resultlen, - (const unsigned char *)buf, n, RSA_PRIVATE_TAG) < 0) - goto done; - - *dest = result; - *len = resultlen; - rv = 0; - done: - if (rv < 0 && result) { - memwipe(result, 0, resultlen); - tor_free(result); - } - memwipe(buf, 0, buflen); - tor_free(buf); - return rv; + return crypto_pk_write_to_string_generic(env, dest, len, true); } -/** Read a PEM-encoded public key from the first len characters of - * src, and store the result in env. Return 0 on success, -1 on - * failure. +/** + * Helper. Read a PEM-encoded RSA from the first len characters of + * src, and store the result in env. If private_key, + * expect a private key; otherwise expect a public key. Return 0 on success, + * -1 on failure. If len is -1, the string is nul-terminated. */ -int -crypto_pk_read_public_key_from_string(crypto_pk_t *env, - const char *src, size_t len) +static int +crypto_pk_read_from_string_generic(crypto_pk_t *env, const char *src, + size_t len, bool private_key) { - if (len == (size_t)-1) + if (len == (size_t)-1) // "-1" indicates "use the length of the string." len = strlen(src); + const char *tag = + private_key ? RSA_PRIVATE_TAG : RSA_PUBLIC_TAG; size_t buflen = len; uint8_t *buf = tor_malloc(buflen); int rv = -1; - int n = pem_decode(buf, buflen, src, len, RSA_PUBLIC_TAG); + int n = pem_decode(buf, buflen, src, len, tag); if (n < 0) goto done; - crypto_pk_t *pk = crypto_pk_asn1_decode((const char*)buf, n); + crypto_pk_t *pk = private_key + ? crypto_pk_asn1_decode_private((const char*)buf, n) + : crypto_pk_asn1_decode((const char*)buf, n); if (! pk) goto done; - crypto_pk_assign_public(env, pk); + if (private_key) + crypto_pk_assign_private(env, pk); + else + crypto_pk_assign_public(env, pk); crypto_pk_free(pk); rv = 0; @@ -509,38 +518,26 @@ crypto_pk_read_public_key_from_string(crypto_pk_t *env, return rv; } -/** Read a PEM-encoded private key from the len-byte string s +/** Read a PEM-encoded public key from the first len characters of + * src, and store the result in env. Return 0 on success, -1 on + * failure. If len is -1, the string is nul-terminated. + */ +int +crypto_pk_read_public_key_from_string(crypto_pk_t *env, + const char *src, size_t len) +{ + return crypto_pk_read_from_string_generic(env, src, len, false); +} + +/** Read a PEM-encoded private key from the len-byte string src * into env. Return 0 on success, -1 on failure. If len is -1, * the string is nul-terminated. */ int crypto_pk_read_private_key_from_string(crypto_pk_t *env, - const char *s, ssize_t len) + const char *src, ssize_t len) { - if (len == -1) - len = strlen(s); - - size_t buflen = len; - uint8_t *buf = tor_malloc(buflen); - int rv = -1; - - int n = pem_decode(buf, buflen, s, len, RSA_PRIVATE_TAG); - if (n < 0) { - goto done; - } - - crypto_pk_t *pk = crypto_pk_asn1_decode_private((const char *)buf, n); - if (! pk) - goto done; - - crypto_pk_assign_private(env, pk); - crypto_pk_free(pk); - rv = 0; - - done: - memwipe(buf, 0, buflen); - tor_free(buf); - return rv; + return crypto_pk_read_from_string_generic(env, src, len, true); } /** Read a PEM-encoded private key from the file named by

1 0

[tor/master] Add unit test for bridge-style TLS initialization.
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 3cdf0497f9672bc281b0aac2606123249d7b9ddc Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 4 12:08:06 2018 -0400 Add unit test for bridge-style TLS initialization. --- src/test/test_tortls.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c index b16a88058..3d0ca5e85 100644 --- a/src/test/test_tortls.c +++ b/src/test/test_tortls.c @@ -265,6 +265,8 @@ test_tortls_double_init(void *arg) r = tor_tls_context_init(TOR_TLS_CTX_IS_PUBLIC_SERVER, pk2, pk1, 86400); tt_int_op(r, OP_EQ, 0); + /* For a public server context, these are the same */ + tt_ptr_op(tor_tls_context_get(0), OP_EQ, tor_tls_context_get(1)); done: crypto_pk_free(pk1); @@ -272,6 +274,26 @@ test_tortls_double_init(void *arg) } static void +test_tortls_bridge_init(void *arg) +{ + (void)arg; + crypto_pk_t *pk1 = NULL, *pk2 = NULL; + pk1 = pk_generate(2); + pk2 = pk_generate(0); + + /* If we pass in a server identity key but not the + TOR_TLS_CTX_IS_PUBLIC_SERVER flag, we should get a bridge-style + configuration, with two distinct contexts. */ + int r = tor_tls_context_init(0 /* flags */, pk1, pk2, 86400); + + tt_int_op(r, OP_EQ, 0); + tt_ptr_op(tor_tls_context_get(0), OP_NE, tor_tls_context_get(1)); + done: + crypto_pk_free(pk1); + crypto_pk_free(pk2); +} + +static void test_tortls_address(void *arg) { (void)arg; @@ -344,5 +366,6 @@ struct testcase_t tortls_tests[] = { LOCAL_TEST_CASE(double_init, TT_FORK), LOCAL_TEST_CASE(address, TT_FORK), LOCAL_TEST_CASE(is_server, 0), + LOCAL_TEST_CASE(bridge_init, TT_FORK), END_OF_TESTCASES };

1 0

[tor/master] Port test_tortls_verify to not depend on openssl internals
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit ad94d43fc50525e8814b6e99f78d4b9635fa80ca Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 4 12:59:47 2018 -0400 Port test_tortls_verify to not depend on openssl internals --- src/lib/tls/x509.c | 9 +++ src/lib/tls/x509_internal.h | 1 + src/lib/tls/x509_nss.c | 10 ++- src/lib/tls/x509_openssl.c | 18 ++--- src/test/include.am | 1 + src/test/test_tortls.c | 157 ++++++++++++++++++++++++++++++++++++++++ src/test/test_tortls.h | 13 ++++ src/test/test_tortls_openssl.c | 161 +---------------------------------------- 8 files changed, 198 insertions(+), 172 deletions(-) diff --git a/src/lib/tls/x509.c b/src/lib/tls/x509.c index c88298b6c..2e7020646 100644 --- a/src/lib/tls/x509.c +++ b/src/lib/tls/x509.c @@ -134,3 +134,12 @@ tor_x509_cert_new,(tor_x509_cert_impl_t *x509_cert)) tor_x509_cert_impl_free_(x509_cert); return NULL; } + +/** Return a new copy of cert. */ +tor_x509_cert_t * +tor_x509_cert_dup(const tor_x509_cert_t *cert) +{ + tor_assert(cert); + tor_assert(cert->cert); + return tor_x509_cert_new(tor_x509_cert_impl_dup_(cert->cert)); +} diff --git a/src/lib/tls/x509_internal.h b/src/lib/tls/x509_internal.h index 0e186ec84..c08705cb2 100644 --- a/src/lib/tls/x509_internal.h +++ b/src/lib/tls/x509_internal.h @@ -41,6 +41,7 @@ int tor_x509_check_cert_lifetime_internal(int severity, int future_tolerance); void tor_x509_cert_impl_free_(tor_x509_cert_impl_t *cert); +tor_x509_cert_impl_t *tor_x509_cert_impl_dup_(tor_x509_cert_impl_t *cert); #ifdef ENABLE_OPENSSL int tor_x509_cert_set_cached_der_encoding(tor_x509_cert_t *cert); #else diff --git a/src/lib/tls/x509_nss.c b/src/lib/tls/x509_nss.c index 68aebbb69..5bb7940c9 100644 --- a/src/lib/tls/x509_nss.c +++ b/src/lib/tls/x509_nss.c @@ -225,11 +225,13 @@ tor_x509_cert_impl_free_(tor_x509_cert_impl_t *cert) CERT_DestroyCertificate(cert); } -tor_x509_cert_t * -tor_x509_cert_dup(const tor_x509_cert_t *cert) +tor_x509_cert_impl_t * +tor_x509_cert_impl_dup_(tor_x509_cert_impl_t *cert) { - tor_assert(cert); - return tor_x509_cert_new(CERT_DupCertificate(cert->cert)); + if (cert) + return CERT_DupCertificate(cert); + else + return NULL; } /** diff --git a/src/lib/tls/x509_openssl.c b/src/lib/tls/x509_openssl.c index f315b88f3..dd74e8441 100644 --- a/src/lib/tls/x509_openssl.c +++ b/src/lib/tls/x509_openssl.c @@ -206,6 +206,15 @@ tor_x509_cert_impl_free_(tor_x509_cert_impl_t *cert) X509_free(cert); } +tor_x509_cert_impl_t * +tor_x509_cert_impl_dup_(tor_x509_cert_impl_t *cert) +{ + if (cert) + return X509_dup(cert); + else + return NULL; +} + /** Set *encoded_out and *size_out to cert's encoded DER * representation and length, respectively. */ void @@ -219,15 +228,6 @@ tor_x509_cert_get_der(const tor_x509_cert_t *cert, *size_out = cert->encoded_len; } -/** Return a new copy of cert. */ -tor_x509_cert_t * -tor_x509_cert_dup(const tor_x509_cert_t *cert) -{ - tor_assert(cert); - X509 *x509 = cert->cert; - return tor_x509_cert_new(X509_dup(x509)); -} - /** Read a DER-encoded X509 cert, of length exactly certificate_len, * from a certificate. Return a newly allocated tor_x509_cert_t on * success and NULL on failure. */ diff --git a/src/test/include.am b/src/test/include.am index 69680f969..c553e6fe4 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -301,6 +301,7 @@ noinst_HEADERS+= \ src/test/test_helpers.h \ src/test/test_dir_common.h \ src/test/test_connection.h \ + src/test/test_tortls.h \ src/test/test_descriptors.inc \ src/test/example_extrainfo.inc \ src/test/failing_routerdescs.inc \ diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c index 3d0ca5e85..b260310ad 100644 --- a/src/test/test_tortls.c +++ b/src/test/test_tortls.c @@ -23,13 +23,120 @@ #include "lib/tls/tortls.h" #include "lib/tls/tortls_st.h" #include "lib/tls/tortls_internal.h" +#include "lib/encoding/pem.h" #include "app/config/or_state_st.h" #include "test/test.h" #include "test/log_test_helpers.h" +#include "test/test_tortls.h" #include "tinytest.h" +const char* notCompletelyValidCertString = + "-----BEGIN CERTIFICATE-----\n" + "MIICVjCCAb8CAg37MA0GCSqGSIb3DQEBBQUAMIGbMQswCQYDVQQGEwJKUDEOMAwG\n" + "A1UECBMFVG9reW8xEDAOBgNVBAcTB0NodW8ta3UxETAPBgNVBAoTCEZyYW5rNERE\n" + "MRgwFgYDVQQLEw9XZWJDZXJ0IFN1cHBvcnQxGDAWBgNVBAMTD0ZyYW5rNEREIFdl\n" + "YiBDQTEjMCEGCSqGSIb3DQEJARYUc3VwcG9ydEBmcmFuazRkZC5jb20wHhcNMTIw\n" + "ODIyMDUyNzIzWhcNMTcwODIxMDUyNzIzWjBKMQswCQYDVQQGEwJKUDEOMAwGA1UE\n" + "CAwFVG9reW8xETAPBgNVBAoMCEZyYW5rNEREMRgwFgYDVQQDDA93d3cuZXhhbXBs\n" + "ZS5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMYBBrx5PlP0WNI/ZdzD\n" + "+6Pktmurn+F2kQYbtc7XQh8/LTBvCo+P6iZoLEmUA9e7EXLRxgU1CVqeAi7QcAn9\n" + "MwBlc8ksFJHB0rtf9pmf8Oza9E0Bynlq/4/Kb1x+d+AyhL7oK9tQwB24uHOueHi1\n" + "C/iVv8CSWKiYe6hzN1txYe8rAgMBAAEwDQYJKoZIhvcNAQEFBQADgYEAASPdjigJ\n" + "kXCqKWpnZ/Oc75EUcMi6HztaW8abUMlYXPIgkV2F7YanHOB7K4f7OOLjiz8DTPFf\n" + "jC9UeuErhaA/zzWi8ewMTFZW/WshOrm3fNvcMrMLKtH534JKvcdMg6qIdjTFINIr\n" + "evnAhf0cwULaebn+lMs8Pdl7y37+sfluVok=\n" + "-----END CERTIFICATE-----\n"; + +const char* validCertString = "-----BEGIN CERTIFICATE-----\n" + "MIIDpTCCAY0CAg3+MA0GCSqGSIb3DQEBBQUAMF4xCzAJBgNVBAYTAlVTMREwDwYD\n" + "VQQIDAhJbGxpbm9pczEQMA4GA1UEBwwHQ2hpY2FnbzEUMBIGA1UECgwLVG9yIFRl\n" + "c3RpbmcxFDASBgNVBAMMC1RvciBUZXN0aW5nMB4XDTE1MDkwNjEzMzk1OVoXDTQz\n" + "MDEyMjEzMzk1OVowVjELMAkGA1UEBhMCVVMxEDAOBgNVBAcMB0NoaWNhZ28xFDAS\n" + "BgNVBAoMC1RvciBUZXN0aW5nMR8wHQYDVQQDDBZ0ZXN0aW5nLnRvcnByb2plY3Qu\n" + "b3JnMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDoT6uyVVhWyOF3wkHjjYbd\n" + "nKaykyRv4JVtKQdZ4OpEErmX1zw4MmyzpQNV6iR4bQnWiyLfzyVJMZDIC/WILBfX\n" + "w2Pza/yuLgUvDc3twMuhOACzOQVO8PrEF/aVv2+hbCCy2udXvKhnYn+CCXl3ozc8\n" + "XcKYvujTXDyvGWY3xwAjlQIDAQABMA0GCSqGSIb3DQEBBQUAA4ICAQCUvnhzQWuQ\n" + "MrN+pERkE+zcTI/9dGS90rUMMLgu8VDNqTa0TUQh8uO0EQ6uDvI8Js6e8tgwS0BR\n" + "UBahqb7ZHv+rejGCBr5OudqD+x4STiiuPNJVs86JTLN8SpM9CHjIBH5WCCN2KOy3\n" + "mevNoRcRRyYJzSFULCunIK6FGulszigMYGscrO4oiTkZiHPh9KvWT40IMiHfL+Lw\n" + "EtEWiLex6064LcA2YQ1AMuSZyCexks63lcfaFmQbkYOKqXa1oLkIRuDsOaSVjTfe\n" + "vec+X6jvf12cFTKS5WIeqkKF2Irt+dJoiHEGTe5RscUMN/f+gqHPzfFz5dR23sxo\n" + "g+HC6MZHlFkLAOx3wW6epPS8A/m1mw3zMPoTnb2U2YYt8T0dJMMlUn/7Y1sEAa+a\n" + "dSTMaeUf6VnJ//11m454EZl1to9Z7oJOgqmFffSrdD4BGIWe8f7hhW6L1Enmqe/J\n" + "BKL3wbzZh80O1W0bndAwhnEEhlzneFY84cbBo9pmVxpODHkUcStpr5Z7pBDrcL21\n" + "Ss/aB/1YrsVXhdvJdOGxl3Mnl9dUY57CympLGlT8f0pPS6GAKOelECOhFMHmJd8L\n" + "dj3XQSmKtYHevZ6IvuMXSlB/fJvSjSlkCuLo5+kJoaqPuRu+i/S1qxeRy3CBwmnE\n" + "LdSNdcX4N79GQJ996PA8+mUCQG7YRtK+WA==\n" + "-----END CERTIFICATE-----\n"; + +const char* caCertString = "-----BEGIN CERTIFICATE-----\n" + "MIIFjzCCA3egAwIBAgIJAKd5WgyfPMYRMA0GCSqGSIb3DQEBCwUAMF4xCzAJBgNV\n" + "BAYTAlVTMREwDwYDVQQIDAhJbGxpbm9pczEQMA4GA1UEBwwHQ2hpY2FnbzEUMBIG\n" + "A1UECgwLVG9yIFRlc3RpbmcxFDASBgNVBAMMC1RvciBUZXN0aW5nMB4XDTE1MDkw\n" + "NjEzMzc0MVoXDTQzMDEyMjEzMzc0MVowXjELMAkGA1UEBhMCVVMxETAPBgNVBAgM\n" + "CElsbGlub2lzMRAwDgYDVQQHDAdDaGljYWdvMRQwEgYDVQQKDAtUb3IgVGVzdGlu\n" + "ZzEUMBIGA1UEAwwLVG9yIFRlc3RpbmcwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAw\n" + "ggIKAoICAQCpLMUEiLW5leUgBZoEJms2V7lZRhIAjnJBhVMHD0e3UubNknmaQoxf\n" + "ARz3rvqOaRd0JlV+qM9qE0DjiYcCVP1cAfqAo9d83uS1vwY3YMVJzADlaIiHfyVW\n" + "uEgBy0vvkeUBqaua24dYlcwsemOiXYLu41yM1wkcGHW1AhBNHppY6cznb8TyLgNM\n" + "2x3SGUdzc5XMyAFx51faKGBA3wjs+Hg1PLY7d30nmCgEOBavpm5I1disM/0k+Mcy\n" + "YmAKEo/iHJX/rQzO4b9znP69juLlR8PDBUJEVIG/CYb6+uw8MjjUyiWXYoqfVmN2\n" + "hm/lH8b6rXw1a2Aa3VTeD0DxaWeacMYHY/i01fd5n7hCoDTRNdSw5KJ0L3Z0SKTu\n" + "0lzffKzDaIfyZGlpW5qdouACkWYzsaitQOePVE01PIdO30vUfzNTFDfy42ccx3Di\n" + "59UCu+IXB+eMtrBfsok0Qc63vtF1linJgjHW1z/8ujk8F7/qkOfODhk4l7wngc2A\n" + "EmwWFIFoGaiTEZHB9qteXr4unbXZ0AHpM02uGGwZEGohjFyebEb73M+J57WKKAFb\n" + "PqbLcGUksL1SHNBNAJcVLttX55sO4nbidOS/kA3m+F1R04MBTyQF9qA6YDDHqdI3\n" + "h/3pw0Z4fxVouTYT4/NfRnX4JTP4u+7Mpcoof28VME0qWqD1LnRhFQIDAQABo1Aw\n" + "TjAdBgNVHQ4EFgQUMoAgIXH7pZ3QMRwTjT+DM9Yo/v0wHwYDVR0jBBgwFoAUMoAg\n" + "IXH7pZ3QMRwTjT+DM9Yo/v0wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC\n" + "AgEAUJxacjXR9sT+Xs6ISFiUsyd0T6WVKMnV46xrYJHirGfx+krWHrjxMY+ZtxYD\n" + "DBDGlo11Qc4v6QrclNf5QUBfIiGQsP9Cm6hHcQ+Tpg9HHCgSqG1YNPwCPReCR4br\n" + "BLvLfrfkcBL2IWM0PdQdCze+59DBfipsULD2mEn9fjYRXQEwb2QWtQ9qRc20Yb/x\n" + "Q4b/+CvUodLkaq7B8MHz0BV8HHcBoph6DYaRmO/N+hPauIuSp6XyaGYcEefGKVKj\n" + "G2+fcsdyXsoijNdL8vNKwm4j2gVwCBnw16J00yfFoV46YcbfqEdJB2je0XSvwXqt\n" + "14AOTngxso2h9k9HLtrfpO1ZG/B5AcCMs1lzbZ2fp5DPHtjvvmvA2RJqgo3yjw4W\n" + "4DHAuTglYFlC3mDHNfNtcGP20JvepcQNzNP2UzwcpOc94hfKikOFw+gf9Vf1qd0y\n" + "h/Sk6OZHn2+JVUPiWHIQV98Vtoh4RmUZDJD+b55ia3fQGTGzt4z1XFzQYSva5sfs\n" + "wocS/papthqWldQU7x+3wofNd5CNU1x6WKXG/yw30IT/4F8ADJD6GeygNT8QJYvt\n" + "u/8lAkbOy6B9xGmSvr0Kk1oq9P2NshA6kalxp1Oz/DTNDdL4AeBXV3JmM6WWCjGn\n" + "Yy1RT69d0rwYc5u/vnqODz1IjvT90smsrkBumGt791FAFeg=\n" + "-----END CERTIFICATE-----\n"; + +tor_x509_cert_impl_t * +read_cert_from(const char *str) +{ + size_t len = strlen(str); + uint8_t *raw_cert = tor_malloc(len); + size_t true_len = pem_decode(raw_cert, len, str, len, "CERTIFICATE"); + tor_x509_cert_t *cert = tor_x509_cert_decode(raw_cert, true_len); + tor_free(raw_cert); + if (! cert) { + return NULL; + } + tor_x509_cert_impl_t *res = tor_x509_cert_impl_dup_(cert->cert); + tor_x509_cert_free(cert); + return res; +} + +static tor_x509_cert_impl_t * + fixed_try_to_extract_certs_from_tls_cert_out_result = NULL; +static tor_x509_cert_impl_t * + fixed_try_to_extract_certs_from_tls_id_cert_out_result = NULL; + +static void +fixed_try_to_extract_certs_from_tls(int severity, tor_tls_t *tls, + tor_x509_cert_impl_t **cert_out, + tor_x509_cert_impl_t **id_cert_out) +{ + (void) severity; + (void) tls; + *cert_out = fixed_try_to_extract_certs_from_tls_cert_out_result; + *id_cert_out = fixed_try_to_extract_certs_from_tls_id_cert_out_result; +} + static void test_tortls_errno_to_tls_error(void *data) { @@ -348,6 +455,55 @@ test_tortls_is_server(void *arg) crypto_pk_free(pk2); } +static void +test_tortls_verify(void *ignored) +{ + (void)ignored; + int ret; + tor_tls_t *tls; + crypto_pk_t *k = NULL; + tor_x509_cert_impl_t *cert1 = NULL, *cert2 = NULL, *invalidCert = NULL, + *validCert = NULL, *caCert = NULL; + + validCert = read_cert_from(validCertString); + caCert = read_cert_from(caCertString); + invalidCert = read_cert_from(notCompletelyValidCertString); + + tls = tor_malloc_zero(sizeof(tor_tls_t)); + + MOCK(try_to_extract_certs_from_tls, fixed_try_to_extract_certs_from_tls); + + fixed_try_to_extract_certs_from_tls_cert_out_result = cert1; + ret = tor_tls_verify(LOG_WARN, tls, &k); + tt_int_op(ret, OP_EQ, -1); + + fixed_try_to_extract_certs_from_tls_id_cert_out_result = cert2; + ret = tor_tls_verify(LOG_WARN, tls, &k); + tt_int_op(ret, OP_EQ, -1); + + fixed_try_to_extract_certs_from_tls_cert_out_result = invalidCert; + fixed_try_to_extract_certs_from_tls_id_cert_out_result = invalidCert; + + ret = tor_tls_verify(LOG_WARN, tls, &k); + tt_int_op(ret, OP_EQ, -1); + + fixed_try_to_extract_certs_from_tls_cert_out_result = validCert; + fixed_try_to_extract_certs_from_tls_id_cert_out_result = caCert; + + ret = tor_tls_verify(LOG_WARN, tls, &k); + tt_int_op(ret, OP_EQ, 0); + tt_assert(k); + + done: + UNMOCK(try_to_extract_certs_from_tls); + if (cert1) + tor_x509_cert_impl_free_(cert1); + if (cert2) + tor_x509_cert_impl_free_(cert2); + tor_free(tls); + crypto_pk_free(k); +} + #define LOCAL_TEST_CASE(name, flags) \ { #name, test_tortls_##name, (flags|TT_FORK), NULL, NULL } @@ -367,5 +523,6 @@ struct testcase_t tortls_tests[] = { LOCAL_TEST_CASE(address, TT_FORK), LOCAL_TEST_CASE(is_server, 0), LOCAL_TEST_CASE(bridge_init, TT_FORK), + LOCAL_TEST_CASE(verify, TT_FORK), END_OF_TESTCASES }; diff --git a/src/test/test_tortls.h b/src/test/test_tortls.h new file mode 100644 index 000000000..c997934eb --- /dev/null +++ b/src/test/test_tortls.h @@ -0,0 +1,13 @@ +/* Copyright (c) 2010-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#ifndef TEST_TORTLS_H +#define TEST_TORTLS_H + +tor_x509_cert_impl_t *read_cert_from(const char *str); + +extern const char *notCompletelyValidCertString; +extern const char *validCertString; +extern const char *caCertString; + +#endif diff --git a/src/test/test_tortls_openssl.c b/src/test/test_tortls_openssl.c index 3ae0f04b7..3abf41265 100644 --- a/src/test/test_tortls_openssl.c +++ b/src/test/test_tortls_openssl.c @@ -44,6 +44,8 @@ ENABLE_GCC_WARNING(redundant-decls) #include "test/test.h" #include "test/log_test_helpers.h" +#include "test/test_tortls.h" + #define NS_MODULE tortls #ifndef HAVE_SSL_STATE @@ -803,164 +805,6 @@ test_tortls_client_is_using_v2_ciphers(void *ignored) } #ifndef OPENSSL_OPAQUE -static X509 *fixed_try_to_extract_certs_from_tls_cert_out_result = NULL; -static X509 *fixed_try_to_extract_certs_from_tls_id_cert_out_result = NULL; - -static void -fixed_try_to_extract_certs_from_tls(int severity, tor_tls_t *tls, - tor_x509_cert_impl_t **cert_out, - tor_x509_cert_impl_t **id_cert_out) -{ - (void) severity; - (void) tls; - *cert_out = fixed_try_to_extract_certs_from_tls_cert_out_result; - *id_cert_out = fixed_try_to_extract_certs_from_tls_id_cert_out_result; -} -#endif /* !defined(OPENSSL_OPAQUE) */ - -#ifndef OPENSSL_OPAQUE -static const char* notCompletelyValidCertString = - "-----BEGIN CERTIFICATE-----\n" - "MIICVjCCAb8CAg37MA0GCSqGSIb3DQEBBQUAMIGbMQswCQYDVQQGEwJKUDEOMAwG\n" - "A1UECBMFVG9reW8xEDAOBgNVBAcTB0NodW8ta3UxETAPBgNVBAoTCEZyYW5rNERE\n" - "MRgwFgYDVQQLEw9XZWJDZXJ0IFN1cHBvcnQxGDAWBgNVBAMTD0ZyYW5rNEREIFdl\n" - "YiBDQTEjMCEGCSqGSIb3DQEJARYUc3VwcG9ydEBmcmFuazRkZC5jb20wHhcNMTIw\n" - "ODIyMDUyNzIzWhcNMTcwODIxMDUyNzIzWjBKMQswCQYDVQQGEwJKUDEOMAwGA1UE\n" - "CAwFVG9reW8xETAPBgNVBAoMCEZyYW5rNEREMRgwFgYDVQQDDA93d3cuZXhhbXBs\n" - "ZS5jb20wgZ8wDQYJKoZIhvcNAQEBBQADgY0AMIGJAoGBAMYBBrx5PlP0WNI/ZdzD\n" - "+6Pktmurn+F2kQYbtc7XQh8/LTBvCo+P6iZoLEmUA9e7EXLRxgU1CVqeAi7QcAn9\n" - "MwBlc8ksFJHB0rtf9pmf8Oza9E0Bynlq/4/Kb1x+d+AyhL7oK9tQwB24uHOueHi1\n" - "C/iVv8CSWKiYe6hzN1txYe8rAgMBAAEwDQYJKoZIhvcNAQEFBQADgYEAASPdjigJ\n" - "kXCqKWpnZ/Oc75EUcMi6HztaW8abUMlYXPIgkV2F7YanHOB7K4f7OOLjiz8DTPFf\n" - "jC9UeuErhaA/zzWi8ewMTFZW/WshOrm3fNvcMrMLKtH534JKvcdMg6qIdjTFINIr\n" - "evnAhf0cwULaebn+lMs8Pdl7y37+sfluVok=\n" - "-----END CERTIFICATE-----\n"; -#endif /* !defined(OPENSSL_OPAQUE) */ - -static const char* validCertString = "-----BEGIN CERTIFICATE-----\n" - "MIIDpTCCAY0CAg3+MA0GCSqGSIb3DQEBBQUAMF4xCzAJBgNVBAYTAlVTMREwDwYD\n" - "VQQIDAhJbGxpbm9pczEQMA4GA1UEBwwHQ2hpY2FnbzEUMBIGA1UECgwLVG9yIFRl\n" - "c3RpbmcxFDASBgNVBAMMC1RvciBUZXN0aW5nMB4XDTE1MDkwNjEzMzk1OVoXDTQz\n" - "MDEyMjEzMzk1OVowVjELMAkGA1UEBhMCVVMxEDAOBgNVBAcMB0NoaWNhZ28xFDAS\n" - "BgNVBAoMC1RvciBUZXN0aW5nMR8wHQYDVQQDDBZ0ZXN0aW5nLnRvcnByb2plY3Qu\n" - "b3JnMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDoT6uyVVhWyOF3wkHjjYbd\n" - "nKaykyRv4JVtKQdZ4OpEErmX1zw4MmyzpQNV6iR4bQnWiyLfzyVJMZDIC/WILBfX\n" - "w2Pza/yuLgUvDc3twMuhOACzOQVO8PrEF/aVv2+hbCCy2udXvKhnYn+CCXl3ozc8\n" - "XcKYvujTXDyvGWY3xwAjlQIDAQABMA0GCSqGSIb3DQEBBQUAA4ICAQCUvnhzQWuQ\n" - "MrN+pERkE+zcTI/9dGS90rUMMLgu8VDNqTa0TUQh8uO0EQ6uDvI8Js6e8tgwS0BR\n" - "UBahqb7ZHv+rejGCBr5OudqD+x4STiiuPNJVs86JTLN8SpM9CHjIBH5WCCN2KOy3\n" - "mevNoRcRRyYJzSFULCunIK6FGulszigMYGscrO4oiTkZiHPh9KvWT40IMiHfL+Lw\n" - "EtEWiLex6064LcA2YQ1AMuSZyCexks63lcfaFmQbkYOKqXa1oLkIRuDsOaSVjTfe\n" - "vec+X6jvf12cFTKS5WIeqkKF2Irt+dJoiHEGTe5RscUMN/f+gqHPzfFz5dR23sxo\n" - "g+HC6MZHlFkLAOx3wW6epPS8A/m1mw3zMPoTnb2U2YYt8T0dJMMlUn/7Y1sEAa+a\n" - "dSTMaeUf6VnJ//11m454EZl1to9Z7oJOgqmFffSrdD4BGIWe8f7hhW6L1Enmqe/J\n" - "BKL3wbzZh80O1W0bndAwhnEEhlzneFY84cbBo9pmVxpODHkUcStpr5Z7pBDrcL21\n" - "Ss/aB/1YrsVXhdvJdOGxl3Mnl9dUY57CympLGlT8f0pPS6GAKOelECOhFMHmJd8L\n" - "dj3XQSmKtYHevZ6IvuMXSlB/fJvSjSlkCuLo5+kJoaqPuRu+i/S1qxeRy3CBwmnE\n" - "LdSNdcX4N79GQJ996PA8+mUCQG7YRtK+WA==\n" - "-----END CERTIFICATE-----\n"; - -static const char* caCertString = "-----BEGIN CERTIFICATE-----\n" - "MIIFjzCCA3egAwIBAgIJAKd5WgyfPMYRMA0GCSqGSIb3DQEBCwUAMF4xCzAJBgNV\n" - "BAYTAlVTMREwDwYDVQQIDAhJbGxpbm9pczEQMA4GA1UEBwwHQ2hpY2FnbzEUMBIG\n" - "A1UECgwLVG9yIFRlc3RpbmcxFDASBgNVBAMMC1RvciBUZXN0aW5nMB4XDTE1MDkw\n" - "NjEzMzc0MVoXDTQzMDEyMjEzMzc0MVowXjELMAkGA1UEBhMCVVMxETAPBgNVBAgM\n" - "CElsbGlub2lzMRAwDgYDVQQHDAdDaGljYWdvMRQwEgYDVQQKDAtUb3IgVGVzdGlu\n" - "ZzEUMBIGA1UEAwwLVG9yIFRlc3RpbmcwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAw\n" - "ggIKAoICAQCpLMUEiLW5leUgBZoEJms2V7lZRhIAjnJBhVMHD0e3UubNknmaQoxf\n" - "ARz3rvqOaRd0JlV+qM9qE0DjiYcCVP1cAfqAo9d83uS1vwY3YMVJzADlaIiHfyVW\n" - "uEgBy0vvkeUBqaua24dYlcwsemOiXYLu41yM1wkcGHW1AhBNHppY6cznb8TyLgNM\n" - "2x3SGUdzc5XMyAFx51faKGBA3wjs+Hg1PLY7d30nmCgEOBavpm5I1disM/0k+Mcy\n" - "YmAKEo/iHJX/rQzO4b9znP69juLlR8PDBUJEVIG/CYb6+uw8MjjUyiWXYoqfVmN2\n" - "hm/lH8b6rXw1a2Aa3VTeD0DxaWeacMYHY/i01fd5n7hCoDTRNdSw5KJ0L3Z0SKTu\n" - "0lzffKzDaIfyZGlpW5qdouACkWYzsaitQOePVE01PIdO30vUfzNTFDfy42ccx3Di\n" - "59UCu+IXB+eMtrBfsok0Qc63vtF1linJgjHW1z/8ujk8F7/qkOfODhk4l7wngc2A\n" - "EmwWFIFoGaiTEZHB9qteXr4unbXZ0AHpM02uGGwZEGohjFyebEb73M+J57WKKAFb\n" - "PqbLcGUksL1SHNBNAJcVLttX55sO4nbidOS/kA3m+F1R04MBTyQF9qA6YDDHqdI3\n" - "h/3pw0Z4fxVouTYT4/NfRnX4JTP4u+7Mpcoof28VME0qWqD1LnRhFQIDAQABo1Aw\n" - "TjAdBgNVHQ4EFgQUMoAgIXH7pZ3QMRwTjT+DM9Yo/v0wHwYDVR0jBBgwFoAUMoAg\n" - "IXH7pZ3QMRwTjT+DM9Yo/v0wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQsFAAOC\n" - "AgEAUJxacjXR9sT+Xs6ISFiUsyd0T6WVKMnV46xrYJHirGfx+krWHrjxMY+ZtxYD\n" - "DBDGlo11Qc4v6QrclNf5QUBfIiGQsP9Cm6hHcQ+Tpg9HHCgSqG1YNPwCPReCR4br\n" - "BLvLfrfkcBL2IWM0PdQdCze+59DBfipsULD2mEn9fjYRXQEwb2QWtQ9qRc20Yb/x\n" - "Q4b/+CvUodLkaq7B8MHz0BV8HHcBoph6DYaRmO/N+hPauIuSp6XyaGYcEefGKVKj\n" - "G2+fcsdyXsoijNdL8vNKwm4j2gVwCBnw16J00yfFoV46YcbfqEdJB2je0XSvwXqt\n" - "14AOTngxso2h9k9HLtrfpO1ZG/B5AcCMs1lzbZ2fp5DPHtjvvmvA2RJqgo3yjw4W\n" - "4DHAuTglYFlC3mDHNfNtcGP20JvepcQNzNP2UzwcpOc94hfKikOFw+gf9Vf1qd0y\n" - "h/Sk6OZHn2+JVUPiWHIQV98Vtoh4RmUZDJD+b55ia3fQGTGzt4z1XFzQYSva5sfs\n" - "wocS/papthqWldQU7x+3wofNd5CNU1x6WKXG/yw30IT/4F8ADJD6GeygNT8QJYvt\n" - "u/8lAkbOy6B9xGmSvr0Kk1oq9P2NshA6kalxp1Oz/DTNDdL4AeBXV3JmM6WWCjGn\n" - "Yy1RT69d0rwYc5u/vnqODz1IjvT90smsrkBumGt791FAFeg=\n" - "-----END CERTIFICATE-----\n"; - -static X509 * -read_cert_from(const char *str) -{ - BIO *bio = BIO_new(BIO_s_mem()); - BIO_write(bio, str, (int) strlen(str)); - X509 *res = PEM_read_bio_X509(bio, NULL, NULL, NULL); - BIO_free(bio); - return res; -} - -#ifndef OPENSSL_OPAQUE -static void -test_tortls_verify(void *ignored) -{ - (void)ignored; - int ret; - tor_tls_t *tls; - crypto_pk_t *k = NULL; - X509 *cert1 = NULL, *cert2 = NULL, *invalidCert = NULL, - *validCert = NULL, *caCert = NULL; - - cert1 = tor_malloc_zero(sizeof(X509)); - cert1->references = 10; - - cert2 = tor_malloc_zero(sizeof(X509)); - cert2->references = 10; - - validCert = read_cert_from(validCertString); - caCert = read_cert_from(caCertString); - invalidCert = read_cert_from(notCompletelyValidCertString); - - tls = tor_malloc_zero(sizeof(tor_tls_t)); - ret = tor_tls_verify(LOG_WARN, tls, &k); - tt_int_op(ret, OP_EQ, -1); - - MOCK(try_to_extract_certs_from_tls, fixed_try_to_extract_certs_from_tls); - - fixed_try_to_extract_certs_from_tls_cert_out_result = cert1; - ret = tor_tls_verify(LOG_WARN, tls, &k); - tt_int_op(ret, OP_EQ, -1); - - fixed_try_to_extract_certs_from_tls_id_cert_out_result = cert2; - ret = tor_tls_verify(LOG_WARN, tls, &k); - tt_int_op(ret, OP_EQ, -1); - - fixed_try_to_extract_certs_from_tls_cert_out_result = invalidCert; - fixed_try_to_extract_certs_from_tls_id_cert_out_result = invalidCert; - - ret = tor_tls_verify(LOG_WARN, tls, &k); - tt_int_op(ret, OP_EQ, -1); - - fixed_try_to_extract_certs_from_tls_cert_out_result = validCert; - fixed_try_to_extract_certs_from_tls_id_cert_out_result = caCert; - - ret = tor_tls_verify(LOG_WARN, tls, &k); - tt_int_op(ret, OP_EQ, 0); - tt_assert(k); - - done: - UNMOCK(try_to_extract_certs_from_tls); - tor_free(cert1); - tor_free(cert2); - tor_free(tls); - tor_free(k); -} -#endif /* !defined(OPENSSL_OPAQUE) */ - -#ifndef OPENSSL_OPAQUE static int fixed_ssl_pending_result = 0; static int @@ -2429,7 +2273,6 @@ struct testcase_t tortls_openssl_tests[] = { INTRUSIVE_TEST_CASE(get_ciphersuite_name, 0), INTRUSIVE_TEST_CASE(classify_client_ciphers, 0), LOCAL_TEST_CASE(client_is_using_v2_ciphers, 0), - INTRUSIVE_TEST_CASE(verify, 0), INTRUSIVE_TEST_CASE(get_pending_bytes, 0), INTRUSIVE_TEST_CASE(SSL_SESSION_get_master_key, 0), INTRUSIVE_TEST_CASE(get_tlssecrets, 0),

1 0

[tor/master] Use FREE_AND_NULL for impl types
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 274efb126324f3f8a7e98b15a697c0038a9803d0 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Sep 4 14:37:42 2018 -0400 Use FREE_AND_NULL for impl types --- src/lib/tls/tortls.c | 17 ++++++----------- src/lib/tls/tortls_internal.h | 6 +++++- src/lib/tls/tortls_nss.c | 6 +++++- src/lib/tls/tortls_openssl.c | 5 ++++- src/lib/tls/x509.c | 5 ++--- src/lib/tls/x509_internal.h | 2 ++ src/test/test_tortls.c | 6 ++---- src/test/test_x509.c | 3 +-- 8 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/lib/tls/tortls.c b/src/lib/tls/tortls.c index 64c26f7e1..923b0db4c 100644 --- a/src/lib/tls/tortls.c +++ b/src/lib/tls/tortls.c @@ -335,12 +335,9 @@ tor_tls_context_init_certificates(tor_tls_context_t *result, tor_free(nickname); tor_free(nn2); - if (cert) - tor_x509_cert_impl_free_(cert); - if (idcert) - tor_x509_cert_impl_free_(idcert); - if (authcert) - tor_x509_cert_impl_free_(authcert); + tor_x509_cert_impl_free(cert); + tor_x509_cert_impl_free(idcert); + tor_x509_cert_impl_free(authcert); crypto_pk_free(rsa); crypto_pk_free(rsa_auth); @@ -379,7 +376,7 @@ tor_tls_free_(tor_tls_t *tls) size_t r,w; tor_tls_get_n_raw_bytes(tls,&r,&w); /* ensure written_by_tls is updated */ } - tor_tls_impl_free_(tls->ssl); + tor_tls_impl_free(tls->ssl); tls->ssl = NULL; #ifdef ENABLE_OPENSSL tls->negotiated_callback = NULL; @@ -424,10 +421,8 @@ tor_tls_verify(int severity, tor_tls_t *tls, crypto_pk_t **identity) rv = 0; done: - if (cert) - tor_x509_cert_impl_free_(cert); - if (id_cert) - tor_x509_cert_impl_free_(id_cert); + tor_x509_cert_impl_free(cert); + tor_x509_cert_impl_free(id_cert); tor_x509_cert_free(peer_x509); tor_x509_cert_free(id_x509); diff --git a/src/lib/tls/tortls_internal.h b/src/lib/tls/tortls_internal.h index b9e01e0c5..2bf221210 100644 --- a/src/lib/tls/tortls_internal.h +++ b/src/lib/tls/tortls_internal.h @@ -28,8 +28,12 @@ int tor_tls_context_init_certificates(tor_tls_context_t *result, unsigned key_lifetime, unsigned flags); void tor_tls_impl_free_(tor_tls_impl_t *ssl); +#define tor_tls_impl_free(tls) \ + FREE_AND_NULL(tor_tls_impl_t, tor_tls_impl_free_, (tls)) -void tor_tls_context_impl_free(tor_tls_context_impl_t *); +void tor_tls_context_impl_free_(tor_tls_context_impl_t *); +#define tor_tls_context_impl_free(ctx) \ + FREE_AND_NULL(tor_tls_context_impl_t, tor_tls_context_impl_free_, (ctx)) #ifdef ENABLE_OPENSSL tor_tls_t *tor_tls_get_by_ssl(const struct ssl_st *ssl); diff --git a/src/lib/tls/tortls_nss.c b/src/lib/tls/tortls_nss.c index 266455049..548a817e6 100644 --- a/src/lib/tls/tortls_nss.c +++ b/src/lib/tls/tortls_nss.c @@ -298,8 +298,10 @@ tor_tls_context_new(crypto_pk_t *identity, } void -tor_tls_context_impl_free(tor_tls_context_impl_t *ctx) +tor_tls_context_impl_free_(tor_tls_context_impl_t *ctx) { + if (!ctx) + return; PR_Close(ctx); } @@ -409,6 +411,8 @@ tor_tls_impl_free_(tor_tls_impl_t *tls) { // XXXX This will close the underlying fd, which our OpenSSL version does // not do! + if (!tls) + return; PR_Close(tls); } diff --git a/src/lib/tls/tortls_openssl.c b/src/lib/tls/tortls_openssl.c index 6ff03ee81..370e7e436 100644 --- a/src/lib/tls/tortls_openssl.c +++ b/src/lib/tls/tortls_openssl.c @@ -496,7 +496,7 @@ tor_tls_cert_matches_key,(const tor_tls_t *tls, const tor_x509_cert_t *cert)) } void -tor_tls_context_impl_free(struct ssl_ctx_st *ctx) +tor_tls_context_impl_free_(struct ssl_ctx_st *ctx) { if (!ctx) return; @@ -1150,6 +1150,9 @@ tor_tls_assert_renegotiation_unblocked(tor_tls_t *tls) void tor_tls_impl_free_(tor_tls_impl_t *ssl) { + if (!ssl) + return; + #ifdef SSL_set_tlsext_host_name SSL_set_tlsext_host_name(ssl, NULL); #endif diff --git a/src/lib/tls/x509.c b/src/lib/tls/x509.c index 2e7020646..442cdf2c7 100644 --- a/src/lib/tls/x509.c +++ b/src/lib/tls/x509.c @@ -76,8 +76,7 @@ tor_x509_cert_free_(tor_x509_cert_t *cert) { if (! cert) return; - if (cert->cert) - tor_x509_cert_impl_free_(cert->cert); + tor_x509_cert_impl_free(cert->cert); #ifdef ENABLE_OPENSSL tor_free(cert->encoded); #endif @@ -131,7 +130,7 @@ tor_x509_cert_new,(tor_x509_cert_impl_t *x509_cert)) err: tor_free(cert); log_err(LD_CRYPTO, "Couldn't wrap encoded X509 certificate."); - tor_x509_cert_impl_free_(x509_cert); + tor_x509_cert_impl_free(x509_cert); return NULL; } diff --git a/src/lib/tls/x509_internal.h b/src/lib/tls/x509_internal.h index c08705cb2..139ecedd2 100644 --- a/src/lib/tls/x509_internal.h +++ b/src/lib/tls/x509_internal.h @@ -41,6 +41,8 @@ int tor_x509_check_cert_lifetime_internal(int severity, int future_tolerance); void tor_x509_cert_impl_free_(tor_x509_cert_impl_t *cert); +#define tor_x509_cert_impl_free(cert) \ + FREE_AND_NULL(tor_x509_cert_impl_t, tor_x509_cert_impl_free_, (cert)) tor_x509_cert_impl_t *tor_x509_cert_impl_dup_(tor_x509_cert_impl_t *cert); #ifdef ENABLE_OPENSSL int tor_x509_cert_set_cached_der_encoding(tor_x509_cert_t *cert); diff --git a/src/test/test_tortls.c b/src/test/test_tortls.c index b260310ad..8e8487a40 100644 --- a/src/test/test_tortls.c +++ b/src/test/test_tortls.c @@ -496,10 +496,8 @@ test_tortls_verify(void *ignored) done: UNMOCK(try_to_extract_certs_from_tls); - if (cert1) - tor_x509_cert_impl_free_(cert1); - if (cert2) - tor_x509_cert_impl_free_(cert2); + tor_x509_cert_impl_free(cert1); + tor_x509_cert_impl_free(cert2); tor_free(tls); crypto_pk_free(k); } diff --git a/src/test/test_x509.c b/src/test/test_x509.c index 9163977bd..9ec0657d8 100644 --- a/src/test/test_x509.c +++ b/src/test/test_x509.c @@ -57,8 +57,7 @@ test_x509_cert_new_failing_digest(void *arg) done: crypto_pk_free(pk1); crypto_pk_free(pk2); - if (impl) - tor_x509_cert_impl_free_(impl); + tor_x509_cert_impl_free(impl); UNMOCK(crypto_digest); teardown_capture_of_logs(); }

1 0