[tor-commits] [tor/maint-0.4.5] Don't warn about missing guard state if controller picked first hop

[asn at torproject.org]

commit 9409c7f2f52f192eaf497c56daa485d75358de65
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Mar 8 13:41:20 2021 -0500

    Don't warn about missing guard state if controller picked first hop
    
    See comments about why this needs a new flag and we can't just use
    CIRCUIT_PURPOSE_CONTROLLER.
    
    Fixes #40285; bugfix on 0.3.2.1-alpha.
---
 changes/bug40285_045              |  4 ++++
 src/core/or/circuitbuild.c        | 18 +++++++++++++-----
 src/core/or/circuitlist.h         |  3 ++-
 src/core/or/origin_circuit_st.h   | 12 ++++++++++++
 src/feature/control/control_cmd.c |  1 +
 5 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/changes/bug40285_045 b/changes/bug40285_045
new file mode 100644
index 0000000000..db4f73cde0
--- /dev/null
+++ b/changes/bug40285_045
@@ -0,0 +1,4 @@
+  o Minor bugfixes (controller):
+    - Fix a "BUG" warning that would appear when a controller chooses the
+      first hop for a circuit, and that circuit completes.  Fixes
+      bug 40285; bugfix on 0.3.2.1-alpha.
diff --git a/src/core/or/circuitbuild.c b/src/core/or/circuitbuild.c
index c0c918abe4..78501c0aa2 100644
--- a/src/core/or/circuitbuild.c
+++ b/src/core/or/circuitbuild.c
@@ -881,14 +881,22 @@ circuit_pick_extend_handshake(uint8_t *cell_type_out,
 }
 
 /**
- * Return true iff <b>purpose</b> is a purpose for a circuit which is
- * allowed to have no guard configured, even if the circuit is multihop
+ * Return true iff <b>circ</b> is allowed
+ * to have no guard configured, even if the circuit is multihop
  * and guards are enabled.
  */
 static int
-circuit_purpose_may_omit_guard(int purpose)
+circuit_may_omit_guard(const origin_circuit_t *circ)
 {
-  switch (purpose) {
+  if (BUG(!circ))
+    return 0;
+
+  if (circ->first_hop_from_controller) {
+    /* The controller picked the first hop: that bypasses the guard system. */
+    return 1;
+  }
+
+  switch (circ->base_.purpose) {
     case CIRCUIT_PURPOSE_TESTING:
     case CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT:
       /* Testing circuits may omit guards because they're measuring
@@ -1019,7 +1027,7 @@ circuit_build_no_more_hops(origin_circuit_t *circ)
   guard_usable_t r;
   if (! circ->guard_state) {
     if (circuit_get_cpath_len(circ) != 1 &&
-        ! circuit_purpose_may_omit_guard(circ->base_.purpose) &&
+        ! circuit_may_omit_guard(circ) &&
         get_options()->UseEntryGuards) {
       log_warn(LD_BUG, "%d-hop circuit %p with purpose %d has no "
                "guard state",
diff --git a/src/core/or/circuitlist.h b/src/core/or/circuitlist.h
index 3178e6cd0d..bd4a117e26 100644
--- a/src/core/or/circuitlist.h
+++ b/src/core/or/circuitlist.h
@@ -118,7 +118,8 @@
  * bandwidth measurement, reachability test and address discovery from an
  * authority using the NETINFO cell. */
 #define CIRCUIT_PURPOSE_TESTING 21
-/** A controller made this circuit and Tor should not use it. */
+/** A controller made this circuit and Tor should not cannibalize it or attach
+ * streams to it without explicitly being told. */
 #define CIRCUIT_PURPOSE_CONTROLLER 22
 /** This circuit is used for path bias probing only */
 #define CIRCUIT_PURPOSE_PATH_BIAS_TESTING 23
diff --git a/src/core/or/origin_circuit_st.h b/src/core/or/origin_circuit_st.h
index a45a6573dc..c40e84aed8 100644
--- a/src/core/or/origin_circuit_st.h
+++ b/src/core/or/origin_circuit_st.h
@@ -170,6 +170,18 @@ struct origin_circuit_t {
    * not try to negotiate further circuit padding. */
   unsigned padding_negotiation_failed : 1;
 
+  /**
+   * If this flag is set, then a controller chose the first hop of this
+   * circuit's path, and it's okay to ignore checks that we'd usually do
+   * on this circuit's first hop.
+   *
+   * This flag is distinct from the CIRCUIT_PURPOSE_CONTROLLER purpose: the
+   * purpose indicates _what tor can use the circuit for_.  Controller-created
+   * circuits can still have the CIRCUIT_PURPOSE_GENERAL purpose if Tor is
+   * allowed to attach streams to them.
+   */
+  unsigned first_hop_from_controller : 1;
+
   /**
    * Tristate variable to guard against pathbias miscounting
    * due to circuit purpose transitions changing the decision
diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c
index 5b75c24692..0456d709f5 100644
--- a/src/feature/control/control_cmd.c
+++ b/src/feature/control/control_cmd.c
@@ -819,6 +819,7 @@ handle_control_extendcircuit(control_connection_t *conn,
   if (zero_circ) {
     /* start a new circuit */
     circ = origin_circuit_init(intended_purpose, 0);
+    circ->first_hop_from_controller = 1;
   }
 
   /* now circ refers to something that is ready to be extended */