From df5c7fedbd938c5d4634eadcf53693e07d2c8182 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Wed, 20 Apr 2011 15:20:10 -0700 Subject: [PATCH 1/4] Don't allow v0 HS auths to act as clients A v0 HS authority stores v0 HS descriptors in the same descriptor cache that its HS client functionality uses. Thus, if the HS authority operator clears its client HS descriptor cache, ALL v0 HS descriptors will be lost. That would be bad. --- changes/forget-rend-descs-on-newnym | 3 +++ src/or/config.c | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/changes/forget-rend-descs-on-newnym b/changes/forget-rend-descs-on-newnym index ab2fd61f34..f8758c2d58 100644 --- a/changes/forget-rend-descs-on-newnym +++ b/changes/forget-rend-descs-on-newnym @@ -2,6 +2,9 @@ - Forget all hidden service descriptors cached as a client when processing a SIGNAL NEWNYM command. Fixes bug 3000. Bugfix on 0.0.6. + o Minor bugfixes: + - Don't allow v0 hidden service authorities to act as clients. + Required by fix for bug 3000. o Code simplifications and refactoring: - Allow rend_client_send_introduction to fail without closing the AP connection permanently. diff --git a/src/or/config.c b/src/or/config.c index f003e4d296..9384b3a68a 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -3078,6 +3078,10 @@ options_validate(or_options_t *old_options, or_options_t *options, REJECT("FetchDirInfoExtraEarly requires that you also set " "FetchDirInfoEarly"); + if (options->HSAuthoritativeDir && proxy_mode(options)) + REJECT("Running as authoritative v0 HS directory, but also configured " + "as a client."); + if (options->ConnLimit <= 0) { tor_asprintf(msg, "ConnLimit must be greater than 0, but was set to %d", From ddd1b7be2dce0845e6c08dcb5404ae65940b9edb Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Wed, 20 Apr 2011 17:01:41 -0700 Subject: [PATCH 2/4] Ignore SIGNAL NEWNYM on relay-only Tor instances --- changes/forget-rend-descs-on-newnym | 2 ++ src/or/main.c | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/changes/forget-rend-descs-on-newnym b/changes/forget-rend-descs-on-newnym index f8758c2d58..0ea09e0800 100644 --- a/changes/forget-rend-descs-on-newnym +++ b/changes/forget-rend-descs-on-newnym @@ -5,6 +5,8 @@ o Minor bugfixes: - Don't allow v0 hidden service authorities to act as clients. Required by fix for bug 3000. + - Ignore SIGNAL NEWNYM commands on relay-only Tor instances. + Required by fix for bug 3000. o Code simplifications and refactoring: - Allow rend_client_send_introduction to fail without closing the AP connection permanently. diff --git a/src/or/main.c b/src/or/main.c index 3bf21693f4..a26be39fdf 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -842,6 +842,13 @@ run_connection_housekeeping(int i, time_t now) static void signewnym_impl(time_t now) { + or_options_t *options = get_options(); + if (!proxy_mode(options)) { + log_info(LD_CONTROL, "Ignoring SIGNAL NEWNYM because client functionality " + "is disabled."); + return; + } + circuit_expire_all_dirty_circs(); addressmap_clear_transient(); rend_cache_purge(); From b8708b5bd30f5ef6dd34f7199a5588627486bcd2 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Tue, 26 Apr 2011 02:21:25 -0700 Subject: [PATCH 3/4] Fix bug 1930 --- changes/forget-rend-descs-on-newnym | 7 ++ src/or/rendclient.c | 103 +++++++++++++++++----------- 2 files changed, 70 insertions(+), 40 deletions(-) diff --git a/changes/forget-rend-descs-on-newnym b/changes/forget-rend-descs-on-newnym index 0ea09e0800..da7afbe201 100644 --- a/changes/forget-rend-descs-on-newnym +++ b/changes/forget-rend-descs-on-newnym @@ -2,6 +2,13 @@ - Forget all hidden service descriptors cached as a client when processing a SIGNAL NEWNYM command. Fixes bug 3000. Bugfix on 0.0.6. + o Major bugfixes: + - When we find that we have extended a hidden service's introduction + circuit to a relay which isn't listed as an introduction point in + the HS descriptor we currently have for the service, we now retry + one of the introduction points in the current HS descriptor. + Previously we would just give up. Bugfix on 0.2.0.10-alpha; fixes + bugs 1024 and 1930. o Minor bugfixes: - Don't allow v0 hidden service authorities to act as clients. Required by fix for bug 3000. diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 88038755e4..8d024d8ebb 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -66,6 +66,50 @@ rend_client_send_establish_rendezvous(origin_circuit_t *circ) return 0; } +/** Extend the introduction circuit circ to another valid + * introduction point for the hidden service it is trying to connect + * to, or mark it and launch a new circuit if we can't extend it. + * Return 0 on success. Return -1 and mark the introduction + * circuit on failure. + * + * On failure, the caller is responsible for marking the associated + * rendezvous circuit for close. */ +static int +rend_client_reextend_intro_circuit(origin_circuit_t *circ) +{ + extend_info_t *extend_info; + int result; + extend_info = rend_client_get_random_intro(circ->rend_data); + if (!extend_info) { + log_warn(LD_REND, + "No usable introduction points left for %s. Closing.", + safe_str_client(circ->rend_data->onion_address)); + circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); + return -1; + } + if (circ->remaining_relay_early_cells) { + log_info(LD_REND, + "Re-extending circ %d, this time to %s.", + circ->_base.n_circ_id, extend_info->nickname); + result = circuit_extend_to_new_exit(circ, extend_info); + } else { + log_info(LD_REND, + "Building a new introduction circuit, this time to %s.", + extend_info->nickname); + circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_FINISHED); + if (!circuit_launch_by_extend_info(CIRCUIT_PURPOSE_C_INTRODUCING, + extend_info, + CIRCLAUNCH_IS_INTERNAL)) { + log_warn(LD_REND, "Building introduction circuit failed."); + result = -1; + } else { + result = 0; + } + } + extend_info_free(extend_info); + return result; +} + /** Called when we're trying to connect an ap conn; sends an INTRODUCE1 cell * down introcirc if possible. */ @@ -120,11 +164,18 @@ rend_client_send_introduction(origin_circuit_t *introcirc, } }); if (!intro_key) { - log_info(LD_REND, "Our introduction point knowledge changed in " - "mid-connect! Could not find intro key; we only have a " - "v2 rend desc with %d intro points. Giving up.", + log_info(LD_REND, "Could not find intro key for %s at %s; we " + "have a v2 rend desc with %d intro points. " + "Trying a different intro point...", + safe_str_client(introcirc->rend_data->onion_address), + introcirc->build_state->chosen_exit->nickname, smartlist_len(entry->parsed->intro_nodes)); - goto perm_err; + + if (rend_client_reextend_intro_circuit(introcirc)) { + goto perm_err; + } else { + return -1; + } } if (crypto_pk_get_digest(intro_key, payload)<0) { log_warn(LD_BUG, "Internal error: couldn't hash public key."); @@ -227,7 +278,8 @@ rend_client_send_introduction(origin_circuit_t *introcirc, return 0; perm_err: - circuit_mark_for_close(TO_CIRCUIT(introcirc), END_CIRC_REASON_INTERNAL); + if (!introcirc->_base.marked_for_close) + circuit_mark_for_close(TO_CIRCUIT(introcirc), END_CIRC_REASON_INTERNAL); circuit_mark_for_close(TO_CIRCUIT(rendcirc), END_CIRC_REASON_INTERNAL); return -2; } @@ -290,45 +342,16 @@ rend_client_introduction_acked(origin_circuit_t *circ, * points. If any remain, extend to a new one and try again. * If none remain, refetch the service descriptor. */ + log_info(LD_REND, "Got nack for %s from %s...", + safe_str_client(circ->rend_data->onion_address), + circ->build_state->chosen_exit->nickname); if (rend_client_remove_intro_point(circ->build_state->chosen_exit, circ->rend_data) > 0) { /* There are introduction points left. Re-extend the circuit to * another intro point and try again. */ - extend_info_t *extend_info; - int result; - extend_info = rend_client_get_random_intro(circ->rend_data); - if (!extend_info) { - log_warn(LD_REND, "No introduction points left for %s. Closing.", - escaped_safe_str_client(circ->rend_data->onion_address)); - circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL); - return -1; - } - if (circ->remaining_relay_early_cells) { - log_info(LD_REND, - "Got nack for %s from %s. Re-extending circ %d, " - "this time to %s.", - escaped_safe_str_client(circ->rend_data->onion_address), - circ->build_state->chosen_exit->nickname, - circ->_base.n_circ_id, extend_info->nickname); - result = circuit_extend_to_new_exit(circ, extend_info); - } else { - log_info(LD_REND, - "Got nack for %s from %s. Building a new introduction " - "circuit, this time to %s.", - escaped_safe_str_client(circ->rend_data->onion_address), - circ->build_state->chosen_exit->nickname, - extend_info->nickname); - circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_FINISHED); - if (!circuit_launch_by_extend_info(CIRCUIT_PURPOSE_C_INTRODUCING, - extend_info, - CIRCLAUNCH_IS_INTERNAL)) { - log_warn(LD_REND, "Building introduction circuit failed."); - result = -1; - } else { - result = 0; - } - } - extend_info_free(extend_info); + int result = rend_client_reextend_intro_circuit(circ); + /* XXXX If that call failed, should we close the rend circuit, + * too? */ return result; } } From 6dfc0d530113e055d91b68969c81595ddc749f07 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 28 Apr 2011 17:45:41 -0400 Subject: [PATCH 4/4] Avoid false positives from proxy_mode() Previously it would erroneously return true if ListenAddr was set for a client port, even if that port itself was 0. This would give false positives, which were not previously harmful... but which were about to become. --- src/or/router.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index 0ef4728a02..65afd49f7f 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1107,10 +1107,10 @@ set_server_advertised(int s) int proxy_mode(or_options_t *options) { - return (options->SocksPort != 0 || options->SocksListenAddress || - options->TransPort != 0 || options->TransListenAddress || - options->NATDPort != 0 || options->NATDListenAddress || - options->DNSPort != 0 || options->DNSListenAddress); + return (options->SocksPort != 0 || + options->TransPort != 0 || + options->NATDPort != 0 || + options->DNSPort != 0); } /** Decide if we're a publishable server. We are a publishable server if: