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

@@ -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 <openssl/bn.h> 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;
 }

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 <openssl/bn.h>
- * (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 modssl_get_dh_params(keylen);
-        return get_dh4096();
-    else if (keylen >= 3072)
-        return get_dh3072();
-    else if (keylen >= 2048)
-        return get_dh2048();
-    else
-        return get_dh1024();
 }
 
 /*

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