Switch to a StrictNodes config option.

This is step one of handling ExcludedNodes better. This first
step is just to make EntryNodes and ExitNodes do what they did
before.

5 files changed, 51 insertions, 66 deletions

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index 7da6272482..9466192cda 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -2174,7 +2174,8 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime,
 
     routersets_get_disjunction(use, supporting, options->ExitNodes,
                                options->_ExcludeExitNodesUnion, 1);
-    if (smartlist_len(use) == 0 && !options->StrictExitNodes) {
+    if (smartlist_len(use) == 0 && options->ExitNodes &&
+        !options->StrictNodes) { /* give up on exitnodes and try again */
       routersets_get_disjunction(use, supporting, NULL,
                                  options->_ExcludeExitNodesUnion, 1);
     }
@@ -2220,12 +2221,14 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime,
 
       routersets_get_disjunction(use, supporting, options->ExitNodes,
                                  options->_ExcludeExitNodesUnion, 1);
-      if (smartlist_len(use) == 0 && !options->StrictExitNodes) {
+      if (smartlist_len(use) == 0 && options->ExitNodes &&
+          !options->StrictNodes) { /* give up on exitnodes and try again */
         routersets_get_disjunction(use, supporting, NULL,
                                    options->_ExcludeExitNodesUnion, 1);
       }
-      /* XXX sometimes the above results in null, when the requested
-       * exit node is down. we should pick it anyway. */
+      /* FFF sometimes the above results in null, when the requested
+       * exit node is considered down by the consensus. we should pick
+       * it anyway, since the user asked for it. */
       router = routerlist_sl_choose_by_bandwidth(use, WEIGHT_FOR_EXIT);
       if (router)
         break;
@@ -2243,10 +2246,10 @@ choose_good_exit_server_general(routerlist_t *dir, int need_uptime,
     log_info(LD_CIRC, "Chose exit server '%s'", router->nickname);
     return router;
   }
-  if (options->StrictExitNodes) {
+  if (options->ExitNodes && options->StrictNodes) {
     log_warn(LD_CIRC,
              "No specified exit routers seem to be running, and "
-             "StrictExitNodes is set: can't choose an exit.");
+             "StrictNodes is set: can't choose an exit.");
   }
   return NULL;
 }
@@ -2277,15 +2280,13 @@ choose_good_exit_server(uint8_t purpose, routerlist_t *dir,
       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, NULL,
-                                         options->ExcludeNodes, flags);
+        return router_choose_random_node(NULL, options->ExcludeNodes, flags);
       else
         return choose_good_exit_server_general(dir,need_uptime,need_capacity);
     case CIRCUIT_PURPOSE_C_ESTABLISH_REND:
       if (options->_AllowInvalid & ALLOW_INVALID_RENDEZVOUS)
         flags |= CRN_ALLOW_INVALID;
-      return router_choose_random_node(NULL, NULL,
-                                       options->ExcludeNodes, flags);
+      return router_choose_random_node(NULL, options->ExcludeNodes, flags);
   }
   log_warn(LD_BUG,"Unhandled purpose %d", purpose);
   tor_fragile_assert();
@@ -2527,8 +2528,7 @@ choose_good_middle_server(uint8_t purpose,
     flags |= CRN_NEED_CAPACITY;
   if (options->_AllowInvalid & ALLOW_INVALID_MIDDLE)
     flags |= CRN_ALLOW_INVALID;
-  choice = router_choose_random_node(NULL,
-                                     excluded, options->ExcludeNodes, flags);
+  choice = router_choose_random_node(excluded, options->ExcludeNodes, flags);
   smartlist_free(excluded);
   return choice;
 }
@@ -2593,11 +2593,7 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state)
   if (options->_AllowInvalid & ALLOW_INVALID_ENTRY)
     flags |= CRN_ALLOW_INVALID;
 
-  choice = router_choose_random_node(
-           NULL,
-           excluded,
-           options->ExcludeNodes,
-           flags);
+  choice = router_choose_random_node(excluded, options->ExcludeNodes, flags);
   smartlist_free(excluded);
   return choice;
 }
@@ -3378,7 +3374,7 @@ entry_guards_prepend_from_config(void)
     add_an_entry_guard(ri, 0);
   });
   /* Finally, the remaining EntryNodes, unless we're strict */
-  if (options->StrictEntryNodes) {
+  if (options->EntryNodes && options->StrictNodes) {
     SMARTLIST_FOREACH(old_entry_guards_not_on_list, entry_guard_t *, e,
                       entry_guard_free(e));
   } else {
@@ -3397,7 +3393,7 @@ entry_guards_prepend_from_config(void)
 int
 entry_list_can_grow(or_options_t *options)
 {
-  if (options->StrictEntryNodes)
+  if (options->EntryNodes && options->StrictNodes)
     return 0;
   if (options->UseBridges)
     return 0;
diff --git a/src/or/config.c b/src/or/config.c
index 0c5da96dae..2e2c89a901 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -66,6 +66,7 @@ static config_abbrev_t _option_abbrevs[] = {
   PLURAL(RendExcludeNode),
   PLURAL(StrictEntryNode),
   PLURAL(StrictExitNode),
+  PLURAL(StrictNode),
   { "l", "Log", 1, 0},
   { "AllowUnverifiedNodes", "AllowInvalidNodes", 0, 0},
   { "AutomapHostSuffixes", "AutomapHostsSuffixes", 0, 0},
@@ -83,10 +84,12 @@ static config_abbrev_t _option_abbrevs[] = {
   { "NumEntryNodes", "NumEntryGuards", 0, 0},
   { "ResolvConf", "ServerDNSResolvConfFile", 0, 1},
   { "SearchDomains", "ServerDNSSearchDomains", 0, 1},
-  { "ServerDNSAllowBrokenResolvConf", "ServerDNSAllowBrokenConfig", 0, 0 },
+  { "ServerDNSAllowBrokenResolvConf", "ServerDNSAllowBrokenConfig", 0, 0},
   { "PreferTunnelledDirConns", "PreferTunneledDirConns", 0, 0},
   { "BridgeAuthoritativeDirectory", "BridgeAuthoritativeDir", 0, 0},
   { "HashedControlPassword", "__HashedControlSessionPassword", 1, 0},
+  { "StrictEntryNodes", "StrictNodes", 0, 1},
+  { "StrictExitNodes", "StrictNodes", 0, 1},
   { NULL, NULL, 0, 0},
 };
 
@@ -320,8 +323,7 @@ static config_var_t _option_vars[] = {
   V(SocksPort,                   UINT,     "9050"),
   V(SocksTimeout,                INTERVAL, "2 minutes"),
   OBSOLETE("StatusFetchPeriod"),
-  V(StrictEntryNodes,            BOOL,     "0"),
-  V(StrictExitNodes,             BOOL,     "0"),
+  V(StrictNodes,                 BOOL,     "0"),
   OBSOLETE("SysLog"),
   V(TestSocks,                   BOOL,     "0"),
   OBSOLETE("TestVia"),
@@ -517,6 +519,9 @@ static config_var_description_t options_description[] = {
   { "ExitNodes", "A list of preferred nodes to use for the last hop in "
     "circuits, when possible." },
   { "ExcludeNodes", "A list of nodes never to use when building a circuit." },
+  { "ExcludeExitNodes", "A list of nodes never to use for the last when "
+    "building a circuit for exit. Other circuits can still end at these "
+    "nodes." },
   { "FascistFirewall", "If set, Tor will only create outgoing connections to "
     "servers running on the ports listed in FirewallPorts." },
   { "FirewallPorts", "A list of ports that we can connect to.  Only used "
@@ -545,10 +550,9 @@ static config_var_description_t options_description[] = {
   { "SOCKSPolicy", "Set an entry policy to limit which addresses can connect "
     "to the SOCKSPort." },
   /* SocksTimeout */
-  { "StrictExitNodes", "If set, Tor will fail to operate when none of the "
-    "configured ExitNodes can be used." },
-  { "StrictEntryNodes", "If set, Tor will fail to operate when none of the "
-    "configured EntryNodes can be used." },
+  { "StrictNodes", "If set, Tor will fail to operate when none of the "
+    "configured EntryNodes, ExitNodes, ExcludeNodes, or ExcludeExitNodes "
+    "can be used." },
   /* TestSocks */
   { "TrackHostsExit", "Hosts and domains which should, if possible, be "
     "accessed from the same exit node each time we connect to them." },
@@ -3194,6 +3198,15 @@ options_validate(or_options_t *old_options, or_options_t *options,
     routerset_union(options->_ExcludeExitNodesUnion,options->ExcludeNodes);
   }
 
+  if (options->ExcludeNodes && options->StrictNodes) {
+    COMPLAIN("You have asked to exclude certain relays from all positions "
+             "in your circuits. Expect hidden services and other Tor "
+             "features to be broken in unpredictable ways.");
+  }
+
+#if 0 /* for now, it's ok to set StrictNodes without setting any actual
+       * preferences. It won't hurt anything. Eventually, either figure
+       * out the logic for the right case to complain, or just delete. -RD */
   if (options->StrictExitNodes &&
       (!options->ExitNodes) &&
       (!old_options ||
@@ -3207,6 +3220,7 @@ options_validate(or_options_t *old_options, or_options_t *options,
        (old_options->StrictEntryNodes != options->StrictEntryNodes) ||
        (!routerset_equal(old_options->EntryNodes,options->EntryNodes))))
     COMPLAIN("StrictEntryNodes set, but no EntryNodes listed.");
+#endif
 
   if (options->EntryNodes && !routerset_is_list(options->EntryNodes)) {
     /* XXXX fix this; see entry_guards_prepend_from_config(). */
diff --git a/src/or/or.h b/src/or/or.h
index 3fd7e0e870..d30b53743d 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -2326,16 +2326,13 @@ typedef struct {
   routerset_t *EntryNodes;/**< Structure containing nicknames, digests,
                            * country codes and IP address patterns of ORs to
                            * consider as entry points. */
-  int StrictExitNodes; /**< Boolean: When none of our ExitNodes are up, do we
-                        * stop building circuits? */
-  int StrictEntryNodes; /**< Boolean: When none of our EntryNodes are up, do we
-                         * stop building circuits? */
-  int DisableAllSwap; /**< Boolean: Attempt to call mlockall() on our
-                               * process for all current and future memory. */
-
+  int StrictNodes; /**< Boolean: When none of our EntryNodes or ExitNodes
+                    * are up, or we need to access a node in ExcludeNodes,
+                    * do we just fail instead? */
   routerset_t *ExcludeNodes;/**< Structure containing nicknames, digests,
                              * country codes and IP address patterns of ORs
-                             * not to use in circuits. */
+                             * not to use in circuits. But see StrictNodes
+                             * above. */
   routerset_t *ExcludeExitNodes;/**< Structure containing nicknames, digests,
                                  * country codes and IP address patterns of
                                  * ORs not to consider as exits. */
@@ -2343,6 +2340,9 @@ typedef struct {
   /** Union of ExcludeNodes and ExcludeExitNodes */
   struct routerset_t *_ExcludeExitNodesUnion;
 
+  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. */
@@ -4949,13 +4949,10 @@ typedef enum {
   CRN_NEED_GUARD = 1<<2,
   CRN_ALLOW_INVALID = 1<<3,
   /* XXXX not used, apparently. */
-  CRN_STRICT_PREFERRED = 1<<4,
-  /* XXXX not used, apparently. */
   CRN_WEIGHT_AS_EXIT = 1<<5
 } router_crn_flags_t;
 
-routerinfo_t *router_choose_random_node(const char *preferred,
-                                        smartlist_t *excludedsmartlist,
+routerinfo_t *router_choose_random_node(smartlist_t *excludedsmartlist,
                                         struct routerset_t *excludedset,
                                         router_crn_flags_t flags);
 
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index 6f81868d99..9fcf248c37 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -1825,7 +1825,7 @@ rend_services_introduce(void)
       router_crn_flags_t flags = CRN_NEED_UPTIME;
       if (get_options()->_AllowInvalid & ALLOW_INVALID_INTRODUCTION)
         flags |= CRN_ALLOW_INVALID;
-      router = router_choose_random_node(NULL, intro_routers,
+      router = router_choose_random_node(intro_routers,
                                          options->ExcludeNodes, flags);
       if (!router) {
         log_warn(LD_REND,
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 133e1933ee..0c242507bc 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -1827,8 +1827,7 @@ routerstatus_sl_choose_by_bandwidth(smartlist_t *sl)
  * node (that is, possibly discounting exit nodes).
  */
 routerinfo_t *
-router_choose_random_node(const char *preferred,
-                          smartlist_t *excludedsmartlist,
+router_choose_random_node(smartlist_t *excludedsmartlist,
                           routerset_t *excludedset,
                           router_crn_flags_t flags)
 {
@@ -1836,7 +1835,6 @@ router_choose_random_node(const char *preferred,
   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 strict = (flags & CRN_STRICT_PREFERRED) != 0;
   const int weight_for_exit = (flags & CRN_WEIGHT_AS_EXIT) != 0;
 
   smartlist_t *sl, *excludednodes;
@@ -1864,22 +1862,7 @@ router_choose_random_node(const char *preferred,
     routerlist_add_family(excludednodes, r);
   }
 
-  /* Try the preferred nodes first. Ignore need_uptime and need_capacity
-   * and need_guard, since the user explicitly asked for these nodes. */
-  if (preferred) {
-    sl = smartlist_create();
-    add_nickname_list_to_smartlist(sl,preferred,1);
-    smartlist_subtract(sl,excludednodes);
-    if (excludedsmartlist)
-      smartlist_subtract(sl,excludedsmartlist);
-    if (excludedset)
-      routerset_subtract_routers(sl,excludedset);
-    choice = smartlist_choose(sl);
-    smartlist_free(sl);
-  }
-  if (!choice && !strict) {
-    /* Then give up on our preferred choices: any node
-     * will do that has the required attributes. */
+  { /* XXX021 reformat */
     sl = smartlist_create();
     router_add_running_routers_to_smartlist(sl, allow_invalid,
                                             need_uptime, need_capacity,
@@ -1906,18 +1889,13 @@ router_choose_random_node(const char *preferred,
                need_guard?", guard":"");
       flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD);
       choice = router_choose_random_node(
-                       NULL, excludedsmartlist, excludedset, flags);
+                       excludedsmartlist, excludedset, flags);
     }
   }
   smartlist_free(excludednodes);
   if (!choice) {
-    if (strict) {
-      log_warn(LD_CIRC, "All preferred nodes were down when trying to choose "
-               "node, and the Strict[...]Nodes option is set. Failing.");
-    } else {
-      log_warn(LD_CIRC,
-               "No available nodes when trying to choose node. Failing.");
-    }
+    log_warn(LD_CIRC,
+             "No available nodes when trying to choose node. Failing.");
   }
   return choice;
 }