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.

diff --git a/changes/bug40285_045 b/changes/bug40285_045
new file mode 100644 (file)
index 0000000..db4f73c
--- /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 c0c918abe480dc0600974fcc3594a5482a8547ea..78501c0aa21625b3a4d20bcf3ce4dfe265294710 100644 (file)
--- 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 3178e6cd0dfe662b44175f8680f1546781a3da83..bd4a117e2600de4028f0347c6c61c8929b313fe7 100644 (file)
--- 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 a45a6573dc85b1c5f5b62ee7bdbf40b549d680d7..c40e84aed886649de7d17ffde9b9eee62b08bb76 100644 (file)
--- 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 5b75c24692e5a809784114e76b96ad363dd2dad7..0456d709f599852bf55f8ca13e8ffa5f8087848b 100644 (file)
--- 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 */