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@torproject.org>

5 files changed, 100 insertions, 0 deletions

diff --git a/changes/ticket33148 b/changes/ticket33148
new file mode 100644
index 0000000000..1f44b4259d
--- /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 a7b41b835f..9cf408ca3e 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 ebe1621e88..bb3c77f224 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 611cc54302..4599bde5bb 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 9e0094d250..8ea550b65f 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
 };