hs-v2: Lookup intro failure cache when picking an intro from descriptor

When picking an intro point from the service descriptor, the client failed to
lookup the failure cache.

It made an HS v2 client re-pick bad intro points for which we already know it
won't work in the first place.

Based on Neel Chauhan original patch.

Fixes #25568

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

4 files changed, 33 insertions, 4 deletions

diff --git a/changes/bug25568 b/changes/bug25568
new file mode 100644
index 0000000000..eeb7235cae
--- /dev/null
+++ b/changes/bug25568
@@ -0,0 +1,5 @@
+  o Minor bugfixes (onion service v2):
+    - When sending the INTRO cell for a v2 Onion Service, look at the failure
+      cache alongside timeout values to check if the intro point is marked
+      as failed. Previously, we only looked at if the relay timeout values.
+      Fixes bug 25568; bugfix on 0.2.7.3-rc. Patch by Neel Chauhan.
diff --git a/src/feature/rend/rendcache.c b/src/feature/rend/rendcache.c
index c3f86d8c82..2d8de2a80d 100644
--- a/src/feature/rend/rendcache.c
+++ b/src/feature/rend/rendcache.c
@@ -228,6 +228,17 @@ rend_cache_entry_free_void(void *p)
   rend_cache_entry_free_(p);
 }
 
+/** Check if a failure cache entry exists for the given intro point. */
+bool
+rend_cache_intro_failure_exists(const char *service_id,
+                                const uint8_t *intro_identity)
+{
+  tor_assert(service_id);
+  tor_assert(intro_identity);
+
+  return cache_failure_intro_lookup(intro_identity, service_id, NULL);
+}
+
 /** Free all storage held by the service descriptor cache. */
 void
 rend_cache_free_all(void)
diff --git a/src/feature/rend/rendcache.h b/src/feature/rend/rendcache.h
index aec97eabb8..c83f36d189 100644
--- a/src/feature/rend/rendcache.h
+++ b/src/feature/rend/rendcache.h
@@ -80,6 +80,8 @@ int rend_cache_store_v2_desc_as_client(const char *desc,
                                        rend_cache_entry_t **entry);
 size_t rend_cache_get_total_allocation(void);
 
+bool rend_cache_intro_failure_exists(const char *service_id,
+                                     const uint8_t *intro_identity);
 void rend_cache_intro_failure_note(rend_intro_point_failure_t failure,
                                    const uint8_t *identity,
                                    const char *service_id);
diff --git a/src/feature/rend/rendclient.c b/src/feature/rend/rendclient.c
index 2540066dfc..bc94c88efb 100644
--- a/src/feature/rend/rendclient.c
+++ b/src/feature/rend/rendclient.c
@@ -1048,18 +1048,29 @@ rend_client_get_random_intro_impl(const rend_cache_entry_t *entry,
   const or_options_t *options = get_options();
   smartlist_t *usable_nodes;
   int n_excluded = 0;
+  char service_id[REND_SERVICE_ID_LEN_BASE32 + 1];
 
   /* We'll keep a separate list of the usable nodes.  If this becomes empty,
    * no nodes are usable.  */
   usable_nodes = smartlist_new();
   smartlist_add_all(usable_nodes, entry->parsed->intro_nodes);
 
+  /* Get service ID so we can use it to query the failure cache. If we fail to
+   * parse it, this cache entry is no good. */
+  if (BUG(rend_get_service_id(entry->parsed->pk, service_id) < 0)) {
+    return NULL;
+  }
+
   /* Remove the intro points that have timed out during this HS
    * connection attempt from our list of usable nodes. */
-  SMARTLIST_FOREACH(usable_nodes, rend_intro_point_t *, ip,
-                    if (ip->timed_out) {
-                      SMARTLIST_DEL_CURRENT(usable_nodes, ip);
-                    });
+  SMARTLIST_FOREACH_BEGIN(usable_nodes, const rend_intro_point_t *, ip) {
+    bool failed_intro =
+      rend_cache_intro_failure_exists(service_id,
+                       (const uint8_t *) ip->extend_info->identity_digest);
+    if (ip->timed_out || failed_intro) {
+      SMARTLIST_DEL_CURRENT(usable_nodes, ip);
+    };
+  } SMARTLIST_FOREACH_END(ip);
 
  again:
   if (smartlist_len(usable_nodes) == 0) {