diff --git a/changes/ticket34200 b/changes/ticket34200 new file mode 100644 index 0000000000..b984bd83bb --- /dev/null +++ b/changes/ticket34200 @@ -0,0 +1,3 @@ + o Code simplification and refactoring: + - Refactor some common node selection code into a single function. + Closes ticket 34200. diff --git a/scripts/maint/practracker/exceptions.txt b/scripts/maint/practracker/exceptions.txt index ca06d94fbd..8a62c260b5 100644 --- a/scripts/maint/practracker/exceptions.txt +++ b/scripts/maint/practracker/exceptions.txt @@ -96,7 +96,7 @@ problem function-size /src/core/or/channeltls.c:channel_tls_process_authenticate problem dependency-violation /src/core/or/channeltls.c 11 problem include-count /src/core/or/circuitbuild.c 53 problem function-size /src/core/or/circuitbuild.c:get_unique_circ_id_by_chan() 128 -problem function-size /src/core/or/circuitbuild.c:choose_good_exit_server_general() 206 +problem function-size /src/core/or/circuitbuild.c:choose_good_exit_server_general() 196 problem dependency-violation /src/core/or/circuitbuild.c 25 problem include-count /src/core/or/circuitlist.c 55 problem function-size /src/core/or/circuitlist.c:HT_PROTOTYPE() 109 @@ -259,7 +259,7 @@ problem function-size /src/feature/nodelist/node_select.c:router_pick_directory_ problem function-size /src/feature/nodelist/node_select.c:compute_weighted_bandwidths() 204 problem function-size /src/feature/nodelist/node_select.c:router_pick_trusteddirserver_impl() 116 problem function-size /src/feature/nodelist/nodelist.c:compute_frac_paths_available() 190 -problem file-size /src/feature/nodelist/routerlist.c 3300 +problem file-size /src/feature/nodelist/routerlist.c 3350 problem function-size /src/feature/nodelist/routerlist.c:router_rebuild_store() 148 problem function-size /src/feature/nodelist/routerlist.c:router_add_to_routerlist() 168 problem function-size /src/feature/nodelist/routerlist.c:routerlist_remove_old_routers() 121 diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c index 4dc3067a76..233ea9bd08 100644 --- a/src/core/or/circuitbuild.c +++ b/src/core/or/circuitbuild.c @@ -1563,7 +1563,23 @@ choose_good_exit_server_general(router_crn_flags_t flags) const node_t *selected_node=NULL; const int need_uptime = (flags & CRN_NEED_UPTIME) != 0; const int need_capacity = (flags & CRN_NEED_CAPACITY) != 0; - const int direct_conn = (flags & CRN_DIRECT_CONN) != 0; + + /* We should not require guard flags on exits. */ + IF_BUG_ONCE(flags & CRN_NEED_GUARD) + return NULL; + + /* We reject single-hop exits for all node positions. */ + IF_BUG_ONCE(flags & CRN_DIRECT_CONN) + return NULL; + + /* This isn't the function for picking rendezvous nodes. */ + IF_BUG_ONCE(flags & CRN_RENDEZVOUS_V3) + return NULL; + + /* We only want exits to extend if we cannibalize the circuit. + * But we don't require IPv6 extends yet. */ + IF_BUG_ONCE(flags & CRN_INITIATE_IPV6_EXTEND) + return NULL; connections = get_connection_array(); @@ -1596,19 +1612,14 @@ choose_good_exit_server_general(router_crn_flags_t flags) */ continue; } - if (!node_has_preferred_descriptor(node, direct_conn)) { + if (!router_can_choose_node(node, flags)) { n_supported[i] = -1; continue; } - if (!node->is_running || node->is_bad_exit) { + if (node->is_bad_exit) { n_supported[i] = -1; continue; /* skip routers that are known to be down or bad exits */ } - if (node_get_purpose(node) != ROUTER_PURPOSE_GENERAL) { - /* never pick a non-general node as a random exit. */ - n_supported[i] = -1; - continue; - } if (routerset_contains_node(options->ExcludeExitNodesUnion_, node)) { n_supported[i] = -1; continue; /* user asked us not to use it, no matter what */ @@ -1618,27 +1629,6 @@ choose_good_exit_server_general(router_crn_flags_t flags) n_supported[i] = -1; continue; /* not one of our chosen exit nodes */ } - - if (node_is_unreliable(node, need_uptime, need_capacity, 0)) { - n_supported[i] = -1; - 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 (!(node->is_valid)) { - /* if it's invalid and we don't want it */ - n_supported[i] = -1; -// log_fn(LOG_DEBUG,"Skipping node %s (index %d) -- invalid router.", -// router->nickname, i); - continue; /* skip invalid routers */ - } - /* We do not allow relays that allow single hop exits by default. Option - * was deprecated in 0.2.9.2-alpha and removed in 0.3.1.0-alpha. */ - if (node_allows_single_hop_exits(node)) { - n_supported[i] = -1; - continue; - } if (node_exit_policy_rejects_all(node)) { n_supported[i] = -1; // log_fn(LOG_DEBUG,"Skipping node %s (index %d) -- it rejects all.", @@ -1795,14 +1785,7 @@ pick_restricted_middle_node(router_crn_flags_t flags, tor_assert(pick_from); /* Add all running nodes to all_live_nodes */ - router_add_running_nodes_to_smartlist(all_live_nodes, - (flags & CRN_NEED_UPTIME) != 0, - (flags & CRN_NEED_CAPACITY) != 0, - (flags & CRN_NEED_GUARD) != 0, - (flags & CRN_NEED_DESC) != 0, - (flags & CRN_PREF_ADDR) != 0, - (flags & CRN_DIRECT_CONN) != 0, - (flags & CRN_INITIATE_IPV6_EXTEND) != 0); + router_add_running_nodes_to_smartlist(all_live_nodes, flags); /* Filter all_live_nodes to only add live *and* whitelisted middles * to the list whitelisted_live_middles. */ @@ -2120,32 +2103,27 @@ circuit_extend_to_new_exit(origin_circuit_t *circ, extend_info_t *exit_ei) return 0; } -/** Return the number of routers in routers that are currently up - * and available for building circuits through. +/** Return the number of routers in nodes that are currently up and + * available for building circuits through. * - * (Note that this function may overcount or undercount, if we have - * descriptors that are not the type we would prefer to use for some - * particular router. See bug #25885.) + * If direct is true, only count nodes that are suitable for direct + * connections. Counts nodes regardless of whether their addresses are + * preferred. */ MOCK_IMPL(STATIC int, count_acceptable_nodes, (const smartlist_t *nodes, int direct)) { int num=0; + int flags = CRN_NEED_DESC; + + if (direct) + flags |= CRN_DIRECT_CONN; SMARTLIST_FOREACH_BEGIN(nodes, const node_t *, node) { // log_debug(LD_CIRC, -// "Contemplating whether router %d (%s) is a new option.", -// i, r->nickname); - if (! node->is_running) -// log_debug(LD_CIRC,"Nope, the directory says %d is not running.",i); - continue; - if (! node->is_valid) -// log_debug(LD_CIRC,"Nope, the directory says %d is not valid.",i); - continue; - if (! node_has_preferred_descriptor(node, direct)) - continue; - /* The node has a descriptor, so we can just check the ntor key directly */ - if (!node_has_curve25519_onion_key(node)) + // "Contemplating whether router %d (%s) is a new option.", + // i, r->nickname); + if (!router_can_choose_node(node, flags)) continue; ++num; } SMARTLIST_FOREACH_END(node); diff --git a/src/feature/nodelist/node_select.c b/src/feature/nodelist/node_select.c index fe88b7230a..25904d4c63 100644 --- a/src/feature/nodelist/node_select.c +++ b/src/feature/nodelist/node_select.c @@ -930,73 +930,67 @@ nodelist_subtract(smartlist_t *sl, const smartlist_t *excluded) bitarray_free(excluded_idx); } +/* Node selection helper for router_choose_random_node(). + * + * Populates a node list based on flags, ignoring nodes in + * excludednodes and excludedset. Chooses the node based on + * rule. */ +static const node_t * +router_choose_random_node_helper(smartlist_t *excludednodes, + routerset_t *excludedset, + router_crn_flags_t flags, + bandwidth_weight_rule_t rule) +{ + smartlist_t *sl=smartlist_new(); + const node_t *choice = NULL; + + router_add_running_nodes_to_smartlist(sl, flags); + log_debug(LD_CIRC, + "We found %d running nodes.", + smartlist_len(sl)); + + nodelist_subtract(sl, excludednodes); + + if (excludedset) { + routerset_subtract_nodes(sl,excludedset); + log_debug(LD_CIRC, + "We removed excludedset, leaving %d nodes.", + smartlist_len(sl)); + } + + // Always weight by bandwidth + choice = node_sl_choose_by_bandwidth(sl, rule); + + smartlist_free(sl); + + return choice; +} + /** Return a random running node from the nodelist. Never pick a node that is * in excludedsmartlist, or which matches excludedset, even if * they are the only nodes available. * - * If the following flags are set: - * - CRN_NEED_UPTIME: if any router has more than a minimum uptime, - * return one of those; - * - CRN_NEED_CAPACITY: weight your choice by the advertised capacity - * of each router; - * - CRN_NEED_GUARD: only consider Guard routers; - * - CRN_WEIGHT_AS_EXIT: we weight bandwidths as if picking an exit - * node, otherwise we weight bandwidths for - * picking a relay node (that is, possibly - * discounting exit nodes); - * - CRN_NEED_DESC: only consider nodes that have a routerinfo or - * microdescriptor -- that is, enough info to be - * used to build a circuit; - * - CRN_DIRECT_CONN: only consider nodes that are suitable for direct - * connections. Check ReachableAddresses, - * ClientUseIPv4 0, and - * fascist_firewall_use_ipv6() == 0); - * - CRN_PREF_ADDR: only consider nodes that have an address that is - * preferred by the ClientPreferIPv6ORPort setting. - * - CRN_RENDEZVOUS_V3: only consider nodes that can become v3 onion - * service rendezvous points. - * - CRN_INITIATE_IPV6_EXTEND: only consider routers than can initiate - * IPv6 extends. + * flags is a set of CRN_* flags, see + * router_add_running_nodes_to_smartlist() for details. */ const node_t * router_choose_random_node(smartlist_t *excludedsmartlist, routerset_t *excludedset, router_crn_flags_t flags) -{ /* XXXX MOVE */ - const int need_uptime = (flags & CRN_NEED_UPTIME) != 0; - const int need_capacity = (flags & CRN_NEED_CAPACITY) != 0; - const int need_guard = (flags & CRN_NEED_GUARD) != 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; - const int direct_conn = (flags & CRN_DIRECT_CONN) != 0; - const int rendezvous_v3 = (flags & CRN_RENDEZVOUS_V3) != 0; - const bool initiate_ipv6_extend = (flags & CRN_INITIATE_IPV6_EXTEND) != 0; - - const smartlist_t *node_list = nodelist_get_list(); - smartlist_t *sl=smartlist_new(), - *excludednodes=smartlist_new(); +{ + /* A limited set of flags, used for fallback node selection. + */ + const bool need_uptime = (flags & CRN_NEED_UPTIME) != 0; + const bool need_capacity = (flags & CRN_NEED_CAPACITY) != 0; + const bool need_guard = (flags & CRN_NEED_GUARD) != 0; + const bool pref_addr = (flags & CRN_PREF_ADDR) != 0; + + smartlist_t *excludednodes=smartlist_new(); const node_t *choice = NULL; const routerinfo_t *r; bandwidth_weight_rule_t rule; - tor_assert(!(weight_for_exit && need_guard)); - rule = weight_for_exit ? WEIGHT_FOR_EXIT : - (need_guard ? WEIGHT_FOR_GUARD : WEIGHT_FOR_MID); - - SMARTLIST_FOREACH_BEGIN(node_list, const node_t *, node) { - if (node_allows_single_hop_exits(node)) { - /* Exclude relays that allow single hop exit circuits. This is an - * obsolete option since 0.2.9.2-alpha and done by default in - * 0.3.1.0-alpha. */ - smartlist_add(excludednodes, (node_t*)node); - } else if (rendezvous_v3 && - !node_supports_v3_rendezvous_point(node)) { - /* Exclude relays that can not become a rendezvous for a hidden service - * version 3. */ - smartlist_add(excludednodes, (node_t*)node); - } - } SMARTLIST_FOREACH_END(node); + rule = (need_guard ? WEIGHT_FOR_GUARD : WEIGHT_FOR_MID); /* If the node_t is not found we won't be to exclude ourself but we * won't be able to pick ourself in router_choose_random_node() so @@ -1004,31 +998,17 @@ router_choose_random_node(smartlist_t *excludedsmartlist, if ((r = router_get_my_routerinfo())) routerlist_add_node_and_family(excludednodes, r); - router_add_running_nodes_to_smartlist(sl, need_uptime, need_capacity, - need_guard, need_desc, pref_addr, - direct_conn, initiate_ipv6_extend); - log_debug(LD_CIRC, - "We found %d running nodes.", - smartlist_len(sl)); - if (excludedsmartlist) { smartlist_add_all(excludednodes, excludedsmartlist); } - nodelist_subtract(sl, excludednodes); - if (excludedset) { - routerset_subtract_nodes(sl,excludedset); - log_debug(LD_CIRC, - "We removed excludedset, leaving %d nodes.", - smartlist_len(sl)); - } + choice = router_choose_random_node_helper(excludednodes, + excludedset, + flags, + rule); - // Always weight by bandwidth - choice = node_sl_choose_by_bandwidth(sl, rule); - - smartlist_free(sl); if (!choice && (need_uptime || need_capacity || need_guard || pref_addr)) { - /* try once more -- recurse but with fewer restrictions. */ + /* try once more, with fewer restrictions. */ log_info(LD_CIRC, "We couldn't find any live%s%s%s%s routers; falling back " "to list of all routers.", @@ -1038,8 +1018,10 @@ router_choose_random_node(smartlist_t *excludedsmartlist, pref_addr?", preferred address":""); flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD| CRN_PREF_ADDR); - choice = router_choose_random_node( - excludedsmartlist, excludedset, flags); + choice = router_choose_random_node_helper(excludednodes, + excludedset, + flags, + rule); } smartlist_free(excludednodes); if (!choice) { diff --git a/src/feature/nodelist/node_select.h b/src/feature/nodelist/node_select.h index 29b6e6f2c8..1776d8ea1a 100644 --- a/src/feature/nodelist/node_select.h +++ b/src/feature/nodelist/node_select.h @@ -14,22 +14,26 @@ /** Flags to be passed to control router_choose_random_node() to indicate what * kind of nodes to pick according to what algorithm. */ typedef enum router_crn_flags_t { + /* Try to choose stable nodes. */ CRN_NEED_UPTIME = 1<<0, + /* Try to choose nodes with a reasonable amount of bandwidth. */ CRN_NEED_CAPACITY = 1<<1, - CRN_NEED_GUARD = 1<<2, - /* XXXX not used, apparently. */ - CRN_WEIGHT_AS_EXIT = 1<<5, - CRN_NEED_DESC = 1<<6, - /* On clients, only provide nodes that satisfy ClientPreferIPv6OR */ - CRN_PREF_ADDR = 1<<7, + /* Only choose nodes if we have downloaded their descriptor or + * microdescriptor. */ + CRN_NEED_DESC = 1<<2, + /* Choose nodes that can be used as Guard relays. */ + CRN_NEED_GUARD = 1<<3, /* On clients, only provide nodes that we can connect to directly, based on - * our firewall rules */ - CRN_DIRECT_CONN = 1<<8, - /* On clients, only provide nodes with HSRend >= 2 protocol version which - * is required for hidden service version >= 3. */ - CRN_RENDEZVOUS_V3 = 1<<9, + * our firewall rules. */ + CRN_DIRECT_CONN = 1<<4, + /* On clients, if choosing a node for a direct connection, only provide + * nodes that satisfy ClientPreferIPv6OR. */ + CRN_PREF_ADDR = 1<<5, + /* On clients, only provide nodes with HSRend=2 protocol version which + * is required for hidden service version 3. */ + CRN_RENDEZVOUS_V3 = 1<<6, /* On clients, only provide nodes that can initiate IPv6 extends. */ - CRN_INITIATE_IPV6_EXTEND = 1<<10, + CRN_INITIATE_IPV6_EXTEND = 1<<7, } router_crn_flags_t; /** Possible ways to weight routers when choosing one randomly. See diff --git a/src/feature/nodelist/node_st.h b/src/feature/nodelist/node_st.h index b1ec4db202..3769f9dc84 100644 --- a/src/feature/nodelist/node_st.h +++ b/src/feature/nodelist/node_st.h @@ -84,12 +84,11 @@ struct node_t { /* Local info: derived. */ - /** True if the IPv6 OR port is preferred over the IPv4 OR port. - * XX/teor - can this become out of date if the torrc changes? */ + /** True if the IPv6 OR port is preferred over the IPv4 OR port. */ unsigned int ipv6_preferred:1; /** According to the geoip db what country is this router in? */ - /* XXXprop186 what is this suppose to mean with multiple OR ports? */ + /* IPv6: what is this supposed to mean with multiple OR ports? */ country_t country; /* The below items are used only by authdirservers for diff --git a/src/feature/nodelist/nodelist.c b/src/feature/nodelist/nodelist.c index a553390628..6b2c0d2016 100644 --- a/src/feature/nodelist/nodelist.c +++ b/src/feature/nodelist/nodelist.c @@ -1158,9 +1158,9 @@ node_get_protover_summary_flags(const node_t *node) * by ed25519 ID during the link handshake. If compatible_with_us, * it needs to be using a link authentication method that we understand. * If not, any plausible link authentication method will do. */ -MOCK_IMPL(int, +MOCK_IMPL(bool, node_supports_ed25519_link_authentication,(const node_t *node, - int compatible_with_us)) + bool compatible_with_us)) { if (! node_get_ed25519_id(node)) return 0; @@ -1175,7 +1175,7 @@ node_supports_ed25519_link_authentication,(const node_t *node, /** Return true iff node supports the hidden service directory version * 3 protocol (proposal 224). */ -int +bool node_supports_v3_hsdir(const node_t *node) { tor_assert(node); @@ -1185,7 +1185,7 @@ node_supports_v3_hsdir(const node_t *node) /** Return true iff node supports ed25519 authentication as an hidden * service introduction point.*/ -int +bool node_supports_ed25519_hs_intro(const node_t *node) { tor_assert(node); @@ -1195,7 +1195,7 @@ node_supports_ed25519_hs_intro(const node_t *node) /** Return true iff node can be a rendezvous point for hidden * service version 3 (HSRend=2). */ -int +bool node_supports_v3_rendezvous_point(const node_t *node) { tor_assert(node); @@ -1210,7 +1210,7 @@ node_supports_v3_rendezvous_point(const node_t *node) /** Return true iff node supports the DoS ESTABLISH_INTRO cell * extenstion. */ -int +bool node_supports_establish_intro_dos_extension(const node_t *node) { tor_assert(node); diff --git a/src/feature/nodelist/nodelist.h b/src/feature/nodelist/nodelist.h index 991cec319b..4ba699d69d 100644 --- a/src/feature/nodelist/nodelist.h +++ b/src/feature/nodelist/nodelist.h @@ -74,17 +74,16 @@ MOCK_DECL(const struct ed25519_public_key_t *,node_get_ed25519_id, (const node_t *node)); int node_ed25519_id_matches(const node_t *node, const struct ed25519_public_key_t *id); -MOCK_DECL(int,node_supports_ed25519_link_authentication, +MOCK_DECL(bool,node_supports_ed25519_link_authentication, (const node_t *node, - int compatible_with_us)); -int node_supports_v3_hsdir(const node_t *node); -int node_supports_ed25519_hs_intro(const node_t *node); -int node_supports_v3_rendezvous_point(const node_t *node); -int node_supports_establish_intro_dos_extension(const node_t *node); + bool compatible_with_us)); +bool node_supports_v3_hsdir(const node_t *node); +bool node_supports_ed25519_hs_intro(const node_t *node); +bool node_supports_v3_rendezvous_point(const node_t *node); +bool node_supports_establish_intro_dos_extension(const node_t *node); bool node_supports_initiating_ipv6_extends(const node_t *node); -bool node_supports_accepting_ipv6_extends( - const node_t *node, - bool need_canonical_ipv6_conn); +bool node_supports_accepting_ipv6_extends(const node_t *node, + bool need_canonical_ipv6_conn); const uint8_t *node_get_rsa_id_digest(const node_t *node); MOCK_DECL(smartlist_t *,node_get_link_specifier_smartlist,(const node_t *node, diff --git a/src/feature/nodelist/routerlist.c b/src/feature/nodelist/routerlist.c index 0d3d1bea3e..ece3379fa5 100644 --- a/src/feature/nodelist/routerlist.c +++ b/src/feature/nodelist/routerlist.c @@ -511,43 +511,108 @@ routers_have_same_or_addrs(const routerinfo_t *r1, const routerinfo_t *r2) r1->ipv6_orport == r2->ipv6_orport; } +/* Returns true if node can be chosen based on flags. + * + * The following conditions are applied to all nodes: + * - is running; + * - is valid; + * - supports EXTEND2 cells; + * - has an ntor circuit crypto key; and + * - does not allow single-hop exits. + * + * If the node has a routerinfo, we're checking for a direct connection, and + * we're using bridges, the following condition is applied: + * - has a bridge-purpose routerinfo; + * and for all other nodes: + * - has a general-purpose routerinfo (or no routerinfo). + * + * Nodes that don't have a routerinfo must be general-purpose nodes, because + * routerstatuses and microdescriptors only come via consensuses. + * + * The flags chech that node: + * - CRN_NEED_UPTIME: has more than a minimum uptime; + * - CRN_NEED_CAPACITY: has more than a minimum capacity; + * - CRN_NEED_GUARD: is a Guard; + * - CRN_NEED_DESC: has a routerinfo or microdescriptor -- that is, + * enough info to be used to build a circuit; + * - CRN_DIRECT_CONN: is suitable for direct connections. Checks + * for the relevant descriptors. Checks the address + * against ReachableAddresses, ClientUseIPv4 0, and + * fascist_firewall_use_ipv6() == 0); + * - CRN_PREF_ADDR: if we are connecting directly to the node, it has + * an address that is preferred by the + * ClientPreferIPv6ORPort setting; + * - CRN_RENDEZVOUS_V3: can become a v3 onion service rendezvous point; + * - CRN_INITIATE_IPV6_EXTEND: can initiate IPv6 extends. + */ +bool +router_can_choose_node(const node_t *node, int flags) +{ + /* The full set of flags used for node selection. */ + const bool need_uptime = (flags & CRN_NEED_UPTIME) != 0; + const bool need_capacity = (flags & CRN_NEED_CAPACITY) != 0; + const bool need_guard = (flags & CRN_NEED_GUARD) != 0; + const bool need_desc = (flags & CRN_NEED_DESC) != 0; + const bool pref_addr = (flags & CRN_PREF_ADDR) != 0; + const bool direct_conn = (flags & CRN_DIRECT_CONN) != 0; + const bool rendezvous_v3 = (flags & CRN_RENDEZVOUS_V3) != 0; + const bool initiate_ipv6_extend = (flags & CRN_INITIATE_IPV6_EXTEND) != 0; + + const or_options_t *options = get_options(); + const bool check_reach = + !router_or_conn_should_skip_reachable_address_check(options, pref_addr); + const bool direct_bridge = direct_conn && options->UseBridges; + + if (!node->is_running || !node->is_valid) + return false; + if (need_desc && !node_has_preferred_descriptor(node, direct_conn)) + return false; + if (node->ri) { + if (direct_bridge && node->ri->purpose != ROUTER_PURPOSE_BRIDGE) + return false; + else if (node->ri->purpose != ROUTER_PURPOSE_GENERAL) + return false; + } + if (node_is_unreliable(node, need_uptime, need_capacity, need_guard)) + return false; + /* Don't choose nodes if we are certain they can't do EXTEND2 cells */ + if (node->rs && !routerstatus_version_supports_extend2_cells(node->rs, 1)) + return false; + /* Don't choose nodes if we are certain they can't do ntor. */ + if ((node->ri || node->md) && !node_has_curve25519_onion_key(node)) + return false; + /* Exclude relays that allow single hop exit circuits. This is an + * obsolete option since 0.2.9.2-alpha and done by default in + * 0.3.1.0-alpha. */ + if (node_allows_single_hop_exits(node)) + return false; + /* Exclude relays that can not become a rendezvous for a hidden service + * version 3. */ + if (rendezvous_v3 && + !node_supports_v3_rendezvous_point(node)) + return false; + /* Choose a node with an OR address that matches the firewall rules */ + if (direct_conn && check_reach && + !fascist_firewall_allows_node(node, + FIREWALL_OR_CONNECTION, + pref_addr)) + return false; + if (initiate_ipv6_extend && !node_supports_initiating_ipv6_extends(node)) + return false; + + return true; +} + /** Add every suitable node from our nodelist to sl, so that - * we can pick a node for a circuit. See router_choose_random_node() - * for details. + * we can pick a node for a circuit based on flags. + * + * See router_can_choose_node() for details of flags. */ void -router_add_running_nodes_to_smartlist(smartlist_t *sl, int need_uptime, - int need_capacity, int need_guard, - int need_desc, int pref_addr, - int direct_conn, - bool initiate_ipv6_extend) -{ - const int check_reach = !router_or_conn_should_skip_reachable_address_check( - get_options(), - pref_addr); - /* XXXX MOVE */ +router_add_running_nodes_to_smartlist(smartlist_t *sl, int flags) +{ SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) { - if (!node->is_running || !node->is_valid) - continue; - if (need_desc && !node_has_preferred_descriptor(node, direct_conn)) - continue; - if (node->ri && node->ri->purpose != ROUTER_PURPOSE_GENERAL) - continue; - if (node_is_unreliable(node, need_uptime, need_capacity, need_guard)) - continue; - /* Don't choose nodes if we are certain they can't do EXTEND2 cells */ - if (node->rs && !routerstatus_version_supports_extend2_cells(node->rs, 1)) - continue; - /* Don't choose nodes if we are certain they can't do ntor. */ - if ((node->ri || node->md) && !node_has_curve25519_onion_key(node)) - continue; - /* Choose a node with an OR address that matches the firewall rules */ - if (direct_conn && check_reach && - !fascist_firewall_allows_node(node, - FIREWALL_OR_CONNECTION, - pref_addr)) - continue; - if (initiate_ipv6_extend && !node_supports_initiating_ipv6_extends(node)) + if (!router_can_choose_node(node, flags)) continue; smartlist_add(sl, (void *)node); } SMARTLIST_FOREACH_END(node); diff --git a/src/feature/nodelist/routerlist.h b/src/feature/nodelist/routerlist.h index 1297bb4b65..98472b2771 100644 --- a/src/feature/nodelist/routerlist.h +++ b/src/feature/nodelist/routerlist.h @@ -58,11 +58,8 @@ int router_dir_conn_should_skip_reachable_address_check( int try_ip_pref); 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 need_uptime, - int need_capacity, int need_guard, - int need_desc, int pref_addr, - int direct_conn, - bool initiate_ipv6_extend); +bool router_can_choose_node(const node_t *node, int flags); +void router_add_running_nodes_to_smartlist(smartlist_t *sl, int flags); const routerinfo_t *routerlist_find_my_routerinfo(void); uint32_t router_get_advertised_bandwidth(const routerinfo_t *router); diff --git a/src/test/test_circuitbuild.c b/src/test/test_circuitbuild.c index 44d286e758..88e46af136 100644 --- a/src/test/test_circuitbuild.c +++ b/src/test/test_circuitbuild.c @@ -281,10 +281,10 @@ mock_node_get_by_id(const char *identity_digest) return mocked_node; } -static int mocked_supports_ed25519_link_authentication = 0; -static int +static bool mocked_supports_ed25519_link_authentication = 0; +static bool mock_node_supports_ed25519_link_authentication(const node_t *node, - int compatible_with_us) + bool compatible_with_us) { (void)node; (void)compatible_with_us;