commit 9278a24729c92b9f5c670b3e1608e2cdbd8bd9a1
Author: David Goulet <dgoulet(a)torproject.org>
Date: Tue Feb 4 09:25:55 2020 -0500
hs-v3: Remove descriptor when we remove client authorization
When the ONION_CLIENT_AUTH_REMOVE command is given to tor, now also remove the
descriptor associated with the client authorization credentials.
Fixes #33148
Signed-off-by: David Goulet <dgoulet(a)torproject.org>
---
changes/ticket33148 | 5 +++++
src/feature/hs/hs_cache.c | 36 ++++++++++++++++++++++++++++++
src/feature/hs/hs_cache.h | 1 +
src/feature/hs/hs_client.c | 3 +++
src/test/test_hs_cache.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 100 insertions(+)
diff --git a/changes/ticket33148 b/changes/ticket33148
new file mode 100644
index 000000000..1f44b4259
--- /dev/null
+++ b/changes/ticket33148
@@ -0,0 +1,5 @@
+ o Minor bugfixes (onion service v3, client authorization):
+ - When removing client authorization credentials using the control port,
+ also remove the associated descriptor so they don't linger and are still
+ usable hence making the onion service behind client authorization
+ reachable. Fixes bug 33148; bugfix on 0.4.3.1-alpha.
diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c
index a7b41b835..9cf408ca3 100644
--- a/src/feature/hs/hs_cache.c
+++ b/src/feature/hs/hs_cache.c
@@ -847,6 +847,42 @@ hs_cache_store_as_client(const char *desc_str,
return ret;
}
+/** Remove and free a client cache descriptor entry for the given onion
+ * service ed25519 public key. If the descriptor is decoded, the intro
+ * circuits are closed if any.
+ *
+ * This does nothing if no descriptor exists for the given key. */
+void
+hs_cache_remove_as_client(const ed25519_public_key_t *key)
+{
+ hs_cache_client_descriptor_t *cached_desc = NULL;
+
+ tor_assert(key);
+
+ cached_desc = lookup_v3_desc_as_client(key->pubkey);
+ if (!cached_desc) {
+ return;
+ }
+ /* If we have a decrypted/decoded descriptor, attempt to close its
+ * introduction circuit(s). We shouldn't have circuit(s) without a
+ * descriptor else it will lead to a failure. */
+ if (cached_desc->desc) {
+ hs_client_close_intro_circuits_from_desc(cached_desc->desc);
+ }
+ /* Remove and free. */
+ remove_v3_desc_as_client(cached_desc);
+ cache_client_desc_free(cached_desc);
+
+ /* Logging. */
+ {
+ char key_b64[BASE64_DIGEST256_LEN + 1];
+ digest256_to_base64(key_b64, (const char *) key);
+ log_info(LD_REND, "Onion service v3 descriptor '%s' removed "
+ "from client cache",
+ safe_str_client(key_b64));
+ }
+}
+
/** Clean all client caches using the current time now. */
void
hs_cache_clean_as_client(time_t now)
diff --git a/src/feature/hs/hs_cache.h b/src/feature/hs/hs_cache.h
index ebe1621e8..bb3c77f22 100644
--- a/src/feature/hs/hs_cache.h
+++ b/src/feature/hs/hs_cache.h
@@ -85,6 +85,7 @@ const char *
hs_cache_lookup_encoded_as_client(const struct ed25519_public_key_t *key);
hs_desc_decode_status_t hs_cache_store_as_client(const char *desc_str,
const struct ed25519_public_key_t *identity_pk);
+void hs_cache_remove_as_client(const struct ed25519_public_key_t *key);
void hs_cache_clean_as_client(time_t now);
void hs_cache_purge_as_client(void);
diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c
index 611cc5430..4599bde5b 100644
--- a/src/feature/hs/hs_client.c
+++ b/src/feature/hs/hs_client.c
@@ -1735,6 +1735,9 @@ hs_client_remove_auth_credentials(const char *hsaddress)
find_and_remove_client_auth_creds_file(cred);
}
+ /* Remove associated descriptor if any. */
+ hs_cache_remove_as_client(&service_identity_pk);
+
client_service_authorization_free(cred);
return REMOVAL_SUCCESS;
}
diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c
index 9e0094d25..8ea550b65 100644
--- a/src/test/test_hs_cache.c
+++ b/src/test/test_hs_cache.c
@@ -645,6 +645,59 @@ test_client_cache_decrypt(void *arg)
UNMOCK(networkstatus_get_live_consensus);
}
+static void
+test_client_cache_remove(void *arg)
+{
+ int ret;
+ ed25519_keypair_t service_kp;
+ hs_descriptor_t *desc1 = NULL;
+
+ (void) arg;
+
+ hs_init();
+
+ MOCK(networkstatus_get_live_consensus,
+ mock_networkstatus_get_live_consensus);
+
+ /* Set consensus time. Lookup will not return the entry if it has expired
+ * and it is checked against the consensus valid_after 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));
+
+ /* Build a 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, HS_DESC_DECODE_OK);
+ tor_free(encoded);
+ tt_assert(hs_cache_lookup_as_client(&service_kp.pubkey));
+ }
+
+ /* Remove the cached entry. */
+ hs_cache_remove_as_client(&service_kp.pubkey);
+ tt_assert(!hs_cache_lookup_as_client(&service_kp.pubkey));
+
+ done:
+ hs_descriptor_free(desc1);
+ hs_free_all();
+
+ UNMOCK(networkstatus_get_live_consensus);
+}
+
struct testcase_t hs_cache[] = {
/* Encoding tests. */
{ "directory", test_directory, TT_FORK,
@@ -659,6 +712,8 @@ struct testcase_t hs_cache[] = {
NULL, NULL },
{ "client_cache_decrypt", test_client_cache_decrypt, TT_FORK,
NULL, NULL },
+ { "client_cache_remove", test_client_cache_remove, TT_FORK,
+ NULL, NULL },
END_OF_TESTCASES
};