commit 87511766873fd22ebeff1bc9dcfa78773a890083
Author: David Goulet <dgoulet(a)torproject.org>
Date: Wed Jun 19 09:22:07 2019 -0400
hs-v3: Close intro circuits when cleaning client cache
Fixes #30921
Signed-off-by: David Goulet <dgoulet(a)torproject.org>
---
changes/ticket30921 | 5 +++
src/feature/hs/hs_cache.c | 5 +++
src/test/test_hs_client.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 99 insertions(+)
diff --git a/changes/ticket30921 b/changes/ticket30921
new file mode 100644
index 000000000..50ec570ff
--- /dev/null
+++ b/changes/ticket30921
@@ -0,0 +1,5 @@
+ o Minor bugfixes (onion service v3):
+ - When purging the client descriptor cache, always also close any
+ introduction point circuits associated with it. This avoids picking those
+ when connecting to them later while not having the descriptor to complete
+ the introduction. Fixes bug 30921; bugfix on 0.3.2.1-alpha.
diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c
index 05f9940ae..9817113b2 100644
--- a/src/feature/hs/hs_cache.c
+++ b/src/feature/hs/hs_cache.c
@@ -710,6 +710,11 @@ cache_clean_v3_as_client(time_t now)
MAP_DEL_CURRENT(key);
entry_size = cache_get_client_entry_size(entry);
bytes_removed += entry_size;
+ /* We just removed an old descriptor. We need to close all intro circuits
+ * so we don't have leftovers that can be selected while lacking a
+ * descriptor. We leave the rendezvous circuits opened because they could
+ * be in use. */
+ hs_client_close_intro_circuits_from_desc(entry->desc);
/* Entry is not in the cache anymore, destroy it. */
cache_client_desc_free(entry);
/* Update our OOM. We didn't use the remove() function because we are in
diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c
index 0d25a98bb..fb497d52a 100644
--- a/src/test/test_hs_client.c
+++ b/src/test/test_hs_client.c
@@ -37,6 +37,7 @@
#include "feature/hs/hs_config.h"
#include "feature/hs/hs_ident.h"
#include "feature/hs/hs_cache.h"
+#include "feature/rend/rendcache.h"
#include "core/or/circuitlist.h"
#include "core/or/circuitbuild.h"
#include "core/mainloop/connection.h"
@@ -1007,6 +1008,92 @@ test_close_intro_circuits_new_desc(void *arg)
UNMOCK(networkstatus_get_live_consensus);
}
+static void
+test_close_intro_circuits_cache_clean(void *arg)
+{
+ int ret;
+ ed25519_keypair_t service_kp;
+ circuit_t *circ = NULL;
+ origin_circuit_t *ocirc = NULL;
+ hs_descriptor_t *desc1 = NULL;
+
+ (void) arg;
+
+ hs_init();
+ rend_cache_init();
+
+ /* This is needed because of the client cache expiration timestamp is based
+ * on having a consensus. See cached_client_descriptor_has_expired(). */
+ MOCK(networkstatus_get_live_consensus,
+ mock_networkstatus_get_live_consensus);
+
+ /* Set consensus time */
+ parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC",
+ &mock_ns.valid_after);
+ parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC",
+ &mock_ns.fresh_until);
+ parse_rfc1123_time("Sat, 26 Oct 1985 16:00:00 UTC",
+ &mock_ns.valid_until);
+
+ /* Generate service keypair */
+ tt_int_op(0, OP_EQ, ed25519_keypair_generate(&service_kp, 0));
+
+ /* Create and add to the global list a dummy client introduction circuits.
+ * We'll then make sure the hs_ident is attached to a dummy descriptor. */
+ circ = dummy_origin_circuit_new(0);
+ tt_assert(circ);
+ circ->purpose = CIRCUIT_PURPOSE_C_INTRODUCING;
+ ocirc = TO_ORIGIN_CIRCUIT(circ);
+
+ /* Build the first descriptor and cache it. */
+ {
+ char *encoded;
+ desc1 = hs_helper_build_hs_desc_with_ip(&service_kp);
+ tt_assert(desc1);
+ ret = hs_desc_encode_descriptor(desc1, &service_kp, NULL, &encoded);
+ tt_int_op(ret, OP_EQ, 0);
+ tt_assert(encoded);
+
+ /* Store it */
+ ret = hs_cache_store_as_client(encoded, &service_kp.pubkey);
+ tt_int_op(ret, OP_EQ, 0);
+ tor_free(encoded);
+ tt_assert(hs_cache_lookup_as_client(&service_kp.pubkey));
+ }
+
+ /* We'll pick one introduction point and associate it with the circuit. */
+ {
+ const hs_desc_intro_point_t *ip =
+ smartlist_get(desc1->encrypted_data.intro_points, 0);
+ tt_assert(ip);
+ ocirc->hs_ident = hs_ident_circuit_new(&service_kp.pubkey,
+ HS_IDENT_CIRCUIT_INTRO);
+ ed25519_pubkey_copy(ô->hs_ident->intro_auth_pk,
+ &ip->auth_key_cert->signed_key);
+ }
+
+ /* Before we are about to clean up the intro circuits, make sure it is
+ * actually there. */
+ tt_assert(circuit_get_next_intro_circ(NULL, true));
+
+ /* Cleanup the client cache. The ns valid after time is what decides if the
+ * descriptor has expired so put it in the future enough (72h) so we are
+ * sure to always expire. */
+ mock_ns.valid_after = approx_time() + (72 * 24 * 60 * 60);
+ hs_cache_clean_as_client(0);
+
+ /* Once stored, our intro circuit should be closed because it is related to
+ * an old introduction point that doesn't exists anymore. */
+ tt_assert(!circuit_get_next_intro_circ(NULL, true));
+
+ done:
+ circuit_free(circ);
+ hs_descriptor_free(desc1);
+ hs_free_all();
+ rend_cache_free_all();
+ UNMOCK(networkstatus_get_live_consensus);
+}
+
struct testcase_t hs_client_tests[] = {
{ "e2e_rend_circuit_setup_legacy", test_e2e_rend_circuit_setup_legacy,
TT_FORK, NULL, NULL },
@@ -1026,6 +1113,8 @@ struct testcase_t hs_client_tests[] = {
TT_FORK, NULL, NULL },
{ "close_intro_circuits_new_desc", test_close_intro_circuits_new_desc,
TT_FORK, NULL, NULL },
+ { "close_intro_circuits_cache_clean", test_close_intro_circuits_cache_clean,
+ TT_FORK, NULL, NULL },
END_OF_TESTCASES
};