September 2018 - tor-commits - lists.torproject.org

[tor/master] Use NSS in crypto_rand.c
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 60705a5719d929a5f3180bc4e6906f972855f84b Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jul 13 14:35:51 2018 -0400 Use NSS in crypto_rand.c This is comparatively straightforward too, except for a couple of twists: * For as long as we're building with two crypto libraries, we want to seed _both_ their RNGs, and use _both_ their RNGs to improve the output of crypto_strongest_rand() * The NSS prng will sometimes refuse to generate huge outputs. When it does, we stretch the output with SHAKE. We only need this for the tests. --- src/lib/crypt_ops/crypto_rand.c | 110 +++++++++++++++++++++++++++++++++++++--- src/test/test-timers.c | 5 ++ 2 files changed, 107 insertions(+), 8 deletions(-) diff --git a/src/lib/crypt_ops/crypto_rand.c b/src/lib/crypt_ops/crypto_rand.c index fb9d0c2c6..980671474 100644 --- a/src/lib/crypt_ops/crypto_rand.c +++ b/src/lib/crypt_ops/crypto_rand.c @@ -35,9 +35,22 @@ #include "lib/testsupport/testsupport.h" #include "lib/fs/files.h" +#ifdef ENABLE_NSS +#include "lib/crypt_ops/crypto_nss_mgt.h" +#include "lib/crypt_ops/crypto_digest.h" +#endif + +#ifdef ENABLE_OPENSSL DISABLE_GCC_WARNING(redundant-decls) #include <openssl/rand.h> ENABLE_GCC_WARNING(redundant-decls) +#endif + +#ifdef ENABLE_NSS +#include <pk11pub.h> +#include <secerr.h> +#include <prerror.h> +#endif #if __GNUC__ && GCC_VERSION >= 402 #if GCC_VERSION >= 406 @@ -324,14 +337,21 @@ crypto_strongest_rand(uint8_t *out, size_t out_len) { #define DLEN SHA512_DIGEST_LENGTH /* We're going to hash DLEN bytes from the system RNG together with some - * bytes from the openssl PRNG, in order to yield DLEN bytes. + * bytes from the PRNGs from our crypto librar(y/ies), in order to yield + * DLEN bytes. */ - uint8_t inp[DLEN*2]; + uint8_t inp[DLEN*3]; uint8_t tmp[DLEN]; tor_assert(out); while (out_len) { - crypto_rand((char*) inp, DLEN); - if (crypto_strongest_rand_raw(inp+DLEN, DLEN) < 0) { + memset(inp, 0, sizeof(inp)); +#ifdef ENABLE_OPENSSL + RAND_bytes(inp, DLEN); +#endif +#ifdef ENABLE_NSS + PK11_GenerateRandom(inp+DLEN, DLEN); +#endif + if (crypto_strongest_rand_raw(inp+DLEN*2, DLEN) < 0) { // LCOV_EXCL_START log_err(LD_CRYPTO, "Failed to load strong entropy when generating an " "important key. Exiting."); @@ -354,12 +374,13 @@ crypto_strongest_rand(uint8_t *out, size_t out_len) #undef DLEN } +#ifdef ENABLE_OPENSSL /** * Seed OpenSSL's random number generator with bytes from the operating * system. Return 0 on success, -1 on failure. **/ -int -crypto_seed_rng(void) +static int +crypto_seed_openssl_rng(void) { int rand_poll_ok = 0, load_entropy_ok = 0; uint8_t buf[ADD_ENTROPY]; @@ -383,6 +404,52 @@ crypto_seed_rng(void) else return -1; } +#endif + +#ifdef ENABLE_NSS +/** + * Seed OpenSSL's random number generator with bytes from the operating + * system. Return 0 on success, -1 on failure. + **/ +static int +crypto_seed_nss_rng(void) +{ + uint8_t buf[ADD_ENTROPY]; + + int load_entropy_ok = !crypto_strongest_rand_raw(buf, sizeof(buf)); + if (load_entropy_ok) { + if (PK11_RandomUpdate(buf, sizeof(buf)) != SECSuccess) { + load_entropy_ok = 0; + } + } + + memwipe(buf, 0, sizeof(buf)); + + return load_entropy_ok ? 0 : -1; +} +#endif + +/** + * Seed the RNG for any and all crypto libraries that we're using with bytes + * from the operating system. Return 0 on success, -1 on failure. + */ +int +crypto_seed_rng(void) +{ + int seeded = 0; +#ifdef ENABLE_NSS + if (crypto_seed_nss_rng() < 0) + return -1; + ++seeded; +#endif +#ifdef ENABLE_OPENSSL + if (crypto_seed_openssl_rng() < 0) + return -1; + ++seeded; +#endif + tor_assert(seeded); + return 0; +} /** * Write n bytes of strong random data to to. Supports mocking @@ -407,17 +474,44 @@ crypto_rand, (char *to, size_t n)) void crypto_rand_unmocked(char *to, size_t n) { - int r; if (n == 0) return; tor_assert(n < INT_MAX); tor_assert(to); - r = RAND_bytes((unsigned char*)to, (int)n); + +#ifdef ENABLE_NSS + SECStatus s = PK11_GenerateRandom((unsigned char*)to, (int)n); + if (s != SECSuccess) { + /* NSS rather sensibly might refuse to generate huge amounts of random + * data at once. Unfortunately, our unit test do this in a couple of + * places. To solve this issue, we use our XOF to stretch a shorter + * output when a longer one is needed. + * + * Yes, this is secure. */ + + /* This is longer than it needs to be; 1600 bits == 200 bytes is the + * state-size of SHA3. */ +#define BUFLEN 512 + tor_assert(PR_GetError() == SEC_ERROR_INVALID_ARGS && n > BUFLEN); + unsigned char buf[BUFLEN]; + s = PK11_GenerateRandom(buf, BUFLEN); + tor_assert(s == SECSuccess); + crypto_xof_t *xof = crypto_xof_new(); + crypto_xof_add_bytes(xof, buf, BUFLEN); + crypto_xof_squeeze_bytes(xof, (unsigned char *)to, n); + crypto_xof_free(xof); + memwipe(buf, 0, BUFLEN); + +#undef BUFLEN + } +#else + int r = RAND_bytes((unsigned char*)to, (int)n); /* We consider a PRNG failure non-survivable. Let's assert so that we get a * stack trace about where it happened. */ tor_assert(r >= 0); +#endif } /** diff --git a/src/test/test-timers.c b/src/test/test-timers.c index f9276c25d..923f51ecc 100644 --- a/src/test/test-timers.c +++ b/src/test/test-timers.c @@ -9,6 +9,7 @@ #include "lib/evloop/compat_libevent.h" #include "lib/evloop/timers.h" +#include "lib/crypt_ops/crypto_init.h" #include "lib/crypt_ops/crypto_rand.h" #include "lib/log/util_bug.h" #include "lib/time/compat_time.h" @@ -62,6 +63,10 @@ main(int argc, char **argv) memset(&cfg, 0, sizeof(cfg)); tor_libevent_initialize(&cfg); timers_initialize(); + init_logging(1); + + if (crypto_global_init(0, NULL, NULL) < 0) + return 1; int i; int ret;

1 0

[tor/master] Make the rust tests link.
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 99beed152e5df41181703645a67fe10a98225d55 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 18 10:40:52 2018 -0400 Make the rust tests link. --- config.rust.in | 2 ++ src/rust/build.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/config.rust.in b/config.rust.in index 4ca5351ae..11b671b98 100644 --- a/config.rust.in +++ b/config.rust.in @@ -5,6 +5,7 @@ BUILDDIR=@BUILDDIR@ TOR_LDFLAGS_zlib=@TOR_LDFLAGS_zlib@ +TOR_LDFLAGS_nss=@TOR_LDFLAGS_nss@ TOR_LDFLAGS_openssl=@TOR_LDFLAGS_openssl@ TOR_LDFLAGS_libevent=@TOR_LDFLAGS_libevent@ TOR_ZLIB_LIBS=@TOR_ZLIB_LIBS@ @@ -20,3 +21,4 @@ TOR_LZMA_LIBS=@TOR_LZMA_LIBS@ TOR_ZSTD_LIBS=@TOR_ZSTD_LIBS@ LIBS=@LIBS@ LDFLAGS=@LDFLAGS@ +NSS_LIBS=@NSS_LIBS@ \ No newline at end of file diff --git a/src/rust/build.rs b/src/rust/build.rs index 2cf85b404..a35e6868c 100644 --- a/src/rust/build.rs +++ b/src/rust/build.rs @@ -173,6 +173,7 @@ pub fn main() { cfg.from_cflags("TOR_ZLIB_LIBS"); cfg.from_cflags("TOR_LIB_MATH"); + cfg.from_cflags("NSS_LIBS"); cfg.from_cflags("TOR_OPENSSL_LIBS"); cfg.from_cflags("TOR_LIBEVENT_LIBS"); cfg.from_cflags("TOR_LIB_WS32");

1 0

[tor/master] Fix "make distcheck."
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 2d80673b9a7965d5b2af895c7527eb58bd6a00ce Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 18 09:52:47 2018 -0400 Fix "make distcheck." --- src/lib/crypt_ops/include.am | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/crypt_ops/include.am b/src/lib/crypt_ops/include.am index 3ebb0b0f6..018730627 100644 --- a/src/lib/crypt_ops/include.am +++ b/src/lib/crypt_ops/include.am @@ -54,6 +54,7 @@ noinst_HEADERS += \ src/lib/crypt_ops/crypto_cipher.h \ src/lib/crypt_ops/crypto_hkdf.h \ src/lib/crypt_ops/crypto_init.h \ + src/lib/crypt_ops/crypto_nss_mgt.h \ src/lib/crypt_ops/crypto_openssl_mgt.h \ src/lib/crypt_ops/crypto_pwbox.h \ src/lib/crypt_ops/crypto_rand.h \

1 0

[tor/master] Extract the OpenSSL DH functionality to a new file.
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 60a5b784803df5a1a9e467e0aa14c17b0991f68c Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 17 09:06:05 2018 -0400 Extract the OpenSSL DH functionality to a new file. --- src/lib/crypt_ops/crypto_dh.c | 515 ++-------------------------------- src/lib/crypt_ops/crypto_dh.h | 4 + src/lib/crypt_ops/crypto_dh_openssl.c | 490 ++++++++++++++++++++++++++++++++ src/lib/crypt_ops/include.am | 1 + 4 files changed, 519 insertions(+), 491 deletions(-) diff --git a/src/lib/crypt_ops/crypto_dh.c b/src/lib/crypt_ops/crypto_dh.c index ab06fd496..8bb66379f 100644 --- a/src/lib/crypt_ops/crypto_dh.c +++ b/src/lib/crypt_ops/crypto_dh.c @@ -7,6 +7,8 @@ /** * \file crypto_dh.c * \brief Block of functions related with DH utilities and operations. + * over Z_p. We aren't using this for any new crypto -- EC is more + * efficient. **/ #include "lib/crypt_ops/compat_openssl.h" @@ -17,495 +19,26 @@ #include "lib/log/log.h" #include "lib/log/util_bug.h" -DISABLE_GCC_WARNING(redundant-decls) - -#include <openssl/dh.h> - -ENABLE_GCC_WARNING(redundant-decls) - -#include <openssl/bn.h> -#include <string.h> - -/** A structure to hold the first half (x, g^x) of a Diffie-Hellman handshake - * while we're waiting for the second.*/ -struct crypto_dh_t { - DH *dh; /**< The openssl DH object */ -}; - -static int tor_check_dh_key(int severity, const BIGNUM *bn); - -/** Used by tortls.c: Get the DH* from a crypto_dh_t. - */ -DH * -crypto_dh_get_dh_(crypto_dh_t *dh) -{ - return dh->dh; -} - /** Our DH 'g' parameter */ -#define DH_GENERATOR 2 - -/** 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. */ -static BIGNUM *dh_param_p_tls = NULL; -/** Shared G parameter for our DH key exchanges. */ -static BIGNUM *dh_param_g = NULL; - -/** Validate a given set of Diffie-Hellman parameters. This is moderately - * computationally expensive (milliseconds), so should only be called when - * the DH parameters change. Returns 0 on success, * -1 on failure. - */ -static int -crypto_validate_dh_params(const BIGNUM *p, const BIGNUM *g) -{ - DH *dh = NULL; - int ret = -1; - - /* 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 /* !(defined(OPENSSL_1_1_API)) */ - if (!(dh->p = BN_dup(p))) - goto out; - if (!(dh->g = BN_dup(g))) - goto out; -#endif /* defined(OPENSSL_1_1_API) */ - - /* Perform the validation. */ - int codes = 0; - if (!DH_check(dh, &codes)) - goto out; - 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(p, 24); - if (residue == 11 || residue == 23) - codes &= ~DH_NOT_SUITABLE_GENERATOR; - } - if (codes != 0) /* Specifics on why the params suck is irrelevant. */ - goto out; - - /* Things are probably not evil. */ - ret = 0; - - out: - if (dh) - DH_free(dh); - return ret; -} - -/** Set the global Diffie-Hellman generator, used for both TLS and internal - * DH stuff. - */ -static void -crypto_set_dh_generator(void) -{ - BIGNUM *generator; - int r; - - if (dh_param_g) - return; - - generator = BN_new(); - tor_assert(generator); - - r = BN_set_word(generator, DH_GENERATOR); - tor_assert(r); - - dh_param_g = generator; -} - -/** Set the global TLS Diffie-Hellman modulus. Use the Apache mod_ssl DH - * modulus. */ -void -crypto_set_tls_dh_prime(void) -{ - BIGNUM *tls_prime = NULL; - int r; - - /* If the space is occupied, free the previous TLS DH prime */ - if (BUG(dh_param_p_tls)) { - /* LCOV_EXCL_START - * - * We shouldn't be calling this twice. - */ - BN_clear_free(dh_param_p_tls); - dh_param_p_tls = NULL; - /* LCOV_EXCL_STOP */ - } - - tls_prime = BN_new(); - tor_assert(tls_prime); - - /* 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. - */ - r = BN_hex2bn(&tls_prime, - "D67DE440CBBBDC1936D693D34AFD0AD50C84D239A45F520BB88174CB98" - "BCE951849F912E639C72FB13B4B4D7177E16D55AC179BA420B2A29FE324A" - "467A635E81FF5901377BEDDCFD33168A461AAD3B72DAE8860078045B07A7" - "DBCA7874087D1510EA9FCC9DDD330507DD62DB88AEAA747DE0F4D6E2BD68" - "B0E7393E0F24218EB3"); - tor_assert(r); - - tor_assert(tls_prime); - - dh_param_p_tls = tls_prime; - crypto_set_dh_generator(); - tor_assert(0 == crypto_validate_dh_params(dh_param_p_tls, dh_param_g)); -} - -/** Initialize dh_param_p and dh_param_g if they are not already - * set. */ -static void -init_dh_param(void) -{ - BIGNUM *circuit_dh_prime; - int r; - if (BUG(dh_param_p && dh_param_g)) - return; // LCOV_EXCL_LINE This function isn't supposed to be called twice. - - circuit_dh_prime = BN_new(); - tor_assert(circuit_dh_prime); - - /* This is from rfc2409, section 6.2. It's a safe prime, and - supposedly it equals: - 2^1024 - 2^960 - 1 + 2^64 * { [2^894 pi] + 129093 }. - */ - r = BN_hex2bn(&circuit_dh_prime, - "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E08" - "8A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B" - "302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9" - "A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE6" - "49286651ECE65381FFFFFFFFFFFFFFFF"); - tor_assert(r); - - /* Set the new values as the global DH parameters. */ - dh_param_p = circuit_dh_prime; - crypto_set_dh_generator(); - tor_assert(0 == crypto_validate_dh_params(dh_param_p, dh_param_g)); - - if (!dh_param_p_tls) { - crypto_set_tls_dh_prime(); - } -} - -/** Number of bits to use when choosing the x or y value in a Diffie-Hellman - * handshake. Since we exponentiate by this value, choosing a smaller one - * lets our handhake go faster. - */ -#define DH_PRIVATE_KEY_BITS 320 - -/** Allocate and return a new DH object for a key exchange. Returns NULL on - * failure. - */ -crypto_dh_t * -crypto_dh_new(int dh_type) -{ - crypto_dh_t *res = tor_malloc_zero(sizeof(crypto_dh_t)); - - tor_assert(dh_type == DH_TYPE_CIRCUIT || dh_type == DH_TYPE_TLS || - dh_type == DH_TYPE_REND); - - 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; - - 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 /* !(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))) - goto err; - - res->dh->length = DH_PRIVATE_KEY_BITS; -#endif /* defined(OPENSSL_1_1_API) */ - - return res; - - /* 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); - return NULL; - /* LCOV_EXCL_STOP */ -} - -/** Return a copy of dh, sharing its internal state. */ -crypto_dh_t * -crypto_dh_dup(const crypto_dh_t *dh) -{ - crypto_dh_t *dh_new = tor_malloc_zero(sizeof(crypto_dh_t)); - tor_assert(dh); - tor_assert(dh->dh); - dh_new->dh = dh->dh; - DH_up_ref(dh->dh); - return dh_new; -} - -/** Return the length of the DH key in dh, in bytes. - */ -int -crypto_dh_get_bytes(crypto_dh_t *dh) -{ - tor_assert(dh); - return DH_size(dh->dh); -} - -/** Generate \<x,g^x\> for our part of the key exchange. Return 0 on - * success, -1 on failure. - */ -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. */ - crypto_openssl_log_errors(LOG_WARN, "generating DH key"); - 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. - */ - 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) { - 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 /* !(defined(OPENSSL_1_1_API)) */ - 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. */ - log_warn(LD_CRYPTO, "Weird! Our own DH key was invalid. I guess once-in-" - "the-universe chances really do happen. Trying again."); - /* Free and clear the keys, so OpenSSL will actually try again. */ - BN_clear_free(dh->dh->pub_key); - BN_clear_free(dh->dh->priv_key); - dh->dh->pub_key = dh->dh->priv_key = NULL; - goto again; - /* LCOV_EXCL_STOP */ - } -#endif /* defined(OPENSSL_1_1_API) */ - return 0; -} - -/** Generate g^x as necessary, and write the g^x for the key exchange - * as a pubkey_len-byte value into pubkey. Return 0 on - * success, -1 on failure. pubkey_len must be \>= DH1024_KEY_LEN. - */ -int -crypto_dh_get_public(crypto_dh_t *dh, char *pubkey, size_t pubkey_len) -{ - int bytes; - tor_assert(dh); - - const BIGNUM *dh_pub; - -#ifdef OPENSSL_1_1_API - const BIGNUM *dh_priv; - DH_get0_key(dh->dh, &dh_pub, &dh_priv); -#else - dh_pub = dh->dh->pub_key; -#endif /* defined(OPENSSL_1_1_API) */ - - 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_pub); - bytes = BN_num_bytes(dh_pub); - 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)", - (int) pubkey_len, bytes); - return -1; - } - - memset(pubkey, 0, pubkey_len); - BN_bn2bin(dh_pub, (unsigned char*)(pubkey+(pubkey_len-bytes))); - - return 0; -} - -/** Check for bad Diffie-Hellman public keys (g^x). Return 0 if the key is - * okay (in the subgroup [2,p-2]), or -1 if it's bad. - * 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) -{ - BIGNUM *x; - char *s; - tor_assert(bn); - x = BN_new(); - tor_assert(x); - if (BUG(!dh_param_p)) - init_dh_param(); //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_sub_word(x, 1); - if (BN_cmp(bn,x)>=0) { - log_fn(severity, LD_CRYPTO, "DH key must be at most p-2."); - goto err; - } - BN_clear_free(x); - return 0; - err: - BN_clear_free(x); - s = BN_bn2hex(bn); - log_fn(severity, LD_CRYPTO, "Rejecting insecure DH key [%s]", s); - OPENSSL_free(s); - return -1; -} - -/** Given a DH key exchange object, and our peer's value of g^y (as a - * pubkey_len-byte value in pubkey) generate - * secret_bytes_out bytes of shared key material and write them - * to secret_out. Return the number of bytes generated on success, - * or -1 on failure. - * - * (We generate key material by computing - * SHA1( g^xy || "\x00" ) || SHA1( g^xy || "\x01" ) || ... - * where || is concatenation.) - */ -ssize_t -crypto_dh_compute_secret(int severity, crypto_dh_t *dh, - const char *pubkey, size_t pubkey_len, - char *secret_out, size_t secret_bytes_out) -{ - char *secret_tmp = NULL; - BIGNUM *pubkey_bn = NULL; - size_t secret_len=0, secret_tmp_len=0; - int result=0; - tor_assert(dh); - tor_assert(secret_bytes_out/DIGEST_LEN <= 255); - tor_assert(pubkey_len < INT_MAX); - - if (!(pubkey_bn = BN_bin2bn((const unsigned char*)pubkey, - (int)pubkey_len, NULL))) - goto error; - if (tor_check_dh_key(severity, pubkey_bn)<0) { - /* Check for invalid public keys. */ - log_fn(severity, LD_CRYPTO,"Rejected invalid g^x"); - goto error; - } - secret_tmp_len = crypto_dh_get_bytes(dh); - secret_tmp = tor_malloc(secret_tmp_len); - result = DH_compute_key((unsigned char*)secret_tmp, pubkey_bn, dh->dh); - if (result < 0) { - log_warn(LD_CRYPTO,"DH_compute_key() failed."); - goto error; - } - secret_len = result; - if (crypto_expand_key_material_TAP((uint8_t*)secret_tmp, secret_len, - (uint8_t*)secret_out, secret_bytes_out)<0) - goto error; - secret_len = secret_bytes_out; - - goto done; - error: - result = -1; - done: - crypto_openssl_log_errors(LOG_WARN, "completing DH handshake"); - if (pubkey_bn) - BN_clear_free(pubkey_bn); - if (secret_tmp) { - memwipe(secret_tmp, 0, secret_tmp_len); - tor_free(secret_tmp); - } - if (result < 0) - return result; - else - return secret_len; -} - -/** Free a DH key exchange object. - */ -void -crypto_dh_free_(crypto_dh_t *dh) -{ - if (!dh) - return; - tor_assert(dh->dh); - DH_free(dh->dh); - tor_free(dh); -} - -void -crypto_dh_free_all(void) -{ - if (dh_param_p) - BN_clear_free(dh_param_p); - if (dh_param_p_tls) - BN_clear_free(dh_param_p_tls); - if (dh_param_g) - BN_clear_free(dh_param_g); - - dh_param_p = dh_param_p_tls = dh_param_g = NULL; -} +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. + */ +const char TLS_DH_PRIME[] = + "D67DE440CBBBDC1936D693D34AFD0AD50C84D239A45F520BB88174CB98" + "BCE951849F912E639C72FB13B4B4D7177E16D55AC179BA420B2A29FE324A" + "467A635E81FF5901377BEDDCFD33168A461AAD3B72DAE8860078045B07A7" + "DBCA7874087D1510EA9FCC9DDD330507DD62DB88AEAA747DE0F4D6E2BD68" + "B0E7393E0F24218EB3"; +/** + * This is from rfc2409, section 6.2. It's a safe prime, and + * supposedly it equals: + * 2^1024 - 2^960 - 1 + 2^64 * { [2^894 pi] + 129093 }. + */ +const char OAKLEY_PRIME_2[] = + "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E08" + "8A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B" + "302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9" + "A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE6" + "49286651ECE65381FFFFFFFFFFFFFFFF"; diff --git a/src/lib/crypt_ops/crypto_dh.h b/src/lib/crypt_ops/crypto_dh.h index 88e8a919a..2d25ba8c8 100644 --- a/src/lib/crypt_ops/crypto_dh.h +++ b/src/lib/crypt_ops/crypto_dh.h @@ -19,6 +19,10 @@ typedef struct crypto_dh_t crypto_dh_t; +extern const unsigned DH_GENERATOR; +extern const char TLS_DH_PRIME[]; +extern const char OAKLEY_PRIME_2[]; + /* Key negotiation */ #define DH_TYPE_CIRCUIT 1 #define DH_TYPE_REND 2 diff --git a/src/lib/crypt_ops/crypto_dh_openssl.c b/src/lib/crypt_ops/crypto_dh_openssl.c new file mode 100644 index 000000000..4457b1814 --- /dev/null +++ b/src/lib/crypt_ops/crypto_dh_openssl.c @@ -0,0 +1,490 @@ +/* Copyright (c) 2001, Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file crypto_dh_openssl.c + * \brief Implement Tor's Z_p diffie-hellman stuff for OpenSSL. + **/ + +#include "lib/crypt_ops/compat_openssl.h" +#include "lib/crypt_ops/crypto_dh.h" +#include "lib/crypt_ops/crypto_digest.h" +#include "lib/crypt_ops/crypto_hkdf.h" +#include "lib/crypt_ops/crypto_util.h" +#include "lib/log/log.h" +#include "lib/log/util_bug.h" + +DISABLE_GCC_WARNING(redundant-decls) + +#include <openssl/dh.h> + +ENABLE_GCC_WARNING(redundant-decls) + +#include <openssl/bn.h> +#include <string.h> + +static int tor_check_dh_key(int severity, const BIGNUM *bn); + +/** A structure to hold the first half (x, g^x) of a Diffie-Hellman handshake + * while we're waiting for the second.*/ +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. */ +static BIGNUM *dh_param_p_tls = NULL; +/** Shared G parameter for our DH key exchanges. */ +static BIGNUM *dh_param_g = NULL; + +/** Validate a given set of Diffie-Hellman parameters. This is moderately + * computationally expensive (milliseconds), so should only be called when + * the DH parameters change. Returns 0 on success, * -1 on failure. + */ +static int +crypto_validate_dh_params(const BIGNUM *p, const BIGNUM *g) +{ + DH *dh = NULL; + int ret = -1; + + /* 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 /* !(defined(OPENSSL_1_1_API)) */ + if (!(dh->p = BN_dup(p))) + goto out; + if (!(dh->g = BN_dup(g))) + goto out; +#endif /* defined(OPENSSL_1_1_API) */ + + /* Perform the validation. */ + int codes = 0; + if (!DH_check(dh, &codes)) + goto out; + 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(p, 24); + if (residue == 11 || residue == 23) + codes &= ~DH_NOT_SUITABLE_GENERATOR; + } + if (codes != 0) /* Specifics on why the params suck is irrelevant. */ + goto out; + + /* Things are probably not evil. */ + ret = 0; + + out: + if (dh) + DH_free(dh); + return ret; +} + +/** Set the global Diffie-Hellman generator, used for both TLS and internal + * DH stuff. + */ +static void +crypto_set_dh_generator(void) +{ + BIGNUM *generator; + int r; + + if (dh_param_g) + return; + + generator = BN_new(); + tor_assert(generator); + + r = BN_set_word(generator, DH_GENERATOR); + tor_assert(r); + + dh_param_g = generator; +} + +/** Set the global TLS Diffie-Hellman modulus. Use the Apache mod_ssl DH + * modulus. */ +void +crypto_set_tls_dh_prime(void) +{ + BIGNUM *tls_prime = NULL; + int r; + + /* If the space is occupied, free the previous TLS DH prime */ + if (BUG(dh_param_p_tls)) { + /* LCOV_EXCL_START + * + * We shouldn't be calling this twice. + */ + BN_clear_free(dh_param_p_tls); + dh_param_p_tls = NULL; + /* LCOV_EXCL_STOP */ + } + + tls_prime = BN_new(); + tor_assert(tls_prime); + + r = BN_hex2bn(&tls_prime, TLS_DH_PRIME); + tor_assert(r); + + tor_assert(tls_prime); + + dh_param_p_tls = tls_prime; + crypto_set_dh_generator(); + tor_assert(0 == crypto_validate_dh_params(dh_param_p_tls, dh_param_g)); +} + +/** Initialize dh_param_p and dh_param_g if they are not already + * set. */ +static void +init_dh_param(void) +{ + BIGNUM *circuit_dh_prime; + int r; + if (BUG(dh_param_p && dh_param_g)) + return; // LCOV_EXCL_LINE This function isn't supposed to be called twice. + + circuit_dh_prime = BN_new(); + tor_assert(circuit_dh_prime); + + r = BN_hex2bn(&circuit_dh_prime, OAKLEY_PRIME_2); + tor_assert(r); + + /* Set the new values as the global DH parameters. */ + dh_param_p = circuit_dh_prime; + crypto_set_dh_generator(); + tor_assert(0 == crypto_validate_dh_params(dh_param_p, dh_param_g)); + + if (!dh_param_p_tls) { + crypto_set_tls_dh_prime(); + } +} + +/** Number of bits to use when choosing the x or y value in a Diffie-Hellman + * handshake. Since we exponentiate by this value, choosing a smaller one + * lets our handhake go faster. + */ +#define DH_PRIVATE_KEY_BITS 320 + +/** Allocate and return a new DH object for a key exchange. Returns NULL on + * failure. + */ +crypto_dh_t * +crypto_dh_new(int dh_type) +{ + crypto_dh_t *res = tor_malloc_zero(sizeof(crypto_dh_t)); + + tor_assert(dh_type == DH_TYPE_CIRCUIT || dh_type == DH_TYPE_TLS || + dh_type == DH_TYPE_REND); + + 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; + + 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 /* !(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))) + goto err; + + res->dh->length = DH_PRIVATE_KEY_BITS; +#endif /* defined(OPENSSL_1_1_API) */ + + return res; + + /* 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); + return NULL; + /* LCOV_EXCL_STOP */ +} + +/** Return a copy of dh, sharing its internal state. */ +crypto_dh_t * +crypto_dh_dup(const crypto_dh_t *dh) +{ + crypto_dh_t *dh_new = tor_malloc_zero(sizeof(crypto_dh_t)); + tor_assert(dh); + tor_assert(dh->dh); + dh_new->dh = dh->dh; + DH_up_ref(dh->dh); + return dh_new; +} + +/** Return the length of the DH key in dh, in bytes. + */ +int +crypto_dh_get_bytes(crypto_dh_t *dh) +{ + tor_assert(dh); + return DH_size(dh->dh); +} + +/** Generate \<x,g^x\> for our part of the key exchange. Return 0 on + * success, -1 on failure. + */ +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. */ + crypto_openssl_log_errors(LOG_WARN, "generating DH key"); + 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. + */ + 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) { + 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 /* !(defined(OPENSSL_1_1_API)) */ + 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. */ + log_warn(LD_CRYPTO, "Weird! Our own DH key was invalid. I guess once-in-" + "the-universe chances really do happen. Trying again."); + /* Free and clear the keys, so OpenSSL will actually try again. */ + BN_clear_free(dh->dh->pub_key); + BN_clear_free(dh->dh->priv_key); + dh->dh->pub_key = dh->dh->priv_key = NULL; + goto again; + /* LCOV_EXCL_STOP */ + } +#endif /* defined(OPENSSL_1_1_API) */ + return 0; +} + +/** Generate g^x as necessary, and write the g^x for the key exchange + * as a pubkey_len-byte value into pubkey. Return 0 on + * success, -1 on failure. pubkey_len must be \>= DH1024_KEY_LEN. + */ +int +crypto_dh_get_public(crypto_dh_t *dh, char *pubkey, size_t pubkey_len) +{ + int bytes; + tor_assert(dh); + + const BIGNUM *dh_pub; + +#ifdef OPENSSL_1_1_API + const BIGNUM *dh_priv; + DH_get0_key(dh->dh, &dh_pub, &dh_priv); +#else + dh_pub = dh->dh->pub_key; +#endif /* defined(OPENSSL_1_1_API) */ + + 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_pub); + bytes = BN_num_bytes(dh_pub); + 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)", + (int) pubkey_len, bytes); + return -1; + } + + memset(pubkey, 0, pubkey_len); + BN_bn2bin(dh_pub, (unsigned char*)(pubkey+(pubkey_len-bytes))); + + return 0; +} + +/** Check for bad Diffie-Hellman public keys (g^x). Return 0 if the key is + * okay (in the subgroup [2,p-2]), or -1 if it's bad. + * 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) +{ + BIGNUM *x; + char *s; + tor_assert(bn); + x = BN_new(); + tor_assert(x); + if (BUG(!dh_param_p)) + init_dh_param(); //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_sub_word(x, 1); + if (BN_cmp(bn,x)>=0) { + log_fn(severity, LD_CRYPTO, "DH key must be at most p-2."); + goto err; + } + BN_clear_free(x); + return 0; + err: + BN_clear_free(x); + s = BN_bn2hex(bn); + log_fn(severity, LD_CRYPTO, "Rejecting insecure DH key [%s]", s); + OPENSSL_free(s); + return -1; +} + +/** Given a DH key exchange object, and our peer's value of g^y (as a + * pubkey_len-byte value in pubkey) generate + * secret_bytes_out bytes of shared key material and write them + * to secret_out. Return the number of bytes generated on success, + * or -1 on failure. + * + * (We generate key material by computing + * SHA1( g^xy || "\x00" ) || SHA1( g^xy || "\x01" ) || ... + * where || is concatenation.) + */ +ssize_t +crypto_dh_compute_secret(int severity, crypto_dh_t *dh, + const char *pubkey, size_t pubkey_len, + char *secret_out, size_t secret_bytes_out) +{ + char *secret_tmp = NULL; + BIGNUM *pubkey_bn = NULL; + size_t secret_len=0, secret_tmp_len=0; + int result=0; + tor_assert(dh); + tor_assert(secret_bytes_out/DIGEST_LEN <= 255); + tor_assert(pubkey_len < INT_MAX); + + if (!(pubkey_bn = BN_bin2bn((const unsigned char*)pubkey, + (int)pubkey_len, NULL))) + goto error; + if (tor_check_dh_key(severity, pubkey_bn)<0) { + /* Check for invalid public keys. */ + log_fn(severity, LD_CRYPTO,"Rejected invalid g^x"); + goto error; + } + secret_tmp_len = crypto_dh_get_bytes(dh); + secret_tmp = tor_malloc(secret_tmp_len); + result = DH_compute_key((unsigned char*)secret_tmp, pubkey_bn, dh->dh); + if (result < 0) { + log_warn(LD_CRYPTO,"DH_compute_key() failed."); + goto error; + } + secret_len = result; + if (crypto_expand_key_material_TAP((uint8_t*)secret_tmp, secret_len, + (uint8_t*)secret_out, secret_bytes_out)<0) + goto error; + secret_len = secret_bytes_out; + + goto done; + error: + result = -1; + done: + crypto_openssl_log_errors(LOG_WARN, "completing DH handshake"); + if (pubkey_bn) + BN_clear_free(pubkey_bn); + if (secret_tmp) { + memwipe(secret_tmp, 0, secret_tmp_len); + tor_free(secret_tmp); + } + if (result < 0) + return result; + else + return secret_len; +} + +/** Free a DH key exchange object. + */ +void +crypto_dh_free_(crypto_dh_t *dh) +{ + if (!dh) + return; + tor_assert(dh->dh); + DH_free(dh->dh); + tor_free(dh); +} + +void +crypto_dh_free_all(void) +{ + if (dh_param_p) + BN_clear_free(dh_param_p); + if (dh_param_p_tls) + BN_clear_free(dh_param_p_tls); + if (dh_param_g) + BN_clear_free(dh_param_g); + + dh_param_p = dh_param_p_tls = dh_param_g = NULL; +} diff --git a/src/lib/crypt_ops/include.am b/src/lib/crypt_ops/include.am index 018730627..a5541d49e 100644 --- a/src/lib/crypt_ops/include.am +++ b/src/lib/crypt_ops/include.am @@ -9,6 +9,7 @@ src_lib_libtor_crypt_ops_a_SOURCES = \ src/lib/crypt_ops/crypto_cipher.c \ src/lib/crypt_ops/crypto_curve25519.c \ src/lib/crypt_ops/crypto_dh.c \ + src/lib/crypt_ops/crypto_dh_openssl.c \ src/lib/crypt_ops/crypto_digest.c \ src/lib/crypt_ops/crypto_ed25519.c \ src/lib/crypt_ops/crypto_format.c \

1 0

[tor/master] Extract the shared part of crypto_dh_compute_secret.
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit ac9a470c641fd3ba826cdad07b1a7a495c00acba Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 17 09:16:11 2018 -0400 Extract the shared part of crypto_dh_compute_secret. --- src/lib/crypt_ops/crypto_dh.c | 47 +++++++++++++++++++++++++++++++++++ src/lib/crypt_ops/crypto_dh.h | 7 +++++- src/lib/crypt_ops/crypto_dh_openssl.c | 34 ++++++++++--------------- 3 files changed, 66 insertions(+), 22 deletions(-) diff --git a/src/lib/crypt_ops/crypto_dh.c b/src/lib/crypt_ops/crypto_dh.c index 8bb66379f..6f763e37a 100644 --- a/src/lib/crypt_ops/crypto_dh.c +++ b/src/lib/crypt_ops/crypto_dh.c @@ -42,3 +42,50 @@ const char OAKLEY_PRIME_2[] = "302B0A6DF25F14374FE1356D6D51C245E485B576625E7EC6F44C42E9" "A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE6" "49286651ECE65381FFFFFFFFFFFFFFFF"; + +/** Given a DH key exchange object, and our peer's value of g^y (as a + * pubkey_len-byte value in pubkey) generate + * secret_bytes_out bytes of shared key material and write them + * to secret_out. Return the number of bytes generated on success, + * or -1 on failure. + * + * (We generate key material by computing + * SHA1( g^xy || "\x00" ) || SHA1( g^xy || "\x01" ) || ... + * where || is concatenation.) + */ +ssize_t +crypto_dh_compute_secret(int severity, crypto_dh_t *dh, + const char *pubkey, size_t pubkey_len, + char *secret_out, size_t secret_bytes_out) +{ + tor_assert(secret_bytes_out/DIGEST_LEN <= 255); + + unsigned char *secret_tmp = NULL; + size_t secret_len=0, secret_tmp_len=0; + secret_tmp_len = crypto_dh_get_bytes(dh); + secret_tmp = tor_malloc(secret_tmp_len); + + ssize_t result = crypto_dh_handshake(severity, dh, pubkey, pubkey_len, + secret_tmp, secret_tmp_len); + if (result < 0) + goto error; + + secret_len = result; + if (crypto_expand_key_material_TAP(secret_tmp, secret_len, + (uint8_t*)secret_out, secret_bytes_out)<0) + goto error; + secret_len = secret_bytes_out; + + goto done; + error: + result = -1; + done: + if (secret_tmp) { + memwipe(secret_tmp, 0, secret_tmp_len); + tor_free(secret_tmp); + } + if (result < 0) + return result; + else + return secret_len; +} diff --git a/src/lib/crypt_ops/crypto_dh.h b/src/lib/crypt_ops/crypto_dh.h index 2d25ba8c8..f8e4e4f43 100644 --- a/src/lib/crypt_ops/crypto_dh.h +++ b/src/lib/crypt_ops/crypto_dh.h @@ -40,7 +40,11 @@ ssize_t crypto_dh_compute_secret(int severity, crypto_dh_t *dh, void crypto_dh_free_(crypto_dh_t *dh); #define crypto_dh_free(dh) FREE_AND_NULL(crypto_dh_t, crypto_dh_free_, (dh)) -/* Crypto DH free */ +ssize_t crypto_dh_handshake(int severity, crypto_dh_t *dh, + const char *pubkey, size_t pubkey_len, + unsigned char *secret_out, + size_t secret_bytes_out); + void crypto_dh_free_all(void); /* Prototypes for private functions only used by tortls.c, crypto.c, and the @@ -48,4 +52,5 @@ void crypto_dh_free_all(void); struct dh_st; struct dh_st *crypto_dh_get_dh_(crypto_dh_t *dh); + #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 4457b1814..395058d92 100644 --- a/src/lib/crypt_ops/crypto_dh_openssl.c +++ b/src/lib/crypt_ops/crypto_dh_openssl.c @@ -405,27 +405,29 @@ tor_check_dh_key(int severity, const BIGNUM *bn) /** Given a DH key exchange object, and our peer's value of g^y (as a * pubkey_len-byte value in pubkey) generate - * secret_bytes_out bytes of shared key material and write them - * to secret_out. Return the number of bytes generated on success, + * g^xy as a big-endian integer in secret_out. + * Return the number of bytes generated on success, * or -1 on failure. * - * (We generate key material by computing - * SHA1( g^xy || "\x00" ) || SHA1( g^xy || "\x01" ) || ... - * where || is concatenation.) + * This function MUST validate that g^y is actually in the group. */ ssize_t -crypto_dh_compute_secret(int severity, crypto_dh_t *dh, - const char *pubkey, size_t pubkey_len, - char *secret_out, size_t secret_bytes_out) +crypto_dh_handshake(int severity, crypto_dh_t *dh, + const char *pubkey, size_t pubkey_len, + unsigned char *secret_out, size_t secret_bytes_out) { - char *secret_tmp = NULL; BIGNUM *pubkey_bn = NULL; - size_t secret_len=0, secret_tmp_len=0; + size_t secret_len=0; int result=0; + tor_assert(dh); tor_assert(secret_bytes_out/DIGEST_LEN <= 255); tor_assert(pubkey_len < INT_MAX); + if (BUG(crypto_dh_get_bytes(dh) > (int)secret_bytes_out)) { + goto error; + } + if (!(pubkey_bn = BN_bin2bn((const unsigned char*)pubkey, (int)pubkey_len, NULL))) goto error; @@ -434,18 +436,12 @@ crypto_dh_compute_secret(int severity, crypto_dh_t *dh, log_fn(severity, LD_CRYPTO,"Rejected invalid g^x"); goto error; } - secret_tmp_len = crypto_dh_get_bytes(dh); - secret_tmp = tor_malloc(secret_tmp_len); - result = DH_compute_key((unsigned char*)secret_tmp, pubkey_bn, dh->dh); + result = DH_compute_key(secret_out, pubkey_bn, dh->dh); if (result < 0) { log_warn(LD_CRYPTO,"DH_compute_key() failed."); goto error; } secret_len = result; - if (crypto_expand_key_material_TAP((uint8_t*)secret_tmp, secret_len, - (uint8_t*)secret_out, secret_bytes_out)<0) - goto error; - secret_len = secret_bytes_out; goto done; error: @@ -454,10 +450,6 @@ crypto_dh_compute_secret(int severity, crypto_dh_t *dh, crypto_openssl_log_errors(LOG_WARN, "completing DH handshake"); if (pubkey_bn) BN_clear_free(pubkey_bn); - if (secret_tmp) { - memwipe(secret_tmp, 0, secret_tmp_len); - tor_free(secret_tmp); - } if (result < 0) return result; else

1 0

[tor/master] Refactor the dependency between tortls and crypto_dh.
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 32bbc8f6b5e7d964439c6ba91f87f7d5fba673cc Author: Nick Mathewson <nickm(a)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;

1 0

[tor/master] Additional tests for NSS DH
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit f5e22358b0d8f71e4d6bad4b72859e2d3f38c700 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 17 13:08:34 2018 -0400 Additional tests for NSS DH Notably, there's a test to make sure that it round-trips with OpenSSL, if OpenSSL is enabled. --- src/test/test_crypto.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 17f23b96b..029e98a63 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -26,6 +26,13 @@ #include <unistd.h> #endif +#if defined(ENABLE_OPENSSL) +#include "lib/crypt_ops/compat_openssl.h" +DISABLE_GCC_WARNING(redundant-decls) +#include <openssl/dh.h> +ENABLE_GCC_WARNING(redundant-decls) +#endif + /** Run unit tests for Diffie-Hellman functionality. */ static void test_crypto_dh(void *arg) @@ -38,6 +45,11 @@ test_crypto_dh(void *arg) char s1[DH1024_KEY_LEN]; char s2[DH1024_KEY_LEN]; ssize_t s1len, s2len; +#ifdef ENABLE_OPENSSL + crypto_dh_t *dh3 = NULL; + DH *dh4 = NULL; + BIGNUM *pubkey_tmp = NULL; +#endif (void)arg; tt_int_op(crypto_dh_get_bytes(dh1),OP_EQ, DH1024_KEY_LEN); @@ -92,6 +104,10 @@ test_crypto_dh(void *arg) s1len = crypto_dh_compute_secret(LOG_WARN, dh1, "\x02", 1, s1, 50); tt_int_op(50, OP_EQ, s1len); + /* 2 a second time is still okay, though weird. */ + s1len = crypto_dh_compute_secret(LOG_WARN, dh1, "\x02", 1, s1, 50); + tt_int_op(50, OP_EQ, s1len); + const char P[] = "FFFFFFFFFFFFFFFFC90FDAA22168C234C4C6628B80DC1CD129024E08" "8A67CC74020BBEA63B139B22514A08798E3404DDEF9519B3CD3A431B" @@ -158,10 +174,50 @@ test_crypto_dh(void *arg) tt_int_op(s1len, OP_EQ, -1); } +#if defined(ENABLE_OPENSSL) + { + /* 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)); + + dh4 = crypto_dh_new_openssl_tls(); + tt_assert(DH_generate_key(dh4)); + const BIGNUM *pk=NULL; +#ifdef OPENSSL_1_1_API + const BIGNUM *sk=NULL; + DH_get0_key(dh4, &pk, &sk); +#else + pk = dh4->pub_key; +#endif + tt_assert(pk); + tt_int_op(BN_num_bytes(pk), OP_LE, DH1024_KEY_LEN); + 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))); + + s1len = crypto_dh_handshake(LOG_WARN, dh3, p2, DH1024_KEY_LEN, + (unsigned char *)s1, sizeof(s1)); + pubkey_tmp = BN_bin2bn((unsigned char *)p1, DH1024_KEY_LEN, NULL); + s2len = DH_compute_key((unsigned char *)s2, pubkey_tmp, dh4); + + tt_int_op(s1len, OP_EQ, s2len); + tt_int_op(s1len, OP_GT, 0); + tt_mem_op(s1, OP_EQ, s2, s1len); + } +#endif + done: crypto_dh_free(dh1); crypto_dh_free(dh2); crypto_dh_free(dh1_dup); +#ifdef ENABLE_OPENSSL + crypto_dh_free(dh3); + if (dh4) + DH_free(dh4); + if (pubkey_tmp) + BN_free(pubkey_tmp); +#endif } static void

1 0

[tor/master] Implement DH in NSS.
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 17ea931ac70af3cc11c1729b09b8e1ff17a53348 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 17 11:27:08 2018 -0400 Implement DH in NSS. --- src/lib/crypt_ops/crypto_dh.c | 22 ++++ src/lib/crypt_ops/crypto_dh.h | 11 +- src/lib/crypt_ops/crypto_dh_nss.c | 207 ++++++++++++++++++++++++++++++++++ src/lib/crypt_ops/crypto_dh_openssl.c | 90 ++++++--------- src/lib/crypt_ops/crypto_init.c | 2 + src/lib/crypt_ops/include.am | 4 +- src/test/testing_common.c | 1 - 7 files changed, 281 insertions(+), 56 deletions(-) diff --git a/src/lib/crypt_ops/crypto_dh.c b/src/lib/crypt_ops/crypto_dh.c index 6f763e37a..673ef311f 100644 --- a/src/lib/crypt_ops/crypto_dh.c +++ b/src/lib/crypt_ops/crypto_dh.c @@ -43,6 +43,28 @@ const char OAKLEY_PRIME_2[] = "A637ED6B0BFF5CB6F406B7EDEE386BFB5A899FA5AE9F24117C4B1FE6" "49286651ECE65381FFFFFFFFFFFFFFFF"; +void +crypto_dh_init(void) +{ +#ifdef ENABLE_OPENSSL + crypto_dh_init_openssl(); +#endif +#ifdef ENABLE_NSS + crypto_dh_init_nss(); +#endif +} + +void +crypto_dh_free_all(void) +{ +#ifdef ENABLE_OPENSSL + crypto_dh_free_all_openssl(); +#endif +#ifdef ENABLE_NSS + crypto_dh_free_all_nss(); +#endif +} + /** Given a DH key exchange object, and our peer's value of g^y (as a * pubkey_len-byte value in pubkey) generate * secret_bytes_out bytes of shared key material and write them diff --git a/src/lib/crypt_ops/crypto_dh.h b/src/lib/crypt_ops/crypto_dh.h index 953362696..6e79a6404 100644 --- a/src/lib/crypt_ops/crypto_dh.h +++ b/src/lib/crypt_ops/crypto_dh.h @@ -27,7 +27,7 @@ extern const char OAKLEY_PRIME_2[]; #define DH_TYPE_CIRCUIT 1 #define DH_TYPE_REND 2 #define DH_TYPE_TLS 3 -void crypto_set_tls_dh_prime(void); +void crypto_dh_init(void); crypto_dh_t *crypto_dh_new(int dh_type); crypto_dh_t *crypto_dh_dup(const crypto_dh_t *dh); int crypto_dh_get_bytes(crypto_dh_t *dh); @@ -52,4 +52,13 @@ void crypto_dh_free_all(void); struct dh_st; struct dh_st *crypto_dh_new_openssl_tls(void); +#ifdef ENABLE_OPENSSL +void crypto_dh_init_openssl(void); +void crypto_dh_free_all_openssl(void); +#endif +#ifdef ENABLE_OPENSSL +void crypto_dh_init_nss(void); +void crypto_dh_free_all_nss(void); +#endif + #endif /* !defined(TOR_CRYPTO_DH_H) */ diff --git a/src/lib/crypt_ops/crypto_dh_nss.c b/src/lib/crypt_ops/crypto_dh_nss.c new file mode 100644 index 000000000..647dfb4e5 --- /dev/null +++ b/src/lib/crypt_ops/crypto_dh_nss.c @@ -0,0 +1,207 @@ +/* Copyright (c) 2001, Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file crypto_dh_nss.h + * + * \brief NSS implementation of Diffie-Hellman over Z_p. + **/ + +#include "lib/crypt_ops/crypto_dh.h" +#include "lib/crypt_ops/crypto_nss_mgt.h" + +#include "lib/encoding/binascii.h" +#include "lib/log/util_bug.h" +#include "lib/malloc/malloc.h" + +#include <cryptohi.h> +#include <keyhi.h> +#include <pk11pub.h> + +static int dh_initialized = 0; +static SECKEYDHParams tls_dh_param, circuit_dh_param; +static unsigned char tls_dh_prime_data[DH1024_KEY_LEN]; +static unsigned char circuit_dh_prime_data[DH1024_KEY_LEN]; +static unsigned char dh_generator_data[1]; + +void +crypto_dh_init_nss(void) +{ + if (dh_initialized) + return; + + int r; + r = base16_decode((char*)tls_dh_prime_data, + sizeof(tls_dh_prime_data), + TLS_DH_PRIME, strlen(TLS_DH_PRIME)); + tor_assert(r == DH1024_KEY_LEN); + r = base16_decode((char*)circuit_dh_prime_data, + sizeof(circuit_dh_prime_data), + OAKLEY_PRIME_2, strlen(OAKLEY_PRIME_2)); + tor_assert(r == DH1024_KEY_LEN); + dh_generator_data[0] = DH_GENERATOR; + + tls_dh_param.prime.data = tls_dh_prime_data; + tls_dh_param.prime.len = DH1024_KEY_LEN; + tls_dh_param.base.data = dh_generator_data; + tls_dh_param.base.len = 1; + + circuit_dh_param.prime.data = circuit_dh_prime_data; + circuit_dh_param.prime.len = DH1024_KEY_LEN; + circuit_dh_param.base.data = dh_generator_data; + circuit_dh_param.base.len = 1; +} + +void +crypto_dh_free_all_nss(void) +{ + dh_initialized = 0; +} + +struct crypto_dh_t { + int dh_type; // XXXX let's remove this later on. + SECKEYPrivateKey *seckey; + SECKEYPublicKey *pubkey; +}; + +crypto_dh_t * +crypto_dh_new(int dh_type) +{ + crypto_dh_t *r = tor_malloc_zero(sizeof(crypto_dh_t)); + r->dh_type = dh_type; + return r; +} + +crypto_dh_t * +crypto_dh_dup(const crypto_dh_t *dh) +{ + tor_assert(dh); + crypto_dh_t *r = crypto_dh_new(dh->dh_type); + if (dh->seckey) + r->seckey = SECKEY_CopyPrivateKey(dh->seckey); + if (dh->pubkey) + r->pubkey = SECKEY_CopyPublicKey(dh->pubkey); + return r; +} + +int +crypto_dh_get_bytes(crypto_dh_t *dh) +{ + (void)dh; + return DH1024_KEY_LEN; +} + +int +crypto_dh_generate_public(crypto_dh_t *dh) +{ + tor_assert(dh); + SECKEYDHParams *p; + if (dh->dh_type == DH_TYPE_TLS) + p = &tls_dh_param; + else + p = &circuit_dh_param; + + dh->seckey = SECKEY_CreateDHPrivateKey(p, &dh->pubkey, NULL); + if (!dh->seckey || !dh->pubkey) + return -1; + else + return 0; +} +int +crypto_dh_get_public(crypto_dh_t *dh, char *pubkey_out, + size_t pubkey_out_len) +{ + tor_assert(dh); + tor_assert(pubkey_out); + if (!dh->pubkey) { + if (crypto_dh_generate_public(dh) < 0) + return -1; + } + + const SECItem *item = &dh->pubkey->u.dh.publicValue; + + if (item->len > pubkey_out_len) + return -1; + + /* Left-pad the result with 0s. */ + memset(pubkey_out, 0, pubkey_out_len); + memcpy(pubkey_out + pubkey_out_len - item->len, + item->data, + item->len); + + return 0; +} + +void +crypto_dh_free_(crypto_dh_t *dh) +{ + if (!dh) + return; + if (dh->seckey) + SECKEY_DestroyPrivateKey(dh->seckey); + if (dh->pubkey) + SECKEY_DestroyPublicKey(dh->pubkey); + tor_free(dh); +} + +ssize_t +crypto_dh_handshake(int severity, crypto_dh_t *dh, + const char *pubkey, size_t pubkey_len, + unsigned char *secret_out, + size_t secret_bytes_out) +{ + tor_assert(dh); + if (pubkey_len > DH1024_KEY_LEN) + return -1; + if (!dh->pubkey || !dh->seckey) + return -1; + if (secret_bytes_out < DH1024_KEY_LEN) + return -1; + + SECKEYPublicKey peer_key; + memset(&peer_key, 0, sizeof(peer_key)); + peer_key.keyType = dhKey; + peer_key.pkcs11ID = CK_INVALID_HANDLE; + + if (dh->dh_type == DH_TYPE_TLS) + peer_key.u.dh.prime.data = tls_dh_prime_data; // should never use this code + else + peer_key.u.dh.prime.data = circuit_dh_prime_data; + peer_key.u.dh.prime.len = DH1024_KEY_LEN; + peer_key.u.dh.base.data = dh_generator_data; + peer_key.u.dh.base.len = 1; + peer_key.u.dh.publicValue.data = (unsigned char *)pubkey; + peer_key.u.dh.publicValue.len = pubkey_len; + + PK11SymKey *sym = PK11_PubDerive(dh->seckey, &peer_key, + PR_FALSE, NULL, NULL, CKM_DH_PKCS_DERIVE, + CKM_GENERIC_SECRET_KEY_GEN /* ??? */, + CKA_DERIVE, 0, NULL); + if (! sym) { + crypto_nss_log_errors(severity, "deriving a DH shared secret"); + return -1; + } + + SECStatus s = PK11_ExtractKeyValue(sym); + if (s != SECSuccess) { + crypto_nss_log_errors(severity, "extracting a DH shared secret"); + PK11_FreeSymKey(sym); + return -1; + } + + SECItem *result = PK11_GetKeyData(sym); + tor_assert(result); // This cannot fail. + if (BUG(result->len > secret_bytes_out)) { + PK11_FreeSymKey(sym); + return -1; + } + + ssize_t len = result->len; + memcpy(secret_out, result->data, len); + PK11_FreeSymKey(sym); + + return len; +} diff --git a/src/lib/crypt_ops/crypto_dh_openssl.c b/src/lib/crypt_ops/crypto_dh_openssl.c index d66031afd..54946458d 100644 --- a/src/lib/crypt_ops/crypto_dh_openssl.c +++ b/src/lib/crypt_ops/crypto_dh_openssl.c @@ -26,14 +26,17 @@ ENABLE_GCC_WARNING(redundant-decls) #include <openssl/bn.h> #include <string.h> +#ifndef ENABLE_NSS 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.*/ struct crypto_dh_t { DH *dh; /**< The openssl DH object */ }; +#endif + +static DH *new_openssl_dh_from_params(BIGNUM *p, BIGNUM *g); /** Shared P parameter for our circuit-crypto DH key exchanges. */ static BIGNUM *dh_param_p = NULL; @@ -96,6 +99,22 @@ crypto_validate_dh_params(const BIGNUM *p, const BIGNUM *g) return ret; } +/** + * Helper: convert hex to a bignum, and return it. Assert that the + * operation was successful. + */ +static BIGNUM * +bignum_from_hex(const char *hex) +{ + BIGNUM *result = BN_new(); + tor_assert(result); + + int r = BN_hex2bn(&result, hex); + tor_assert(r); + tor_assert(result); + return result; +} + /** Set the global Diffie-Hellman generator, used for both TLS and internal * DH stuff. */ @@ -117,62 +136,23 @@ crypto_set_dh_generator(void) dh_param_g = generator; } -/** Set the global TLS Diffie-Hellman modulus. Use the Apache mod_ssl DH - * modulus. */ +/** Initialize our DH parameters. Idempotent. */ void -crypto_set_tls_dh_prime(void) +crypto_dh_init_openssl(void) { - BIGNUM *tls_prime = NULL; - int r; - - /* If the space is occupied, free the previous TLS DH prime */ - if (BUG(dh_param_p_tls)) { - /* LCOV_EXCL_START - * - * We shouldn't be calling this twice. - */ - BN_clear_free(dh_param_p_tls); - dh_param_p_tls = NULL; - /* LCOV_EXCL_STOP */ - } - - tls_prime = BN_new(); - tor_assert(tls_prime); - - r = BN_hex2bn(&tls_prime, TLS_DH_PRIME); - tor_assert(r); + if (dh_param_p && dh_param_g && dh_param_p_tls) + return; - tor_assert(tls_prime); + tor_assert(dh_param_g == NULL); + tor_assert(dh_param_p == NULL); + tor_assert(dh_param_p_tls == NULL); - dh_param_p_tls = tls_prime; crypto_set_dh_generator(); - tor_assert(0 == crypto_validate_dh_params(dh_param_p_tls, dh_param_g)); -} - -/** Initialize dh_param_p and dh_param_g if they are not already - * set. */ -static void -init_dh_param(void) -{ - BIGNUM *circuit_dh_prime; - int r; - if (BUG(dh_param_p && dh_param_g)) - return; // LCOV_EXCL_LINE This function isn't supposed to be called twice. + dh_param_p = bignum_from_hex(OAKLEY_PRIME_2); + dh_param_p_tls = bignum_from_hex(TLS_DH_PRIME); - circuit_dh_prime = BN_new(); - tor_assert(circuit_dh_prime); - - r = BN_hex2bn(&circuit_dh_prime, OAKLEY_PRIME_2); - tor_assert(r); - - /* Set the new values as the global DH parameters. */ - dh_param_p = circuit_dh_prime; - crypto_set_dh_generator(); tor_assert(0 == crypto_validate_dh_params(dh_param_p, dh_param_g)); - - if (!dh_param_p_tls) { - crypto_set_tls_dh_prime(); - } + tor_assert(0 == crypto_validate_dh_params(dh_param_p_tls, dh_param_g)); } /** Number of bits to use when choosing the x or y value in a Diffie-Hellman @@ -189,6 +169,7 @@ crypto_dh_new_openssl_tls(void) return new_openssl_dh_from_params(dh_param_p_tls, dh_param_g); } +#ifndef ENABLE_NSS /** Allocate and return a new DH object for a key exchange. Returns NULL on * failure. */ @@ -201,7 +182,7 @@ crypto_dh_new(int dh_type) dh_type == DH_TYPE_REND); if (!dh_param_p) - init_dh_param(); + crypto_dh_init(); BIGNUM *dh_p = NULL; if (dh_type == DH_TYPE_TLS) { @@ -215,6 +196,7 @@ crypto_dh_new(int dh_type) tor_free(res); // sets res to NULL. return res; } +#endif /** Create and return a new openssl DH from a given prime and generator. */ static DH * @@ -260,6 +242,7 @@ new_openssl_dh_from_params(BIGNUM *p, BIGNUM *g) /* LCOV_EXCL_STOP */ } +#ifndef ENABLE_NSS /** Return a copy of dh, sharing its internal state. */ crypto_dh_t * crypto_dh_dup(const crypto_dh_t *dh) @@ -386,7 +369,7 @@ tor_check_dh_key(int severity, const BIGNUM *bn) x = BN_new(); tor_assert(x); if (BUG(!dh_param_p)) - init_dh_param(); //LCOV_EXCL_LINE we already checked whether we did this. + 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."); @@ -472,9 +455,10 @@ crypto_dh_free_(crypto_dh_t *dh) DH_free(dh->dh); tor_free(dh); } +#endif void -crypto_dh_free_all(void) +crypto_dh_free_all_openssl(void) { if (dh_param_p) BN_clear_free(dh_param_p); diff --git a/src/lib/crypt_ops/crypto_init.c b/src/lib/crypt_ops/crypto_init.c index b651474cf..620fe8e1b 100644 --- a/src/lib/crypt_ops/crypto_init.c +++ b/src/lib/crypt_ops/crypto_init.c @@ -83,6 +83,8 @@ crypto_global_init(int useAccel, const char *accelName, const char *accelDir) crypto_global_initialized_ = 1; + crypto_dh_init(); + #ifdef ENABLE_OPENSSL if (crypto_openssl_late_init(useAccel, accelName, accelDir) < 0) return -1; diff --git a/src/lib/crypt_ops/include.am b/src/lib/crypt_ops/include.am index a5541d49e..05451cc0e 100644 --- a/src/lib/crypt_ops/include.am +++ b/src/lib/crypt_ops/include.am @@ -25,10 +25,12 @@ src_lib_libtor_crypt_ops_a_SOURCES = \ if USE_NSS src_lib_libtor_crypt_ops_a_SOURCES += \ src/lib/crypt_ops/aes_nss.c \ + src/lib/crypt_ops/crypto_dh_nss.c \ src/lib/crypt_ops/crypto_nss_mgt.c else src_lib_libtor_crypt_ops_a_SOURCES += \ - src/lib/crypt_ops/aes_openssl.c + src/lib/crypt_ops/aes_openssl.c \ + src/lib/crypt_ops/crypto_dh_openssl.c endif if USE_OPENSSL diff --git a/src/test/testing_common.c b/src/test/testing_common.c index 1611a54b6..6957342c6 100644 --- a/src/test/testing_common.c +++ b/src/test/testing_common.c @@ -292,7 +292,6 @@ main(int c, const char **v) printf("Can't initialize crypto subsystem; exiting.\n"); return 1; } - crypto_set_tls_dh_prime(); if (crypto_seed_rng() < 0) { printf("Couldn't seed RNG; exiting.\n"); return 1;

1 0

[tor/master] Only link crypto_dh_openssl.c once
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit 17f922d3719837fade1888dfa7cc99ac801ad800 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Jul 18 10:19:20 2018 -0400 Only link crypto_dh_openssl.c once (We do this unconditionally, since we still need it for tortls.c) --- src/lib/crypt_ops/include.am | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib/crypt_ops/include.am b/src/lib/crypt_ops/include.am index 05451cc0e..803d44885 100644 --- a/src/lib/crypt_ops/include.am +++ b/src/lib/crypt_ops/include.am @@ -29,8 +29,7 @@ src_lib_libtor_crypt_ops_a_SOURCES += \ src/lib/crypt_ops/crypto_nss_mgt.c else src_lib_libtor_crypt_ops_a_SOURCES += \ - src/lib/crypt_ops/aes_openssl.c \ - src/lib/crypt_ops/crypto_dh_openssl.c + src/lib/crypt_ops/aes_openssl.c endif if USE_OPENSSL

1 0

[tor/master] Merge branch 'nss_dh_squashed' into nss_dh_squashed_merged
by nickm＠torproject.org 05 Sep '18

05 Sep '18

commit fdaa483098d723b4be24a4e861c4280a67a3d4b0 Merge: 7e4ac0283 17f922d37 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jul 31 19:56:23 2018 -0400 Merge branch 'nss_dh_squashed' into nss_dh_squashed_merged Makefile.am | 8 + changes/feature26815 | 3 + changes/feature26816 | 11 + config.rust.in | 2 + configure.ac | 22 + src/app/include.am | 8 +- src/core/crypto/onion_ntor.c | 2 +- src/core/crypto/relay_crypto.c | 2 +- src/core/mainloop/main.c | 1 + src/core/or/or.h | 3 +- src/ext/tinytest.c | 8 + src/feature/nodelist/parsecommon.c | 2 +- src/feature/relay/router.c | 1 + src/feature/rend/rendmid.c | 2 +- src/lib/crypt_ops/aes_nss.c | 106 +++++ src/lib/crypt_ops/{aes.c => aes_openssl.c} | 4 +- src/lib/crypt_ops/crypto.c | 509 ------------------------ src/lib/crypt_ops/crypto_cipher.c | 190 +++++++++ src/lib/crypt_ops/{crypto.h => crypto_cipher.h} | 25 +- src/lib/crypt_ops/crypto_dh.c | 484 ++-------------------- src/lib/crypt_ops/crypto_dh.h | 23 +- src/lib/crypt_ops/crypto_dh_nss.c | 207 ++++++++++ src/lib/crypt_ops/crypto_dh_openssl.c | 471 ++++++++++++++++++++++ src/lib/crypt_ops/crypto_digest.c | 258 +++++++++++- src/lib/crypt_ops/crypto_digest.h | 3 + src/lib/crypt_ops/crypto_init.c | 141 +++++++ src/lib/crypt_ops/crypto_init.h | 29 ++ src/lib/crypt_ops/crypto_nss_mgt.c | 102 +++++ src/lib/crypt_ops/crypto_nss_mgt.h | 33 ++ src/lib/crypt_ops/crypto_openssl_mgt.c | 228 ++++++++++- src/lib/crypt_ops/crypto_openssl_mgt.h | 15 +- src/lib/crypt_ops/crypto_pwbox.c | 2 +- src/lib/crypt_ops/crypto_rand.c | 110 ++++- src/lib/crypt_ops/crypto_rsa.c | 48 ++- src/lib/crypt_ops/crypto_rsa.h | 5 + src/lib/crypt_ops/crypto_s2k.c | 2 +- src/lib/crypt_ops/crypto_util.c | 23 -- src/lib/crypt_ops/crypto_util.h | 9 - src/lib/crypt_ops/include.am | 29 +- src/lib/include.libdonna.am | 2 +- src/lib/tls/include.am | 5 +- src/lib/tls/tortls.c | 8 +- src/rust/build.rs | 1 + src/test/bench.c | 6 +- src/test/fuzz/fuzzing_common.c | 15 +- src/test/fuzz/include.am | 4 +- src/test/include.am | 22 +- src/test/test-timers.c | 5 + src/test/test_crypto.c | 57 +++ src/test/test_hs_client.c | 2 +- src/test/test_hs_ntor_cl.c | 7 +- src/test/test_introduce.c | 2 +- src/test/test_ntor_cl.c | 8 +- src/test/test_relaycell.c | 2 +- src/test/test_routerkeys.c | 2 +- src/test/test_util_slow.c | 2 +- src/test/test_workqueue.c | 1 + src/test/testing_common.c | 9 +- src/tools/include.am | 10 +- src/tools/tor-gencert.c | 14 +- 60 files changed, 2208 insertions(+), 1107 deletions(-) diff --cc Makefile.am index 3df35ad3f,01ed8c935..adf02122f --- a/Makefile.am +++ b/Makefile.am @@@ -131,8 -126,15 +131,16 @@@ TOR_INTERNAL_TESTING_LIBS = $(TOR_UTIL_TESTING_LIBS) \ src/trunnel/libor-trunnel-testing.a \ src/lib/libtor-trace.a +endif + TOR_LDFLAGS_CRYPTLIB=@TOR_LDFLAGS_openssl@ + TOR_LIBS_CRYPTLIB=@TOR_OPENSSL_LIBS@ + TOR_CFLAGS_CRYPTLIB= + if USE_NSS + TOR_CFLAGS_CRYPTLIB+=@NSS_CFLAGS@ + TOR_LIBS_CRYPTLIB+=@NSS_LIBS@ + endif + # All libraries used to link tor-cov include src/include.am diff --cc src/lib/crypt_ops/include.am index 017d7956d,803d44885..8647a91e8 --- a/src/lib/crypt_ops/include.am +++ b/src/lib/crypt_ops/include.am @@@ -14,8 -14,7 +14,9 @@@ src_lib_libtor_crypt_ops_a_SOURCES = src/lib/crypt_ops/crypto_ed25519.c \ src/lib/crypt_ops/crypto_format.c \ src/lib/crypt_ops/crypto_hkdf.c \ + src/lib/crypt_ops/crypto_init.c \ + src/lib/crypt_ops/crypto_ope.c \ + src/lib/crypt_ops/crypto_openssl_mgt.c \ src/lib/crypt_ops/crypto_pwbox.c \ src/lib/crypt_ops/crypto_rand.c \ src/lib/crypt_ops/crypto_rsa.c \ @@@ -36,10 -53,11 +55,12 @@@ noinst_HEADERS += src/lib/crypt_ops/crypto_digest.h \ src/lib/crypt_ops/crypto_ed25519.h \ src/lib/crypt_ops/crypto_format.h \ - src/lib/crypt_ops/crypto.h \ + src/lib/crypt_ops/crypto_cipher.h \ src/lib/crypt_ops/crypto_hkdf.h \ + src/lib/crypt_ops/crypto_init.h \ + src/lib/crypt_ops/crypto_nss_mgt.h \ src/lib/crypt_ops/crypto_openssl_mgt.h \ + src/lib/crypt_ops/crypto_ope.h \ src/lib/crypt_ops/crypto_pwbox.h \ src/lib/crypt_ops/crypto_rand.h \ src/lib/crypt_ops/crypto_rsa.h \ diff --cc src/tools/include.am index d5924dda5,7c5d3f0bc..cdd5616fb --- a/src/tools/include.am +++ b/src/tools/include.am @@@ -26,18 -26,9 +26,18 @@@ src_tools_tor_gencert_LDADD = $(TOR_CRYPTO_LIBS) \ $(TOR_UTIL_LIBS) \ $(rust_ldadd) \ - @TOR_LIB_MATH@ @TOR_ZLIB_LIBS@ @TOR_OPENSSL_LIBS@ \ + @TOR_LIB_MATH@ @TOR_ZLIB_LIBS@ $(TOR_LIBS_CRYPTLIB) \ @TOR_LIB_WS32@ @TOR_LIB_IPHLPAPI@ @TOR_LIB_GDI@ @TOR_LIB_USERENV@ @CURVE25519_LIBS@ +src_tools_tor_print_ed_signing_cert_SOURCES = src/tools/tor-print-ed-signing-cert.c +src_tools_tor_print_ed_signing_cert_LDFLAGS = @TOR_LDFLAGS_zlib@ @TOR_LDFLAGS_openssl@ +src_tools_tor_print_ed_signing_cert_LDADD = \ + src/trunnel/libor-trunnel.a \ + $(TOR_CRYPTO_LIBS) \ + $(TOR_UTIL_LIBS) \ - @TOR_LIB_MATH@ @TOR_OPENSSL_LIBS@ \ ++ @TOR_LIB_MATH@ $(TOR_LIBS_CRYPTLIB) \ + @TOR_LIB_WS32@ @TOR_LIB_USERENV@ + if COVERAGE_ENABLED src_tools_tor_cov_gencert_SOURCES = src/tools/tor-gencert.c src_tools_tor_cov_gencert_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_CPPFLAGS)

1 0