]> granicus.if.org Git - apache/commitdiff
Merge r1597349,1598107,1603915,1605827,1605829 from trunk:
authorJeff Trawick <trawick@apache.org>
Sat, 12 Jul 2014 18:08:09 +0000 (18:08 +0000)
committerJeff Trawick <trawick@apache.org>
Sat, 12 Jul 2014 18:08:09 +0000 (18:08 +0000)
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
STATUS
docs/manual/mod/mod_ssl.xml
modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c
modules/ssl/ssl_private.h

diff --git a/CHANGES b/CHANGES
index c001481b6de901cbdecb6975948023ae93a4449d..8f7a52ac4a4f6c189f9cf0552eb94e9f713b5ee8 100644 (file)
--- 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 4e935f519c1dc46578927fc8f47054e7f55cfb20..bc784975a5fa7fa237bafb17d9b2d298f6a8b9d5 100644 (file)
--- 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
index 0ee747eab58875c79ddf6c620195daecf004d8de..799e77404fb5cb1b41c4731f6122534e34a1b1d0 100644 (file)
@@ -850,6 +850,8 @@ are applied independently of the authentication algorithm type.</p>
 <p>
 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 <a href="http://www.ietf.org/rfc/rfc3526.txt">RFC 3526</a>), 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
index 70930ea740f847e6ad445c34d628ce7330246f12..d47317e63328fba2c4b413eedc124b9e0f47d52d 100644 (file)
 #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 = 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;
 }
index 1b83520f16c56c3d930857c2d8bc8568b27e5d33..a6ca7cc041de1656268e7c75ba2dc4d83192070a 100644 (file)
@@ -1297,34 +1297,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.
- */
-#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);
 }
 
 /*
index 101ac40663e95eaa5de77963e5646b195ce48b85..e8e2cff08b0bad231c7094351b9a8369e61c0367 100644 (file)
@@ -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 */
 /** @} */