Merge branch 'maint-0.4.8'

changes/bug40876

@@ -0,0 +1,8 @@
+  o Major bugfixes (guard usage):
+    - When Tor excluded a guard due to temporary circuit restrictions,
+      it considered *additional* primary guards for potential usage
+      by that circuit. This could result in more than the specified number
+      of guards (currently 2) being used, long-term, by the tor client.
+      This could happen when a Guard was also selected as an Exit node,
+      but it was exacerbated by the Conflux guard restrictions. Both
+      instances have been fixed. Fixes bug 40876; bugfix on 0.3.0.1-alpha.

src/feature/client/entrynodes.c

@@ -1681,6 +1681,17 @@ guard_obeys_md_dirserver_restriction(const entry_guard_t *guard)
   return 1;
 }
 
+/**
+ * Return true if a restriction is reachability related, such that it should
+ * cause us to consider additional primary guards when selecting one.
+ */
+static bool
+entry_guard_restriction_is_reachability(const entry_guard_restriction_t *rst)
+{
+  tor_assert(rst);
+  return (rst->type == RST_OUTDATED_MD_DIRSERVER);
+}
+
 /**
  * Return true iff <b>guard</b> obeys the restrictions defined in <b>rst</b>.
  * (If <b>rst</b> is NULL, there are no restrictions.)
@@ -2127,14 +2138,44 @@ select_primary_guard_for_circuit(guard_selection_t *gs,
   const int need_descriptor = (usage == GUARD_USAGE_TRAFFIC);
   entry_guard_t *chosen_guard = NULL;
 
-  int num_entry_guards = get_n_primary_guards_to_use(usage);
+  int num_entry_guards_to_consider = get_n_primary_guards_to_use(usage);
   smartlist_t *usable_primary_guards = smartlist_new();
+  int num_entry_guards_considered = 0;
 
   SMARTLIST_FOREACH_BEGIN(gs->primary_entry_guards, entry_guard_t *, guard) {
     entry_guard_consider_retry(guard);
     if (!entry_guard_obeys_restriction(guard, rst)) {
       log_info(LD_GUARD, "Entry guard %s doesn't obey restriction, we test the"
           " next one", entry_guard_describe(guard));
+      if (!entry_guard_restriction_is_reachability(rst)) {
+        log_info(LD_GUARD,
+                 "Skipping guard %s due to circuit path restriction. "
+                 "Have %d, considered: %d, to consider: %d",
+                 entry_guard_describe(guard),
+                 smartlist_len(usable_primary_guards),
+                 num_entry_guards_considered,
+                 num_entry_guards_to_consider);
+        /* If the restriction is a circuit path restriction (as opposed to a
+         * reachability restriction), count this as considered. */
+        num_entry_guards_considered++;
+
+        /* If we have considered enough guards, *and* we actually have a guard,
+         * then proceed to select one from the list. */
+        if (num_entry_guards_considered >= num_entry_guards_to_consider) {
+          /* This should not happen with 2-leg conflux unless there is a
+           * race between removing a failed leg and a retry, but check
+           * anyway and log. */
+          if (smartlist_len(usable_primary_guards) == 0) {
+            static ratelim_t guardlog = RATELIM_INIT(60);
+            log_fn_ratelim(&guardlog, LOG_NOTICE, LD_GUARD,
+                           "All current guards excluded by path restriction "
+                           "type %d; using an additonal guard.",
+                           rst->type);
+          } else {
+            break;
+          }
+        }
+      }
       continue;
     }
     if (guard->is_reachable != GUARD_REACHABLE_NO) {
@@ -2146,9 +2187,17 @@ select_primary_guard_for_circuit(guard_selection_t *gs,
       *state_out = GUARD_CIRC_STATE_USABLE_ON_COMPLETION;
       guard->last_tried_to_connect = approx_time();
       smartlist_add(usable_primary_guards, guard);
-      if (smartlist_len(usable_primary_guards) >= num_entry_guards)
+      num_entry_guards_considered++;
+
+      /* If we have considered enough guards, then proceed to select
+       * one from the list. */
+      if (num_entry_guards_considered >= num_entry_guards_to_consider) {
         break;
       }
+    } else {
+      log_info(LD_GUARD, "Guard %s is not reachable",
+          entry_guard_describe(guard));
+    }
   } SMARTLIST_FOREACH_END(guard);
 
   if (smartlist_len(usable_primary_guards)) {
@@ -2157,6 +2206,10 @@ select_primary_guard_for_circuit(guard_selection_t *gs,
         "Selected primary guard %s for circuit from a list size of %d.",
         entry_guard_describe(chosen_guard),
         smartlist_len(usable_primary_guards));
+    /* Describe each guard in the list: */
+    SMARTLIST_FOREACH_BEGIN(usable_primary_guards, entry_guard_t *, guard) {
+      log_info(LD_GUARD, "  %s", entry_guard_describe(guard));
+    } SMARTLIST_FOREACH_END(guard);
     smartlist_free(usable_primary_guards);
   }
 
@@ -2262,16 +2315,22 @@ select_entry_guard_for_circuit(guard_selection_t *gs,
   /* "If any entry in PRIMARY_GUARDS has {is_reachable} status of
       <maybe> or <yes>, return the first such guard." */
   chosen_guard = select_primary_guard_for_circuit(gs, usage, rst, state_out);
-  if (chosen_guard)
+  if (chosen_guard) {
+    log_info(LD_GUARD, "Selected primary guard %s for circuit.",
+             entry_guard_describe(chosen_guard));
     return chosen_guard;
+  }
 
   /* "Otherwise, if the ordered intersection of {CONFIRMED_GUARDS}
       and {USABLE_FILTERED_GUARDS} is nonempty, return the first
       entry in that intersection that has {is_pending} set to
       false." */
   chosen_guard = select_confirmed_guard_for_circuit(gs, usage, rst, state_out);
-  if (chosen_guard)
+  if (chosen_guard) {
+     log_info(LD_GUARD, "Selected confirmed guard %s for circuit.",
+             entry_guard_describe(chosen_guard));
     return chosen_guard;
+  }
 
   /* "Otherwise, if there is no such entry, select a member
    * {USABLE_FILTERED_GUARDS} following the sample ordering" */
@@ -2284,6 +2343,8 @@ select_entry_guard_for_circuit(guard_selection_t *gs,
     return NULL;
   }
 
+  log_info(LD_GUARD, "Selected filtered guard %s for circuit.",
+             entry_guard_describe(chosen_guard));
   return chosen_guard;
 }