Always generate DH keys for ephemeral DH cipher suites

Modified version of the commit ffaef3f15 in the master branch by Stephen
Henson. This makes the SSL_OP_SINGLE_DH_USE option a no-op and always
generates a new DH key for every handshake regardless.

CVE-2016-0701 (fix part 2 or 2)

Issue reported by Antonio Sanso

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>

diff --git a/doc/ssl/SSL_CTX_set_tmp_dh_callback.pod b/doc/ssl/SSL_CTX_set_tmp_dh_callback.pod
index b754c16a86e66eaeab66df876000ffd760dd7c70..234fbc845002f33bf188f1e0aff5c8b1975c01bf 100644 (file)
--- a/doc/ssl/SSL_CTX_set_tmp_dh_callback.pod
+++ b/doc/ssl/SSL_CTX_set_tmp_dh_callback.pod
@@ -48,25 +48,8 @@ even if he gets hold of the normal (certified) key, as this key was
 only used for signing.
 
 In order to perform a DH key exchange the server must use a DH group
-(DH parameters) and generate a DH key.
-The server will always generate a new DH key during the negotiation
-if either the DH parameters are supplied via callback or the
-SSL_OP_SINGLE_DH_USE option of SSL_CTX_set_options(3) is set (or both).
-It will  immediately create a DH key if DH parameters are supplied via
-SSL_CTX_set_tmp_dh() and SSL_OP_SINGLE_DH_USE is not set.
-In this case,
-it may happen that a key is generated on initialization without later
-being needed, while on the other hand the computer time during the
-negotiation is being saved.
-
-If "strong" primes were used to generate the DH parameters, it is not strictly
-necessary to generate a new key for each handshake but it does improve forward
-secrecy. If it is not assured that "strong" primes were used,
-SSL_OP_SINGLE_DH_USE must be used in order to prevent small subgroup
-attacks. Always using SSL_OP_SINGLE_DH_USE has an impact on the
-computer time needed during negotiation, but it is not very large, so
-application authors/users should consider always enabling this option.
-The option is required to implement perfect forward secrecy (PFS).
+(DH parameters) and generate a DH key. The server will always generate
+a new DH key during the negotiation.
 
 As generating DH parameters is extremely time consuming, an application
 should not generate the parameters on the fly but supply the parameters.
@@ -93,10 +76,9 @@ can supply the DH parameters via a callback function.
 Previous versions of the callback used B<is_export> and B<keylength>
 parameters to control parameter generation for export and non-export
 cipher suites. Modern servers that do not support export ciphersuites
-are advised to either use SSL_CTX_set_tmp_dh() in combination with
-SSL_OP_SINGLE_DH_USE, or alternatively, use the callback but ignore
-B<keylength> and B<is_export> and simply supply at least 2048-bit
-parameters in the callback.
+are advised to either use SSL_CTX_set_tmp_dh() or alternatively, use
+the callback but ignore B<keylength> and B<is_export> and simply
+supply at least 2048-bit parameters in the callback.
 
 =head1 EXAMPLES
 
@@ -128,7 +110,6 @@ partly left out.)
  if (SSL_CTX_set_tmp_dh(ctx, dh_2048) != 1) {
    /* Error. */
  }
- SSL_CTX_set_options(ctx, SSL_OP_SINGLE_DH_USE);
  ...
 
 =head1 RETURN VALUES

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index b7c5db3aa5e009633830044ff4927ff4128b4fa2..f846cb5b7b01994b77c21b2972ab70bbaa4b662b 100644 (file)
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3206,13 +3206,6 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg)
                 SSLerr(SSL_F_SSL3_CTRL, ERR_R_DH_LIB);
                 return (ret);
             }
-            if (!(s->options & SSL_OP_SINGLE_DH_USE)) {
-                if (!DH_generate_key(dh)) {
-                    DH_free(dh);
-                    SSLerr(SSL_F_SSL3_CTRL, ERR_R_DH_LIB);
-                    return (ret);
-                }
-            }
             if (s->cert->dh_tmp != NULL)
                 DH_free(s->cert->dh_tmp);
             s->cert->dh_tmp = dh;
@@ -3713,13 +3706,6 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg)
                 SSLerr(SSL_F_SSL3_CTX_CTRL, ERR_R_DH_LIB);
                 return 0;
             }
-            if (!(ctx->options & SSL_OP_SINGLE_DH_USE)) {
-                if (!DH_generate_key(new)) {
-                    SSLerr(SSL_F_SSL3_CTX_CTRL, ERR_R_DH_LIB);
-                    DH_free(new);
-                    return 0;
-                }
-            }
             if (cert->dh_tmp != NULL)
                 DH_free(cert->dh_tmp);
             cert->dh_tmp = new;

diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 9b05f189d52a66005d7de0d7f3c54450d63e0213..ab28702ee972dac73060a4f5d0098d918b47c0b1 100644 (file)
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -1687,20 +1687,9 @@ int ssl3_send_server_key_exchange(SSL *s)
             }
 
             s->s3->tmp.dh = dh;
-            if ((dhp->pub_key == NULL ||
-                 dhp->priv_key == NULL ||
-                 (s->options & SSL_OP_SINGLE_DH_USE))) {
-                if (!DH_generate_key(dh)) {
-                    SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE, ERR_R_DH_LIB);
-                    goto err;
-                }
-            } else {
-                dh->pub_key = BN_dup(dhp->pub_key);
-                dh->priv_key = BN_dup(dhp->priv_key);
-                if ((dh->pub_key == NULL) || (dh->priv_key == NULL)) {
-                    SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE, ERR_R_DH_LIB);
-                    goto err;
-                }
+            if (!DH_generate_key(dh)) {
+                SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE, ERR_R_DH_LIB);
+                goto err;
             }
             r[0] = dh->p;
             r[1] = dh->g;

diff --git a/ssl/ssl.h b/ssl/ssl.h
index a31c08571148a2f5ecd51d582a8d88fbccb07ad4..ae8c92575e03e028e3deb2f78ddf783a9cf1cd37 100644 (file)
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -625,7 +625,7 @@ struct ssl_session_st {
 # define SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION        0x00040000L
 /* If set, always create a new key when using tmp_ecdh parameters */
 # define SSL_OP_SINGLE_ECDH_USE                          0x00080000L
-/* If set, always create a new key when using tmp_dh parameters */
+/* Does nothing: retained for compatibility */
 # define SSL_OP_SINGLE_DH_USE                            0x00100000L
 /* Does nothing: retained for compatibiity */
 # define SSL_OP_EPHEMERAL_RSA                            0x0