From 9b09627edd2e1dcaed4ca8382bde3cf608ce6a81 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 15 Jan 2011 11:22:25 -0500 Subject: [PATCH 1/2] Zero out some more key data before freeing it Found by cypherpunks; fixes bug 2384. --- changes/bug2384 | 5 +++++ src/common/crypto.c | 12 +++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) create mode 100644 changes/bug2384 diff --git a/changes/bug2384 b/changes/bug2384 new file mode 100644 index 0000000000..5321814424 --- /dev/null +++ b/changes/bug2384 @@ -0,0 +1,5 @@ + o Minor bugfixes + - Zero out a few more keys in memory before freeing them. Fixes bug + 2384. Found by cypherpunks. Bugfix on 0.0.2pre9. + + diff --git a/src/common/crypto.c b/src/common/crypto.c index 208e1c5fe1..29137a834d 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -490,6 +490,7 @@ crypto_pk_read_private_key_from_filename(crypto_pk_env_t *env, /* Try to parse it. */ r = crypto_pk_read_private_key_from_string(env, contents, -1); + memset(contents, 0, strlen(contents)); tor_free(contents); if (r) return -1; /* read_private_key_from_string already warned, so we don't.*/ @@ -627,6 +628,7 @@ crypto_pk_write_private_key_to_filename(crypto_pk_env_t *env, s[len]='\0'; r = write_str_to_file(fname, s, 0); BIO_free(bio); + memset(s, 0, strlen(s)); tor_free(s); return r; } @@ -1688,7 +1690,7 @@ crypto_dh_compute_secret(crypto_dh_env_t *dh, { char *secret_tmp = NULL; BIGNUM *pubkey_bn = NULL; - size_t secret_len=0; + size_t secret_len=0, secret_tmp_len=0; int result=0; tor_assert(dh); tor_assert(secret_bytes_out/DIGEST_LEN <= 255); @@ -1702,7 +1704,8 @@ crypto_dh_compute_secret(crypto_dh_env_t *dh, log_warn(LD_CRYPTO,"Rejected invalid g^x"); goto error; } - secret_tmp = tor_malloc(crypto_dh_get_bytes(dh)); + 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."); @@ -1721,7 +1724,10 @@ crypto_dh_compute_secret(crypto_dh_env_t *dh, crypto_log_errors(LOG_WARN, "completing DH handshake"); if (pubkey_bn) BN_free(pubkey_bn); - tor_free(secret_tmp); + if (secret_tmp) { + memset(secret_tmp, 0, secret_tmp_len); + tor_free(secret_tmp); + } if (result < 0) return result; else From ef6fa07e4830dde86fce2d06bf9da44d5c1c79b9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 15 Jan 2011 11:32:56 -0500 Subject: [PATCH 2/2] Fix a couple of non-cleared key issues in hidden services we need to do more hunting, but this fixes the ones mentioned in 2385. --- changes/bug2384 | 3 ++- src/or/rendclient.c | 2 ++ src/or/rendservice.c | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/changes/bug2384 b/changes/bug2384 index 5321814424..ded5eee992 100644 --- a/changes/bug2384 +++ b/changes/bug2384 @@ -1,5 +1,6 @@ o Minor bugfixes - Zero out a few more keys in memory before freeing them. Fixes bug - 2384. Found by cypherpunks. Bugfix on 0.0.2pre9. + 2384 and part of bug 2385. These key instances found by + "cypherpunks". Bugfix on 0.0.2pre9. diff --git a/src/or/rendclient.c b/src/or/rendclient.c index ab18d35298..95875465cb 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -717,8 +717,10 @@ rend_client_receive_rendezvous(origin_circuit_t *circ, const uint8_t *request, * attach only the connections that are waiting on this circuit, rather * than trying to attach them all. See comments bug 743. */ connection_ap_attach_pending(); + memset(keys, 0, sizeof(keys)); return 0; err: + memset(keys, 0, sizeof(keys)); circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_TORPROTOCOL); return -1; } diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 07f01aead9..a650eda405 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1214,8 +1214,10 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, memcpy(cpath->handshake_digest, keys, DIGEST_LEN); if (extend_info) extend_info_free(extend_info); + memset(keys, 0, sizeof(keys)); return 0; err: + memset(keys, 0, sizeof(keys)); if (dh) crypto_dh_free(dh); if (launched) circuit_mark_for_close(TO_CIRCUIT(launched), reason);