commit 2b9823b3106df2cf23e2de13ae9a9b6d12607ce4 Author: David Goulet dgoulet@torproject.org Date: Tue Apr 25 13:22:35 2017 -0400

config: Remove AllowInvalidNodes option

Deprecated in 0.2.9.2-alpha, this commits changes it as OBSOLETE() and cleans up the code associated with it.

Partially fixes #22060

Signed-off-by: David Goulet dgoulet@torproject.org --- changes/bug22060 | 3 +++ doc/tor.1.txt | 7 ------- src/or/circuitbuild.c | 17 +---------------- src/or/config.c | 26 +------------------------ src/or/or.h | 14 -------------- src/or/rendservice.c | 2 -- src/or/routerlist.c | 17 ++++++----------- src/or/routerlist.h | 8 ++++---- src/test/test_options.c | 51 ------------------------------------------------- 9 files changed, 15 insertions(+), 130 deletions(-)

diff --git a/changes/bug22060 b/changes/bug22060 new file mode 100644 index 0000000..d839c9b --- /dev/null +++ b/changes/bug22060 @@ -0,0 +1,3 @@ + o Remove configuration option (confic): + - AllowInvalidNodes was deprecated in 0.2.9.2-alpha and now has been + rendered obsolete. Code has been removed and feature no longer exists. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index c769237..012c5b9 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -772,13 +772,6 @@ CLIENT OPTIONS The following options are useful only for clients (that is, if **SocksPort**, **TransPort**, **DNSPort**, or **NATDPort** is non-zero):

-[[AllowInvalidNodes]] **AllowInvalidNodes** **entry**|**exit**|**middle**|**introduction**|**rendezvous**|**...**:: - If some Tor servers are obviously not working right, the directory - authorities can manually mark them as invalid, meaning that it's not - recommended you use them for entry or exit positions in your circuits. You - can opt to use them in some circuit positions, though. The default is - "middle,rendezvous", and other choices are not advised. - [[ExcludeSingleHopRelays]] **ExcludeSingleHopRelays** **0**|**1**:: This option controls whether circuits built by Tor will include relays with the AllowSingleHopExits flag set to true. If ExcludeSingleHopRelays is set diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index f8b3609..faf2e3d 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1828,7 +1828,7 @@ choose_good_exit_server_general(int need_uptime, int need_capacity) * we'll retry later in this function with need_update and * need_capacity set to 0. */ } - if (!(node->is_valid || options->AllowInvalid_ & ALLOW_INVALID_EXIT)) { + 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.", @@ -1968,7 +1968,6 @@ pick_tor2web_rendezvous_node(router_crn_flags_t flags, const or_options_t *options) { const node_t *rp_node = NULL; - const int allow_invalid = (flags & CRN_ALLOW_INVALID) != 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; @@ -1980,7 +1979,6 @@ pick_tor2web_rendezvous_node(router_crn_flags_t flags,

/* Add all running nodes to all_live_nodes */ router_add_running_nodes_to_smartlist(all_live_nodes, - allow_invalid, 0, 0, 0, need_desc, pref_addr, @@ -2022,9 +2020,6 @@ pick_rendezvous_node(router_crn_flags_t flags) { const or_options_t *options = get_options();

- if (options->AllowInvalid_ & ALLOW_INVALID_RENDEZVOUS) - flags |= CRN_ALLOW_INVALID; - #ifdef ENABLE_TOR2WEB_MODE /* We want to connect directly to the node if we can */ router_crn_flags_t direct_flags = flags; @@ -2081,8 +2076,6 @@ choose_good_exit_server(uint8_t purpose,

switch (purpose) { case CIRCUIT_PURPOSE_C_GENERAL: - if (options->AllowInvalid_ & ALLOW_INVALID_MIDDLE) - flags |= CRN_ALLOW_INVALID; if (is_internal) /* pick it like a middle hop */ return router_choose_random_node(NULL, options->ExcludeNodes, flags); else @@ -2280,10 +2273,6 @@ count_acceptable_nodes, (smartlist_t *nodes)) if (! node->is_running) // log_debug(LD_CIRC,"Nope, the directory says %d is not running.",i); continue; - /* XXX This clause makes us count incorrectly: if AllowInvalidRouters - * allows this node in some places, then we're getting an inaccurate - * count. For now, be conservative and don't count it. But later we - * should try to be smarter. */ if (! node->is_valid) // log_debug(LD_CIRC,"Nope, the directory says %d is not valid.",i); continue; @@ -2354,8 +2343,6 @@ choose_good_middle_server(uint8_t purpose, flags |= CRN_NEED_UPTIME; if (state->need_capacity) flags |= CRN_NEED_CAPACITY; - if (options->AllowInvalid_ & ALLOW_INVALID_MIDDLE) - flags |= CRN_ALLOW_INVALID; choice = router_choose_random_node(excluded, options->ExcludeNodes, flags); smartlist_free(excluded); return choice; @@ -2408,8 +2395,6 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state, if (state->need_capacity) flags |= CRN_NEED_CAPACITY; } - if (options->AllowInvalid_ & ALLOW_INVALID_ENTRY) - flags |= CRN_ALLOW_INVALID;

choice = router_choose_random_node(excluded, options->ExcludeNodes, flags); smartlist_free(excluded); diff --git a/src/or/config.c b/src/or/config.c index a73f397..6edfac3 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -205,7 +205,7 @@ static config_var_t option_vars_[] = { V(AccountingStart, STRING, NULL), V(Address, STRING, NULL), V(AllowDotExit, BOOL, "0"), - V(AllowInvalidNodes, CSV, "middle,rendezvous"), + OBSOLETE("AllowInvalidNodes"), V(AllowNonRFC953Hostnames, BOOL, "0"), V(AllowSingleHopCircuits, BOOL, "0"), V(AllowSingleHopExits, BOOL, "0"), @@ -662,8 +662,6 @@ static const config_deprecation_t option_deprecation_notes_[] = { /* Deprecated since 0.2.9.2-alpha... */ { "AllowDotExit", "Unrestricted use of the .exit notation can be used for " "a wide variety of application-level attacks." }, - { "AllowInvalidNodes", "There is no reason to enable this option; at best " - "it will make you easier to track." }, { "AllowSingleHopCircuits", "Almost no relays actually allow single-hop " "exits, making this option pointless." }, { "AllowSingleHopExits", "Turning this on will make your relay easier " @@ -3372,28 +3370,6 @@ options_validate(or_options_t *old_options, or_options_t *options, server_mode(options)); options->MaxMemInQueues_low_threshold = (options->MaxMemInQueues / 4) * 3;

- options->AllowInvalid_ = 0; - - if (options->AllowInvalidNodes) { - SMARTLIST_FOREACH_BEGIN(options->AllowInvalidNodes, const char *, cp) { - if (!strcasecmp(cp, "entry")) - options->AllowInvalid_ |= ALLOW_INVALID_ENTRY; - else if (!strcasecmp(cp, "exit")) - options->AllowInvalid_ |= ALLOW_INVALID_EXIT; - else if (!strcasecmp(cp, "middle")) - options->AllowInvalid_ |= ALLOW_INVALID_MIDDLE; - else if (!strcasecmp(cp, "introduction")) - options->AllowInvalid_ |= ALLOW_INVALID_INTRODUCTION; - else if (!strcasecmp(cp, "rendezvous")) - options->AllowInvalid_ |= ALLOW_INVALID_RENDEZVOUS; - else { - tor_asprintf(msg, - "Unrecognized value '%s' in AllowInvalidNodes", cp); - return -1; - } - } SMARTLIST_FOREACH_END(cp); - } - if (!options->SafeLogging || !strcasecmp(options->SafeLogging, "0")) { options->SafeLogging_ = SAFELOG_SCRUB_NONE; diff --git a/src/or/or.h b/src/or/or.h index 9e3e409..53a8710 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3470,15 +3470,6 @@ static inline const origin_circuit_t *CONST_TO_ORIGIN_CIRCUIT( return DOWNCAST(origin_circuit_t, x); }

-/** Bitfield type: things that we're willing to use invalid routers for. */ -typedef enum invalid_router_usage_t { - ALLOW_INVALID_ENTRY =1, - ALLOW_INVALID_EXIT =2, - ALLOW_INVALID_MIDDLE =4, - ALLOW_INVALID_RENDEZVOUS =8, - ALLOW_INVALID_INTRODUCTION=16, -} invalid_router_usage_t; - /* limits for TCP send and recv buffer size used for constrained sockets */ #define MIN_CONSTRAINED_TCP_BUFFER 2048 #define MAX_CONSTRAINED_TCP_BUFFER 262144 /* 256k */ @@ -3604,10 +3595,6 @@ typedef struct { int DisableAllSwap; /**< Boolean: Attempt to call mlockall() on our * process for all current and future memory. */

- /** List of "entry", "middle", "exit", "introduction", "rendezvous". */ - smartlist_t *AllowInvalidNodes; - /** Bitmask; derived from AllowInvalidNodes. */ - invalid_router_usage_t AllowInvalid_; config_line_t *ExitPolicy; /**< Lists of exit policy components. */ int ExitPolicyRejectPrivate; /**< Should we not exit to reserved private * addresses, and our own published addresses? @@ -5356,7 +5343,6 @@ typedef enum { CRN_NEED_UPTIME = 1<<0, CRN_NEED_CAPACITY = 1<<1, CRN_NEED_GUARD = 1<<2, - CRN_ALLOW_INVALID = 1<<3, /* XXXX not used, apparently. */ CRN_WEIGHT_AS_EXIT = 1<<5, CRN_NEED_DESC = 1<<6, diff --git a/src/or/rendservice.c b/src/or/rendservice.c index c19c85f..281736c 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -4179,8 +4179,6 @@ rend_consider_services_intro_points(void) const node_t *node; rend_intro_point_t *intro; router_crn_flags_t flags = CRN_NEED_UPTIME|CRN_NEED_DESC; - if (get_options()->AllowInvalid_ & ALLOW_INVALID_INTRODUCTION) - flags |= CRN_ALLOW_INVALID; router_crn_flags_t direct_flags = flags; direct_flags |= CRN_PREF_ADDR; direct_flags |= CRN_DIRECT_CONN; diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 35fe501..b3b959a 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -2317,17 +2317,16 @@ routerlist_add_node_and_family(smartlist_t *sl, const routerinfo_t *router) * we can pick a node for a circuit. */ 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 pref_addr, int direct_conn) +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) { const int check_reach = !router_skip_or_reachability(get_options(), pref_addr); /* XXXX MOVE */ SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) { - if (!node->is_running || - (!node->is_valid && !allow_invalid)) + if (!node->is_running || !node->is_valid) continue; if (need_desc && !(node->ri || (node->rs && node->md))) continue; @@ -2773,8 +2772,6 @@ node_sl_choose_by_bandwidth(const smartlist_t *sl, * a minimum uptime, return one of those. * If <b>CRN_NEED_CAPACITY</b> is set in flags, weight your choice by the * advertised capacity of each router. - * If <b>CRN_ALLOW_INVALID</b> is not set in flags, consider only Valid - * routers. * If <b>CRN_NEED_GUARD</b> is set in flags, consider only Guard routers. * If <b>CRN_WEIGHT_AS_EXIT</b> is set in flags, we weight bandwidths as if * picking an exit node, otherwise we weight bandwidths for picking a relay @@ -2795,7 +2792,6 @@ router_choose_random_node(smartlist_t *excludedsmartlist, 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 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; @@ -2823,8 +2819,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist, if ((r = routerlist_find_my_routerinfo())) routerlist_add_node_and_family(excludednodes, r);

- router_add_running_nodes_to_smartlist(sl, allow_invalid, - need_uptime, need_capacity, + router_add_running_nodes_to_smartlist(sl, need_uptime, need_capacity, need_guard, need_desc, pref_addr, direct_conn); log_debug(LD_CIRC, diff --git a/src/or/routerlist.h b/src/or/routerlist.h index 5c1f76c..25b2f64 100644 --- a/src/or/routerlist.h +++ b/src/or/routerlist.h @@ -62,10 +62,10 @@ int router_skip_or_reachability(const or_options_t *options, int try_ip_pref); int router_get_my_share_of_directory_requests(double *v3_share_out); 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 pref_addr, int direct_conn); +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);

const routerinfo_t *routerlist_find_my_routerinfo(void); uint32_t router_get_advertised_bandwidth(const routerinfo_t *router); diff --git a/src/test/test_options.c b/src/test/test_options.c index 1ba6a05..43aa907 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -2008,56 +2008,6 @@ test_options_validate__entry_nodes(void *ignored) }

static void -test_options_validate__invalid_nodes(void *ignored) -{ - (void)ignored; - int ret; - char *msg; - options_test_data_t *tdata = get_options_test_data( - "AllowInvalidNodes something_stupid\n" - "MaxClientCircuitsPending 1\n" - "ConnLimit 1\n" - "SchedulerHighWaterMark__ 42\n" - "SchedulerLowWaterMark__ 10\n"); - - ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, - "Unrecognized value 'something_stupid' in AllowInvalidNodes"); - tor_free(msg); - - free_options_test_data(tdata); - tdata = get_options_test_data("AllowInvalidNodes entry, middle, exit\n" - "MaxClientCircuitsPending 1\n" - "ConnLimit 1\n" - "SchedulerHighWaterMark__ 42\n" - "SchedulerLowWaterMark__ 10\n"); - - ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_int_op(tdata->opt->AllowInvalid_, OP_EQ, ALLOW_INVALID_ENTRY | - ALLOW_INVALID_EXIT | ALLOW_INVALID_MIDDLE); - tor_free(msg); - - free_options_test_data(tdata); - tdata = get_options_test_data("AllowInvalidNodes introduction, rendezvous\n" - "MaxClientCircuitsPending 1\n" - "ConnLimit 1\n" - "SchedulerHighWaterMark__ 42\n" - "SchedulerLowWaterMark__ 10\n"); - - ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_int_op(ret, OP_EQ, -1); - tt_int_op(tdata->opt->AllowInvalid_, OP_EQ, ALLOW_INVALID_INTRODUCTION | - ALLOW_INVALID_RENDEZVOUS); - tor_free(msg); - - done: - free_options_test_data(tdata); - tor_free(msg); -} - -static void test_options_validate__safe_logging(void *ignored) { (void)ignored; @@ -4530,7 +4480,6 @@ struct testcase_t options_tests[] = { LOCAL_VALIDATE_TEST(reachable_addresses), LOCAL_VALIDATE_TEST(use_bridges), LOCAL_VALIDATE_TEST(entry_nodes), - LOCAL_VALIDATE_TEST(invalid_nodes), LOCAL_VALIDATE_TEST(safe_logging), LOCAL_VALIDATE_TEST(publish_server_descriptor), LOCAL_VALIDATE_TEST(testing),