From d3eac27e32ff73c711856343033d089a25b58ea4 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Wed, 28 May 2014 19:14:28 +0000 Subject: [PATCH] Create DH parameters from OpenSSL at module init, avoiding (very minor) race and leaks: * modules/ssl/ssl_engine_init.c (make_dh_params): Moved/rejigged variant of make_get_dh() macro. (init_dh_params, free_dh_params): New functions. (modssl_get_dh_params): Split out from ssl_callback_TmpDH. (ssl_init_Module, ssl_init_ModuleKill): Use new init_/free_. * modules/ssl/ssl_engine_kernel.c: Moved out DH parameter handling. (ssl_callback_TmpDH): Use modssl_get_dh_params. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1598107 13f79535-47bb-0310-9956-ffa450edef68 --- modules/ssl/ssl_engine_init.c | 70 +++++++++++++++++++++++++++++++++ modules/ssl/ssl_engine_kernel.c | 50 +---------------------- modules/ssl/ssl_private.h | 5 +++ 3 files changed, 76 insertions(+), 49 deletions(-) diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index e4f234630e..592df604e2 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -47,6 +47,72 @@ APR_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(ssl, SSL, int, init_server, #define KEYTYPES "RSA or DSA" #endif +/* + * Grab well-defined DH parameters from OpenSSL, see the get_rfc* + * functions in for all available primes. + */ +static DH *make_dh_params(BIGNUM *(*prime)(BIGNUM *), const char *gen) +{ + DH *dh = NULL; + DH *dh_tmp; + + if (dh) { + return dh; + } + if (!(dh_tmp = DH_new())) { + return NULL; + } + dh_tmp->p = prime(NULL); + BN_dec2bn(&dh_tmp->g, gen); + if (!dh_tmp->p || !dh_tmp->g) { + DH_free(dh_tmp); + return NULL; + } + dh = dh_tmp; + return dh; +} + +static DH *dhparam1024, *dhparam2048, *dhparam3072, *dhparam4096; + +static void init_dh_params(void) +{ + /* + * Prepare DH parameters from 1024 to 4096 bits, in 1024-bit increments + */ + dhparam1024 = make_dh_params(get_rfc2409_prime_1024, "2"); + dhparam2048 = make_dh_params(get_rfc3526_prime_2048, "2"); + dhparam3072 = make_dh_params(get_rfc3526_prime_3072, "2"); + dhparam4096 = make_dh_params(get_rfc3526_prime_4096, "2"); +} + +static void free_dh_params(void) +{ + DH_free(dhparam1024); + DH_free(dhparam2048); + DH_free(dhparam3072); + DH_free(dhparam4096); + dhparam1024 = dhparam2048 = dhparam3072 = dhparam4096 = NULL; +} + +/* Hand out the same DH structure though once generated as we leak + * memory otherwise and freeing the structure up after use would be + * hard to track and in fact is not needed at all as it is safe to + * use the same parameters over and over again security wise (in + * contrast to the keys itself) and code safe as the returned structure + * is duplicated by OpenSSL anyway. Hence no modification happens + * to our copy. */ +DH *modssl_get_dh_params(unsigned keylen) +{ + if (keylen >= 4096) + return dhparam4096; + else if (keylen >= 3072) + return dhparam3072; + else if (keylen >= 2048) + return dhparam2048; + else + return dhparam1024; +} + static void ssl_add_version_components(apr_pool_t *p, server_rec *s) { @@ -277,6 +343,8 @@ apr_status_t ssl_init_Module(apr_pool_t *p, apr_pool_t *plog, SSL_init_app_data2_idx(); /* for SSL_get_app_data2() at request time */ + init_dh_params(); + return OK; } @@ -1706,5 +1774,7 @@ apr_status_t ssl_init_ModuleKill(void *data) ssl_init_ctx_cleanup(sc->server); } + free_dh_params(); + return APR_SUCCESS; } diff --git a/modules/ssl/ssl_engine_kernel.c b/modules/ssl/ssl_engine_kernel.c index 475018bd8f..3587f9b6dd 100644 --- a/modules/ssl/ssl_engine_kernel.c +++ b/modules/ssl/ssl_engine_kernel.c @@ -1310,47 +1310,6 @@ const authz_provider ssl_authz_provider_verify_client = ** _________________________________________________________________ */ -/* - * Grab well-defined DH parameters from OpenSSL, see - * (get_rfc*) for all available primes. - * Hand out the same DH structure though once generated as we leak - * memory otherwise and freeing the structure up after use would be - * hard to track and in fact is not needed at all as it is safe to - * use the same parameters over and over again security wise (in - * contrast to the keys itself) and code safe as the returned structure - * is duplicated by OpenSSL anyway. Hence no modification happens - * to our copy. - */ -#define make_get_dh(rfc,size,gen) \ -static DH *get_dh##size(void) \ -{ \ - static DH *dh = NULL; \ - DH *dh_tmp; \ -\ - if (dh) { \ - return dh; \ - } \ - if (!(dh_tmp = DH_new())) { \ - return NULL; \ - } \ - dh_tmp->p = get_##rfc##_prime_##size(NULL); \ - BN_dec2bn(&dh_tmp->g, #gen); \ - if (!dh_tmp->p || !dh_tmp->g) { \ - DH_free(dh_tmp); \ - return NULL; \ - } \ - dh = dh_tmp; \ - return dh; \ -} - -/* - * Prepare DH parameters from 1024 to 4096 bits, in 1024-bit increments - */ -make_get_dh(rfc2409, 1024, 2) -make_get_dh(rfc3526, 2048, 2) -make_get_dh(rfc3526, 3072, 2) -make_get_dh(rfc3526, 4096, 2) - /* * Hand out standard DH parameters, based on the authentication strength */ @@ -1390,14 +1349,7 @@ DH *ssl_callback_TmpDH(SSL *ssl, int export, int keylen) ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, "handing out built-in DH parameters for %d-bit authenticated connection", keylen); - if (keylen >= 4096) - return get_dh4096(); - else if (keylen >= 3072) - return get_dh3072(); - else if (keylen >= 2048) - return get_dh2048(); - else - return get_dh1024(); + return modssl_get_dh_params(keylen); } /* diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h index 797e0f47ec..a0100d0387 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -931,6 +931,11 @@ OCSP_RESPONSE *modssl_dispatch_ocsp_request(const apr_uri_t *uri, conn_rec *c, apr_pool_t *p); #endif +/* Retrieve DH parameters for given key length. Return value should + * be treated as unmutable, since it is stored in process-global + * memory. */ +DH *modssl_get_dh_params(unsigned keylen); + #if HAVE_VALGRIND extern int ssl_running_on_valgrind; #endif -- 2.40.0