- tor-commits - lists.torproject.org

[tor/maint-0.2.2] 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/maint-0.2.2] 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/maint-0.2.2] When there is a transition in permitted nodes, apply it to trackexithosts map
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 80adb3de507db4cd67208396e1ea301f131c1228 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Apr 3 19:43:47 2011 -0400 When there is a transition in permitted nodes, apply it to trackexithosts map IOW, if we were using TrackExitHosts, and we added an excluded node or removed a node from exitnodes, we wouldn't actually remove the mapping that points us at the new node. Also, note with an XXX022 comment a place that I think we are looking at the wrong string. --- src/or/config.c | 1 + src/or/connection_edge.c | 54 +++++++++++++++++++++++++++++++++++++++++++-- src/or/connection_edge.h | 1 + src/or/routerlist.c | 2 +- src/or/routerlist.h | 1 + 5 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 44cde85..f003e4d 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -1271,6 +1271,7 @@ options_act(or_options_t *old_options) "excluded node lists. Abandoning previous circuits."); circuit_mark_all_unused_circs(); circuit_expire_all_dirty_circs(); + addressmap_clear_excluded_trackexithosts(options); } /* How long should we delay counting bridge stats after becoming a bridge? diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index f435976..5f322ca 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -789,9 +789,6 @@ addressmap_ent_remove(const char *address, addressmap_entry_t *ent) static void clear_trackexithost_mappings(const char *exitname) { - /* XXXX022-1090 We need a variant of this that clears all mappings no longer - permitted because of changes to the ExcludeNodes, ExitNodes, or - ExcludeExitNodes settings. */ char *suffix; size_t suffix_len; if (!addressmap || !exitname) @@ -802,6 +799,7 @@ clear_trackexithost_mappings(const char *exitname) tor_strlower(suffix); STRMAP_FOREACH_MODIFY(addressmap, address, addressmap_entry_t *, ent) { + /* XXXX022 HEY! Shouldn't this look at ent->new_address? */ if (ent->source == ADDRMAPSRC_TRACKEXIT && !strcmpend(address, suffix)) { addressmap_ent_remove(address, ent); MAP_DEL_CURRENT(address); @@ -811,6 +809,56 @@ clear_trackexithost_mappings(const char *exitname) tor_free(suffix); } +/** Remove all TRACKEXIT mappings from the addressmap for which the target + * host is unknown or no longer allowed. */ +void +addressmap_clear_excluded_trackexithosts(or_options_t *options) +{ + const routerset_t *allow_nodes = options->ExitNodes; + const routerset_t *exclude_nodes = options->_ExcludeExitNodesUnion; + + if (!addressmap) + return; + if (routerset_is_empty(allow_nodes)) + allow_nodes = NULL; + if (allow_nodes == NULL && routerset_is_empty(exclude_nodes)) + return; + + STRMAP_FOREACH_MODIFY(addressmap, address, addressmap_entry_t *, ent) { + size_t len; + const char *target = ent->new_address, *dot; + char *nodename; + routerinfo_t *ri; /* XXX023 Use node_t. */ + + if (strcmpend(target, ".exit")) { + /* Not a .exit mapping */ + continue; + } else if (ent->source != ADDRMAPSRC_TRACKEXIT) { + /* Not a trackexit mapping. */ + continue; + } + len = strlen(target); + 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 */ + if (!dot) { + nodename = tor_strndup(target, len-5); + } else { + nodename = tor_strndup(dot+1, strlen(dot+1)-5); + } + ri = router_get_by_nickname(nodename, 0); + tor_free(nodename); + if (!ri || + (allow_nodes && !routerset_contains_router(allow_nodes, ri)) || + routerset_contains_router(exclude_nodes, ri)) { + /* We don't know this one, or we want to be rid of it. */ + addressmap_ent_remove(address, ent); + MAP_DEL_CURRENT(address); + } + } STRMAP_FOREACH_END; +} + /** Remove all entries from the addressmap that were set via the * configuration file or the command line. */ void diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index 0b08dd0..c4ae467 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -62,6 +62,7 @@ int connection_ap_process_transparent(edge_connection_t *conn); int address_is_invalid_destination(const char *address, int client); void addressmap_init(void); +void addressmap_clear_excluded_trackexithosts(or_options_t *options); void addressmap_clean(time_t now); void addressmap_clear_configured(void); void addressmap_clear_transient(void); diff --git a/src/or/routerlist.c b/src/or/routerlist.c index d5e8a6b..d9f099b 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -5474,7 +5474,7 @@ routerset_needs_geoip(const routerset_t *set) } /** Return true iff there are no entries in set. */ -static int +int routerset_is_empty(const routerset_t *set) { return !set || smartlist_len(set->list) == 0; diff --git a/src/or/routerlist.h b/src/or/routerlist.h index 3bbdc42..fec1870 100644 --- a/src/or/routerlist.h +++ b/src/or/routerlist.h @@ -167,6 +167,7 @@ int routerset_parse(routerset_t *target, const char *s, void routerset_union(routerset_t *target, const routerset_t *source); int routerset_is_list(const routerset_t *set); int routerset_needs_geoip(const routerset_t *set); +int routerset_is_empty(const routerset_t *set); int routerset_contains_router(const routerset_t *set, routerinfo_t *ri); int routerset_contains_routerstatus(const routerset_t *set, routerstatus_t *rs);

1 0

[tor/maint-0.2.2] 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/maint-0.2.2] Add a big changelog entry for bug 1090 fixes
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit c49f660c1a9f196c8cbb4863bb377bdb6e642539 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Apr 27 13:33:38 2011 -0400 Add a big changelog entry for bug 1090 fixes --- changes/bug1090-general | 73 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 73 insertions(+), 0 deletions(-) diff --git a/changes/bug1090-general b/changes/bug1090-general new file mode 100644 index 0000000..4656315 --- /dev/null +++ b/changes/bug1090-general @@ -0,0 +1,73 @@ + o Major features and bugfixes (node selection) + + - Revise and unify the meaning of the ExitNodes, EntryNodes, + ExcludeEntryNodes, ExcludeExitNodes, ExcludeNodes, and + StrictNodes options. Previously, we had been ambiguous in + describing what counted as an "exit" node, and what operations + exactly "StrictNodes 0" would permit. This created confusion + when people saw nodes built through unexpected circuits, and + made it hard to tell real bugs from surprises. We now stipulate + that the intended behavior is: + + . "Exit", in the context of ExitNodes and ExcludeExitNodes, + means a node that delivers user traffic outside the Tor + network. + . "Entry", in the context of EntryNodes and ExcludeEntryNodes, + means a node used as the first hop of a multihop circuit: + it doesn't include direct connections to directory servers. + . "ExcludeNodes" applies to all nodes. + . "StrictNodes" changes the behavior of ExcludeNodes only. + When StrictNodes is set, Tor should avoid all nodes listed + in ExcludeNodes, even when it will make user requests + fail. When StrictNodes is *not* set, then Tor should + follow ExcludeNodes whenever it can, except when it must + use an excluded node to perform self-tests, connect to a + hidden service, provide a hidden service, fulfill a .exit + request, upload directory information, or fetch directory + information. + + Collectively, the changes to implement the behavior are a fix for + bug 1090. + + - ExcludeNodes now takes precedence over EntryNodes and ExitNodes: + if a node is listed in both, it's treated as excluded. + + - ExcludeNodes now applies to directory nodes: as a preference if + StrictNodes is 0, or an absolute requirement if StrictNodes is 1. + (Don't exclude all the directory authorities and set StrictNodes + to 1 unless you really want your Tor to break.) + + - ExcludeNodes and ExcludeExitNodes now override exit enclaving. + + - ExcludeExitNodes now overrides .exit requests. + + - We don't use bridges from ExcludeNodes. + + - When StrictNodes is 1: + . We now apply ExcludeNodes to hidden service introduction points + and to rendezvous points selected by hidden service users. + This can make your hidden service less reliable: use it with + caution! + . If we have used ExcludeNodes on ourself, do not try self-tests. + . If we have excluded all the directory authorities, we will + not even try to upload our descriptor if we're a server. + . Do not honor .exit requests to an excluded node. + + - Remove a misfeature that caused us to ignore the Fast/Stable flags + if ExitNodes was set. Bugfix on 0.2.2.7-alpha. + + - When the set of permitted nodes changes, we now remove any + mappings introduced via TrackExitHosts to now-excluded nodes. + Bugfix on 0.1.0.1-rc. + + - We never cannibalize a circuit that had excluded nodes on it, + even if StrictNodes is 0. Bugfix on 0.1.0.1-rc. + + - Improve log messages related to excluded nodes. + + - Revert a change where we would be laxer about attaching streams to + circuits than when building the circuits. This was meant to + prevent a set of bugs where streams were never attachable, but our + improved code here should make this unnecessary. Bugfix on + 0.2.2.7-alpha. +

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] If ExitNodes and Exclude{Exit}Nodes overlap, obey Exclude{Exit}Nodes.
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 0ad3836f73cbb6f0aa8b643c43e799d69d378aea Author: Roger Dingledine <arma(a)torproject.org> Date: Fri Mar 11 04:35:08 2011 -0500 If ExitNodes and Exclude{Exit}Nodes overlap, obey Exclude{Exit}Nodes. Also, ExitNodes are always strict. --- src/or/circuitbuild.c | 56 ++++++++++++++++++++++-------------------------- src/or/routerlist.c | 4 +++ src/or/routerlist.h | 2 + 3 files changed, 32 insertions(+), 30 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 86406cb..b8a82e8 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2685,13 +2685,29 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime, n_supported[i] = -1; continue; /* skip routers that are known to be down or bad exits */ } + + if (options->_ExcludeExitNodesUnion && + routerset_contains_router(options->_ExcludeExitNodesUnion, router)) { + n_supported[i] = -1; + continue; /* user asked us not to use it, no matter what */ + } + if (options->ExitNodes && + !routerset_contains_router(options->ExitNodes, router)) { + n_supported[i] = -1; + continue; /* not one of our chosen exit nodes */ + } + if (router_is_unreliable(router, need_uptime, need_capacity, 0) && - (!options->ExitNodes || - !routerset_contains_router(options->ExitNodes, router))) { + !options->ExitNodes) { /* FFFF Someday, differentiate between a routerset that names * routers, and a routerset that names countries, and only do this * check if they've asked for specific exit relays. Or if the country * they ask for is rare. Or something. */ + /* XXX022-1090 We need to pick a tradeoff here: if we throw it out because + * it's unreliable, users might end up with no exit options even + * though some options are up. If we don't throw it out, users who + * set ExitNodes will have partitioning problems because they'll be + * the only folks willing to use this node. */ n_supported[i] = -1; continue; /* skip routers that are not suitable, unless we have * ExitNodes set, in which case we asked for it */ @@ -2753,21 +2769,13 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime, /* If any routers definitely support any pending connections, choose one * at random. */ if (best_support > 0) { - smartlist_t *supporting = smartlist_create(), *use = smartlist_create(); + smartlist_t *supporting = smartlist_create(); for (i = 0; i < smartlist_len(dir->routers); i++) if (n_supported[i] == best_support) smartlist_add(supporting, smartlist_get(dir->routers, i)); - routersets_get_disjunction(use, supporting, options->ExitNodes, - options->_ExcludeExitNodesUnion, 1); - if (smartlist_len(use) == 0 && options->ExitNodes && - !options->StrictNodes) { /* give up on exitnodes and try again */ - routersets_get_disjunction(use, supporting, NULL, - options->_ExcludeExitNodesUnion, 1); - } - router = routerlist_sl_choose_by_bandwidth(use, WEIGHT_FOR_EXIT); - smartlist_free(use); + router = routerlist_sl_choose_by_bandwidth(supporting, WEIGHT_FOR_EXIT); smartlist_free(supporting); } else { /* Either there are no pending connections, or no routers even seem to @@ -2775,7 +2783,7 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime, * at least one predicted exit port. */ int attempt; - smartlist_t *needed_ports, *supporting, *use; + smartlist_t *needed_ports, *supporting; if (best_support == -1) { if (need_uptime || need_capacity) { @@ -2792,7 +2800,6 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime, options->_ExcludeExitNodesUnion ? " or are Excluded" : ""); } supporting = smartlist_create(); - use = smartlist_create(); needed_ports = circuit_get_unhandled_ports(time(NULL)); for (attempt = 0; attempt < 2; attempt++) { /* try once to pick only from routers that satisfy a needed port, @@ -2807,25 +2814,13 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime, } } - routersets_get_disjunction(use, supporting, options->ExitNodes, - options->_ExcludeExitNodesUnion, 1); - if (smartlist_len(use) == 0 && options->ExitNodes && - !options->StrictNodes) { /* give up on exitnodes and try again */ - routersets_get_disjunction(use, supporting, NULL, - options->_ExcludeExitNodesUnion, 1); - } - /* FFF sometimes the above results in null, when the requested - * exit node is considered down by the consensus. we should pick - * it anyway, since the user asked for it. */ - router = routerlist_sl_choose_by_bandwidth(use, WEIGHT_FOR_EXIT); + router = routerlist_sl_choose_by_bandwidth(supporting, WEIGHT_FOR_EXIT); if (router) break; smartlist_clear(supporting); - smartlist_clear(use); } SMARTLIST_FOREACH(needed_ports, uint16_t *, cp, tor_free(cp)); smartlist_free(needed_ports); - smartlist_free(use); smartlist_free(supporting); } @@ -2834,10 +2829,11 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime, log_info(LD_CIRC, "Chose exit server '%s'", router->nickname); return router; } - if (options->ExitNodes && options->StrictNodes) { + if (options->ExitNodes) { log_warn(LD_CIRC, - "No specified exit routers seem to be running, and " - "StrictNodes is set: can't choose an exit."); + "No specified %sexit routers seem to be running: " + "can't choose an exit.", + options->_ExcludeExitNodesUnion ? "non-excluded " : ""); } return NULL; } diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 523596d..a9a216b 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -5473,12 +5473,14 @@ routerset_needs_geoip(const routerset_t *set) return set && smartlist_len(set->country_names); } +#if 0 /** Return true iff there are no entries in set. */ static int routerset_is_empty(const routerset_t *set) { return !set || smartlist_len(set->list) == 0; } +#endif /** Helper. Return true iff set contains a router based on the other * provided fields. Return higher values for more specific subentries: a @@ -5594,6 +5596,7 @@ routerset_get_all_routers(smartlist_t *out, const routerset_t *routerset, } } +#if 0 /** Add to target every routerinfo_t from source except: * * 1) Don't add it if include is non-empty and the relay isn't in @@ -5624,6 +5627,7 @@ routersets_get_disjunction(smartlist_t *target, } }); } +#endif /** Remove every routerinfo_t from lst that is in routerset. */ void diff --git a/src/or/routerlist.h b/src/or/routerlist.h index cd0eb95..3bbdc42 100644 --- a/src/or/routerlist.h +++ b/src/or/routerlist.h @@ -175,9 +175,11 @@ int routerset_contains_extendinfo(const routerset_t *set, void routerset_get_all_routers(smartlist_t *out, const routerset_t *routerset, const routerset_t *excludeset, int running_only); +#if 0 void routersets_get_disjunction(smartlist_t *target, const smartlist_t *source, const routerset_t *include, const routerset_t *exclude, int running_only); +#endif void routerset_subtract_routers(smartlist_t *out, const routerset_t *routerset); char *routerset_to_string(const routerset_t *routerset);

1 0

[tor/maint-0.2.2] the new entrynodes behavior is always strict
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 5d12495d982d076b74ca2c0c88b78b00d2810e71 Author: Roger Dingledine <arma(a)torproject.org> Date: Fri Mar 11 04:21:25 2011 -0500 the new entrynodes behavior is always strict --- src/or/circuitbuild.c | 36 +++++++++++------------------------- 1 files changed, 11 insertions(+), 25 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index ebbda21..86406cb 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4006,14 +4006,10 @@ entry_guards_prepend_from_config(or_options_t *options) SMARTLIST_FOREACH(entry_routers, routerinfo_t *, ri, { add_an_entry_guard(ri, 0); }); - /* Finally, the remaining previously configured guards that are not in - * EntryNodes, unless we're strict in which case we drop them */ - if (options->StrictNodes) { - SMARTLIST_FOREACH(old_entry_guards_not_on_list, entry_guard_t *, e, - entry_guard_free(e)); - } else { - smartlist_add_all(entry_guards, old_entry_guards_not_on_list); - } + /* Finally, free the remaining previously configured guards that are not in + * EntryNodes. */ + SMARTLIST_FOREACH(old_entry_guards_not_on_list, entry_guard_t *, e, + entry_guard_free(e)); smartlist_free(entry_routers); smartlist_free(entry_fps); @@ -4024,7 +4020,7 @@ entry_guards_prepend_from_config(or_options_t *options) /** Return 0 if we're fine adding arbitrary routers out of the * directory to our entry guard list, or return 1 if we have a - * list already and we'd prefer to stick to it. + * list already and we must stick to it. */ int entry_list_is_constrained(or_options_t *options) @@ -4036,18 +4032,6 @@ entry_list_is_constrained(or_options_t *options) return 0; } -/* Are we dead set against changing our entry guard list, or would we - * change it if it means keeping Tor usable? */ -static int -entry_list_is_totally_static(or_options_t *options) -{ - if (options->EntryNodes && options->StrictNodes) - return 1; - if (options->UseBridges) - return 1; - return 0; -} - /** Pick a live (up and listed) entry guard from entry_guards. If * state is non-NULL, this is for a specific circuit -- * make sure not to pick this circuit's exit or any node in the @@ -4092,6 +4076,7 @@ choose_random_entry(cpath_build_state_t *state) continue; /* don't pick the same node for entry and exit */ if (consider_exit_family && smartlist_isin(exit_family, r)) continue; /* avoid relays that are family members of our exit */ +#if 0 /* since EntryNodes is always strict now, this clause is moot */ if (options->EntryNodes && !routerset_contains_router(options->EntryNodes, r)) { /* We've come to the end of our preferred entry nodes. */ @@ -4106,6 +4091,7 @@ choose_random_entry(cpath_build_state_t *state) "No relays from EntryNodes available. Using others."); } } +#endif smartlist_add(live_entry_guards, r); if (!entry->made_contact) { /* Always start with the first not-yet-contacted entry @@ -4131,7 +4117,7 @@ choose_random_entry(cpath_build_state_t *state) } if (smartlist_len(live_entry_guards) < preferred_min) { - if (!entry_list_is_totally_static(options)) { + if (!entry_list_is_constrained(options)) { /* still no? try adding a new entry then */ /* XXX if guard doesn't imply fast and stable, then we need * to tell add_an_entry_guard below what we want, or it might @@ -4157,9 +4143,9 @@ choose_random_entry(cpath_build_state_t *state) goto retry; } if (!r && entry_list_is_constrained(options) && consider_exit_family) { - /* still no? if we're using bridges or have StrictNodes - * set, and our chosen exit is in the same family as all our - * bridges/entry guards, then be flexible about families. */ + /* 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. */ consider_exit_family = 0; goto retry; }

1 0

[tor/maint-0.2.2] Note another place that we need to fix a 1090 issue.
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit ed7c267743f2471a5a16c5ec437efda665f4c6af Author: Nick Mathewson <nickm(a)torproject.org> Date: Sun Apr 3 17:08:29 2011 -0400 Note another place that we need to fix a 1090 issue. --- src/or/connection_edge.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 4c74426..1f5eb70 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -789,6 +789,9 @@ addressmap_ent_remove(const char *address, addressmap_entry_t *ent) static void clear_trackexithost_mappings(const char *exitname) { + /* XXXX022-1090 We need a variant of this that clears all mappings no longer + permitted because of changes to the ExcludeNodes, ExitNodes, or + ExcludeExitNodes settings. */ char *suffix; size_t suffix_len; if (!addressmap || !exitname)

1 0

[tor/maint-0.2.2] Do not automatically ignore Fast/Stable for exits when ExitNodes is set
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 4851de554d5fc473cc9418b15bfb752e45b7d81d Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 28 17:29:59 2011 -0400 Do not automatically ignore Fast/Stable for exits when ExitNodes is set This once maybe made sense when ExitNodes meant "Here are 3 exits; use them all", but now it more typically means "Here are 3 countries; exit from there." Using non-Fast/Stable exits created a potential partitioning opportunity and an annoying stability problem. (Don't worry about the case where all of our ExitNodes are non-Fast or non-Stable: we handle that later in the function by retrying with need_capacity and need_uptime set to 0.) --- changes/exitnodes_reliable | 7 +++++++ src/or/circuitbuild.c | 18 +++++------------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/changes/exitnodes_reliable b/changes/exitnodes_reliable new file mode 100644 index 0000000..62ef03a --- /dev/null +++ b/changes/exitnodes_reliable @@ -0,0 +1,7 @@ + o Minor features: + - If ExitNodes is set, still pay attention to the Fast/Stable + status of exits when picking exit nodes. (We used to ignore + these flags when ExitNodes was set, on the grounds that people + who set exitnodes wanted all of those nodes to get used, but + with the ability to pick exits by country and IP range, this + doesn't necessarily make sense any more.) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index b6627a0..714d636 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -2697,20 +2697,12 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime, continue; /* not one of our chosen exit nodes */ } - if (router_is_unreliable(router, need_uptime, need_capacity, 0) && - !options->ExitNodes) { - /* FFFF Someday, differentiate between a routerset that names - * routers, and a routerset that names countries, and only do this - * check if they've asked for specific exit relays. Or if the country - * they ask for is rare. Or something. */ - /* XXX022-1090 We need to pick a tradeoff here: if we throw it out because - * it's unreliable, users might end up with no exit options even - * though some options are up. If we don't throw it out, users who - * set ExitNodes will have partitioning problems because they'll be - * the only folks willing to use this node. */ + if (router_is_unreliable(router, need_uptime, need_capacity, 0)) { n_supported[i] = -1; - continue; /* skip routers that are not suitable, unless we have - * ExitNodes set, in which case we asked for it */ + continue; /* skip routers that are not suitable. Don't worry if + * this makes us reject all the possible routers: if so, + * we'll retry later in this function with need_update and + * need_capacity set to 0. */ } if (!(router->is_valid || options->_AllowInvalid & ALLOW_INVALID_EXIT)) { /* if it's invalid and we don't want it */

1 0