- tor-commits - lists.torproject.org

[tor/master] better user-facing warnings for unexpected last hops
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit b8b557dcb2bf719976858589345f08fffbab13e3 Author: Roger Dingledine <arma(a)torproject.org> Date: Tue Apr 26 22:48:00 2011 -0400 better user-facing warnings for unexpected last hops these still aren't perfect, but we won't know how to correct them until we start experiencing surprised users. --- src/or/circuitbuild.c | 32 ++++++++++++++++++-------------- 1 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 6401e71..90572d5 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2878,7 +2878,6 @@ warn_if_last_router_excluded(origin_circuit_t *circ, const extend_info_t *exit) or_options_t *options = get_options(); routerset_t *rs = options->ExcludeNodes; const char *description; - int domain = LD_CIRC; uint8_t purpose = circ->_base.purpose; if (circ->build_state->onehop_tunnel) @@ -2914,7 +2913,6 @@ warn_if_last_router_excluded(origin_circuit_t *circ, const extend_info_t *exit) case CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED: case CIRCUIT_PURPOSE_C_REND_JOINED: description = "chosen rendezvous point"; - domain = LD_BUG; break; case CIRCUIT_PURPOSE_CONTROLLER: rs = options->_ExcludeExitNodesUnion; @@ -2924,18 +2922,24 @@ warn_if_last_router_excluded(origin_circuit_t *circ, const extend_info_t *exit) if (routerset_contains_extendinfo(rs, exit)) { /* 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); - circuit_log_path(LOG_WARN, domain, circ); + if (options->StrictNodes) { + log_warn(LD_BUG, "Using %s '%s' which is listed in ExcludeNodes%s, " + "even though StrictNodes is set. Please report. " + "(Circuit purpose: %s)", + description, exit->nickname, + rs==options->ExcludeNodes?"":" or ExcludeExitNodes", + circuit_purpose_to_string(purpose)); + } else { + log_warn(LD_CIRC, "Using %s '%s' which is listed in " + "ExcludeNodes%s, because no better options were available. To " + "prevent this (and possibly break your Tor functionality), " + "set the StrictNodes configuration option. " + "(Circuit purpose: %s)", + description, exit->nickname, + rs==options->ExcludeNodes?"":" or ExcludeExitNodes", + circuit_purpose_to_string(purpose)); + } + circuit_log_path(LOG_WARN, LD_CIRC, circ); } return;

1 0

[tor/master] Add a circuit_purpose_to_string() function, and use it
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 8ee92f28e056fd32f1faef62ae1523ad4d553a64 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 8 13:27:25 2011 -0400 Add a circuit_purpose_to_string() function, and use it We had a circuit_purpose_to_controller_string() function, but it was pretty coarse-grained and didn't try to be human-readable. --- src/or/circuitbuild.c | 10 +++++--- src/or/circuitlist.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++ src/or/circuitlist.h | 1 + src/or/circuituse.c | 22 ++++++++++-------- 4 files changed, 75 insertions(+), 14 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 681f524..b963f1a 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2051,8 +2051,9 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) */ if (timediff < 0 || timediff > 2*circ_times.close_ms+1000) { log_notice(LD_CIRC, "Strange value for circuit build time: %ldmsec. " - "Assuming clock jump. Purpose %d", timediff, - circ->_base.purpose); + "Assuming clock jump. Purpose %d (%s)", timediff, + circ->_base.purpose, + circuit_purpose_to_string(circ->_base.purpose)); } else if (!circuit_build_times_disabled()) { /* Only count circuit times if the network is live */ if (circuit_build_times_network_check_live(&circ_times)) { @@ -2890,8 +2891,9 @@ warn_if_last_router_excluded(origin_circuit_t *circ, const extend_info_t *exit) case CIRCUIT_PURPOSE_INTRO_POINT: case CIRCUIT_PURPOSE_REND_POINT_WAITING: case CIRCUIT_PURPOSE_REND_ESTABLISHED: - log_warn(LD_BUG, "Called on non-origin circuit (purpose %d)", - (int)purpose); + log_warn(LD_BUG, "Called on non-origin circuit (purpose %d, %s)", + (int)purpose, + circuit_purpose_to_string(purpose)); return; case CIRCUIT_PURPOSE_C_GENERAL: if (circ->build_state->is_internal) diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index ce324ca..33dc8f0 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -378,6 +378,62 @@ circuit_purpose_to_controller_string(uint8_t purpose) } } +/** Return a human-readable string for the circuit purpose purpose. */ +const char * +circuit_purpose_to_string(uint8_t purpose) +{ + static char buf[32]; + + switch (purpose) + { + case CIRCUIT_PURPOSE_OR: + return "Circuit at relay"; + case CIRCUIT_PURPOSE_INTRO_POINT: + return "Acting as intro point"; + case CIRCUIT_PURPOSE_REND_POINT_WAITING: + return "Acting as rendevous (pending)"; + case CIRCUIT_PURPOSE_REND_ESTABLISHED: + return "Acting as rendevous (established)"; + case CIRCUIT_PURPOSE_C_GENERAL: + return "General-purpose client"; + case CIRCUIT_PURPOSE_C_INTRODUCING: + return "Hidden service client: Connecting to intro point"; + case CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT: + return "Hidden service client: Waiting for ack from intro point"; + case CIRCUIT_PURPOSE_C_INTRODUCE_ACKED: + return "Hidden service client: Received ack from intro point"; + case CIRCUIT_PURPOSE_C_ESTABLISH_REND: + return "Hidden service client: Establishing rendezvous point"; + case CIRCUIT_PURPOSE_C_REND_READY: + return "Hidden service client: Pending rendezvous point"; + case CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED: + return "Hidden service client: Pending rendezvous point (ack received)"; + case CIRCUIT_PURPOSE_C_REND_JOINED: + return "Hidden service client: Active rendezvous point"; + case CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT: + return "Measuring circuit timeout"; + + case CIRCUIT_PURPOSE_S_ESTABLISH_INTRO: + return "Hidden service: Establishing introduction point"; + case CIRCUIT_PURPOSE_S_INTRO: + return "Hidden service: Introduction point"; + case CIRCUIT_PURPOSE_S_CONNECT_REND: + return "Hidden service: Connecting to rendezvous point"; + case CIRCUIT_PURPOSE_S_REND_JOINED: + return "Hidden service: Active rendezvous point"; + + case CIRCUIT_PURPOSE_TESTING: + return "Testing circuit"; + + case CIRCUIT_PURPOSE_CONTROLLER: + return "Circuit made by controller"; + + default: + tor_snprintf(buf, sizeof(buf), "UNKNOWN_%d", (int)purpose); + return buf; + } +} + /** Pick a reasonable package_window to start out for our circuits. * Originally this was hard-coded at 1000, but now the consensus votes * on the answer. See proposal 168. */ diff --git a/src/or/circuitlist.h b/src/or/circuitlist.h index ef6fc3a..7b01ca3 100644 --- a/src/or/circuitlist.h +++ b/src/or/circuitlist.h @@ -15,6 +15,7 @@ circuit_t * _circuit_get_global_list(void); const char *circuit_state_to_string(int state); const char *circuit_purpose_to_controller_string(uint8_t purpose); +const char *circuit_purpose_to_string(uint8_t purpose); void circuit_dump_by_conn(connection_t *conn, int severity); void circuit_set_p_circid_orconn(or_circuit_t *circ, circid_t id, or_connection_t *conn); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 7b20f7f..530941c 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -392,10 +392,11 @@ circuit_expire_building(void) TO_ORIGIN_CIRCUIT(victim)->cpath->state == CPATH_STATE_OPEN; if (TO_ORIGIN_CIRCUIT(victim)->p_streams != NULL) { - log_warn(LD_BUG, "Circuit %d (purpose %d) has timed out, " + log_warn(LD_BUG, "Circuit %d (purpose %d, %s) has timed out, " "yet has attached streams!", TO_ORIGIN_CIRCUIT(victim)->global_identifier, - victim->purpose); + victim->purpose, + circuit_purpose_to_string(victim->purpose)); tor_fragile_assert(); continue; } @@ -426,9 +427,10 @@ circuit_expire_building(void) if (timercmp(&victim->timestamp_created, &extremely_old_cutoff, <)) { log_notice(LD_CIRC, "Extremely large value for circuit build timeout: %lds. " - "Assuming clock jump. Purpose %d", + "Assuming clock jump. Purpose %d (%s)", (long)(now.tv_sec - victim->timestamp_created.tv_sec), - victim->purpose); + victim->purpose, + circuit_purpose_to_string(victim->purpose)); } else if (circuit_build_times_count_close(&circ_times, first_hop_succeeded, victim->timestamp_created.tv_sec)) { @@ -794,12 +796,11 @@ circuit_expire_old_circuits_clientside(void) circ->purpose != CIRCUIT_PURPOSE_S_INTRO) { log_notice(LD_CIRC, "Ancient non-dirty circuit %d is still around after " - "%ld milliseconds. Purpose: %d", + "%ld milliseconds. Purpose: %d (%s)", TO_ORIGIN_CIRCUIT(circ)->global_identifier, tv_mdiff(&circ->timestamp_created, &now), - circ->purpose); - /* FFFF implement a new circuit_purpose_to_string() so we don't - * just print out a number for circ->purpose */ + circ->purpose, + circuit_purpose_to_string(circ->purpose)); TO_ORIGIN_CIRCUIT(circ)->is_ancient = 1; } } @@ -1136,8 +1137,9 @@ circuit_launch_by_extend_info(uint8_t purpose, * internal circs rather than exit circs? -RD */ circ = circuit_find_to_cannibalize(purpose, extend_info, flags); if (circ) { - log_info(LD_CIRC,"Cannibalizing circ '%s' for purpose %d", - build_state_get_exit_nickname(circ->build_state), purpose); + log_info(LD_CIRC,"Cannibalizing circ '%s' for purpose %d (%s)", + build_state_get_exit_nickname(circ->build_state), purpose, + circuit_purpose_to_string(purpose)); circ->_base.purpose = purpose; /* reset the birth date of this circ, else expire_building * will see it and think it's been trying to build since it

1 0

[tor/master] Revise the manpage to contain the actual intended *Nodes behavior
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 2ac768e89f16230d9af1ea5dd84856568d4ceeef Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Mar 10 18:25:51 2011 -0500 Revise the manpage to contain the actual intended *Nodes behavior This is a squashed version of my former desired_nodes_behavior branch that we used to specify the intended results wrt bug 1090. --- doc/tor.1.txt | 74 ++++++++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 58 insertions(+), 16 deletions(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index f1734d2..866a702 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -489,32 +489,74 @@ The following options are useful only for clients (that is, if **ExcludeNodes** __node__,__node__,__...__:: A list of identity fingerprints, nicknames, country codes and address - patterns of nodes to never use when building a circuit. (Example: - ExcludeNodes SlowServer, $ EFFFFFFFFFFFFFFF, \{cc}, 255.254.0.0/8) + patterns of nodes to avoid when building a circuit. + (Example: + ExcludeNodes SlowServer, $ EFFFFFFFFFFFFFFF, \{cc}, 255.254.0.0/8) + ++ + By default, this option is treated as a preference that Tor is allowed + to override in order to keep working. + For example, if you try to connect to a hidden service, + but you have excluded all of the hidden service's introduction points, + Tor will connect to one of them anyway. If you do not want this + behavior, set the StrictNodes option (documented below). + ++ + Note also that if you are a relay, this (and the other node selection + options below) only affects your own circuits that Tor builds for you. + Clients can still build circuits through you to any node. Controllers + can tell Tor to build circuits through any node. + **ExcludeExitNodes** __node__,__node__,__...__:: A list of identity fingerprints, nicknames, country codes and address - patterns of nodes to never use when picking an exit node. Note that any + patterns of nodes to never use when picking an exit node---that is, a + node that delivers traffic for you outside the Tor network. Note that any node listed in ExcludeNodes is automatically considered to be part of this - list. + list too. See also the caveats on the "ExitNodes" option below -**EntryNodes** __node__,__node__,__...__:: - A list of identity fingerprints, nicknames and address - patterns of nodes to use for the first hop in normal circuits. These are - treated only as preferences unless StrictNodes (see below) is also set. **ExitNodes** __node__,__node__,__...__:: A list of identity fingerprints, nicknames, country codes and address - patterns of nodes to use for the last hop in normal exit circuits. These - are treated only as preferences unless StrictNodes (see below) is also set. + patterns of nodes to use as exit node---that is, a + node that delivers traffic for you outside the Tor network. + ++ + Note that if you list too few nodes here, or if you exclude too many exit + nodes with ExcludeExitNodes, you can degrade functionality. For example, + if none of the exits you list allows traffic on port 80 or 443, you won't + be able to browse the web. + ++ + Note also that not every circuit is used to deliver traffic outside of + the Tor network. It is normal to see non-exit circuits (such as those + used to connect to hidden services, those that do directory fetches, + those used for self-tests, and so on) that end at a non-exit node. To + keep a node from being used entirely, see ExcludeNodes and StrictNodes. + ++ + The ExcludeNodes option overrides this option: any node listed in both + ExitNodes and ExcludeNodes is treated as excluded. + ++ + The .exit address notation, if enabled, overrides this option. + +**EntryNodes** __node__,__node__,__...__:: + A list of identity fingerprints and nicknames of nodes + to use for the first hop in your normal circuits. (Country codes and + address patterns are not yet supported.) This includes all + circuits except for direct connections to directory servers. The Bridge + option overrides this option; if you have configured bridges and + UseBridges is 1, the Bridges are used as your entry nodes. + ++ + The ExcludeNodes option overrides this option: any node listed in both + EntryNodes and ExcludeNodes is treated as excluded. **StrictNodes** **0**|**1**:: - If 1 and EntryNodes config option is set, Tor will never use any nodes - besides those listed in EntryNodes for the first hop of a normal circuit. - If 1 and ExitNodes config option is set, Tor will never use any nodes - besides those listed in ExitNodes for the last hop of a normal exit - circuit. Note that Tor might still use these nodes for non-exit circuits - such as one-hop directory fetches or hidden service support circuits. + If StrictNodes is set to 1, Tor will treat the ExcludeNodes option as a + requirement to follow for all the circuits you generate, even if doing so + will break functionality for you. If StrictNodes is set to 0, Tor will + still try to avoid nodes in the ExcludeNodes list, but it will err on the + side of avoiding unexpected errors. Specifically, StrictNodes 0 tells + Tor that it is okay to use an excluded node when it is *necessary* to + perform self-tests, connect to + a hidden service, provide a hidden service to a client, fulfill a .exit + request, upload directory information, or download directory information. + (Default: 0) **FascistFirewall** **0**|**1**:: If 1, Tor will only create outgoing connections to ORs running on ports

1 0

[tor/master] 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/master] Instead of checking whether we have unremoved intro points, check for usable ones
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 748350ace11fa55758fb8bafe5e5556867dd9c23 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 20 16:49:41 2011 -0400 Instead of checking whether we have unremoved intro points, check for usable ones --- src/or/rendclient.c | 44 +++++++++++++++++++++++++++++--------------- src/or/rendclient.h | 1 + src/or/rendcommon.c | 2 +- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 90304c3..d444611 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -23,7 +23,8 @@ #include "routerlist.h" static extend_info_t *rend_client_get_random_intro_impl( - const rend_data_t *rend_query, const int strict); + const rend_cache_entry_t *rend_query, + const int strict, const int warnings); /** Called when we've established a circuit to an introduction point: * send the introduction request. */ @@ -562,7 +563,7 @@ rend_client_remove_intro_point(extend_info_t *failed_intro, } } - if (smartlist_len(ent->parsed->intro_nodes) == 0) { + if (! rend_client_any_intro_points_usable(ent)) { log_info(LD_REND, "No more intro points remain for %s. Re-fetching descriptor.", escaped_safe_str_client(rend_query->onion_address)); @@ -708,7 +709,7 @@ rend_client_desc_trynow(const char *query) assert_connection_ok(TO_CONN(conn), now); if (rend_cache_lookup_entry(conn->rend_data->onion_address, -1, &entry) == 1 && - smartlist_len(entry->parsed->intro_nodes) > 0) { + rend_client_any_intro_points_usable(entry)) { /* either this fetch worked, or it failed but there was a * valid entry from before which we should reuse */ log_info(LD_REND,"Rend desc is usable. Launching circuits."); @@ -743,13 +744,22 @@ extend_info_t * rend_client_get_random_intro(const rend_data_t *rend_query) { extend_info_t *result; + rend_cache_entry_t *entry; + + if (rend_cache_lookup_entry(rend_query->onion_address, -1, &entry) < 1) { + log_warn(LD_REND, + "Query '%s' didn't have valid rend desc in cache. Failing.", + safe_str_client(rend_query->onion_address)); + return NULL; + } + /* See if we can get a node that complies with ExcludeNodes */ - if ((result = rend_client_get_random_intro_impl(rend_query, 1))) + if ((result = rend_client_get_random_intro_impl(entry, 1, 1))) return result; /* If not, and StrictNodes is not set, see if we can return any old node */ if (!get_options()->StrictNodes) - return rend_client_get_random_intro_impl(rend_query, 0); + return rend_client_get_random_intro_impl(entry, 0, 1); return NULL; } @@ -757,23 +767,18 @@ rend_client_get_random_intro(const rend_data_t *rend_query) * iff strict is true. */ static extend_info_t * -rend_client_get_random_intro_impl(const rend_data_t *rend_query, - const int strict) +rend_client_get_random_intro_impl(const rend_cache_entry_t *entry, + const int strict, + const int warnings) { int i; - rend_cache_entry_t *entry; + rend_intro_point_t *intro; routerinfo_t *router; or_options_t *options = get_options(); smartlist_t *usable_nodes; int n_excluded = 0; - if (rend_cache_lookup_entry(rend_query->onion_address, -1, &entry) < 1) { - log_warn(LD_REND, - "Query '%s' didn't have valid rend desc in cache. Failing.", - safe_str_client(rend_query->onion_address)); - return NULL; - } /* We'll keep a separate list of the usable nodes. If this becomes empty, * no nodes are usable. */ usable_nodes = smartlist_create(); @@ -781,7 +786,7 @@ rend_client_get_random_intro_impl(const rend_data_t *rend_query, again: if (smartlist_len(usable_nodes) == 0) { - if (n_excluded && get_options()->StrictNodes) { + if (n_excluded && get_options()->StrictNodes && warnings) { /* We only want to warn if StrictNodes is really set. Otherwise * we're just about to retry anyways. */ @@ -822,6 +827,15 @@ rend_client_get_random_intro_impl(const rend_data_t *rend_query, return extend_info_dup(intro->extend_info); } +/** Return true iff any introduction points still listed in entry are + * usable. */ +int +rend_client_any_intro_points_usable(const rend_cache_entry_t *entry) +{ + return rend_client_get_random_intro_impl( + entry, get_options()->StrictNodes, 0) != NULL; +} + /** Client-side authorizations for hidden services; map of onion address to * rend_service_authorization_t*. */ static strmap_t *auth_hid_servs = NULL; diff --git a/src/or/rendclient.h b/src/or/rendclient.h index 56ccde1..3f2e58e 100644 --- a/src/or/rendclient.h +++ b/src/or/rendclient.h @@ -29,6 +29,7 @@ int rend_client_receive_rendezvous(origin_circuit_t *circ, void rend_client_desc_trynow(const char *query); extend_info_t *rend_client_get_random_intro(const rend_data_t *rend_query); +int rend_client_any_intro_points_usable(const rend_cache_entry_t *entry); int rend_client_send_introduction(origin_circuit_t *introcirc, origin_circuit_t *rendcirc); diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index f4c8888..9d6a89e 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -934,7 +934,7 @@ rend_cache_lookup_entry(const char *query, int version, rend_cache_entry_t **e) tor_assert((*e)->parsed && (*e)->parsed->intro_nodes); /* XXX023 hack for now, to return "not found" if there are no intro * points remaining. See bug 997. */ - if (smartlist_len((*e)->parsed->intro_nodes) == 0) + if (! rend_client_any_intro_points_usable(*e)) return 0; return 1; }

1 0

[tor/master] explain an argument in a function comment
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 0c40dda3ad824d8a782dafae4c4233a949837df3 Author: Roger Dingledine <arma(a)torproject.org> Date: Wed Apr 27 13:43:11 2011 -0400 explain an argument in a function comment --- src/or/rendclient.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/or/rendclient.c b/src/or/rendclient.c index d444611..65e632f 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -764,7 +764,8 @@ rend_client_get_random_intro(const rend_data_t *rend_query) } /** As rend_client_get_random_intro, except assume that StrictNodes is set - * iff strict is true. + * iff strict is true. If warnings is false, don't complain + * to the user when we're out of nodes, even if StrictNodes is true. */ static extend_info_t * rend_client_get_random_intro_impl(const rend_cache_entry_t *entry,

1 0

[tor/master] Merge branch 'bug1090-manpage-squashed' into bug1090-part1-squashed
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 8f6cb933b6a8ee63f2e208ec43933ad27afa2af0 Merge: c49f660 2ac768e Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 27 13:44:55 2011 -0400 Merge branch 'bug1090-manpage-squashed' into bug1090-part1-squashed doc/tor.1.txt | 74 ++++++++++++++++++++++++++++++++++++++++++++------------ 1 files changed, 58 insertions(+), 16 deletions(-)

1 0

[tor/master] 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/master] 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/master] 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