From 9f226a5f5183e8814ff30e7436a128a823315532 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben@chromium.org>
Date: Sun, 26 Apr 2015 22:12:32 -0400
Subject: [PATCH] Always set SSL_OP_SINGLE_DH_USE.

This is an API wart that makes it easy to accidentally reuse the server
DHE half for every handshake. It's much simpler to have only one mode.
This mirrors the change made to the ECDHE code; align with that logic.

Change-Id: I47cccbb354d70127ab458f99a6d390b213e4e515
Reviewed-on: https://boringssl-review.googlesource.com/4565
Reviewed-by: Adam Langley <agl@google.com>
---
 include/openssl/ssl.h |  2 +-
 ssl/s3_lib.c          | 47 ++++++++++++-------------------------------
 ssl/s3_srvr.c         | 19 ++++-------------
 3 files changed, 18 insertions(+), 50 deletions(-)

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 90443bf8..27c7449e 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -467,7 +467,7 @@ typedef struct timeval OPENSSL_timeval;
 #define SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION 0x00040000L
 /* SSL_OP_SINGLE_ECDH_USE does nothing. */
 #define SSL_OP_SINGLE_ECDH_USE 0x00080000L
-/* If set, always create a new key when using tmp_dh parameters */
+/* SSL_OP_SINGLE_DH_USE does nothing. */
 #define SSL_OP_SINGLE_DH_USE 0x00100000L
 /* Set on servers to choose the cipher according to the server's preferences */
 #define SSL_OP_CIPHER_SERVER_PREFERENCE 0x00400000L
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index dbd0eb56..d31d69d0 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -607,27 +607,16 @@ long ssl3_ctrl(SSL *s, int cmd, long larg, void *parg) {
       OPENSSL_PUT_ERROR(SSL, ssl3_ctrl, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
       break;
 
-    case SSL_CTRL_SET_TMP_DH: {
-      DH *dh = (DH *)parg;
-      if (dh == NULL) {
-        OPENSSL_PUT_ERROR(SSL, ssl3_ctrl, ERR_R_PASSED_NULL_PARAMETER);
-        return ret;
-      }
-      dh = DHparams_dup(dh);
-      if (dh == NULL) {
-        OPENSSL_PUT_ERROR(SSL, ssl3_ctrl, ERR_R_DH_LIB);
-        return ret;
-      }
-      if (!(s->options & SSL_OP_SINGLE_DH_USE) && !DH_generate_key(dh)) {
-        DH_free(dh);
-        OPENSSL_PUT_ERROR(SSL, ssl3_ctrl, ERR_R_DH_LIB);
-        return ret;
-      }
+    case SSL_CTRL_SET_TMP_DH:
       DH_free(s->cert->dh_tmp);
-      s->cert->dh_tmp = dh;
+      s->cert->dh_tmp = DHparams_dup((DH *)parg);
+      if (s->cert->dh_tmp == NULL) {
+        OPENSSL_PUT_ERROR(SSL, ssl3_ctx_ctrl, ERR_R_DH_LIB);
+        ret = 0;
+        break;
+      }
       ret = 1;
       break;
-    }
 
     case SSL_CTRL_SET_TMP_ECDH: {
       /* For historical reasons, this API expects an |EC_KEY|, but only the
@@ -825,24 +814,14 @@ long ssl3_ctx_ctrl(SSL_CTX *ctx, int cmd, long larg, void *parg) {
       OPENSSL_PUT_ERROR(SSL, ssl3_ctx_ctrl, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
       return 0;
 
-    case SSL_CTRL_SET_TMP_DH: {
-      DH *new = NULL, *dh;
-
-      dh = (DH *)parg;
-      new = DHparams_dup(dh);
-      if (new == NULL) {
-        OPENSSL_PUT_ERROR(SSL, ssl3_ctx_ctrl, ERR_R_DH_LIB);
-        return 0;
-      }
-      if (!(ctx->options & SSL_OP_SINGLE_DH_USE) && !DH_generate_key(new)) {
-        OPENSSL_PUT_ERROR(SSL, ssl3_ctx_ctrl, ERR_R_DH_LIB);
-        DH_free(new);
-        return 0;
-      }
+    case SSL_CTRL_SET_TMP_DH:
       DH_free(cert->dh_tmp);
-      cert->dh_tmp = new;
+      cert->dh_tmp = DHparams_dup((DH *)parg);
+      if (cert->dh_tmp == NULL) {
+        OPENSSL_PUT_ERROR(SSL, ssl3_ctx_ctrl, ERR_R_DH_LIB);
+        return 0;
+      }
       return 1;
-    }
 
     case SSL_CTRL_SET_TMP_ECDH: {
       /* For historical reasons, this API expects an |EC_KEY|, but only the
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 8716f7c7..77286229 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -1377,27 +1377,16 @@ int ssl3_send_server_key_exchange(SSL *s) {
                           ERR_R_INTERNAL_ERROR);
         goto err;
       }
-
       dh = DHparams_dup(dhp);
       if (dh == NULL) {
         OPENSSL_PUT_ERROR(SSL, ssl3_send_server_key_exchange, ERR_R_DH_LIB);
         goto err;
       }
-
       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)) {
-          OPENSSL_PUT_ERROR(SSL, 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) {
-          OPENSSL_PUT_ERROR(SSL, ssl3_send_server_key_exchange, ERR_R_DH_LIB);
-          goto err;
-        }
+
+      if (!DH_generate_key(dh)) {
+        OPENSSL_PUT_ERROR(SSL, ssl3_send_server_key_exchange, ERR_R_DH_LIB);
+        goto err;
       }
 
       r[0] = dh->p;