From 727153b8fe4ddca848bdda6c83045de3fb38f273 Mon Sep 17 00:00:00 2001 From: Jeff Trawick Date: Sat, 12 Jul 2014 18:08:09 +0000 Subject: [PATCH] Merge r1597349,1598107,1603915,1605827,1605829 from trunk: mod_ssl: Fix tmp DH parameter leak, adjust selection to prefer larger keys and support up to 8192-bit keys. Submitted by: rpluem, jorton Reviewed by: ylavic, kbrand git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1610014 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 4 ++ STATUS | 10 ----- docs/manual/mod/mod_ssl.xml | 2 + modules/ssl/ssl_engine_init.c | 77 +++++++++++++++++++++++++++++++++ modules/ssl/ssl_engine_kernel.c | 37 +--------------- modules/ssl/ssl_private.h | 5 +++ 6 files changed, 89 insertions(+), 46 deletions(-) diff --git a/CHANGES b/CHANGES index c001481b6d..8f7a52ac4a 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,10 @@ Changes with Apache 2.4.10 + *) mod_ssl: Fix tmp DH parameter leak, adjust selection to prefer + larger keys and support up to 8192-bit keys. [Ruediger Pluem, + Joe Orton] + *) mod_dav: Fix improper encoding in PROPFIND responses. PR 56480. [Ben Reser] diff --git a/STATUS b/STATUS index 4e935f519c..bc784975a5 100644 --- a/STATUS +++ b/STATUS @@ -177,16 +177,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: 2.4.x patch: http://people.apache.org/~covener/patches/httpd-2.4.x-ldap-connttl-conservative.diff +1 covener, jim - * mod_ssl: Fix tmp DH parameter leak, adjust selection to prefer - larger keys and support up to 8192-bit keys. - trunk patch: http://svn.apache.org/r1597349 - http://svn.apache.org/r1598107 - http://svn.apache.org/r1603915 - http://svn.apache.org/r1605827 - 2.4.x patch: http://people.apache.org/~jorton/ms_tmpdh-2.4.x.diff - +1: jorton, ylavic, kbrand - kbrand: please backport the doc change (r1605829), too - * mod_ssl: Fix issue with redirects to error documents when handling SNI errors. trunk patch: http://svn.apache.org/r1609914 diff --git a/docs/manual/mod/mod_ssl.xml b/docs/manual/mod/mod_ssl.xml index 0ee747eab5..799e77404f 100644 --- a/docs/manual/mod/mod_ssl.xml +++ b/docs/manual/mod/mod_ssl.xml @@ -850,6 +850,8 @@ are applied independently of the authentication algorithm type.

Beginning with version 2.4.7, mod_ssl makes use of standardized DH parameters with prime lengths of 2048, 3072 and 4096 bits +and with additional prime lengths of 6144 and 8192 bits beginning with +version 2.4.10 (from RFC 3526), and hands them out to clients based on the length of the certificate's RSA/DSA key. With Java-based clients in particular (Java 7 or earlier), this may lead diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c index 70930ea740..d47317e633 100644 --- a/modules/ssl/ssl_engine_init.c +++ b/modules/ssl/ssl_engine_init.c @@ -41,6 +41,79 @@ #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 = DH_new(); + + if (!dh) { + return NULL; + } + dh->p = prime(NULL); + BN_dec2bn(&dh->g, gen); + if (!dh->p || !dh->g) { + DH_free(dh); + return NULL; + } + return dh; +} + +/* Storage and initialization for DH parameters. */ +static struct dhparam { + BIGNUM *(*const prime)(BIGNUM *); /* function to generate... */ + DH *dh; /* ...this, used for keys.... */ + const unsigned int min; /* ...of length >= this. */ +} dhparams[] = { + { get_rfc3526_prime_8192, NULL, 6145 }, + { get_rfc3526_prime_6144, NULL, 4097 }, + { get_rfc3526_prime_4096, NULL, 3073 }, + { get_rfc3526_prime_3072, NULL, 2049 }, + { get_rfc3526_prime_2048, NULL, 1025 }, + { get_rfc2409_prime_1024, NULL, 0 } +}; + +static void init_dh_params(void) +{ + unsigned n; + + for (n = 0; n < sizeof(dhparams)/sizeof(dhparams[0]); n++) + dhparams[n].dh = make_dh_params(dhparams[n].prime, "2"); +} + +static void free_dh_params(void) +{ + unsigned n; + + /* DH_free() is a noop for a NULL parameter, so these are harmless + * in the (unexpected) case where these variables are already + * NULL. */ + for (n = 0; n < sizeof(dhparams)/sizeof(dhparams[0]); n++) { + DH_free(dhparams[n].dh); + dhparams[n].dh = 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) +{ + unsigned n; + + for (n = 0; n < sizeof(dhparams)/sizeof(dhparams[0]); n++) + if (keylen >= dhparams[n].min) + return dhparams[n].dh; + + return NULL; /* impossible to reach. */ +} + static void ssl_add_version_components(apr_pool_t *p, server_rec *s) { @@ -256,6 +329,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; } @@ -1681,5 +1756,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 1b83520f16..a6ca7cc041 100644 --- a/modules/ssl/ssl_engine_kernel.c +++ b/modules/ssl/ssl_engine_kernel.c @@ -1297,34 +1297,6 @@ const authz_provider ssl_authz_provider_verify_client = ** _________________________________________________________________ */ -/* - * Grab well-defined DH parameters from OpenSSL, see - * (get_rfc*) for all available primes. - */ -#define make_get_dh(rfc,size,gen) \ -static DH *get_dh##size(void) \ -{ \ - DH *dh; \ - if (!(dh = DH_new())) { \ - return NULL; \ - } \ - dh->p = get_##rfc##_prime_##size(NULL); \ - BN_dec2bn(&dh->g, #gen); \ - if (!dh->p || !dh->g) { \ - DH_free(dh); \ - return NULL; \ - } \ - 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 */ @@ -1364,14 +1336,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 101ac40663..e8e2cff08b 100644 --- a/modules/ssl/ssl_private.h +++ b/modules/ssl/ssl_private.h @@ -933,6 +933,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); + #endif /* SSL_PRIVATE_H */ /** @} */ -- 2.50.1