Change our TLS finite-field diffie-hellman group to ffdhe2048.

(We should never actually _use_ finite-field Diffie-Hellman
with TLS.)

diff --git a/changes/ticket41067 b/changes/ticket41067
index d72442b8ced29a04b7d0b9f02e87ea12c410f596..c018d728b5cc7ab3015975f62ea20c7c85f00357 100644 (file)
--- a/changes/ticket41067
+++ b/changes/ticket41067
@@ -3,3 +3,6 @@
       be required in the near future.)  Part of ticket 41067.
     - Update TLS 1.2 client cipher list to match current Firefox.
       Part of ticket 41067.
+    - Increase the size of our finite-field Diffie Hellman TLS group
+      (which we should never actually use!) to 2048 bits.
+      Part of ticket 41067.

diff --git a/src/lib/crypt_ops/crypto_dh.c b/src/lib/crypt_ops/crypto_dh.c
index d0805d834d7e483373baedfaaa91e8bcc02a758b..2a273e14e61ec277c3c2b1bf982dee54f54d549b 100644 (file)
--- a/src/lib/crypt_ops/crypto_dh.c
+++ b/src/lib/crypt_ops/crypto_dh.c
@@ -21,16 +21,20 @@
 
 /** Our DH 'g' parameter */
 const unsigned DH_GENERATOR = 2;
-/** This is the 1024-bit safe prime that Apache uses for its DH stuff; see
- * modules/ssl/ssl_engine_dh.c; Apache also uses a generator of 2 with this
- * prime.
+/** This is ffdhe2048 from RFC 7919.
  */
 const char TLS_DH_PRIME[] =
-  "D67DE440CBBBDC1936D693D34AFD0AD50C84D239A45F520BB88174CB98"
-  "BCE951849F912E639C72FB13B4B4D7177E16D55AC179BA420B2A29FE324A"
-  "467A635E81FF5901377BEDDCFD33168A461AAD3B72DAE8860078045B07A7"
-  "DBCA7874087D1510EA9FCC9DDD330507DD62DB88AEAA747DE0F4D6E2BD68"
-  "B0E7393E0F24218EB3";
+  "FFFFFFFFFFFFFFFFADF85458A2BB4A9AAFDC5620273D3CF1"
+  "D8B9C583CE2D3695A9E13641146433FBCC939DCE249B3EF9"
+  "7D2FE363630C75D8F681B202AEC4617AD3DF1ED5D5FD6561"
+  "2433F51F5F066ED0856365553DED1AF3B557135E7F57C935"
+  "984F0C70E0E68B77E2A689DAF3EFE8721DF158A136ADE735"
+  "30ACCA4F483A797ABC0AB182B324FB61D108A94BB2C8E3FB"
+  "B96ADAB760D7F4681D4F42A3DE394DF4AE56EDE76372BB19"
+  "0B07A7C8EE0A6D709E02FCE1CDF7E2ECC03404CD28342F61"
+  "9172FE9CE98583FF8E4F1232EEF28183C3FE3B1B4C6FAD73"
+  "3BB5FCBC2EC22005C58EF1837D1683B2C6F34A26C1B2EFFA"
+  "886B423861285C97FFFFFFFFFFFFFFFF";
 /**
  * This is from rfc2409, section 6.2.  It's a safe prime, and
  * supposedly it equals:

diff --git a/src/lib/crypt_ops/crypto_dh.h b/src/lib/crypt_ops/crypto_dh.h
index 60835663068cc0180dd22bb021e7f7482fbf4c71..4d602ae3744267976977420028c5062a86b521b9 100644 (file)
--- a/src/lib/crypt_ops/crypto_dh.h
+++ b/src/lib/crypt_ops/crypto_dh.h
@@ -61,4 +61,6 @@ void crypto_dh_init_nss(void);
 void crypto_dh_free_all_nss(void);
 #endif
 
+#define DH_TLS_KEY_BITS 2048
+
 #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 1578cdf2248304f87beaa6adff0385e1ba0c0cac..7b91f80d95bb644306af77d7ace7e5539c79f421 100644 (file)
--- a/src/lib/crypt_ops/crypto_dh_openssl.c
+++ b/src/lib/crypt_ops/crypto_dh_openssl.c
@@ -27,7 +27,8 @@ ENABLE_GCC_WARNING("-Wredundant-decls")
 #include <string.h>
 
 #ifndef ENABLE_NSS
-static int tor_check_dh_key(int severity, const BIGNUM *bn);
+static int tor_check_dh_key(int severity, const BIGNUM *bn,
+                            const BIGNUM *dh_p);
 
 /** A structure to hold the first half (x, g^x) of a Diffie-Hellman handshake
  * while we're waiting for the second.*/
@@ -277,7 +278,7 @@ crypto_dh_generate_public(crypto_dh_t *dh)
    */
   const BIGNUM *pub_key, *priv_key;
   DH_get0_key(dh->dh, &pub_key, &priv_key);
-  if (tor_check_dh_key(LOG_WARN, pub_key)<0) {
+  if (tor_check_dh_key(LOG_WARN, pub_key, DH_get0_p(dh->dh))<0) {
     log_warn(LD_CRYPTO, "Weird! Our own DH key was invalid.  I guess once-in-"
              "the-universe chances really do happen.  Treating as a failure.");
     return -1;
@@ -314,7 +315,7 @@ crypto_dh_get_public(crypto_dh_t *dh, char *pubkey, size_t pubkey_len)
   tor_assert(bytes >= 0);
   if (pubkey_len < (size_t)bytes) {
     log_warn(LD_CRYPTO,
-             "Weird! pubkey_len (%d) was smaller than DH1024_KEY_LEN (%d)",
+             "Weird! pubkey_len (%d) was smaller than key length (%d)",
              (int) pubkey_len, bytes);
     return -1;
   }
@@ -330,21 +331,19 @@ crypto_dh_get_public(crypto_dh_t *dh, char *pubkey, size_t pubkey_len)
  * See http://www.cl.cam.ac.uk/ftp/users/rja14/psandqs.ps.gz for some tips.
  */
 static int
-tor_check_dh_key(int severity, const BIGNUM *bn)
+tor_check_dh_key(int severity, const BIGNUM *bn, const BIGNUM *dh_p)
 {
   BIGNUM *x;
   char *s;
   tor_assert(bn);
   x = BN_new();
   tor_assert(x);
-  if (BUG(!dh_param_p))
-    crypto_dh_init(); //LCOV_EXCL_LINE we already checked whether we did this.
   BN_set_word(x, 1);
   if (BN_cmp(bn,x)<=0) {
     log_fn(severity, LD_CRYPTO, "DH key must be at least 2.");
     goto err;
   }
-  BN_copy(x,dh_param_p);
+  BN_copy(x,dh_p);
   BN_sub_word(x, 1);
   if (BN_cmp(bn,x)>=0) {
     log_fn(severity, LD_CRYPTO, "DH key must be at most p-2.");
@@ -388,7 +387,7 @@ crypto_dh_handshake(int severity, crypto_dh_t *dh,
   if (!(pubkey_bn = BN_bin2bn((const unsigned char*)pubkey,
                               (int)pubkey_len, NULL)))
     goto error;
-  if (tor_check_dh_key(severity, pubkey_bn)<0) {
+  if (tor_check_dh_key(severity, pubkey_bn, DH_get0_p(dh->dh))<0) {
     /* Check for invalid public keys. */
     log_fn(severity, LD_CRYPTO,"Rejected invalid g^x");
     goto error;

diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index 4e3099bba121660d6cb74328930e1fe63deba475..20556be25d5c3dabb1b1c5ed75dc0a103a0bd2a4 100644 (file)
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -43,10 +43,10 @@ test_crypto_dh(void *arg)
   crypto_dh_t *dh1 = crypto_dh_new(DH_TYPE_CIRCUIT);
   crypto_dh_t *dh1_dup = NULL;
   crypto_dh_t *dh2 = crypto_dh_new(DH_TYPE_CIRCUIT);
-  char p1[DH1024_KEY_LEN];
-  char p2[DH1024_KEY_LEN];
-  char s1[DH1024_KEY_LEN];
-  char s2[DH1024_KEY_LEN];
+  char p1[DH1024_KEY_LEN * 2];
+  char p2[DH1024_KEY_LEN * 2];
+  char s1[DH1024_KEY_LEN * 2];
+  char s2[DH1024_KEY_LEN * 2];
   ssize_t s1len, s2len;
 #ifdef ENABLE_OPENSSL
   crypto_dh_t *dh3 = NULL;
@@ -182,7 +182,7 @@ test_crypto_dh(void *arg)
   {
     /* Make sure that our crypto library can handshake with openssl. */
     dh3 = crypto_dh_new(DH_TYPE_TLS);
-    tt_assert(!crypto_dh_get_public(dh3, p1, DH1024_KEY_LEN));
+    tt_assert(!crypto_dh_get_public(dh3, p1, sizeof(p1)));
 
     dh4 = crypto_dh_new_openssl_tls();
     tt_assert(DH_generate_key(dh4));
@@ -190,15 +190,15 @@ test_crypto_dh(void *arg)
     const BIGNUM *sk=NULL;
     DH_get0_key(dh4, &pk, &sk);
     tt_assert(pk);
-    tt_int_op(BN_num_bytes(pk), OP_LE, DH1024_KEY_LEN);
+    tt_int_op(BN_num_bytes(pk), OP_LE, DH_TLS_KEY_BITS / 8);
     tt_int_op(BN_num_bytes(pk), OP_GT, 0);
     memset(p2, 0, sizeof(p2));
     /* right-pad. */
-    BN_bn2bin(pk, (unsigned char *)(p2+DH1024_KEY_LEN-BN_num_bytes(pk)));
+    BN_bn2bin(pk, (unsigned char *)(p2+sizeof(p2)-BN_num_bytes(pk)));
 
-    s1len = crypto_dh_handshake(LOG_WARN, dh3, p2, DH1024_KEY_LEN,
+    s1len = crypto_dh_handshake(LOG_WARN, dh3, p2, DH_TLS_KEY_BITS / 8,
                                 (unsigned char *)s1, sizeof(s1));
-    pubkey_tmp = BN_bin2bn((unsigned char *)p1, DH1024_KEY_LEN, NULL);
+    pubkey_tmp = BN_bin2bn((unsigned char *)p1, DH_TLS_KEY_BITS / 8, NULL);
     s2len = DH_compute_key((unsigned char *)s2, pubkey_tmp, dh4);
 
     tt_int_op(s1len, OP_EQ, s2len);