hs-v3: Close client intro circuits if the descriptor is replaced

When storing a descriptor in the client cache, if we are about to replace an
existing descriptor, make sure to close every introduction circuits of the old
descriptor so we don't have leftovers lying around.

Ticket 27471 describes a situation where tor is sending an INTRODUCE1 cell on
an introduction circuit for which it doesn't have a matching intro point
object (taken from the descriptor).

The main theory is that, after a new descriptor showed up, the introduction
points changed which led to selecting an introduction circuit not used by the
service anymore thus for which we are unable to find the corresponding
introduction point within the descriptor we just fetched.

Closes #27471.

Signed-off-by: David Goulet <dgoulet@torproject.org>

4 files changed, 41 insertions, 1 deletions

diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c
index b9bcb446a1..afd69e1bec 100644
--- a/src/feature/hs/hs_cache.c
+++ b/src/feature/hs/hs_cache.c
@@ -647,6 +647,13 @@ cache_store_as_client(hs_cache_client_descriptor_t *client_desc)
     }
     /* Remove old entry. Make space for the new one! */
     remove_v3_desc_as_client(cache_entry);
+
+    /* We just removed an old descriptor and will replace it. We'll close all
+     * intro circuits related to this old one so we don't have leftovers. We
+     * leave the rendezvous circuits opened because they could be in use. */
+    hs_client_close_intro_circuits_from_desc(cache_entry->desc);
+
+    /* Free it. */
     cache_client_desc_free(cache_entry);
   }
 
diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c
index 11e24a3660..dfad216abb 100644
--- a/src/feature/hs/hs_client.c
+++ b/src/feature/hs/hs_client.c
@@ -1844,6 +1844,38 @@ hs_client_reextend_intro_circuit(origin_circuit_t *circ)
   return ret;
 }
 
+/* Close all client introduction circuits related to the given descriptor.
+ * This is called with a descriptor that is about to get replaced in the
+ * client cache.
+ *
+ * Even though the introduction point might be exactly the same, we'll rebuild
+ * them if needed but the odds are very low that an existing matching
+ * introduction circuit exists at that stage. */
+void
+hs_client_close_intro_circuits_from_desc(const hs_descriptor_t *desc)
+{
+  origin_circuit_t *ocirc = NULL;
+
+  tor_assert(desc);
+
+  /* We iterate over all client intro circuits because they aren't kept in the
+   * HS circuitmap. That is probably something we want to do one day. */
+  while ((ocirc = circuit_get_next_intro_circ(ocirc, true))) {
+    if (ocirc->hs_ident == NULL) {
+      /* Not a v3 circuit, ignore it. */
+      continue;
+    }
+
+    /* Does it match any IP in the given descriptor? If not, ignore. */
+    if (find_desc_intro_point_by_ident(ocirc->hs_ident, desc) == NULL) {
+      continue;
+    }
+
+    /* We have a match. Close the circuit as consider it expired. */
+    circuit_mark_for_close(TO_CIRCUIT(ocirc), END_CIRC_REASON_FINISHED);
+  }
+}
+
 /* Release all the storage held by the client subsystem. */
 void
 hs_client_free_all(void)
diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h
index fb4f9e9e9f..f6fb167ea2 100644
--- a/src/feature/hs/hs_client.h
+++ b/src/feature/hs/hs_client.h
@@ -77,6 +77,7 @@ int hs_config_client_authorization(const or_options_t *options,
                                    int validate_only);
 
 int hs_client_reextend_intro_circuit(origin_circuit_t *circ);
+void hs_client_close_intro_circuits_from_desc(const hs_descriptor_t *desc);
 
 void hs_client_purge_state(void);
 
diff --git a/src/feature/rend/rendservice.c b/src/feature/rend/rendservice.c
index bae9da3fe5..d135581061 100644
--- a/src/feature/rend/rendservice.c
+++ b/src/feature/rend/rendservice.c
@@ -631,7 +631,7 @@ rend_service_prune_list_impl_(void)
 
   /* For every service introduction circuit we can find, see if we have a
    * matching surviving configured service. If not, close the circuit. */
-  while ((ocirc = circuit_get_next_service_intro_circ(ocirc))) {
+  while ((ocirc = circuit_get_next_intro_circ(ocirc, false))) {
     int keep_it = 0;
     if (ocirc->rend_data == NULL) {
       /* This is a v3 circuit, ignore it. */