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: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ee0789f9b70a9976dc3a5cb7077de6bff6414808;p=thirdparty%2Fapache%2Fhttpd.git 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 b5bda84fa7d..b4761769132 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 1fe109d1fda..21b19e4f030 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 bf9edb28d8c..f58e145f193 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 129a01ff545..392c9ac7d94 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 e60ac304346..5683c552d06 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 08de3a1bf43..b6483a127f1 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 052d23e8220..9e4e719f4bc 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 */