[tor-commits] [tor/master] circ: Don't cannibalize a circuit if the guard state is unusable

[nickm at torproject.org]

commit fb93c6fc517f8717891fa8bfb41c9381a12dc491
Author: David Goulet <dgoulet at torproject.org>
Date:   Wed Jan 31 15:23:45 2018 -0500

    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 at torproject.org>
---
 changes/bug24469     |  4 ++++
 src/or/circuitlist.c |  7 +++++++
 src/or/entrynodes.c  | 16 ++++++++++++++++
 src/or/entrynodes.h  |  2 ++
 4 files changed, 29 insertions(+)

diff --git a/changes/bug24469 b/changes/bug24469
new file mode 100644
index 000000000..2e137b49b
--- /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 27d8c62b5..8c02cd1c1 100644
--- 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 292a393e5..2b6ff38c9 100644
--- 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 aa9c8fe19..b7b110eeb 100644
--- 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