Don't cannibalize one-hop circuits

In rare cases, we could cannibalize a one-hop circuit, ending up
with a two-hop circuit. This circuit would not be actually used,
but we should prevent its creation in the first place.

Thanks to outofwords and swissknife for helping to analyse this.

diff --git a/changes/dont_cannibalize_onehop_circuits b/changes/dont_cannibalize_onehop_circuits
new file mode 100644 (file)
index 0000000..10e5fe5
--- /dev/null
+++ b/changes/dont_cannibalize_onehop_circuits
@@ -0,0 +1,6 @@
+ o Refactorings:
+   - Make it explicit that we don't cannibalize one-hop circuits. This happens
+     in the wild, but doesn't turn out to be a problem because we fortunately
+     don't use those circuits. Many thanks to outofwords for the initial
+     analysis and to swissknife who confirmed that two-hop circuits are
+     actually created.

diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index 73e2e06cce5027d5599b4b2836fc2d8f84137c20..d71d6a21b0c55bb75fc218fd51897e913d59f22e 100644 (file)
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -905,6 +905,10 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info,
   int need_capacity = (flags & CIRCLAUNCH_NEED_CAPACITY) != 0;
   int internal = (flags & CIRCLAUNCH_IS_INTERNAL) != 0;
 
+  /* Make sure we're not trying to create a onehop circ by
+   * cannibalization. */
+  tor_assert(!(flags & CIRCLAUNCH_ONEHOP_TUNNEL));
+
   log_debug(LD_CIRC,
             "Hunting for a circ to cannibalize: purpose %d, uptime %d, "
             "capacity %d, internal %d",
@@ -920,7 +924,8 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info,
       if ((!need_uptime || circ->build_state->need_uptime) &&
           (!need_capacity || circ->build_state->need_capacity) &&
           (internal == circ->build_state->is_internal) &&
-          circ->remaining_relay_early_cells) {
+          circ->remaining_relay_early_cells &&
+          !circ->build_state->onehop_tunnel) {
         if (info) {
           /* need to make sure we don't duplicate hops */
           crypt_path_t *hop = circ->cpath;