Bug 19406: OpenSSL made RSA and DH opaque in 1.1.0.

There's accessors to get at things, but it ends up being rather
cumbersome.  The only place where behavior should change is that the
code will fail instead of attempting to generate a new DH key if our
internal sanity check fails.

Like the previous commit, this probably breaks snapshots prior to pre5.

diff --git a/src/common/crypto.c b/src/common/crypto.c
index ca04e4ec3ff7a4153ac5a894f35f7ff584742fde..614f9b5de911ecdc6ca9ae9b7981d6697b848bdd 100644 (file)
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -94,11 +94,6 @@
 /** Largest strong entropy request */
 #define MAX_STRONGEST_RAND_SIZE 256
 
-/** Macro: is k a valid RSA public or private key? */
-#define PUBLIC_KEY_OK(k) ((k) && (k)->key && (k)->key->n)
-/** Macro: is k a valid RSA private key? */
-#define PRIVATE_KEY_OK(k) ((k) && (k)->key && (k)->key->p)
-
 #ifndef NEW_THREAD_API
 /** A number of preallocated mutexes for use by OpenSSL. */
 static tor_mutex_t **openssl_mutexes_ = NULL;
@@ -440,6 +435,24 @@ crypto_thread_cleanup(void)
 #endif
 }
 
+/** used internally: quicly validate a crypto_pk_t object as a private key.
+ * Return 1 iff the public key is valid, 0 if obviously invalid.
+ */
+static int
+crypto_pk_private_ok(const crypto_pk_t *k)
+{
+#ifdef OPENSSL_1_1_API
+  if (!k || !k->key)
+    return 0;
+
+  BIGNUM *p, *q;
+  RSA_get0_factors(k->key, &p, &q);
+  return p != NULL; /* XXX/yawning: Should we check q? */
+#else
+  return k && k->key && k->key->p;
+#endif
+}
+
 /** used by tortls.c: wrap an RSA* in a crypto_pk_t. */
 crypto_pk_t *
 crypto_new_pk_from_rsa_(RSA *rsa)
@@ -802,7 +815,7 @@ crypto_pk_write_private_key_to_filename(crypto_pk_t *env,
   char *s;
   int r;
 
-  tor_assert(PRIVATE_KEY_OK(env));
+  tor_assert(crypto_pk_private_ok(env));
 
   if (!(bio = BIO_new(BIO_s_mem())))
     return -1;
@@ -844,7 +857,7 @@ int
 crypto_pk_key_is_private(const crypto_pk_t *key)
 {
   tor_assert(key);
-  return PRIVATE_KEY_OK(key);
+  return crypto_pk_private_ok(key);
 }
 
 /** Return true iff <b>env</b> contains a public key whose public exponent
@@ -856,7 +869,15 @@ crypto_pk_public_exponent_ok(crypto_pk_t *env)
   tor_assert(env);
   tor_assert(env->key);
 
-  return BN_is_word(env->key->e, 65537);
+  BIGNUM *e;
+
+#ifdef OPENSSL_1_1_API
+  BIGNUM *n, *d;
+  RSA_get0_key(env->key, &n, &e, &d);
+#else
+  e = env->key->e;
+#endif
+  return BN_is_word(e, 65537);
 }
 
 /** Compare the public-key components of a and b.  Return less than 0
@@ -877,12 +898,27 @@ crypto_pk_cmp_keys(const crypto_pk_t *a, const crypto_pk_t *b)
   if (an_argument_is_null)
     return result;
 
-  tor_assert(PUBLIC_KEY_OK(a));
-  tor_assert(PUBLIC_KEY_OK(b));
-  result = BN_cmp((a->key)->n, (b->key)->n);
+  BIGNUM *a_n, *a_e;
+  BIGNUM *b_n, *b_e;
+
+#ifdef OPENSSL_1_1_API
+  BIGNUM *a_d, *b_d;
+  RSA_get0_key(a->key, &a_n, &a_e, &a_d);
+  RSA_get0_key(b->key, &b_n, &b_e, &b_d);
+#else
+  a_n = a->key->n;
+  a_e = a->key->e;
+  b_n = b->key->n;
+  b_e = b->key->e;
+#endif
+
+  tor_assert(a_n != NULL && a_e != NULL);
+  tor_assert(b_n != NULL && b_e != NULL);
+
+  result = BN_cmp(a_n, b_n);
   if (result)
     return result;
-  return BN_cmp((a->key)->e, (b->key)->e);
+  return BN_cmp(a_e, b_e);
 }
 
 /** Compare the public-key components of a and b.  Return non-zero iff
@@ -913,9 +949,20 @@ crypto_pk_num_bits(crypto_pk_t *env)
 {
   tor_assert(env);
   tor_assert(env->key);
-  tor_assert(env->key->n);
 
+#ifdef OPENSSL_1_1_API
+  /* It's so stupid that there's no other way to check that n is valid
+   * before calling RSA_bits().
+   */
+  BIGNUM *n, *e, *d;
+  RSA_get0_key(env->key, &n, &e, &d);
+  tor_assert(n != NULL);
+
+  return RSA_bits(env->key);
+#else
+  tor_assert(env->key->n);
   return BN_num_bits(env->key->n);
+#endif
 }
 
 /** Increase the reference count of <b>env</b>, and return it.
@@ -940,7 +987,7 @@ crypto_pk_copy_full(crypto_pk_t *env)
   tor_assert(env);
   tor_assert(env->key);
 
-  if (PRIVATE_KEY_OK(env)) {
+  if (crypto_pk_private_ok(env)) {
     new_key = RSAPrivateKey_dup(env->key);
     privatekey = 1;
   } else {
@@ -1009,7 +1056,7 @@ crypto_pk_private_decrypt(crypto_pk_t *env, char *to,
   tor_assert(env->key);
   tor_assert(fromlen<INT_MAX);
   tor_assert(tolen >= crypto_pk_keysize(env));
-  if (!env->key->p)
+  if (!crypto_pk_key_is_private(env))
     /* Not a private key */
     return -1;
 
@@ -1115,7 +1162,7 @@ crypto_pk_private_sign(const crypto_pk_t *env, char *to, size_t tolen,
   tor_assert(to);
   tor_assert(fromlen < INT_MAX);
   tor_assert(tolen >= crypto_pk_keysize(env));
-  if (!env->key->p)
+  if (!crypto_pk_key_is_private(env))
     /* Not a private key */
     return -1;
 
@@ -2108,25 +2155,35 @@ crypto_validate_dh_params(const BIGNUM *p, const BIGNUM *g)
   DH *dh = NULL;
   int ret = -1;
 
-  /* Copy into a temporary DH object. */
+  /* Copy into a temporary DH object, just so that DH_check() can be called. */
   if (!(dh = DH_new()))
       goto out;
+#ifdef OPENSSL_1_1_API
+  BIGNUM *dh_p, *dh_g;
+  if (!(dh_p = BN_dup(p)))
+    goto out;
+  if (!(dh_g = BN_dup(g)))
+    goto out;
+  if (!DH_set0_pqg(dh, dh_p, NULL, dh_g))
+    goto out;
+#else
   if (!(dh->p = BN_dup(p)))
     goto out;
   if (!(dh->g = BN_dup(g)))
     goto out;
+#endif
 
   /* Perform the validation. */
   int codes = 0;
   if (!DH_check(dh, &codes))
     goto out;
-  if (BN_is_word(dh->g, DH_GENERATOR_2)) {
+  if (BN_is_word(g, DH_GENERATOR_2)) {
     /* Per https://wiki.openssl.org/index.php/Diffie-Hellman_parameters
      *
      * OpenSSL checks the prime is congruent to 11 when g = 2; while the
      * IETF's primes are congruent to 23 when g = 2.
      */
-    BN_ULONG residue = BN_mod_word(dh->p, 24);
+    BN_ULONG residue = BN_mod_word(p, 24);
     if (residue == 11 || residue == 23)
       codes &= ~DH_NOT_SUITABLE_GENERATOR;
   }
@@ -2257,6 +2314,30 @@ crypto_dh_new(int dh_type)
   if (!(res->dh = DH_new()))
     goto err;
 
+#ifdef OPENSSL_1_1_API
+  BIGNUM *dh_p = NULL, *dh_g = NULL;
+
+  if (dh_type == DH_TYPE_TLS) {
+    dh_p = BN_dup(dh_param_p_tls);
+  } else {
+    dh_p = BN_dup(dh_param_p);
+  }
+  if (!dh_p)
+    goto err;
+
+  dh_g = BN_dup(dh_param_g);
+  if (!dh_g) {
+    BN_free(dh_p);
+    goto err;
+  }
+
+  if (!DH_set0_pqg(res->dh, dh_p, NULL, dh_g)) {
+    goto err;
+  }
+
+  if (!DH_set_length(res->dh, DH_PRIVATE_KEY_BITS))
+    goto err;
+#else
   if (dh_type == DH_TYPE_TLS) {
     if (!(res->dh->p = BN_dup(dh_param_p_tls)))
       goto err;
@@ -2269,6 +2350,7 @@ crypto_dh_new(int dh_type)
     goto err;
 
   res->dh->length = DH_PRIVATE_KEY_BITS;
+#endif
 
   return res;
  err:
@@ -2305,11 +2387,26 @@ crypto_dh_get_bytes(crypto_dh_t *dh)
 int
 crypto_dh_generate_public(crypto_dh_t *dh)
 {
+#ifndef OPENSSL_1_1_API
  again:
+#endif
   if (!DH_generate_key(dh->dh)) {
     crypto_log_errors(LOG_WARN, "generating DH key");
     return -1;
   }
+#ifdef OPENSSL_1_1_API
+  /* OpenSSL 1.1.x doesn't appear to let you regenerate a DH key, without
+   * recreating the DH object.  I have no idea what sort of aliasing madness
+   * can occur here, so do the check, and just bail on failure.
+   */
+  BIGNUM *pub_key, *priv_key;
+  DH_get0_key(dh->dh, &pub_key, &priv_key);
+  if (tor_check_dh_key(LOG_WARN, pub_key)<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;
+  }
+#else
   if (tor_check_dh_key(LOG_WARN, dh->dh->pub_key)<0) {
     log_warn(LD_CRYPTO, "Weird! Our own DH key was invalid.  I guess once-in-"
              "the-universe chances really do happen.  Trying again.");
@@ -2319,6 +2416,7 @@ crypto_dh_generate_public(crypto_dh_t *dh)
     dh->dh->pub_key = dh->dh->priv_key = NULL;
     goto again;
   }
+#endif
   return 0;
 }
 
@@ -2331,13 +2429,30 @@ crypto_dh_get_public(crypto_dh_t *dh, char *pubkey, size_t pubkey_len)
 {
   int bytes;
   tor_assert(dh);
-  if (!dh->dh->pub_key) {
+
+  BIGNUM *dh_pub;
+
+#ifdef OPENSSL_1_1_API
+  BIGNUM *dh_priv;
+  DH_get0_key(dh->dh, &dh_pub, &dh_priv);
+#else
+  dh_pub = dh->dh->pub_key;
+#endif
+
+  if (!dh_pub) {
     if (crypto_dh_generate_public(dh)<0)
       return -1;
+    else {
+#ifdef OPENSSL_1_1_API
+      DH_get0_key(dh->dh, &dh_pub, &dh_priv);
+#else
+      dh_pub = dh->dh->pub_key;
+#endif
+    }
   }
 
-  tor_assert(dh->dh->pub_key);
-  bytes = BN_num_bytes(dh->dh->pub_key);
+  tor_assert(dh_pub);
+  bytes = BN_num_bytes(dh_pub);
   tor_assert(bytes >= 0);
   if (pubkey_len < (size_t)bytes) {
     log_warn(LD_CRYPTO,
@@ -2347,7 +2462,7 @@ crypto_dh_get_public(crypto_dh_t *dh, char *pubkey, size_t pubkey_len)
   }
 
   memset(pubkey, 0, pubkey_len);
-  BN_bn2bin(dh->dh->pub_key, (unsigned char*)(pubkey+(pubkey_len-bytes)));
+  BN_bn2bin(dh_pub, (unsigned char*)(pubkey+(pubkey_len-bytes)));
 
   return 0;
 }

diff --git a/src/common/tortls.c b/src/common/tortls.c
index 4ffc672546c90ec69b05aa9696b234b92e4979b7..7d070c54cd74d0ce62f44ab7dd0beb3866d4ed7b 100644 (file)
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -904,7 +904,11 @@ tor_tls_cert_is_valid(int severity,
   cert_key = X509_get_pubkey(cert->cert);
   if (check_rsa_1024 && cert_key) {
     RSA *rsa = EVP_PKEY_get1_RSA(cert_key);
+#ifdef OPENSSL_1_1_API
+    if (rsa && RSA_bits(rsa) == 1024)
+#else
     if (rsa && BN_num_bits(rsa->n) == 1024)
+#endif
       key_ok = 1;
     if (rsa)
       RSA_free(rsa);

diff --git a/src/tools/tor-checkkey.c b/src/tools/tor-checkkey.c
index ed68bdf52cad6cc62a7f7765ae499dffc185bade..8e957c254058fe07c5cc9dbc98fe2255d529e157 100644 (file)
--- a/src/tools/tor-checkkey.c
+++ b/src/tools/tor-checkkey.c
@@ -9,6 +9,7 @@
 #include "torlog.h"
 #include "util.h"
 #include "compat.h"
+#include "compat_openssl.h"
 #include <openssl/bn.h>
 #include <openssl/rsa.h>
 
@@ -70,7 +71,15 @@ main(int c, char **v)
     printf("%s\n",digest);
   } else {
     rsa = crypto_pk_get_rsa_(env);
-    str = BN_bn2hex(rsa->n);
+
+    BIGNUM *rsa_n;
+#ifdef OPENSSL_1_1_API
+    BIGNUM *rsa_e, *rsa_d;
+    RSA_get0_key(rsa, &rsa_n, &rsa_e, &rsa_d);
+#else
+    rsa_n = rsa->n;
+#endif
+    str = BN_bn2hex(rsa_n);
 
     printf("%s\n", str);
   }