- tor-commits - lists.torproject.org

[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 * <b>state</b> 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] Add an XXX022-1090 to note consider_exit_fmily b0rkenness
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit affdec8d044513fb87a6dd8931be49974326e8e7 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 25 15:02:37 2011 -0400 Add an XXX022-1090 to note consider_exit_fmily b0rkenness --- src/or/circuitbuild.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 4fa87c0..b6627a0 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4142,6 +4142,8 @@ choose_random_entry(cpath_build_state_t *state) /* 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; }

1 0

[tor/maint-0.2.2] If we're excluded, and StrictNodes is set, do not do self-tests.
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit ca74badbe95be77bd990c9c4f9c1b26052d4159e Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 28 14:14:45 2011 -0400 If we're excluded, and StrictNodes is set, do not do self-tests. --- src/or/router.c | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index 6993e1e..64cb079 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -850,14 +850,33 @@ consider_testing_reachability(int test_or, int test_dir) routerinfo_t *me = router_get_my_routerinfo(); int orport_reachable = check_whether_orport_reachable(); tor_addr_t addr; + or_options_t *options = get_options(); if (!me) return; + if (routerset_contains_router(options->ExcludeNodes, me) && + options->StrictNodes) { + /* If we've excluded ourself, and StrictNodes is set, we can't test + * ourself. */ + if (test_or || test_dir) { +#define SELF_EXCLUDED_WARN_INTERVAL 3600 + static ratelim_t warning_limit=RATELIM_INIT(SELF_EXCLUDED_WARN_INTERVAL); + char *msg; + 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 " + "be able to advertise ourself.%s", msg); + tor_free(msg); + } + } + return; + } + if (test_or && (!orport_reachable || !circuit_enough_testing_circs())) { log_info(LD_CIRC, "Testing %s of my ORPort: %s:%d.", !orport_reachable ? "reachability" : "bandwidth", me->address, me->or_port); - /* XXX022-1090 If we ExcludeNodes ourself, should this fail? -RD */ circuit_launch_by_router(CIRCUIT_PURPOSE_TESTING, me, CIRCLAUNCH_NEED_CAPACITY|CIRCLAUNCH_IS_INTERNAL); } @@ -868,7 +887,6 @@ consider_testing_reachability(int test_or, int test_dir) CONN_TYPE_DIR, &addr, me->dir_port, DIR_PURPOSE_FETCH_SERVERDESC)) { /* ask myself, via tor, for my server descriptor. */ - /* XXX022-1090 If we ExcludeNodes ourself, should this fail? -RD */ directory_initiate_command(me->address, &addr, me->or_port, me->dir_port, 0, /* does not matter */

1 0

[tor/maint-0.2.2] three more cases where maybe we want to exclude
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit 5710ea64757cf0b36ab6d97409bbb9213116f949 Author: Roger Dingledine <arma(a)torproject.org> Date: Fri Mar 11 06:19:15 2011 -0500 three more cases where maybe we want to exclude --- src/or/circuitbuild.c | 1 + src/or/router.c | 2 ++ 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index b8a82e8..4fa87c0 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4570,6 +4570,7 @@ 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 */ directory_initiate_command(address, &bridge->addr, bridge->port, 0, 0, /* does not matter */ diff --git a/src/or/router.c b/src/or/router.c index c15b9b2..6993e1e 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -857,6 +857,7 @@ consider_testing_reachability(int test_or, int test_dir) log_info(LD_CIRC, "Testing %s of my ORPort: %s:%d.", !orport_reachable ? "reachability" : "bandwidth", me->address, me->or_port); + /* XXX022-1090 If we ExcludeNodes ourself, should this fail? -RD */ circuit_launch_by_router(CIRCUIT_PURPOSE_TESTING, me, CIRCLAUNCH_NEED_CAPACITY|CIRCLAUNCH_IS_INTERNAL); } @@ -867,6 +868,7 @@ consider_testing_reachability(int test_or, int test_dir) CONN_TYPE_DIR, &addr, me->dir_port, DIR_PURPOSE_FETCH_SERVERDESC)) { /* ask myself, via tor, for my server descriptor. */ + /* XXX022-1090 If we ExcludeNodes ourself, should this fail? -RD */ directory_initiate_command(me->address, &addr, me->or_port, me->dir_port, 0, /* does not matter */

1 0

[tor/maint-0.2.2] Note that circuit purpose changing can violate ExcludeNodes
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit db2fd28308fb019e59280219cf95f09f5208092f Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 28 16:44:40 2011 -0400 Note that circuit purpose changing can violate ExcludeNodes --- src/or/rendservice.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 88f1ba3..5b85394 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1352,6 +1352,9 @@ rend_service_intro_has_opened(origin_circuit_t *circuit) 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;

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

[tor/maint-0.2.2] Note a slightly less likely way to violate ExcludeNodes
by nickm＠torproject.org 27 Apr '11

27 Apr '11

commit e4689d840266088739eee39e9bef84e13c988ce9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 28 16:51:00 2011 -0400 Note a slightly less likely way to violate ExcludeNodes --- src/or/circuitlist.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index d11b457..42073fb 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -933,6 +933,11 @@ 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 &&

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