From 0121ed98b5dde62145ab0fa408d413316e009758 Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Tue, 24 Jul 2018 11:01:59 +0000 Subject: [PATCH] crypto: follow up to r1836439: restore apr_crypto_lib_init/term(). Also restores apr_crypto_init()'s global pool in testcrypto to avoid segfaults because of openssl inability to re-init. git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1836539 13f79535-47bb-0310-9956-ffa450edef68 --- crypto/apr_crypto_commoncrypto.c | 13 +--- crypto/apr_crypto_nss.c | 116 +------------------------------ crypto/apr_crypto_openssl.c | 27 +------ test/testcrypto.c | 14 ++-- 4 files changed, 12 insertions(+), 158 deletions(-) diff --git a/crypto/apr_crypto_commoncrypto.c b/crypto/apr_crypto_commoncrypto.c index f2095bcd7ad..421b0581a44 100644 --- a/crypto/apr_crypto_commoncrypto.c +++ b/crypto/apr_crypto_commoncrypto.c @@ -123,12 +123,7 @@ static apr_status_t crypto_error(const apu_err_t **result, */ static apr_status_t crypto_shutdown(void) { - return APR_SUCCESS; -} - -static apr_status_t crypto_shutdown_helper(void *data) -{ - return crypto_shutdown(); + return apr_crypto_lib_term("commoncrypto"); } /** @@ -137,11 +132,7 @@ static apr_status_t crypto_shutdown_helper(void *data) static apr_status_t crypto_init(apr_pool_t *pool, const char *params, const apu_err_t **result) { - - apr_pool_cleanup_register(pool, pool, crypto_shutdown_helper, - apr_pool_cleanup_null); - - return APR_SUCCESS; + return apr_crypto_lib_init("commoncrypto", params, result, pool); } /** diff --git a/crypto/apr_crypto_nss.c b/crypto/apr_crypto_nss.c index 37ffe086231..807ae58c54b 100644 --- a/crypto/apr_crypto_nss.c +++ b/crypto/apr_crypto_nss.c @@ -135,20 +135,7 @@ static apr_status_t crypto_error(const apu_err_t **result, */ static apr_status_t crypto_shutdown(void) { - if (NSS_IsInitialized()) { - SECStatus s = NSS_Shutdown(); - if (s != SECSuccess) { - fprintf(stderr, "NSS failed to shutdown, possible leak: %d: %s", - PR_GetError(), PR_ErrorToName(s)); - return APR_EINIT; - } - } - return APR_SUCCESS; -} - -static apr_status_t crypto_shutdown_helper(void *data) -{ - return crypto_shutdown(); + return apr_crypto_lib_term("nss"); } /** @@ -157,106 +144,7 @@ static apr_status_t crypto_shutdown_helper(void *data) static apr_status_t crypto_init(apr_pool_t *pool, const char *params, const apu_err_t **result) { - SECStatus s; - const char *dir = NULL; - const char *keyPrefix = NULL; - const char *certPrefix = NULL; - const char *secmod = NULL; - int noinit = 0; - PRUint32 flags = 0; - - struct { - const char *field; - const char *value; - int set; - } fields[] = { - { "dir", NULL, 0 }, - { "key3", NULL, 0 }, - { "cert7", NULL, 0 }, - { "secmod", NULL, 0 }, - { "noinit", NULL, 0 }, - { NULL, NULL, 0 } - }; - const char *ptr; - size_t klen; - char **elts = NULL; - char *elt; - int i = 0, j; - apr_status_t status; - - if (params) { - if (APR_SUCCESS != (status = apr_tokenize_to_argv(params, &elts, pool))) { - return status; - } - while ((elt = elts[i])) { - ptr = strchr(elt, '='); - if (ptr) { - for (klen = ptr - elt; klen && apr_isspace(elt[klen - 1]); --klen) - ; - ptr++; - } - else { - for (klen = strlen(elt); klen && apr_isspace(elt[klen - 1]); --klen) - ; - } - elt[klen] = 0; - - for (j = 0; fields[j].field != NULL; ++j) { - if (klen && !strcasecmp(fields[j].field, elt)) { - fields[j].set = 1; - if (ptr) { - fields[j].value = ptr; - } - break; - } - } - - i++; - } - dir = fields[0].value; - keyPrefix = fields[1].value; - certPrefix = fields[2].value; - secmod = fields[3].value; - noinit = fields[4].set; - } - - /* if we've been asked to bypass, do so here */ - if (noinit) { - return APR_SUCCESS; - } - - /* sanity check - we can only initialise NSS once */ - if (NSS_IsInitialized()) { - return APR_EREINIT; - } - - if (keyPrefix || certPrefix || secmod) { - s = NSS_Initialize(dir, certPrefix, keyPrefix, secmod, flags); - } - else if (dir) { - s = NSS_InitReadWrite(dir); - } - else { - s = NSS_NoDB_Init(NULL); - } - if (s != SECSuccess) { - if (result) { - /* Note: all memory must be owned by the caller, in case we're unloaded */ - apu_err_t *err = apr_pcalloc(pool, sizeof(apu_err_t)); - err->rc = PR_GetError(); - err->msg = apr_pstrdup(pool, PR_ErrorToName(s)); - err->reason = apr_pstrdup(pool, "Error during 'nss' initialisation"); - *result = err; - } - - return APR_ECRYPT; - } - - apr_pool_cleanup_register(pool, pool, crypto_shutdown_helper, - apr_pool_cleanup_null); - - return APR_SUCCESS; - + return apr_crypto_lib_init("nss", params, result, pool); } /** diff --git a/crypto/apr_crypto_openssl.c b/crypto/apr_crypto_openssl.c index 82b3842f0a7..5186e6b42e9 100644 --- a/crypto/apr_crypto_openssl.c +++ b/crypto/apr_crypto_openssl.c @@ -34,6 +34,7 @@ #include #include #include +#include #define LOG_PREFIX "apr_crypto_openssl: " @@ -143,15 +144,7 @@ static apr_status_t crypto_error(const apu_err_t **result, */ static apr_status_t crypto_shutdown(void) { - ERR_free_strings(); - EVP_cleanup(); - ENGINE_cleanup(); - return APR_SUCCESS; -} - -static apr_status_t crypto_shutdown_helper(void *data) -{ - return crypto_shutdown(); + return apr_crypto_lib_term("openssl"); } /** @@ -160,21 +153,7 @@ static apr_status_t crypto_shutdown_helper(void *data) static apr_status_t crypto_init(apr_pool_t *pool, const char *params, const apu_err_t **result) { -#if APR_USE_OPENSSL_PRE_1_1_API - (void)CRYPTO_malloc_init(); -#else - OPENSSL_malloc_init(); -#endif - ERR_load_crypto_strings(); - /* SSL_load_error_strings(); */ - OpenSSL_add_all_algorithms(); - ENGINE_load_builtin_engines(); - ENGINE_register_all_complete(); - - apr_pool_cleanup_register(pool, pool, crypto_shutdown_helper, - apr_pool_cleanup_null); - - return APR_SUCCESS; + return apr_crypto_lib_init("openssl", params, result, pool); } #if OPENSSL_VERSION_NUMBER < 0x0090802fL diff --git a/test/testcrypto.c b/test/testcrypto.c index 9261d1887cd..4c93a8b2caa 100644 --- a/test/testcrypto.c +++ b/test/testcrypto.c @@ -1075,7 +1075,7 @@ static void test_crypto_init(abts_case *tc, void *data) apr_pool_create(&pool, NULL); - rv = apr_crypto_init(pool); + rv = apr_crypto_init(apr_pool_parent_get(pool)); ABTS_ASSERT(tc, "failed to init apr_crypto", rv == APR_SUCCESS); apr_pool_destroy(pool); @@ -2461,10 +2461,8 @@ static void test_crypto_prng(abts_case *tc, void *data) rv == APR_SUCCESS); } - if (cprng) { - rv = apr_crypto_prng_bytes(cprng, randbytes, 128 - 32); - ABTS_ASSERT(tc, "apr_crypto_prng_bytes failed", rv == APR_SUCCESS); - } + rv = apr_crypto_prng_bytes(cprng, randbytes, 128 - 32); + ABTS_ASSERT(tc, "apr_crypto_prng_bytes failed", rv == APR_SUCCESS); /* Should match the first time only */ if (i != 0) { @@ -2478,10 +2476,8 @@ static void test_crypto_prng(abts_case *tc, void *data) memcmp(randbytes, test_PRNG_kat0 + 32, 128 - 32) == 0); } - if (cprng) { - rv = apr_crypto_prng_destroy(cprng); - ABTS_ASSERT(tc, "apr_crypto_prng_destroy failed", rv == APR_SUCCESS); - } + rv = apr_crypto_prng_destroy(cprng); + ABTS_ASSERT(tc, "apr_crypto_prng_destroy failed", rv == APR_SUCCESS); } apr_pool_destroy(pool);