[tor-commits] [tor/master] hs-v3: Handle client rendezvous circuit timeout

[asn at torproject.org]

commit fc32349adc33ceb0323b7149d661fba71b7f7b98
Author: David Goulet <dgoulet at torproject.org>
Date:   Tue Dec 3 11:08:13 2019 -0500

    hs-v3: Handle client rendezvous circuit timeout
    
    With v3, the "pending_final_cpath" of a circuit is always NULL which means
    that for v3, established client rendezvous circuit waiting for the intro point
    to ACK, will always end up timing out quickly.
    
    This can increase the delays to which you connect to a service since in order
    to succeed, the rendezvous circuit needs to fully established
    (CIRCUIT_PURPOSE_C_REND_JOINED) within the cutoff of the introduction circuit
    as well which is these days around 2-3 seconds.
    
    Fixes #32021
    
    Signed-off-by: David Goulet <dgoulet at torproject.org>
---
 changes/ticket32021         |  7 +++++++
 src/core/or/circuituse.c    | 13 ++++--------
 src/feature/hs/hs_circuit.c | 48 +++++++++++++++++++++++++++++++++++++++++++++
 src/feature/hs/hs_circuit.h |  2 ++
 4 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/changes/ticket32021 b/changes/ticket32021
new file mode 100644
index 000000000..24a6d9d98
--- /dev/null
+++ b/changes/ticket32021
@@ -0,0 +1,7 @@
+  o Minor bugfixes (onion services v3, client):
+    - Properly handle the client rendezvous circuit timeout. This results in
+      better reachability because tor doesn't timeout a rendezvous circuit
+      awaiting the introduction ACK and thus preventing tor to re-establish all
+      circuits because the rendezvous circuit timed out too early. Fixes bug
+      32021; bugfix on 0.3.2.1-alpha.
+
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c
index ad9841cc3..e5013fe96 100644
--- a/src/core/or/circuituse.c
+++ b/src/core/or/circuituse.c
@@ -775,16 +775,11 @@ circuit_expire_building(void)
     if (!(TO_ORIGIN_CIRCUIT(victim)->hs_circ_has_timed_out)) {
       switch (victim->purpose) {
       case CIRCUIT_PURPOSE_C_REND_READY:
-        /* We only want to spare a rend circ if it has been specified in
-         * an INTRODUCE1 cell sent to a hidden service.  A circ's
-         * pending_final_cpath field is non-NULL iff it is a rend circ
-         * and we have tried to send an INTRODUCE1 cell specifying it.
-         * Thus, if the pending_final_cpath field *is* NULL, then we
-         * want to not spare it. */
-        if (TO_ORIGIN_CIRCUIT(victim)->build_state &&
-            TO_ORIGIN_CIRCUIT(victim)->build_state->pending_final_cpath ==
-            NULL)
+        /* We only want to spare a rend circ iff it has been specified in an
+         * INTRODUCE1 cell sent to a hidden service. */
+        if (!hs_circ_is_rend_sent_in_intro1(CONST_TO_ORIGIN_CIRCUIT(victim))) {
           break;
+        }
         /* fallthrough! */
       case CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT:
       case CIRCUIT_PURPOSE_C_REND_READY_INTRO_ACKED:
diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c
index a43e7f0e2..2a420394f 100644
--- a/src/feature/hs/hs_circuit.c
+++ b/src/feature/hs/hs_circuit.c
@@ -1296,3 +1296,51 @@ hs_circ_cleanup_on_repurpose(circuit_t *circ)
     hs_circuitmap_remove_circuit(circ);
   }
 }
+
+/** Return true iff the given established client rendezvous circuit was sent
+ * into the INTRODUCE1 cell. This is called so we can take a decision on
+ * expiring or not the circuit.
+ *
+ * The caller MUST make sure the circuit is an established client rendezvous
+ * circuit (purpose: CIRCUIT_PURPOSE_C_REND_READY).
+ *
+ * This function supports all onion service versions. */
+bool
+hs_circ_is_rend_sent_in_intro1(const origin_circuit_t *circ)
+{
+  tor_assert(circ);
+  /* This can only be called for a rendezvous circuit that is an established
+   * confirmed rendezsvous circuit but without an introduction ACK. */
+  tor_assert(TO_CIRCUIT(circ)->purpose == CIRCUIT_PURPOSE_C_REND_READY);
+
+  /* The v2 and v3 circuit are handled differently:
+   *
+   * v2: A circ's pending_final_cpath field is non-NULL iff it is a rend circ
+   * and we have tried to send an INTRODUCE1 cell specifying it. Thus, if the
+   * pending_final_cpath field *is* NULL, then we want to not spare it.
+   *
+   * v3: When the INTRODUCE1 cell is sent, the introduction encryption public
+   * key is copied in the rendezvous circuit hs identifier. If it is a valid
+   * key, we know that this circuit is waiting the ACK on the introduction
+   * circuit. We want to _not_ spare the circuit if the key was never set. */
+
+  if (circ->rend_data) {
+    /* v2. */
+    if (circ->build_state && circ->build_state->pending_final_cpath != NULL) {
+      return true;
+    }
+  } else if (circ->hs_ident) {
+    /* v3. */
+    if (curve25519_public_key_is_ok(&circ->hs_ident->intro_enc_pk)) {
+      return true;
+    }
+  } else {
+    /* A circuit with an HS purpose without an hs_ident or rend_data in theory
+     * can not happen. In case, scream loudly and return false to the caller
+     * that the rendezvous was not sent in the INTRO1 cell. */
+    tor_assert_nonfatal_unreached();
+  }
+
+  /* The rendezvous has not been specified in the INTRODUCE1 cell. */
+  return false;
+}
diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h
index 42e5ca134..c044ad89c 100644
--- a/src/feature/hs/hs_circuit.h
+++ b/src/feature/hs/hs_circuit.h
@@ -66,6 +66,8 @@ int hs_circuit_setup_e2e_rend_circ(origin_circuit_t *circ,
 int hs_circuit_setup_e2e_rend_circ_legacy_client(origin_circuit_t *circ,
                                           const uint8_t *rend_cell_body);
 
+bool hs_circ_is_rend_sent_in_intro1(const origin_circuit_t *circ);
+
 #ifdef HS_CIRCUIT_PRIVATE
 
 STATIC hs_ident_circuit_t *