circ: Don't cannibalize a circuit if the guard state is unusable

Tor preemptiely builds circuits and they can be cannibalized later in their
lifetime. A Guard node can become unusable (from our guard state) but we can
still have circuits using that node opened. It is important to not pick those
circuits for any usage through the cannibalization process.

Fixes #24469

Signed-off-by: David Goulet <dgoulet@torproject.org>

diff --git a/changes/bug24469 b/changes/bug24469
new file mode 100644 (file)
index 0000000..2e137b4
--- /dev/null
+++ b/changes/bug24469
@@ -0,0 +1,4 @@
+  o Minor bugfixes (circuit, cannibalization):
+    - Don't cannibalize circuits for which we don't know the first hop which
+      can happen if our Guard relay went off the consensus after the circuit
+      was created preemptively. Fixes bug 24469; bugfix on 0.0.6.

diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 27d8c62b5b14dfd53ea88ec7aa6e5379ae52c062..8c02cd1c19eebfe8de20afacab554a3ca7d72429 100644 (file)
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -1803,6 +1803,13 @@ circuit_find_to_cannibalize(uint8_t purpose_to_produce, extend_info_t *info,
         goto next;
       }
 
+      /* Ignore any circuits for which we can't use the Guard. It is possible
+       * that the Guard was removed from the samepled set after the circuit
+       * was created so avoid using it. */
+      if (!entry_guard_could_succeed(circ->guard_state)) {
+        goto next;
+      }
+
       if ((!need_uptime || circ->build_state->need_uptime) &&
           (!need_capacity || circ->build_state->need_capacity) &&
           (internal == circ->build_state->is_internal) &&

diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c
index 292a393e51edf374b83118b0b9a5b07a5bb29373..2b6ff38c9cfe89846093c7034f5b84dfd1a69008 100644 (file)
--- a/src/or/entrynodes.c
+++ b/src/or/entrynodes.c
@@ -3307,6 +3307,22 @@ entry_guards_update_state(or_state_t *state)
   entry_guards_dirty = 0;
 }
 
+/** Return true iff the circuit's guard can succeed that is can be used. */
+int
+entry_guard_could_succeed(const circuit_guard_state_t *guard_state)
+{
+  if (!guard_state) {
+    return 0;
+  }
+
+  entry_guard_t *guard = entry_guard_handle_get(guard_state->guard);
+  if (!guard || BUG(guard->in_selection == NULL)) {
+    return 0;
+  }
+
+  return 1;
+}
+
 /**
  * Format a single entry guard in the format expected by the controller.
  * Return a newly allocated string.

diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h
index aa9c8fe1936d5c6b2892efb7c24c95426f95a45e..b7b110eeb74f5fbe3a0d904909f605fc41c2a0c8 100644 (file)
--- a/src/or/entrynodes.h
+++ b/src/or/entrynodes.h
@@ -386,6 +386,8 @@ void entry_guards_note_internet_connectivity(guard_selection_t *gs);
 
 int update_guard_selection_choice(const or_options_t *options);
 
+int entry_guard_could_succeed(const circuit_guard_state_t *guard_state);
+
 MOCK_DECL(int,num_bridges_usable,(int use_maybe_reachable));
 
 #ifdef ENTRYNODES_PRIVATE