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

@@ -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.

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

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);

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;

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,

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;

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,

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);

src/test/test_options.c

@@ -2007,56 +2007,6 @@ test_options_validate__entry_nodes(void *ignored)
   tor_free(msg);
 }
 
-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)
 {
@@ -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),