April 2011 - tor-commits - lists.torproject.org

[tor/maint-0.2.2] Fix a check-spaces complaint
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit e03e90bf591f649f7883807f99354634e837ee48 Author: Sebastian Hahn <sebastian(a)torproject.org> Date: Fri Apr 8 04:16:26 2011 +0200 Fix a check-spaces complaint --- src/or/connection_edge.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 5f322ca..1ee57ab 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -841,7 +841,7 @@ addressmap_clear_excluded_trackexithosts(or_options_t *options) if (len < 6) continue; /* malformed. */ dot = target + len - 6; /* dot now points to just before .exit */ - dot = strrchr(dot, '.'); /* dot now points to the . before .exit, or NULL */ + dot = strrchr(dot, '.'); /* dot now points to the . before .exit or NULL */ if (!dot) { nodename = tor_strndup(target, len-5); } else {

1 0

[tor/maint-0.2.2] warn if we launch too many circuits for a given stream
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit f7a5bc16d689e8b919285c66cd0f07a6694bcc69 Author: Roger Dingledine <arma(a)torproject.org> Date: Tue Apr 26 22:18:01 2011 -0400 warn if we launch too many circuits for a given stream --- changes/bug1090-launch-warning | 5 +++++ src/or/circuituse.c | 13 ++++++++++++- src/or/or.h | 7 +++++++ 3 files changed, 24 insertions(+), 1 deletions(-) diff --git a/changes/bug1090-launch-warning b/changes/bug1090-launch-warning new file mode 100644 index 0000000..3f3fbcb --- /dev/null +++ b/changes/bug1090-launch-warning @@ -0,0 +1,5 @@ + o Minor features: + - Keep track of how many times we launch a new circuit to handle + a given stream. Too many launches could indicate an inconsistency + between our "launch a circuit to handle this stream" logic and our + "attach our stream to one of the available circuits" logic. diff --git a/src/or/circuituse.c b/src/or/circuituse.c index cc4a739..fd1cf6b 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1409,7 +1409,18 @@ circuit_get_open_circ_or_launch(edge_connection_t *conn, extend_info_free(extend_info); - if (desired_circuit_purpose != CIRCUIT_PURPOSE_C_GENERAL) { + if (desired_circuit_purpose == CIRCUIT_PURPOSE_C_GENERAL) { + /* We just caused a circuit to get built because of this stream. + * If this stream has caused a _lot_ of circuits to be built, that's + * a bad sign: we should tell the user. */ + if (conn->num_circuits_launched < NUM_CIRCUITS_LAUNCHED_THRESHOLD && + ++conn->num_circuits_launched == NUM_CIRCUITS_LAUNCHED_THRESHOLD) + log_warn(LD_BUG, "The application request to %s:%d has launched " + "%d circuits without finding one it likes.", + escaped_safe_str_client(conn->socks_request->address), + conn->socks_request->port, + conn->num_circuits_launched); + } else { /* help predict this next time */ rep_hist_note_used_internal(time(NULL), need_uptime, 1); if (circ) { diff --git a/src/or/or.h b/src/or/or.h index 50a1223..7d354c8 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -1151,6 +1151,13 @@ typedef struct edge_connection_t { * already retried several times. */ uint8_t num_socks_retries; +#define NUM_CIRCUITS_LAUNCHED_THRESHOLD 10 + /** Number of times we've launched a circuit to handle this stream. If + * it gets too high, that could indicate an inconsistency between our + * "launch a circuit to handle this stream" logic and our "attach our + * stream to one of the available circuits" logic. */ + unsigned int num_circuits_launched:4; + /** True iff this connection is for a DNS request only. */ unsigned int is_dns_request:1;

1 0

[tor/maint-0.2.2] When cannibalizing a circuit, make sure it has no ExcludeNodes on it
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 6afad6b691d577fba2fe88f2fe9ed76a2f80002d Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Apr 3 19:58:28 2011 -0400 When cannibalizing a circuit, make sure it has no ExcludeNodes on it This could happen if StrictNodes was 0 and we were forced to pick an excluded node as the last hop of the circuit. --- src/or/circuitlist.c | 19 ++++++++++++++----- 1 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 42073fb..ce324ca 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -923,6 +923,7 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, int need_uptime = (flags & CIRCLAUNCH_NEED_UPTIME) != 0; int need_capacity = (flags & CIRCLAUNCH_NEED_CAPACITY) != 0; int internal = (flags & CIRCLAUNCH_IS_INTERNAL) != 0; + or_options_t *options = get_options(); /* Make sure we're not trying to create a onehop circ by * cannibalization. */ @@ -933,11 +934,6 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, "capacity %d, internal %d", purpose, need_uptime, need_capacity, internal); - /* XXX022-1090 We should make sure that when we cannibalize a circuit, it - * contains no excluded nodes. (This is possible if StrictNodes is 0, and - * we thought we needed to use an excluded exit node for, say, a directory - * operation.) -NM */ - for (_circ=global_circuitlist; _circ; _circ = _circ->next) { if (CIRCUIT_IS_ORIGIN(_circ) && _circ->state == CIRCUIT_STATE_OPEN && @@ -966,6 +962,19 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info, hop=hop->next; } while (hop!=circ->cpath); } + if (options->ExcludeNodes) { + /* Make sure no existing nodes in the circuit are excluded for + * general use. (This may be possible if StrictNodes is 0, and we + * thought we needed to use an otherwise excluded node for, say, a + * directory operation.) */ + crypt_path_t *hop = circ->cpath; + do { + if (routerset_contains_extendinfo(options->ExcludeNodes, + hop->extend_info)) + goto next; + hop = hop->next; + } while (hop != circ->cpath); + } if (!best || (best->build_state->need_uptime && !need_uptime)) best = circ; next: ;

1 0

[tor/maint-0.2.2] Fix a log msg
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 8e2904e269e039713c2b2e7be10b82e11f239cd0 Author: Sebastian Hahn <sebastian(a)torproject.org> Date: Fri Apr 8 04:03:50 2011 +0200 Fix a log msg --- src/or/router.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index 64cb079..0ef4728 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -865,7 +865,7 @@ consider_testing_reachability(int test_or, int test_dir) if ((msg = rate_limit_log(&warning_limit, approx_time()))) { log_warn(LD_CIRC, "Can't peform self-tests for this relay: we have " "listed ourself in ExcludeNodes, and StrictNodes is set. " - "We will cannot learn whether we are usable, and will not " + "We cannot learn whether we are usable, and will not " "be able to advertise ourself.%s", msg); tor_free(msg); }

1 0

[tor/maint-0.2.2] Explain the "using anyway" log message better
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 92ec36a061de17dd4e378d5ce34d82f91eee9f86 Author: Sebastian Hahn <sebastian(a)torproject.org> Date: Fri Apr 8 04:01:23 2011 +0200 Explain the "using anyway" log message better Also add a request to report any cases where we are not honoring StrictNodes to the developers: These should now all be bugs. --- src/or/circuitbuild.c | 18 ++++++++++++------ 1 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index c58509f..681f524 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2896,7 +2896,7 @@ warn_if_last_router_excluded(origin_circuit_t *circ, const extend_info_t *exit) case CIRCUIT_PURPOSE_C_GENERAL: if (circ->build_state->is_internal) return; - description = "Requested exit node"; + description = "requested exit node"; rs = options->_ExcludeExitNodesUnion; break; case CIRCUIT_PURPOSE_C_INTRODUCING: @@ -2911,22 +2911,28 @@ warn_if_last_router_excluded(origin_circuit_t *circ, const extend_info_t *exit) case CIRCUIT_PURPOSE_C_REND_READY: case CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED: case CIRCUIT_PURPOSE_C_REND_JOINED: - description = "Chosen rendezvous point"; + description = "chosen rendezvous point"; domain = LD_BUG; break; case CIRCUIT_PURPOSE_CONTROLLER: rs = options->_ExcludeExitNodesUnion; - description = "Controller-selected circuit target"; + description = "controller-selected circuit target"; break; } if (routerset_contains_extendinfo(rs, exit)) { - log_fn(LOG_WARN, domain, "%s '%s' is in ExcludeNodes%s. Using anyway " - "(circuit purpose %d).", + /* We should never get here if StrictNodes is set to 1. */ + if (options->StrictNodes) + log_warn(LD_BUG, "Using an excluded node with StrictNodes set. " + "Please report the following log message to the " + "developers."); + log_fn(LOG_WARN, domain, "Using %s '%s' which is listed in " + "ExcludeNodes%s, because no other options were available. To " + "prevent this, set the StrictNodes configuration option." + "(Circuit purpose is %d)", description,exit->nickname, rs==options->ExcludeNodes?"":" or ExcludeExitNodes", (int)purpose); - /* XXX022-1090 "using anyway" is freaking people out -RD */ circuit_log_path(LOG_WARN, domain, circ); }

1 0

[tor/maint-0.2.2] Check transition of circuit purpose from INTRO->GENERAL if nodes are constrained
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 79a3b3cd3719b3b87b0edbab62b256e42c7b42de Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Apr 3 20:06:31 2011 -0400 Check transition of circuit purpose from INTRO->GENERAL if nodes are constrained This looked at first like another fun way around our node selection logic: if we had introduction circuits, and we wound up building too many, we would turn extras into general-purpose circuits. But when we did so, we wouldn't necessarily check whether the general-purpose circuits conformed to our node constraints. For example, the last node could totally be in ExcludedExitNodes and we wouldn't have cared... ...except that the circuit should already be internal, so it won't get user streams attached to it, so the transition should generally be allowed. Add an assert to make sure we're right about this, and have it not check whether ExitNodes is set, since that's irrelevant to internal circuits. --- src/or/rendservice.c | 29 +++++++++++++++++++---------- 1 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 5b85394..cd8f9ea 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1347,17 +1347,26 @@ rend_service_intro_has_opened(origin_circuit_t *circuit) } /* If we already have enough introduction circuits for this service, - * redefine this one as a general circuit. */ + * redefine this one as a general circuit or close it, depending. */ if (count_established_intro_points(serviceid) > NUM_INTRO_POINTS) { - log_info(LD_CIRC|LD_REND, "We have just finished an introduction " - "circuit, but we already have enough. Redefining purpose to " - "general."); - /* XXX022-1090: This can wind up violating ExcludeNodes/ - * ExitNodes/ExcludeExitNodes restrictions. - */ - TO_CIRCUIT(circuit)->purpose = CIRCUIT_PURPOSE_C_GENERAL; - circuit_has_opened(circuit); - return; + or_options_t *options = get_options(); + if (options->ExcludeNodes) { + /* XXXX in some future version, we can test whether the transition is + allowed or not given the actual nodes in the circuit. But for now, + this case, we might as well close the thing. */ + log_info(LD_CIRC|LD_REND, "We have just finished an introduction " + "circuit, but we already have enough. Closing it."); + circuit_mark_for_close(TO_CIRCUIT(circuit), END_CIRC_REASON_NONE); + return; + } else { + tor_assert(circuit->build_state->is_internal); + log_info(LD_CIRC|LD_REND, "We have just finished an introduction " + "circuit, but we already have enough. Redefining purpose to " + "general; leaving as internal."); + TO_CIRCUIT(circuit)->purpose = CIRCUIT_PURPOSE_C_GENERAL; + circuit_has_opened(circuit); + return; + } } log_info(LD_REND,

1 0

[tor/maint-0.2.2] revert most of ef81649d2fc
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit f962dda8c188ebe17be1ad7d6548f303b6425943 Author: Roger Dingledine <arma(a)torproject.org> Date: Tue Apr 26 19:55:34 2011 -0400 revert most of ef81649d2fc Now we believe it to be the case that we never build a circuit for our stream that has an unsuitable exit, so we'll never need to use such a circuit. The risk is that we have some code that builds the circuit, but now we refuse to use it, meaning we just build a bazillion circuits and ignore them all. --- src/or/circuitbuild.c | 2 +- src/or/circuituse.c | 18 ++++++++++++------ src/or/connection_edge.c | 19 ++----------------- src/or/connection_edge.h | 3 +-- 4 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index b963f1a..6401e71 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2728,7 +2728,7 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime, { if (!ap_stream_wants_exit_attention(conn)) continue; /* Skip everything but APs in CIRCUIT_WAIT */ - if (connection_ap_can_use_exit(TO_EDGE_CONN(conn), router, 1)) { + if (connection_ap_can_use_exit(TO_EDGE_CONN(conn), router)) { ++n_supported[i]; // log_fn(LOG_DEBUG,"%s is supported. n_supported[%d] now %d.", // router->nickname, i, n_supported[i]); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 530941c..cc4a739 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -127,7 +127,7 @@ circuit_is_acceptable(circuit_t *circ, edge_connection_t *conn, return 0; } } - if (exitrouter && !connection_ap_can_use_exit(conn, exitrouter, 0)) { + if (exitrouter && !connection_ap_can_use_exit(conn, exitrouter)) { /* can't exit from this router */ return 0; } @@ -166,6 +166,10 @@ circuit_is_better(circuit_t *a, circuit_t *b, uint8_t purpose) return 1; if (CIRCUIT_IS_ORIGIN(b) && TO_ORIGIN_CIRCUIT(b)->build_state->is_internal) + /* XXX023 what the heck is this internal thing doing here. I + * think we can get rid of it. circuit_is_acceptable() already + * makes sure that is_internal is exactly what we need it to + * be. -RD */ return 1; } break; @@ -511,7 +515,7 @@ circuit_stream_is_being_handled(edge_connection_t *conn, if (exitrouter && (!need_uptime || build_state->need_uptime)) { int ok; if (conn) { - ok = connection_ap_can_use_exit(conn, exitrouter, 0); + ok = connection_ap_can_use_exit(conn, exitrouter); } else { addr_policy_result_t r = compare_addr_to_addr_policy( 0, port, exitrouter->exit_policy); @@ -1291,9 +1295,10 @@ circuit_get_open_circ_or_launch(edge_connection_t *conn, * refactor into a single function? */ routerinfo_t *router = router_get_by_nickname(conn->chosen_exit_name, 1); int opt = conn->chosen_exit_optional; - if (router && !connection_ap_can_use_exit(conn, router, 0)) { + if (router && !connection_ap_can_use_exit(conn, router)) { log_fn(opt ? LOG_INFO : LOG_WARN, LD_APP, - "Requested exit point '%s' would refuse request. %s.", + "Requested exit point '%s' is excluded or " + "would refuse request. %s.", conn->chosen_exit_name, opt ? "Trying others" : "Closing"); if (opt) { conn->chosen_exit_optional = 0; @@ -1611,9 +1616,10 @@ connection_ap_handshake_attach_circuit(edge_connection_t *conn) } return -1; } - if (router && !connection_ap_can_use_exit(conn, router, 0)) { + if (router && !connection_ap_can_use_exit(conn, router)) { log_fn(opt ? LOG_INFO : LOG_WARN, LD_APP, - "Requested exit point '%s' would refuse request. %s.", + "Requested exit point '%s' is excluded or " + "would refuse request. %s.", conn->chosen_exit_name, opt ? "Trying others" : "Closing"); if (opt) { conn->chosen_exit_optional = 0; diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 1ee57ab..082cd5f 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -3044,15 +3044,9 @@ connection_edge_is_rendezvous_stream(edge_connection_t *conn) * to exit from it, or 0 if it probably will not allow it. * (We might be uncertain if conn's destination address has not yet been * resolved.) - * - * If excluded_means_no is 1 and Exclude*Nodes is set and excludes - * this relay, return 0. - * XXX022-1090 This StrictNodes business needs more work, a la bug 1090. See - * also git commit ef81649d. */ int -connection_ap_can_use_exit(edge_connection_t *conn, routerinfo_t *exit, - int excluded_means_no) +connection_ap_can_use_exit(edge_connection_t *conn, routerinfo_t *exit) { or_options_t *options = get_options(); @@ -3102,17 +3096,8 @@ connection_ap_can_use_exit(edge_connection_t *conn, routerinfo_t *exit, return 0; } if (options->_ExcludeExitNodesUnion && - (options->StrictNodes || excluded_means_no) && routerset_contains_router(options->_ExcludeExitNodesUnion, exit)) { - /* If we are trying to avoid this node as exit, and we have StrictNodes - * set, then this is not a suitable exit. Refuse it. - * - * If we don't have StrictNodes set, then this function gets called in - * two contexts. First, we've got a circuit open and we want to know - * whether we can use it. In that case, we somehow built this circuit - * despite having the last hop in ExcludeExitNodes, so we should be - * willing to use it. Second, we are evaluating whether this is an - * acceptable exit for a new circuit. In that case, skip it. */ + /* Not a suitable exit. Refuse it. */ return 0; } diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index c4ae467..70d0dd2 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -47,8 +47,7 @@ int connection_exit_begin_conn(cell_t *cell, circuit_t *circ); int connection_exit_begin_resolve(cell_t *cell, or_circuit_t *circ); void connection_exit_connect(edge_connection_t *conn); int connection_edge_is_rendezvous_stream(edge_connection_t *conn); -int connection_ap_can_use_exit(edge_connection_t *conn, routerinfo_t *exit, - int excluded_means_no); +int connection_ap_can_use_exit(edge_connection_t *conn, routerinfo_t *exit); void connection_ap_expire_beginning(void); void connection_ap_attach_pending(void); void connection_ap_fail_onehop(const char *failed_digest,

1 0

[tor/maint-0.2.2] Do not try to download descriptors for bridges in ExcludeNodes.
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit b59a289365169cca228566826f19838403320cb2 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Apr 3 18:12:26 2011 -0400 Do not try to download descriptors for bridges in ExcludeNodes. --- src/or/circuitbuild.c | 33 ++++++++++++++++++++++++++++++++- 1 files changed, 32 insertions(+), 1 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 714d636..4832cae 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4539,6 +4539,24 @@ bridge_add_from_config(const tor_addr_t *addr, uint16_t port, char *digest) smartlist_add(bridge_list, b); } +/** Return true iff routerset contains the bridge bridge. */ +static int +routerset_contains_bridge(const routerset_t *routerset, + const bridge_info_t *bridge) +{ + int result; + extend_info_t *extinfo; + tor_assert(bridge); + if (!routerset) + return 0; + + extinfo = extend_info_alloc( + NULL, bridge->identity, NULL, &bridge->addr, bridge->port); + result = routerset_contains_extendinfo(routerset, extinfo); + extend_info_free(extinfo); + return result; +} + /** If digest is one of our known bridges, return it. */ static bridge_info_t * find_bridge_by_digest(const char *digest) @@ -4557,6 +4575,7 @@ static void launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge) { char *address; + or_options_t *options = get_options(); if (connection_get_by_type_addr_port_purpose( CONN_TYPE_DIR, &bridge->addr, bridge->port, @@ -4564,7 +4583,13 @@ launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge) return; /* it's already on the way */ address = tor_dup_addr(&bridge->addr); - /* XXX022-1090 if we ExcludeNodes this bridge, should this step fail? -RD */ + if (routerset_contains_bridge(options->ExcludeNodes, bridge)) { + download_status_mark_impossible(&bridge->fetch_status); + log_warn(LD_APP, "Not using bridge at %s: it is in ExcludeNodes.", + safe_str_client(fmt_addr(&bridge->addr))); + return; + } + directory_initiate_command(address, &bridge->addr, bridge->port, 0, 0, /* does not matter */ @@ -4605,6 +4630,12 @@ fetch_bridge_descriptors(or_options_t *options, time_t now) if (!download_status_is_ready(&bridge->fetch_status, now, IMPOSSIBLE_TO_DOWNLOAD)) continue; /* don't bother, no need to retry yet */ + if (routerset_contains_bridge(options->ExcludeNodes, bridge)) { + download_status_mark_impossible(&bridge->fetch_status); + log_warn(LD_APP, "Not using bridge at %s: it is in ExcludeNodes.", + safe_str_client(fmt_addr(&bridge->addr))); + continue; + } /* schedule another fetch as if this one will fail, in case it does */ download_status_failed(&bridge->fetch_status, 0);

1 0

[tor/maint-0.2.2] If we have chosen an exit that shares a family with all bridges, fail the circuit
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 84f0e87c6a6629d047a39f658b7f7c96767219ca Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Apr 3 18:20:19 2011 -0400 If we have chosen an exit that shares a family with all bridges, fail the circuit We could probably do something smarter here, but the situation is unusual enough that it's okay to just fail the circuit. --- src/or/circuitbuild.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 4832cae..c58509f 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4130,15 +4130,18 @@ choose_random_entry(cpath_build_state_t *state) need_capacity = 0; goto retry; } +#if 0 + /* Removing this retry logic: if we only allow one exit, and it is in the + same family as all our entries, then we are just plain not going to win + here. */ if (!r && entry_list_is_constrained(options) && consider_exit_family) { /* still no? if we're using bridges, * and our chosen exit is in the same family as all our * bridges, then be flexible about families. */ - /* XXXX022-1090 This is probably not what people want. Better to choose - * a new exit. */ consider_exit_family = 0; goto retry; } +#endif /* live_entry_guards may be empty below. Oh well, we tried. */ }

1 0

[tor/maint-0.2.2] Correct the behavior of .exit with ExcludeNodes, StrictNodes, etc.
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit ad78bafb71fbd66e83b29aba612d17f7d03e575b Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Apr 3 17:08:59 2011 -0400 Correct the behavior of .exit with ExcludeNodes, StrictNodes, etc. ExcludeExitNodes foo now means that foo.exit doesn't work. If StrictNodes is set, then ExcludeNodes foo also overrides foo.exit. foo.exit , however, still works even if foo is not listed in ExitNodes. --- src/or/connection_edge.c | 56 +++++++++++++++++++++++++++++++-------------- 1 files changed, 38 insertions(+), 18 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 1f5eb70..f435976 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1497,9 +1497,13 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn, hostname_type_t addresstype; or_options_t *options = get_options(); struct in_addr addr_tmp; + /* We set this to true if this is an address we should automatically + * remap to a local address in VirtualAddrNetwork */ int automap = 0; char orig_address[MAX_SOCKS_ADDR_LEN]; time_t map_expires = TIME_MAX; + /* This will be set to true iff the address starts out as a non-.exit + address, and we remap it to one because of an entry in the addressmap. */ int remapped_to_exit = 0; time_t now = time(NULL); @@ -1610,18 +1614,24 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn, /* foo.exit -- modify conn->chosen_exit_node to specify the exit * node, and conn->address to hold only the address portion. */ char *s = strrchr(socks->address,'.'); + + /* If StrictNodes is not set, then .exit overrides ExcludeNodes. */ + routerset_t *excludeset = options->StrictNodes ? + options->_ExcludeExitNodesUnion : options->ExcludeExitNodes; + /*XXX023 make this a node_t. */ + routerinfo_t *router; + tor_assert(!automap); if (s) { + /* The address was of the form "(stuff).(name).exit */ if (s[1] != '\0') { - /* XXX022-1090 we should look this up as a relay and see if it's - * in our excluded set, and refuse it here if so. But first, - * figure out what's up with this 'remapped_to_exit' business - * and whether that needs careful treatment. -RD */ conn->chosen_exit_name = tor_strdup(s+1); + router = router_get_by_nickname(conn->chosen_exit_name, 1); if (remapped_to_exit) /* 5 tries before it expires the addressmap */ conn->chosen_exit_retries = TRACKHOSTEXITS_RETRIES; *s = 0; } else { + /* Oops, the address was (stuff)..exit. That's not okay. */ log_warn(LD_APP,"Malformed exit address '%s.exit'. Refusing.", safe_str_client(socks->address)); control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", @@ -1630,23 +1640,33 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn, return -1; } } else { - routerinfo_t *r; + /* It looks like they just asked for "foo.exit". */ conn->chosen_exit_name = tor_strdup(socks->address); - r = router_get_by_nickname(conn->chosen_exit_name, 1); - *socks->address = 0; - if (r && (!options->_ExcludeExitNodesUnion || - !routerset_contains_router(options->_ExcludeExitNodesUnion, - r))) { - strlcpy(socks->address, r->address, sizeof(socks->address)); - } else { - log_warn(LD_APP, - "%s relay in exit address '%s.exit'. Refusing.", - r ? "Excluded" : "Unrecognized", - safe_str_client(socks->address)); - connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); - return -1; + router = router_get_by_nickname(conn->chosen_exit_name, 1); + if (router) { + *socks->address = 0; + strlcpy(socks->address, router->address, sizeof(socks->address)); } } + /* Now make sure that the chosen exit exists... */ + if (!router) { + log_warn(LD_APP, + "Unrecognized relay in exit address '%s.exit'. Refusing.", + safe_str_client(socks->address)); + connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); + return -1; + } + /* ...and make sure that it isn't excluded. */ + if (routerset_contains_router(excludeset, router)) { + log_warn(LD_APP, + "Excluded relay in exit address '%s.exit'. Refusing.", + safe_str_client(socks->address)); + connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); + return -1; + } + /* XXXX022-1090 Should we also allow foo.bar.exit if ExitNodes is set and + Bar is not listed in it? I say yes, but our revised manpage branch + implies no. */ } if (addresstype != ONION_HOSTNAME) {

1 0