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 +}; +