Commit b563a3a0 authored by Yawning Angel's avatar Yawning Angel Committed by Nick Mathewson
Browse files

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.
parent 86f0b806

src/common/crypto.c

+138 −23
Original line number Diff line number Diff line
@@ -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;

}

src/common/tortls.c

+4 −0
Original line number Diff line number Diff line
@@ -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);

src/tools/tor-checkkey.c

+10 −1
Original line number Diff line number Diff line
@@ -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);

  }