From: Yann Ylavic Date: Fri, 24 Mar 2017 13:31:03 +0000 (+0000) Subject: Merge r1781187, r1781190, r1781312 from trunk: X-Git-Tag: 2.4.26~233 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ee0789f9b70a9976dc3a5cb7077de6bff6414808;p=apache Merge r1781187, r1781190, r1781312 from trunk: mod_ssl: work around leaks on (graceful) restart. Tested with valgrind and --with-ssl shared/static. mod_ssl: follow up to r1781187. The ssl_util_thread_*() functions are not necessary with openssl-1.1+ mod_ssl: follow up to r1781187. Address SSL_CTX leak in (merged) proxy_ctx. Reviewed by: ylavic, jim, wrowe git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1788442 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index b5bda84fa7..b476176913 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,8 @@ Changes with Apache 2.4.26 + *) mod_ssl: work around leaks on (graceful) restart. [Yann Ylavic] + *) mod_ssl: Add support for OpenSSL 1.1.0. [Rainer Jung] *) Don't set SO_REUSEPORT unless ListenCoresBucketsRatio is greater diff --git a/STATUS b/STATUS index 1fe109d1fd..21b19e4f03 100644 --- a/STATUS +++ b/STATUS @@ -150,13 +150,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - *) mod_ssl: work around leaks on (graceful) restart. - trunk patch: http://svn.apache.org/r1781187 - http://svn.apache.org/r1781190 - http://svn.apache.org/r1781312 - 2.4.x patch: http://home.apache.org/~ylavic/patches/httpd-2.4.x-mod_ssl-restart_leaks-v2.patch - +1: ylavic, jim, wrowe - *) core: %{DOCUMENT_URI} used in nested SSI expressions should point to the URI originally requsted by the user, not the nested documents URI. This restores the behavior of this variable to match the "legacy" SSI parser. diff --git a/modules/ssl/mod_ssl.c b/modules/ssl/mod_ssl.c index bf9edb28d8..f58e145f19 100644 --- a/modules/ssl/mod_ssl.c +++ b/modules/ssl/mod_ssl.c @@ -30,9 +30,12 @@ #include "util_md5.h" #include "util_mutex.h" #include "ap_provider.h" +#include "http_config.h" #include +static int modssl_running_statically = 0; + APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(ssl, SSL, int, pre_handshake, (conn_rec *c,SSL *ssl,int is_proxy), (c,ssl,is_proxy), OK, DECLINED); @@ -297,21 +300,40 @@ static const command_rec ssl_config_cmds[] = { /* * the various processing hooks */ +static int modssl_is_prelinked(void) +{ + apr_size_t i = 0; + const module *mod; + while ((mod = ap_prelinked_modules[i++])) { + if (strcmp(mod->name, "mod_ssl.c") == 0) { + return 1; + } + } + return 0; +} + static apr_status_t ssl_cleanup_pre_config(void *data) { /* * Try to kill the internals of the SSL library. */ - /* Corresponds to OPENSSL_load_builtin_modules(): - * XXX: borrowed from apps.h, but why not CONF_modules_free() - * which also invokes CONF_modules_finish()? - */ - CONF_modules_unload(1); +#ifdef HAVE_FIPS + FIPS_mode_set(0); +#endif + /* Corresponds to OBJ_create()s */ + OBJ_cleanup(); + /* Corresponds to OPENSSL_load_builtin_modules() */ + CONF_modules_free(); /* Corresponds to SSL_library_init: */ EVP_cleanup(); #if HAVE_ENGINE_LOAD_BUILTIN_ENGINES ENGINE_cleanup(); #endif +#if OPENSSL_VERSION_NUMBER >= 0x1000200fL + SSL_COMP_free_compression_methods(); +#endif + + /* Usually needed per thread, but this parent process is single-threaded */ #if OPENSSL_VERSION_NUMBER < 0x10100000L #if OPENSSL_VERSION_NUMBER >= 0x1000000fL ERR_remove_thread_state(NULL); @@ -327,11 +349,14 @@ static apr_status_t ssl_cleanup_pre_config(void *data) ERR_free_strings(); #endif - /* Also don't call CRYPTO_cleanup_all_ex_data here; any registered - * ex_data indices may have been cached in static variables in - * OpenSSL; removing them may cause havoc. Notably, with OpenSSL + /* Also don't call CRYPTO_cleanup_all_ex_data when linked statically here; + * any registered ex_data indices may have been cached in static variables + * in OpenSSL; removing them may cause havoc. Notably, with OpenSSL * versions >= 0.9.8f, COMP_CTX cleanups would not be run, which * could result in a per-connection memory leak (!). */ + if (!modssl_running_statically) { + CRYPTO_cleanup_all_ex_data(); + } /* * TODO: determine somewhere we can safely shove out diagnostics @@ -345,6 +370,15 @@ static int ssl_hook_pre_config(apr_pool_t *pconf, apr_pool_t *plog, apr_pool_t *ptemp) { + modssl_running_statically = modssl_is_prelinked(); + + /* Some OpenSSL internals are allocated per-thread, make sure they + * are associated to the/our same thread-id until cleaned up. + */ +#if APR_HAS_THREADS && OPENSSL_VERSION_NUMBER < 0x10100000L + ssl_util_thread_id_setup(pconf); +#endif + /* We must register the library in full, to ensure our configuration * code can successfully test the SSL environment. */ @@ -367,6 +401,9 @@ static int ssl_hook_pre_config(apr_pool_t *pconf, "SRVName otherName form"); } + /* Start w/o errors (e.g. OBJ_txt2nid() above) */ + ERR_clear_error(); + /* * Let us cleanup the ssl library when the module is unloaded */ diff --git a/modules/ssl/ssl_engine_config.c b/modules/ssl/ssl_engine_config.c index 129a01ff54..392c9ac7d9 100644 --- a/modules/ssl/ssl_engine_config.c +++ b/modules/ssl/ssl_engine_config.c @@ -98,6 +98,14 @@ BOOL ssl_config_global_isfixed(SSLModConfigRec *mc) ** _________________________________________________________________ */ +#ifdef HAVE_SSL_CONF_CMD +static apr_status_t modssl_ctx_config_cleanup(void *ctx) +{ + SSL_CONF_CTX_free(ctx); + return APR_SUCCESS; +} +#endif + static void modssl_ctx_init(modssl_ctx_t *mctx, apr_pool_t *p) { mctx->sc = NULL; /* set during module init */ @@ -157,6 +165,9 @@ static void modssl_ctx_init(modssl_ctx_t *mctx, apr_pool_t *p) #endif #ifdef HAVE_SSL_CONF_CMD mctx->ssl_ctx_config = SSL_CONF_CTX_new(); + apr_pool_cleanup_register(p, mctx->ssl_ctx_config, + modssl_ctx_config_cleanup, + apr_pool_cleanup_null); SSL_CONF_CTX_set_flags(mctx->ssl_ctx_config, SSL_CONF_FLAG_FILE); SSL_CONF_CTX_set_flags(mctx->ssl_ctx_config, SSL_CONF_FLAG_SERVER); SSL_CONF_CTX_set_flags(mctx->ssl_ctx_config, SSL_CONF_FLAG_CERTIFICATE); diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index e60ac30434..5683c552d0 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -257,11 +257,9 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, #endif } -#if OPENSSL_VERSION_NUMBER < 0x10100000L -#if APR_HAS_THREADS +#if APR_HAS_THREADS && OPENSSL_VERSION_NUMBER < 0x10100000L ssl_util_thread_setup(p); #endif -#endif /* #if OPENSSL_VERSION_NUMBER < 0x10100000L */ /* * SSL external crypto device ("engine") support @@ -1641,7 +1639,6 @@ static apr_status_t ssl_init_server_ctx(server_rec *s, ssl_log_ssl_error(SSLLOG_MARK, APLOG_EMERG, s); return ssl_die(s); } - SSL_CONF_CTX_free(cctx); #endif if (SSL_CTX_check_private_key(sc->server->ssl_ctx) != 1) { diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index 08de3a1bf4..b6483a127f 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -922,9 +922,12 @@ void ssl_util_ppclose(server_rec *, apr_pool_t *, apr_file_t *); char *ssl_util_readfilter(server_rec *, apr_pool_t *, const char *, const char * const *); BOOL ssl_util_path_check(ssl_pathcheck_t, const char *, apr_pool_t *); +#if APR_HAS_THREADS #if OPENSSL_VERSION_NUMBER < 0x10100000L void ssl_util_thread_setup(apr_pool_t *); #endif +void ssl_util_thread_id_setup(apr_pool_t *); +#endif int ssl_init_ssl_connection(conn_rec *c, request_rec *r); BOOL ssl_util_vhost_matches(const char *servername, server_rec *s); diff --git a/modules/ssl/ssl_util.c b/modules/ssl/ssl_util.c index 052d23e822..9e4e719f4b 100644 --- a/modules/ssl/ssl_util.c +++ b/modules/ssl/ssl_util.c @@ -383,6 +383,12 @@ static void ssl_util_thr_id(CRYPTO_THREADID *id) #endif } +static apr_status_t ssl_util_thr_id_cleanup(void *old) +{ + CRYPTO_THREADID_set_callback(old); + return APR_SUCCESS; +} + #else static unsigned long ssl_util_thr_id(void) @@ -403,16 +409,17 @@ static unsigned long ssl_util_thr_id(void) #endif } +static apr_status_t ssl_util_thr_id_cleanup(void *old) +{ + CRYPTO_set_id_callback(old); + return APR_SUCCESS; +} + #endif static apr_status_t ssl_util_thread_cleanup(void *data) { CRYPTO_set_locking_callback(NULL); -#if OPENSSL_VERSION_NUMBER >= 0x10000000L - CRYPTO_THREADID_set_callback(NULL); -#else - CRYPTO_set_id_callback(NULL); -#endif CRYPTO_set_dynlock_create_callback(NULL); CRYPTO_set_dynlock_lock_callback(NULL); @@ -436,12 +443,6 @@ void ssl_util_thread_setup(apr_pool_t *p) apr_thread_mutex_create(&(lock_cs[i]), APR_THREAD_MUTEX_DEFAULT, p); } -#if OPENSSL_VERSION_NUMBER >= 0x10000000L - CRYPTO_THREADID_set_callback(ssl_util_thr_id); -#else - CRYPTO_set_id_callback(ssl_util_thr_id); -#endif - CRYPTO_set_locking_callback(ssl_util_thr_lock); /* Set up dynamic locking scaffolding for OpenSSL to use at its @@ -455,5 +456,16 @@ void ssl_util_thread_setup(apr_pool_t *p) apr_pool_cleanup_register(p, NULL, ssl_util_thread_cleanup, apr_pool_cleanup_null); } + +void ssl_util_thread_id_setup(apr_pool_t *p) +{ +#if OPENSSL_VERSION_NUMBER >= 0x10000000L + CRYPTO_THREADID_set_callback(ssl_util_thr_id); +#else + CRYPTO_set_id_callback(ssl_util_thr_id); +#endif + apr_pool_cleanup_register(p, NULL, ssl_util_thr_id_cleanup, + apr_pool_cleanup_null); +} #endif /* #if OPENSSL_VERSION_NUMBER < 0x10100000L */ #endif /* #if APR_HAS_THREADS */