[tor-commits] [tor/master] Fix: don't report timeout when closing parallel intro points

nickm at torproject.org nickm at torproject.org
Wed Nov 12 15:25:14 UTC 2014


commit 34eb007d2201bad44bd6b72681f2c3552445dfc4
Author: David Goulet <dgoulet at ev0ke.net>
Date:   Mon Nov 10 14:38:53 2014 -0500

    Fix: don't report timeout when closing parallel intro points
    
    When closing parallel introduction points, the given reason (timeout)
    was actually changed to "no reason" thus when the circuit purpose was
    CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT, we were reporting an introduction
    point failure and flagging it "unreachable". After three times, that
    intro point gets removed from the rend cache object.
    
    In the case of CIRCUIT_PURPOSE_C_INTRODUCING, the intro point was
    flagged has "timed out" and thus not used until the connection to the HS
    is closed where that flag gets reset.
    
    This commit adds an internal circuit reason called
    END_CIRC_REASON_IP_NOW_REDUNDANT which tells the closing circuit
    mechanism to not report any intro point failure.
    
    This has been observed while opening hundreds of connections to an HS on
    different circuit for each connection. This fix makes this use case to
    work like a charm.
    
    Fixes #13698.
    
    Signed-off-by: David Goulet <dgoulet at ev0ke.net>
---
 changes/bug13698     |    4 ++++
 src/or/circuitlist.c |   41 +++++++++++++++++++++++------------------
 src/or/or.h          |    4 ++++
 src/or/rendclient.c  |    5 ++---
 4 files changed, 33 insertions(+), 21 deletions(-)

diff --git a/changes/bug13698 b/changes/bug13698
new file mode 100644
index 0000000..71e018b
--- /dev/null
+++ b/changes/bug13698
@@ -0,0 +1,4 @@
+  o Bugfixes:
+	- When closing an introduction circuit that was opened in parallel, don't
+	  mark it unreachable which can causes it to be removed from the rend cache
+	  object thus not usable anymore.
diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c
index c7b15e4..6f12df8 100644
--- a/src/or/circuitlist.c
+++ b/src/or/circuitlist.c
@@ -1417,30 +1417,35 @@ circuit_mark_for_close_(circuit_t *circ, int reason, int line,
     tor_assert(circ->state == CIRCUIT_STATE_OPEN);
     tor_assert(ocirc->build_state->chosen_exit);
     tor_assert(ocirc->rend_data);
-    /* treat this like getting a nack from it */
-    log_info(LD_REND, "Failed intro circ %s to %s (awaiting ack). %s",
-           safe_str_client(ocirc->rend_data->onion_address),
-           safe_str_client(build_state_get_exit_nickname(ocirc->build_state)),
-           timed_out ? "Recording timeout." : "Removing from descriptor.");
-    rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
-                                           ocirc->rend_data,
-                                           timed_out ?
-                                           INTRO_POINT_FAILURE_TIMEOUT :
-                                           INTRO_POINT_FAILURE_GENERIC);
+    if (orig_reason != END_CIRC_REASON_IP_NOW_REDUNDANT) {
+      /* treat this like getting a nack from it */
+      log_info(LD_REND, "Failed intro circ %s to %s (awaiting ack). %s",
+          safe_str_client(ocirc->rend_data->onion_address),
+          safe_str_client(build_state_get_exit_nickname(ocirc->build_state)),
+          timed_out ? "Recording timeout." : "Removing from descriptor.");
+      rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
+                                             ocirc->rend_data,
+                                             timed_out ?
+                                             INTRO_POINT_FAILURE_TIMEOUT :
+                                             INTRO_POINT_FAILURE_GENERIC);
+    }
   } else if (circ->purpose == CIRCUIT_PURPOSE_C_INTRODUCING &&
              reason != END_CIRC_REASON_TIMEOUT) {
     origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
     if (ocirc->build_state->chosen_exit && ocirc->rend_data) {
-      log_info(LD_REND, "Failed intro circ %s to %s "
-               "(building circuit to intro point). "
-               "Marking intro point as possibly unreachable.",
-               safe_str_client(ocirc->rend_data->onion_address),
-           safe_str_client(build_state_get_exit_nickname(ocirc->build_state)));
-      rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
-                                             ocirc->rend_data,
-                                             INTRO_POINT_FAILURE_UNREACHABLE);
+      if (orig_reason != END_CIRC_REASON_IP_NOW_REDUNDANT) {
+        log_info(LD_REND, "Failed intro circ %s to %s "
+            "(building circuit to intro point). "
+            "Marking intro point as possibly unreachable.",
+            safe_str_client(ocirc->rend_data->onion_address),
+            safe_str_client(build_state_get_exit_nickname(ocirc->build_state)));
+        rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
+                                               ocirc->rend_data,
+                                               INTRO_POINT_FAILURE_UNREACHABLE);
+      }
     }
   }
+
   if (circ->n_chan) {
     circuit_clear_cell_queue(circ, circ->n_chan);
     /* Only send destroy if the channel isn't closing anyway */
diff --git a/src/or/or.h b/src/or/or.h
index 34f055c..f3e7b80 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -659,6 +659,10 @@ typedef enum {
 
 /* Negative reasons are internal: we never send them in a DESTROY or TRUNCATE
  * call; they only go to the controller for tracking  */
+
+/* Closing introduction point that were opened in parallel. */
+#define END_CIRC_REASON_IP_NOW_REDUNDANT -4
+
 /** Our post-timeout circuit time measurement period expired.
  * We must give up now */
 #define END_CIRC_REASON_MEASUREMENT_EXPIRED -3
diff --git a/src/or/rendclient.c b/src/or/rendclient.c
index 7abbfd6..4ff000d 100644
--- a/src/or/rendclient.c
+++ b/src/or/rendclient.c
@@ -368,8 +368,7 @@ rend_client_rendcirc_has_opened(origin_circuit_t *circ)
 }
 
 /**
- * Called to close other intro circuits we launched in parallel
- * due to timeout.
+ * Called to close other intro circuits we launched in parallel.
  */
 static void
 rend_client_close_other_intros(const char *onion_address)
@@ -387,7 +386,7 @@ rend_client_close_other_intros(const char *onion_address)
         log_info(LD_REND|LD_CIRC, "Closing introduction circuit %d that we "
                  "built in parallel (Purpose %d).", oc->global_identifier,
                  c->purpose);
-        circuit_mark_for_close(c, END_CIRC_REASON_TIMEOUT);
+        circuit_mark_for_close(c, END_CIRC_REASON_IP_NOW_REDUNDANT);
       }
     }
   }





More information about the tor-commits mailing list