Merge remote-tracking branch 'tor-github/pr/1053'

10 files changed, 129 insertions, 0 deletions

diff --git a/changes/bug29034 b/changes/bug29034
new file mode 100644
index 0000000000..816b615009
--- /dev/null
+++ b/changes/bug29034
@@ -0,0 +1,5 @@
+  o Major bugfixes (Onion service reachability):
+    - Properly clean up the introduction point map and associated state when
+      circuits change purpose from onion service circuits to pathbias,
+      measurement, or other circuit types. This should fix some instances of
+      introduction point failure. Fixes bug 29034; bugfix on 0.3.2.1-alpha.
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c
index 485c389054..b3d5c6bb81 100644
--- a/src/core/or/circuituse.c
+++ b/src/core/or/circuituse.c
@@ -3068,6 +3068,12 @@ circuit_change_purpose(circuit_t *circ, uint8_t new_purpose)
 
   if (circ->purpose == new_purpose) return;
 
+  /* Take specific actions if we are repurposing a hidden service circuit. */
+  if (circuit_purpose_is_hidden_service(circ->purpose) &&
+      !circuit_purpose_is_hidden_service(new_purpose)) {
+    hs_circ_repurpose(circ);
+  }
+
   if (CIRCUIT_IS_ORIGIN(circ)) {
     char old_purpose_desc[80] = "";
 
diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c
index a6e86c5ab3..79377eb731 100644
--- a/src/feature/hs/hs_circuit.c
+++ b/src/feature/hs/hs_circuit.c
@@ -25,6 +25,7 @@
 #include "feature/nodelist/describe.h"
 #include "feature/nodelist/nodelist.h"
 #include "feature/rend/rendservice.h"
+#include "feature/rend/rendcommon.h"
 #include "feature/stats/rephist.h"
 #include "lib/crypt_ops/crypto_dh.h"
 #include "lib/crypt_ops/crypto_rand.h"
@@ -1192,3 +1193,33 @@ hs_circ_cleanup(circuit_t *circ)
     hs_circuitmap_remove_circuit(circ);
   }
 }
+
+/* The given circuit will be repurposed so take the appropriate actions. A
+ * cleanup from the HS maps and of all HS related structures is done.
+ *
+ * Once this function returns, the circuit can be safely repurposed. */
+void
+hs_circ_repurpose(circuit_t *circ)
+{
+  origin_circuit_t *origin_circ;
+
+  tor_assert(circ);
+
+  /* Only repurposing an origin circuit is possible for HS. */
+  if (!CIRCUIT_IS_ORIGIN(circ)) {
+    return;
+  }
+  origin_circ = TO_ORIGIN_CIRCUIT(circ);
+
+  /* First, cleanup the circuit from the HS maps. */
+  hs_circ_cleanup(circ);
+
+  /* Depending on the version, different cleanup is done. */
+  if (origin_circ->rend_data) {
+    /* v2. */
+    rend_circ_cleanup(origin_circ);
+  } else if (origin_circ->hs_ident) {
+    /* v3. */
+    hs_ident_circuit_free(origin_circ->hs_ident);
+  }
+}
diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h
index b8d8b25add..0786f3ee45 100644
--- a/src/feature/hs/hs_circuit.h
+++ b/src/feature/hs/hs_circuit.h
@@ -16,6 +16,7 @@
 
 /* Cleanup function when the circuit is closed or/and freed. */
 void hs_circ_cleanup(circuit_t *circ);
+void hs_circ_repurpose(circuit_t *circ);
 
 /* Circuit API. */
 int hs_circ_service_intro_has_opened(hs_service_t *service,
diff --git a/src/feature/rend/rendcommon.c b/src/feature/rend/rendcommon.c
index 777de2984c..265ee368f1 100644
--- a/src/feature/rend/rendcommon.c
+++ b/src/feature/rend/rendcommon.c
@@ -1046,3 +1046,14 @@ rend_circuit_pk_digest_eq(const origin_circuit_t *ocirc,
  match:
   return 1;
 }
+
+/* Cleanup the given circuit of all HS v2 data structure. */
+void
+rend_circ_cleanup(origin_circuit_t *circ)
+{
+  tor_assert(circ);
+
+  /* Both fields are set to NULL with these. */
+  crypto_pk_free(circ->intro_key);
+  rend_data_free(circ->rend_data);
+}
diff --git a/src/feature/rend/rendcommon.h b/src/feature/rend/rendcommon.h
index f136863c7a..c9a04846d7 100644
--- a/src/feature/rend/rendcommon.h
+++ b/src/feature/rend/rendcommon.h
@@ -71,6 +71,8 @@ int rend_non_anonymous_mode_enabled(const or_options_t *options);
 void assert_circ_anonymity_ok(const origin_circuit_t *circ,
                               const or_options_t *options);
 
+void rend_circ_cleanup(origin_circuit_t *circ);
+
 #ifdef RENDCOMMON_PRIVATE
 
 STATIC int
diff --git a/src/test/include.am b/src/test/include.am
index 85f9c9f880..fdfe96c559 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -145,6 +145,7 @@ src_test_test_SOURCES += \
 	src/test/test_hs_common.c \
 	src/test/test_hs_config.c \
 	src/test/test_hs_cell.c \
+	src/test/test_hs_circ.c \
 	src/test/test_hs_ntor.c \
 	src/test/test_hs_service.c \
 	src/test/test_hs_client.c  \
diff --git a/src/test/test.c b/src/test/test.c
index cac98dd839..d7682b6619 100644
--- a/src/test/test.c
+++ b/src/test/test.c
@@ -871,6 +871,7 @@ struct testgroup_t testgroups[] = {
   { "guardfraction/", guardfraction_tests },
   { "hs_cache/", hs_cache },
   { "hs_cell/", hs_cell_tests },
+  { "hs_circ/", hs_circ_tests },
   { "hs_client/", hs_client_tests },
   { "hs_common/", hs_common_tests },
   { "hs_config/", hs_config_tests },
diff --git a/src/test/test.h b/src/test/test.h
index 167fd090ac..b108d795ea 100644
--- a/src/test/test.h
+++ b/src/test/test.h
@@ -221,6 +221,7 @@ extern struct testcase_t guardfraction_tests[];
 extern struct testcase_t handle_tests[];
 extern struct testcase_t hs_cache[];
 extern struct testcase_t hs_cell_tests[];
+extern struct testcase_t hs_circ_tests[];
 extern struct testcase_t hs_client_tests[];
 extern struct testcase_t hs_common_tests[];
 extern struct testcase_t hs_config_tests[];
diff --git a/src/test/test_hs_circ.c b/src/test/test_hs_circ.c
new file mode 100644
index 0000000000..af28af2573
--- /dev/null
+++ b/src/test/test_hs_circ.c
@@ -0,0 +1,70 @@
+/* Copyright (c) 2017-2019, The Tor Project, Inc. */
+/* See LICENSE for licensing information */
+
+/**
+ * \file test_hs_circ.c
+ * \brief Test hidden service circuit functionality.
+ */
+
+#define CIRCUITLIST_PRIVATE
+
+#include "test/test.h"
+#include "test/test_helpers.h"
+#include "test/log_test_helpers.h"
+
+#include "core/or/circuitbuild.h"
+#include "core/or/circuitlist.h"
+#include "core/or/circuituse.h"
+#include "core/or/origin_circuit_st.h"
+
+#include "feature/hs/hs_circuit.h"
+#include "feature/hs/hs_circuitmap.h"
+
+static void
+test_circuit_repurpose(void *arg)
+{
+  origin_circuit_t *intro_circ = NULL;
+  const origin_circuit_t *search;
+  ed25519_keypair_t kp;
+
+  (void) arg;
+
+  hs_init();
+
+  intro_circ = origin_circuit_init(CIRCUIT_PURPOSE_S_ESTABLISH_INTRO, 0);
+  tt_assert(intro_circ);
+  ed25519_keypair_generate(&kp, 0);
+
+  /* Register circuit in global map and make sure it is actually there. */
+  hs_circuitmap_register_intro_circ_v3_service_side(intro_circ,
+                                                    &kp.pubkey);
+  tt_assert(TO_CIRCUIT(intro_circ)->hs_token);
+  search = hs_circuitmap_get_intro_circ_v3_service_side(&kp.pubkey);
+  tt_mem_op(search, OP_EQ, intro_circ, sizeof(origin_circuit_t));
+
+  /* Setup circuit HS ident. We don't care about the service pubkey. */
+  intro_circ->hs_ident = hs_ident_circuit_new(&kp.pubkey,
+                                              HS_IDENT_CIRCUIT_INTRO);
+  tt_assert(intro_circ->hs_ident);
+
+  /* Trigger a repurpose. State should be cleaned up. */
+  hs_circ_repurpose(TO_CIRCUIT(intro_circ));
+
+  /* Removed from map. */
+  search = hs_circuitmap_get_intro_circ_v3_service_side(&kp.pubkey);
+  tt_assert(!search);
+  /* HS identifier has been removed. */
+  tt_assert(!intro_circ->hs_ident);
+
+ done:
+  circuit_free_(TO_CIRCUIT(intro_circ));
+  hs_free_all();
+}
+
+struct testcase_t hs_circ_tests[] = {
+  { "repurpose", test_circuit_repurpose, TT_FORK,
+    NULL, NULL },
+
+  END_OF_TESTCASES
+};
+