[tor-commits] [tor/master] Refactor the dependency between tortls and crypto_dh.

[nickm at torproject.org]

commit 32bbc8f6b5e7d964439c6ba91f87f7d5fba673cc
Author: Nick Mathewson <nickm at torproject.org>
Date:   Tue Jul 17 11:23:53 2018 -0400

    Refactor the dependency between tortls and crypto_dh.
    
    We only ever need this to get us a DH ephemeral key object,
    so make a function that does just that.
---
 src/lib/crypt_ops/crypto_dh.h         |  3 +-
 src/lib/crypt_ops/crypto_dh_openssl.c | 73 +++++++++++++++++++----------------
 src/lib/tls/tortls.c                  |  6 +--
 3 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/src/lib/crypt_ops/crypto_dh.h b/src/lib/crypt_ops/crypto_dh.h
index f8e4e4f43..953362696 100644
--- a/src/lib/crypt_ops/crypto_dh.h
+++ b/src/lib/crypt_ops/crypto_dh.h
@@ -50,7 +50,6 @@ void crypto_dh_free_all(void);
 /* Prototypes for private functions only used by tortls.c, crypto.c, and the
  * unit tests. */
 struct dh_st;
-struct dh_st *crypto_dh_get_dh_(crypto_dh_t *dh);
-
+struct dh_st *crypto_dh_new_openssl_tls(void);
 
 #endif /* !defined(TOR_CRYPTO_DH_H) */
diff --git a/src/lib/crypt_ops/crypto_dh_openssl.c b/src/lib/crypt_ops/crypto_dh_openssl.c
index 395058d92..d66031afd 100644
--- a/src/lib/crypt_ops/crypto_dh_openssl.c
+++ b/src/lib/crypt_ops/crypto_dh_openssl.c
@@ -27,6 +27,7 @@ ENABLE_GCC_WARNING(redundant-decls)
 #include <string.h>
 
 static int tor_check_dh_key(int severity, const BIGNUM *bn);
+static DH *new_openssl_dh_from_params(BIGNUM *p, BIGNUM *g);
 
 /** A structure to hold the first half (x, g^x) of a Diffie-Hellman handshake
  * while we're waiting for the second.*/
@@ -34,14 +35,6 @@ struct crypto_dh_t {
   DH *dh; /**< The openssl DH object */
 };
 
-/** Used by tortls.c: Get the DH* from a crypto_dh_t.
- */
-DH *
-crypto_dh_get_dh_(crypto_dh_t *dh)
-{
-  return dh->dh;
-}
-
 /** Shared P parameter for our circuit-crypto DH key exchanges. */
 static BIGNUM *dh_param_p = NULL;
 /** Shared P parameter for our TLS DH key exchanges. */
@@ -188,6 +181,14 @@ init_dh_param(void)
  */
 #define DH_PRIVATE_KEY_BITS 320
 
+/** Used by tortls.c: Get the DH* for use with TLS.
+ */
+DH *
+crypto_dh_new_openssl_tls(void)
+{
+  return new_openssl_dh_from_params(dh_param_p_tls, dh_param_g);
+}
+
 /** Allocate and return a new DH object for a key exchange. Returns NULL on
  * failure.
  */
@@ -202,55 +203,59 @@ crypto_dh_new(int dh_type)
   if (!dh_param_p)
     init_dh_param();
 
-  if (!(res->dh = DH_new()))
-    goto err;
-
-#ifdef OPENSSL_1_1_API
-  BIGNUM *dh_p = NULL, *dh_g = NULL;
-
+  BIGNUM *dh_p = NULL;
   if (dh_type == DH_TYPE_TLS) {
-    dh_p = BN_dup(dh_param_p_tls);
+    dh_p = dh_param_p_tls;
   } else {
-    dh_p = BN_dup(dh_param_p);
+    dh_p = dh_param_p;
   }
+
+  res->dh = new_openssl_dh_from_params(dh_p, dh_param_g);
+  if (res->dh == NULL)
+    tor_free(res); // sets res to NULL.
+  return res;
+}
+
+/** Create and return a new openssl DH from a given prime and generator. */
+static DH *
+new_openssl_dh_from_params(BIGNUM *p, BIGNUM *g)
+{
+  DH *res_dh;
+  if (!(res_dh = DH_new()))
+    goto err;
+
+  BIGNUM *dh_p = NULL, *dh_g = NULL;
+  dh_p = BN_dup(p);
   if (!dh_p)
     goto err;
 
-  dh_g = BN_dup(dh_param_g);
+  dh_g = BN_dup(g);
   if (!dh_g) {
     BN_free(dh_p);
     goto err;
   }
 
-  if (!DH_set0_pqg(res->dh, dh_p, NULL, dh_g)) {
-    goto err;
-  }
+#ifdef OPENSSL_1_1_API
 
-  if (!DH_set_length(res->dh, DH_PRIVATE_KEY_BITS))
+  if (!DH_set0_pqg(res_dh, dh_p, NULL, dh_g)) {
     goto err;
-#else /* !(defined(OPENSSL_1_1_API)) */
-  if (dh_type == DH_TYPE_TLS) {
-    if (!(res->dh->p = BN_dup(dh_param_p_tls)))
-      goto err;
-  } else {
-    if (!(res->dh->p = BN_dup(dh_param_p)))
-      goto err;
   }
 
-  if (!(res->dh->g = BN_dup(dh_param_g)))
+  if (!DH_set_length(res_dh, DH_PRIVATE_KEY_BITS))
     goto err;
-
-  res->dh->length = DH_PRIVATE_KEY_BITS;
+#else /* !(defined(OPENSSL_1_1_API)) */
+  res_dh->p = dh_p;
+  res_dh->g = dh_g;
+  res_dh->length = DH_PRIVATE_KEY_BITS;
 #endif /* defined(OPENSSL_1_1_API) */
 
-  return res;
+  return res_dh;
 
   /* LCOV_EXCL_START
    * This error condition is only reached when an allocation fails */
  err:
   crypto_openssl_log_errors(LOG_WARN, "creating DH object");
-  if (res->dh) DH_free(res->dh); /* frees p and g too */
-  tor_free(res);
+  if (res_dh) DH_free(res_dh); /* frees p and g too */
   return NULL;
   /* LCOV_EXCL_STOP */
 }
diff --git a/src/lib/tls/tortls.c b/src/lib/tls/tortls.c
index 8d284dde1..875ed95f8 100644
--- a/src/lib/tls/tortls.c
+++ b/src/lib/tls/tortls.c
@@ -1280,10 +1280,10 @@ tor_tls_context_new(crypto_pk_t *identity, unsigned int key_lifetime,
       goto error;
   }
   {
-    crypto_dh_t *dh = crypto_dh_new(DH_TYPE_TLS);
+    DH *dh = crypto_dh_new_openssl_tls();
     tor_assert(dh);
-    SSL_CTX_set_tmp_dh(result->ctx, crypto_dh_get_dh_(dh));
-    crypto_dh_free(dh);
+    SSL_CTX_set_tmp_dh(result->ctx, dh);
+    DH_free(dh);
   }
   if (! is_client) {
     int nid;