commit 268608c0a0605e596d1a884ee35d432c88bac38b Author: teor (Tim Wilson-Brown) <teor2345@gmail.com> Date: Fri Dec 18 11:28:54 2015 +1100 Choose OR Entry Guards using IPv4/IPv6 preferences Update unit tests. --- src/or/circuitbuild.c | 15 +++----- src/or/or.h | 4 +- src/or/routerlist.c | 18 +++++++-- src/or/routerlist.h | 3 +- src/test/test_entrynodes.c | 93 +++++++++++++++++++++++++++++++++++++++++----- 5 files changed, 108 insertions(+), 25 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index d44fcd7..dcb9de3 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1778,7 +1778,7 @@ pick_tor2web_rendezvous_node(router_crn_flags_t flags, router_add_running_nodes_to_smartlist(all_live_nodes, allow_invalid, 0, 0, 0, - need_desc); + need_desc, 0); /* Filter all_live_nodes to only add live *and* whitelisted RPs to * the list whitelisted_live_rps. */ @@ -2144,7 +2144,9 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state) const node_t *choice; smartlist_t *excluded; const or_options_t *options = get_options(); - router_crn_flags_t flags = CRN_NEED_GUARD|CRN_NEED_DESC; + /* If possible, choose an entry server with a preferred address, + * otherwise, choose one with an allowed address */ + router_crn_flags_t flags = CRN_NEED_GUARD|CRN_NEED_DESC|CRN_PREF_ADDR; const node_t *node; if (state && options->UseEntryGuards && @@ -2161,12 +2163,6 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state) * family. */ nodelist_add_node_and_family(excluded, node); } - /* Exclude all ORs that we can't reach through our firewall */ - smartlist_t *nodes = nodelist_get_list(); - SMARTLIST_FOREACH(nodes, const node_t *, node, { - if (!fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION, 0)) - smartlist_add(excluded, (void*)node); - }); /* and exclude current entry guards and their families, * unless we're in a test network, and excluding guards * would exclude all nodes (i.e. we're in an incredibly small tor network, @@ -2332,8 +2328,9 @@ extend_info_from_node(const node_t *node, int for_direct_connect) if (node->ri == NULL && (node->rs == NULL || node->md == NULL)) return NULL; + /* Choose a preferred address first, but fall back to an allowed address*/ if (for_direct_connect) - node_get_pref_orport(node, &ap); + fascist_firewall_choose_address_node(node, FIREWALL_OR_CONNECTION, 0, &ap); else node_get_prim_orport(node, &ap); diff --git a/src/or/or.h b/src/or/or.h index b1765d1..412789c 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -5221,7 +5221,9 @@ typedef enum { CRN_ALLOW_INVALID = 1<<3, /* XXXX not used, apparently. */ CRN_WEIGHT_AS_EXIT = 1<<5, - CRN_NEED_DESC = 1<<6 + CRN_NEED_DESC = 1<<6, + /* On clients, only provide nodes that satisfy ClientPreferIPv6OR */ + CRN_PREF_ADDR = 1<<7 } router_crn_flags_t; /** Return value for router_add_to_routerlist() and dirserv_add_descriptor() */ diff --git a/src/or/routerlist.c b/src/or/routerlist.c index c45854c..804ff29 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1826,7 +1826,8 @@ routerlist_add_node_and_family(smartlist_t *sl, const routerinfo_t *router) void router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid, int need_uptime, int need_capacity, - int need_guard, int need_desc) + int need_guard, int need_desc, + int pref_addr) { /* XXXX MOVE */ SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) { if (!node->is_running || @@ -1838,6 +1839,9 @@ router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid, continue; if (node_is_unreliable(node, need_uptime, need_capacity, need_guard)) continue; + /* Choose a node with a preferred OR address */ + if (!fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION, pref_addr)) + continue; smartlist_add(sl, (void *)node); } SMARTLIST_FOREACH_END(node); @@ -2299,6 +2303,10 @@ node_sl_choose_by_bandwidth(const smartlist_t *sl, * If <b>CRN_NEED_DESC</b> is set in flags, we only consider nodes that * have a routerinfo or microdescriptor -- that is, enough info to be * used to build a circuit. + * If <b>CRN_PREF_ADDR</b> is set in flags, we only consider nodes that + * have an address that is preferred by the ClientPreferIPv6ORPort setting + * (regardless of this flag, we exclude nodes that aren't allowed by the + * firewall, including ClientUseIPv4 0 and ClientUseIPv6 0). */ const node_t * router_choose_random_node(smartlist_t *excludedsmartlist, @@ -2311,6 +2319,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist, const int allow_invalid = (flags & CRN_ALLOW_INVALID) != 0; const int weight_for_exit = (flags & CRN_WEIGHT_AS_EXIT) != 0; const int need_desc = (flags & CRN_NEED_DESC) != 0; + const int pref_addr = (flags & CRN_PREF_ADDR) != 0; smartlist_t *sl=smartlist_new(), *excludednodes=smartlist_new(); @@ -2336,7 +2345,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist, router_add_running_nodes_to_smartlist(sl, allow_invalid, need_uptime, need_capacity, - need_guard, need_desc); + need_guard, need_desc, pref_addr); log_debug(LD_CIRC, "We found %d running nodes.", smartlist_len(sl)); @@ -2365,7 +2374,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist, choice = node_sl_choose_by_bandwidth(sl, rule); smartlist_free(sl); - if (!choice && (need_uptime || need_capacity || need_guard)) { + if (!choice && (need_uptime || need_capacity || need_guard || pref_addr)) { /* try once more -- recurse but with fewer restrictions. */ log_info(LD_CIRC, "We couldn't find any live%s%s%s routers; falling back " @@ -2373,7 +2382,8 @@ router_choose_random_node(smartlist_t *excludedsmartlist, need_capacity?", fast":"", need_uptime?", stable":"", need_guard?", guard":""); - flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD); + flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD| + CRN_PREF_ADDR); choice = router_choose_random_node( excludedsmartlist, excludedset, flags); } diff --git a/src/or/routerlist.h b/src/or/routerlist.h index 339e34a..4c828d6 100644 --- a/src/or/routerlist.h +++ b/src/or/routerlist.h @@ -61,7 +61,8 @@ void router_reset_status_download_failures(void); int routers_have_same_or_addrs(const routerinfo_t *r1, const routerinfo_t *r2); void router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid, int need_uptime, int need_capacity, - int need_guard, int need_desc); + int need_guard, int need_desc, + int pref_addr); const routerinfo_t *routerlist_find_my_routerinfo(void); uint32_t router_get_advertised_bandwidth(const routerinfo_t *router); diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index 87276db..e4947e0 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -70,6 +70,14 @@ fake_network_setup(const struct testcase_t *testcase) return dummy_state; } +static or_options_t mocked_options; + +static const or_options_t * +mock_get_options(void) +{ + return &mocked_options; +} + /** Test choose_random_entry() with none of our routers being guard nodes. */ static void test_choose_random_entry_no_guards(void *arg) @@ -78,6 +86,14 @@ test_choose_random_entry_no_guards(void *arg) (void) arg; + MOCK(get_options, mock_get_options); + + /* Check that we get a guard if it passes preferred + * address settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 1; + mocked_options.ClientPreferIPv6ORPort = 0; + /* Try to pick an entry even though none of our routers are guards. */ chosen_entry = choose_random_entry(NULL); @@ -86,8 +102,35 @@ test_choose_random_entry_no_guards(void *arg) can't find a proper entry guard. */ tt_assert(chosen_entry); + /* And with the other IP version active */ + mocked_options.ClientUseIPv6 = 1; + chosen_entry = choose_random_entry(NULL); + tt_assert(chosen_entry); + + /* Check that we don't get a guard if it doesn't pass mandatory address + * settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 0; + mocked_options.ClientPreferIPv6ORPort = 0; + + chosen_entry = choose_random_entry(NULL); + + /* If we don't allow IPv4 at all, we don't get a guard*/ + tt_assert(!chosen_entry); + + /* Check that we get a guard if it passes allowed but not preferred address + * settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 1; + mocked_options.ClientUseIPv6 = 1; + mocked_options.ClientPreferIPv6ORPort = 1; + + chosen_entry = choose_random_entry(NULL); + tt_assert(chosen_entry); + done: - ; + memset(&mocked_options, 0, sizeof(mocked_options)); + UNMOCK(get_options); } /** Test choose_random_entry() with only one of our routers being a @@ -101,17 +144,55 @@ test_choose_random_entry_one_possible_guard(void *arg) (void) arg; + MOCK(get_options, mock_get_options); + /* Set one of the nodes to be a guard. */ our_nodelist = nodelist_get_list(); the_guard = smartlist_get(our_nodelist, 4); /* chosen by fair dice roll */ the_guard->is_possible_guard = 1; + /* Check that we get the guard if it passes preferred + * address settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 1; + mocked_options.ClientPreferIPv6ORPort = 0; + /* Pick an entry. Make sure we pick the node we marked as guard. */ chosen_entry = choose_random_entry(NULL); tt_ptr_op(chosen_entry, OP_EQ, the_guard); + /* And with the other IP version active */ + mocked_options.ClientUseIPv6 = 1; + chosen_entry = choose_random_entry(NULL); + tt_ptr_op(chosen_entry, OP_EQ, the_guard); + + /* Check that we don't get a guard if it doesn't pass mandatory address + * settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 0; + mocked_options.ClientPreferIPv6ORPort = 0; + + chosen_entry = choose_random_entry(NULL); + + /* If we don't allow IPv4 at all, we don't get a guard*/ + tt_assert(!chosen_entry); + + /* Check that we get a node if it passes allowed but not preferred + * address settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 1; + mocked_options.ClientUseIPv6 = 1; + mocked_options.ClientPreferIPv6ORPort = 1; + + chosen_entry = choose_random_entry(NULL); + + /* We disable the guard check and the preferred address check at the same + * time, so we can't be sure we get the guard */ + tt_assert(chosen_entry); + done: - ; + memset(&mocked_options, 0, sizeof(mocked_options)); + UNMOCK(get_options); } /** Helper to conduct tests for populate_live_entry_guards(). @@ -624,14 +705,6 @@ test_entry_is_live(void *arg) ; /* XXX */ } -static or_options_t mocked_options; - -static const or_options_t * -mock_get_options(void) -{ - return &mocked_options; -} - #define TEST_IPV4_ADDR "123.45.67.89" #define TEST_IPV6_ADDR "[1234:5678:90ab:cdef::]"