commit 511ca9b91cec03f4ef6f23adccd5cdd47a245e5f Author: Nick Mathewson nickm@torproject.org Date: Sat Mar 14 12:40:55 2015 -0400

Remove DynamicDHGroups as obsoleted by PluggableTransports or P256.

Closes ticket 13736. --- changes/bug13736 | 5 + src/common/crypto.c | 251 +-------------------------------------------- src/common/crypto.h | 4 +- src/or/config.c | 25 +---- src/or/or.h | 2 - src/test/testing_common.c | 2 +- 6 files changed, 14 insertions(+), 275 deletions(-)

diff --git a/changes/bug13736 b/changes/bug13736 new file mode 100644 index 0000000..686d2f1 --- /dev/null +++ b/changes/bug13736 @@ -0,0 +1,5 @@ + o Removed features: + - Remove the (seldom-used) DynamicDHGroups feature. For + anti-fingerprinting we now recommend pluggable transports; for + forward-secrecy in TLS, we now use the P-256 group. + Closes ticket 13736. diff --git a/src/common/crypto.c b/src/common/crypto.c index 218c7be..55f816a 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -1768,235 +1768,12 @@ static BIGNUM *dh_param_p_tls = NULL; /** Shared G parameter for our DH key exchanges. */ static BIGNUM *dh_param_g = NULL;

-/** Generate and return a reasonable and safe DH parameter p. */ -static BIGNUM * -crypto_generate_dynamic_dh_modulus(void) -{ - BIGNUM *dynamic_dh_modulus; - DH *dh_parameters; - int r, dh_codes; - char *s; - - dynamic_dh_modulus = BN_new(); - tor_assert(dynamic_dh_modulus); - - dh_parameters = DH_new(); - tor_assert(dh_parameters); - - r = DH_generate_parameters_ex(dh_parameters, - DH_BYTES*8, DH_GENERATOR, NULL); - tor_assert(r == 0); - - r = DH_check(dh_parameters, &dh_codes); - tor_assert(r && !dh_codes); - - BN_copy(dynamic_dh_modulus, dh_parameters->p); - tor_assert(dynamic_dh_modulus); - - DH_free(dh_parameters); - - { /* log the dynamic DH modulus: */ - s = BN_bn2hex(dynamic_dh_modulus); - tor_assert(s); - log_info(LD_OR, "Dynamic DH modulus generated: [%s]", s); - OPENSSL_free(s); - } - - return dynamic_dh_modulus; -} - -/** Store our dynamic DH modulus (and its group parameters) to - <b>fname</b> for future use. */ -static int -crypto_store_dynamic_dh_modulus(const char *fname) -{ - int len, new_len; - DH *dh = NULL; - unsigned char *dh_string_repr = NULL; - char *base64_encoded_dh = NULL; - char *file_string = NULL; - int retval = -1; - static const char file_header[] = "# This file contains stored Diffie-" - "Hellman parameters for future use.\n# You *do not* need to edit this " - "file.\n\n"; - - tor_assert(fname); - - if (!dh_param_p_tls) { - log_info(LD_CRYPTO, "Tried to store a DH modulus that does not exist."); - goto done; - } - - if (!(dh = DH_new())) - goto done; - if (!(dh->p = BN_dup(dh_param_p_tls))) - goto done; - if (!(dh->g = BN_new())) - goto done; - if (!BN_set_word(dh->g, DH_GENERATOR)) - goto done; - - len = i2d_DHparams(dh, &dh_string_repr); - if ((len < 0) || (dh_string_repr == NULL)) { - log_warn(LD_CRYPTO, "Error occured while DER encoding DH modulus (2)."); - goto done; - } - - base64_encoded_dh = tor_calloc(len, 2); /* should be enough */ - new_len = base64_encode(base64_encoded_dh, len * 2, - (char *)dh_string_repr, len); - if (new_len < 0) { - log_warn(LD_CRYPTO, "Error occured while base64-encoding DH modulus."); - goto done; - } - - /* concatenate file header and the dh parameters blob */ - new_len = tor_asprintf(&file_string, "%s%s", file_header, base64_encoded_dh); - - /* write to file */ - if (write_bytes_to_new_file(fname, file_string, new_len, 0) < 0) { - log_info(LD_CRYPTO, "'%s' was already occupied.", fname); - goto done; - } - - retval = 0; - - done: - if (dh) - DH_free(dh); - if (dh_string_repr) - OPENSSL_free(dh_string_repr); - tor_free(base64_encoded_dh); - tor_free(file_string); - - return retval; -} - -/** Return the dynamic DH modulus stored in <b>fname</b>. If there is no - dynamic DH modulus stored in <b>fname</b>, return NULL. */ -static BIGNUM * -crypto_get_stored_dynamic_dh_modulus(const char *fname) -{ - int retval; - char *contents = NULL; - const char *contents_tmp = NULL; - int dh_codes; - DH *stored_dh = NULL; - BIGNUM *dynamic_dh_modulus = NULL; - int length = 0; - unsigned char *base64_decoded_dh = NULL; - const unsigned char *cp = NULL; - - tor_assert(fname); - - contents = read_file_to_str(fname, RFTS_IGNORE_MISSING, NULL); - if (!contents) { - log_info(LD_CRYPTO, "Could not open file '%s'", fname); - goto done; /*usually means that ENOENT. don't try to move file to broken.*/ - } - - /* skip the file header */ - contents_tmp = eat_whitespace(contents); - if (!*contents_tmp) { - log_warn(LD_CRYPTO, "Stored dynamic DH modulus file " - "seems corrupted (eat_whitespace)."); - goto err; - } - - /* 'fname' contains the DH parameters stored in base64-ed DER - * format. We are only interested in the DH modulus. - * NOTE: We allocate more storage here than we need. Since we're already - * doing that, we can also add 1 byte extra to appease Coverity's - * scanner. */ - - cp = base64_decoded_dh = tor_malloc_zero(strlen(contents_tmp) + 1); - length = base64_decode((char *)base64_decoded_dh, strlen(contents_tmp), - contents_tmp, strlen(contents_tmp)); - if (length < 0) { - log_warn(LD_CRYPTO, "Stored dynamic DH modulus seems corrupted (base64)."); - goto err; - } - - stored_dh = d2i_DHparams(NULL, &cp, length); - if ((!stored_dh) || (cp - base64_decoded_dh != length)) { - log_warn(LD_CRYPTO, "Stored dynamic DH modulus seems corrupted (d2i)."); - goto err; - } - - { /* check the cryptographic qualities of the stored dynamic DH modulus: */ - retval = DH_check(stored_dh, &dh_codes); - if (!retval || dh_codes) { - log_warn(LD_CRYPTO, "Stored dynamic DH modulus is not a safe prime."); - goto err; - } - - retval = DH_size(stored_dh); - if (retval < DH_BYTES) { - log_warn(LD_CRYPTO, "Stored dynamic DH modulus is smaller " - "than '%d' bits.", DH_BYTES*8); - goto err; - } - - if (!BN_is_word(stored_dh->g, 2)) { - log_warn(LD_CRYPTO, "Stored dynamic DH parameters do not use '2' " - "as the group generator."); - goto err; - } - } - - { /* log the dynamic DH modulus: */ - char *s = BN_bn2hex(stored_dh->p); - tor_assert(s); - log_info(LD_OR, "Found stored dynamic DH modulus: [%s]", s); - OPENSSL_free(s); - } - - goto done; - - err: - - { - /* move broken prime to $filename.broken */ - char *fname_new=NULL; - tor_asprintf(&fname_new, "%s.broken", fname); - - log_warn(LD_CRYPTO, "Moving broken dynamic DH prime to '%s'.", fname_new); - - if (replace_file(fname, fname_new)) - log_notice(LD_CRYPTO, "Error while moving '%s' to '%s'.", - fname, fname_new); - - tor_free(fname_new); - } - - if (stored_dh) { - DH_free(stored_dh); - stored_dh = NULL; - } - - done: - tor_free(contents); - tor_free(base64_decoded_dh); - - if (stored_dh) { - dynamic_dh_modulus = BN_dup(stored_dh->p); - DH_free(stored_dh); - } - - return dynamic_dh_modulus; -} - -/** Set the global TLS Diffie-Hellman modulus. - * If <b>dynamic_dh_modulus_fname</b> is set, try to read a dynamic DH modulus - * off it and use it as the DH modulus. If that's not possible, - * generate a new dynamic DH modulus. - * If <b>dynamic_dh_modulus_fname</b> is NULL, use the Apache mod_ssl DH +/** Set the global TLS Diffie-Hellman modulus. Use the Apache mod_ssl DH * modulus. */ void -crypto_set_tls_dh_prime(const char *dynamic_dh_modulus_fname) +crypto_set_tls_dh_prime(void) { BIGNUM *tls_prime = NULL; - int store_dh_prime_afterwards = 0; int r;

/* If the space is occupied, free the previous TLS DH prime */ @@ -2005,18 +1782,7 @@ crypto_set_tls_dh_prime(const char *dynamic_dh_modulus_fname) dh_param_p_tls = NULL; }

- if (dynamic_dh_modulus_fname) { /* use dynamic DH modulus: */ - log_info(LD_OR, "Using stored dynamic DH modulus."); - tls_prime = crypto_get_stored_dynamic_dh_modulus(dynamic_dh_modulus_fname); - - if (!tls_prime) { - log_notice(LD_OR, "Generating fresh dynamic DH modulus. " - "This might take a while..."); - tls_prime = crypto_generate_dynamic_dh_modulus(); - - store_dh_prime_afterwards++; - } - } else { /* use the static DH prime modulus used by Apache in mod_ssl: */ + if (1) { tls_prime = BN_new(); tor_assert(tls_prime);

@@ -2036,13 +1802,6 @@ crypto_set_tls_dh_prime(const char *dynamic_dh_modulus_fname) tor_assert(tls_prime);

dh_param_p_tls = tls_prime; - - if (store_dh_prime_afterwards) - /* save the new dynamic DH modulus to disk. */ - if (crypto_store_dynamic_dh_modulus(dynamic_dh_modulus_fname)) { - log_notice(LD_CRYPTO, "Failed while storing dynamic DH modulus. " - "Make sure your data directory is sane."); - } }

/** Initialize dh_param_p and dh_param_g if they are not already @@ -2079,10 +1838,8 @@ init_dh_param(void) dh_param_p = circuit_dh_prime; dh_param_g = generator;

- /* Ensure that we have TLS DH parameters set up, too, even if we're - going to change them soon. */ if (!dh_param_p_tls) { - crypto_set_tls_dh_prime(NULL); + crypto_set_tls_dh_prime(); } }

diff --git a/src/common/crypto.h b/src/common/crypto.h index d305bc1..440ff23 100644 --- a/src/common/crypto.h +++ b/src/common/crypto.h @@ -122,8 +122,7 @@ int crypto_global_cleanup(void); crypto_pk_t *crypto_pk_new(void); void crypto_pk_free(crypto_pk_t *env);

-void crypto_set_tls_dh_prime(const char *dynamic_dh_modulus_fname); - +void crypto_set_tls_dh_prime(void); crypto_cipher_t *crypto_cipher_new(const char *key); crypto_cipher_t *crypto_cipher_new_with_iv(const char *key, const char *iv); void crypto_cipher_free(crypto_cipher_t *env); @@ -297,4 +296,3 @@ struct dh_st *crypto_dh_get_dh_(crypto_dh_t *dh); void crypto_add_spaces_to_fp(char *out, size_t outlen, const char *in);

#endif - diff --git a/src/or/config.c b/src/or/config.c index fca350c..a29cbce 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -225,7 +225,7 @@ static config_var_t option_vars_[] = { V(DisableDebuggerAttachment, BOOL, "1"), V(DisableIOCP, BOOL, "1"), OBSOLETE("DisableV2DirectoryInfo_"), - V(DynamicDHGroups, BOOL, "0"), + OBSOLETE("DynamicDHGroups"), VPORT(DNSPort, LINELIST, NULL), V(DNSListenAddress, LINELIST, NULL), V(DownloadExtraInfo, BOOL, "0"), @@ -1318,10 +1318,6 @@ options_transition_requires_fresh_tls_context(const or_options_t *old_options, if (!old_options) return 0;

- if ((old_options->DynamicDHGroups != new_options->DynamicDHGroups)) { - return 1; - } - if (!opt_streq(old_options->TLSECGroup, new_options->TLSECGroup)) return 1;

@@ -1503,23 +1499,8 @@ options_act(const or_options_t *old_options) finish_daemon(options->DataDirectory); }

- /* If needed, generate a new TLS DH prime according to the current torrc. */ - if (server_mode(options) && options->DynamicDHGroups) { - char *keydir = get_datadir_fname("keys"); - if (check_private_dir(keydir, CPD_CREATE, options->User)) { - tor_free(keydir); - return -1; - } - tor_free(keydir); - - if (!old_options || !old_options->DynamicDHGroups) { - char *fname = get_datadir_fname2("keys", "dynamic_dh_params"); - crypto_set_tls_dh_prime(fname); - tor_free(fname); - } - } else { /* clients don't need a dynamic DH prime. */ - crypto_set_tls_dh_prime(NULL); - } + /* Probably not needed any longer XXXX */ + crypto_set_tls_dh_prime();

/* We want to reinit keys as needed before we do much of anything else: keys are important, and other things can depend on them. */ diff --git a/src/or/or.h b/src/or/or.h index 6723f93..612743e 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3384,8 +3384,6 @@ typedef struct { char *Address; /**< OR only: configured address for this onion router. */ char *PidFile; /**< Where to store PID of Tor process. */

- int DynamicDHGroups; /**< Dynamic generation of prime moduli for use in DH.*/ - routerset_t *ExitNodes; /**< Structure containing nicknames, digests, * country codes and IP address patterns of ORs to * consider as exits. */ diff --git a/src/test/testing_common.c b/src/test/testing_common.c index 403c83b..60be460 100644 --- a/src/test/testing_common.c +++ b/src/test/testing_common.c @@ -269,7 +269,7 @@ main(int c, const char **v) printf("Can't initialize crypto subsystem; exiting.\n"); return 1; } - crypto_set_tls_dh_prime(NULL); + crypto_set_tls_dh_prime(); crypto_seed_rng(1); rep_hist_init(); network_init();