commit 9409c7f2f52f192eaf497c56daa485d75358de65 Author: Nick Mathewson <nickm@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 */