Merge branch 'maint-0.2.8'

changes/bug19406

@@ -0,0 +1,4 @@
+  o Minor features (build):
+    - Tor now again builds with the recent OpenSSL 1.1 development branch
+      (tested against 1.1.0-pre5 and 1.1.0-pre6-dev).
+

src/common/crypto.c

@@ -72,13 +72,18 @@
 #define DISABLE_ENGINES
 #endif
 
-#if OPENSSL_VERSION_NUMBER >= OPENSSL_VER(1,1,0,0,4) && \
+#if OPENSSL_VERSION_NUMBER >= OPENSSL_VER(1,1,0,0,5) && \
   !defined(LIBRESSL_VERSION_NUMBER)
-/* OpenSSL as of 1.1.0-pre4 has an "new" thread API, which doesn't require
+/* OpenSSL as of 1.1.0pre4 has an "new" thread API, which doesn't require
  * seting up various callbacks.
  *
- * Note: Yes, using OPENSSL_VER is naughty, but this was introduced in the
- * pre-release series.
+ * OpenSSL 1.1.0pre4 has a messed up `ERR_remove_thread_state()` prototype,
+ * while the previous one was restored in pre5, and the function made a no-op
+ * (along with a deprecated annotation, which produces a compiler warning).
+ *
+ * While it is possible to support all three versions of the thread API,
+ * a version that existed only for one snapshot pre-release is kind of
+ * pointless, so let's not.
  */
 #define NEW_THREAD_API
 #endif
@@ -89,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;
@@ -426,13 +426,29 @@ crypto_global_init(int useAccel, const char *accelName, const char *accelDir)
 void
 crypto_thread_cleanup(void)
 {
-#ifdef NEW_THREAD_API
-  ERR_remove_thread_state();
-#else
+#ifndef NEW_THREAD_API
   ERR_remove_thread_state(NULL);
 #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)
@@ -795,7 +811,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;
@@ -837,7 +853,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
@@ -849,7 +865,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
@@ -870,12 +894,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
@@ -906,9 +945,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.
@@ -933,7 +983,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 {
@@ -1007,7 +1057,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;
 
@@ -1113,7 +1163,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;
 
@@ -2106,25 +2156,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;
   }
@@ -2260,6 +2320,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;
@@ -2272,6 +2356,7 @@ crypto_dh_new(int dh_type)
     goto err;
 
   res->dh->length = DH_PRIVATE_KEY_BITS;
+#endif
 
   return res;
  err:
@@ -2311,7 +2396,9 @@ 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)) {
     /* LCOV_EXCL_START
      * To test this we would need some way to tell openssl to break DH. */
@@ -2319,6 +2406,19 @@ crypto_dh_generate_public(crypto_dh_t *dh)
     return -1;
     /* LCOV_EXCL_STOP */
   }
+#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) {
     /* LCOV_EXCL_START
      * If this happens, then openssl's DH implementation is busted. */
@@ -2331,6 +2431,7 @@ crypto_dh_generate_public(crypto_dh_t *dh)
     goto again;
     /* LCOV_EXCL_STOP */
   }
+#endif
   return 0;
 }
 
@@ -2343,13 +2444,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,
@@ -2359,7 +2477,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;
 }
@@ -3232,9 +3350,7 @@ int
 crypto_global_cleanup(void)
 {
   EVP_cleanup();
-#ifdef NEW_THREAD_API
-  ERR_remove_thread_state();
-#else
+#ifndef NEW_THREAD_API
   ERR_remove_thread_state(NULL);
 #endif
   ERR_free_strings();

src/common/tortls.c

@@ -215,7 +215,9 @@ tor_tls_log_one_error(tor_tls_t *tls, unsigned long err,
     case SSL_R_HTTP_REQUEST:
     case SSL_R_HTTPS_PROXY_REQUEST:
     case SSL_R_RECORD_LENGTH_MISMATCH:
+#ifndef OPENSSL_1_1_API
     case SSL_R_RECORD_TOO_LARGE:
+#endif
     case SSL_R_UNKNOWN_PROTOCOL:
     case SSL_R_UNSUPPORTED_PROTOCOL:
       severity = LOG_INFO;
@@ -891,7 +893,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);

src/test/test_tortls.c

@@ -381,10 +381,12 @@ test_tortls_log_one_error(void *ignored)
                         LOG_WARN, 0, NULL);
   expect_log_severity(LOG_INFO);
 
+#ifndef OPENSSL_1_1_API
   mock_clean_saved_logs();
   tor_tls_log_one_error(tls, ERR_PACK(1, 2, SSL_R_RECORD_TOO_LARGE),
                         LOG_WARN, 0, NULL);
   expect_log_severity(LOG_INFO);
+#endif
 
   mock_clean_saved_logs();
   tor_tls_log_one_error(tls, ERR_PACK(1, 2, SSL_R_UNKNOWN_PROTOCOL),
@@ -679,7 +681,7 @@ test_tortls_get_my_client_auth_key(void *ignored)
   crypto_pk_t *ret;
   crypto_pk_t *expected;
   tor_tls_context_t *ctx;
-  RSA *k = tor_malloc_zero(sizeof(RSA));
+  RSA *k = RSA_new();
 
   ctx = tor_malloc_zero(sizeof(tor_tls_context_t));
   expected = crypto_new_pk_from_rsa_(k);
@@ -694,8 +696,8 @@ test_tortls_get_my_client_auth_key(void *ignored)
   tt_assert(ret == expected);
 
  done:
+  RSA_free(k);
   tor_free(expected);
-  tor_free(k);
   tor_free(ctx);
 }
 

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);
   }

src/tools/tor-gencert.c

@@ -13,6 +13,20 @@
 #include <unistd.h>
 #endif
 
+#ifdef __GNUC__
+#define GCC_VERSION (__GNUC__ * 100 + __GNUC_MINOR__)
+#endif
+
+#if __GNUC__ && GCC_VERSION >= 402
+#if GCC_VERSION >= 406
+#pragma GCC diagnostic push
+#endif
+/* Some versions of OpenSSL declare X509_STORE_CTX_set_verify_cb twice in
+ * x509.h and x509_vfy.h. Suppress the GCC warning so we can build with
+ * -Wredundant-decl. */
+#pragma GCC diagnostic ignored "-Wredundant-decls"
+#endif
+
 #include <openssl/evp.h>
 #include <openssl/pem.h>
 #include <openssl/rsa.h>
@@ -20,6 +34,14 @@
 #include <openssl/obj_mac.h>
 #include <openssl/err.h>
 
+#if __GNUC__ && GCC_VERSION >= 402
+#if GCC_VERSION >= 406
+#pragma GCC diagnostic pop
+#else
+#pragma GCC diagnostic warning "-Wredundant-decls"
+#endif
+#endif
+
 #include <errno.h>
 #if 0
 #include <stdlib.h>