Refactor rend_cache_lookup_entry() and how it's used

Here is why:

1) v0 descriptors are deprecated since 0.2.2.1 and not suppose to be alive
in the network anymore. This function should only serve v2 version for now
as the default.

2) It should return different error code depending on what's the actual
error is. Right now, there is no distinction between a cache entry not found
and an invalid query.

3) This function should NOT test if the intro points are usable or not. This
adds some load on a function that should be "O(1)" and do one job.
Furthermore, multiple callsites actually already test that doing twice the
job...

4) While adding control event, it would be useful to be able to lookup a
cache entry without having it checking the intro points. There are also
places in the code that do want to lookup the cache entry without doing
that.

Fixes #14391

Signed-off-by: David Goulet <dgoulet@ev0ke.net>

1 files changed, 36 insertions, 23 deletions

diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index f541249992..3833d166df 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -1502,23 +1502,45 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,
     log_info(LD_REND,"Got a hidden service request for ID '%s'",
              safe_str_client(rend_data->onion_address));
 
-    /* see if we already have a hidden service descriptor cached for this
-     * address. */
+    /* Lookup the given onion address. If invalid, stop right now else we
+     * might have it in the cache or not, it will be tested later on. */
+    unsigned int refetch_desc = 0;
     rend_cache_entry_t *entry = NULL;
     const int rend_cache_lookup_result =
       rend_cache_lookup_entry(rend_data->onion_address, -1, &entry);
     if (rend_cache_lookup_result < 0) {
-      /* We should already have rejected this address! */
-      log_warn(LD_BUG,"Invalid service name '%s'",
-               safe_str_client(rend_data->onion_address));
-      connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL);
-      return -1;
+      switch (-rend_cache_lookup_result) {
+      case EINVAL:
+        /* We should already have rejected this address! */
+        log_warn(LD_BUG,"Invalid service name '%s'",
+            safe_str_client(rend_data->onion_address));
+        connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL);
+        return -1;
+      case ENOENT:
+        refetch_desc = 1;
+        break;
+      default:
+        log_warn(LD_BUG, "Unknown cache lookup error %d",
+            rend_cache_lookup_result);
+        return -1;
+      }
     }
 
     /* Help predict this next time. We're not sure if it will need
      * a stable circuit yet, but we know we'll need *something*. */
     rep_hist_note_used_internal(now, 0, 1);
 
+    /* Now we have a descriptor but is it usable or not? If not, refetch.
+     * Also, a fetch could have been requested if the onion address was not
+     * found in the cache previously. */
+    if (refetch_desc || !rend_client_any_intro_points_usable(entry)) {
+      base_conn->state = AP_CONN_STATE_RENDDESC_WAIT;
+      log_info(LD_REND, "Unknown descriptor %s. Fetching.",
+          safe_str_client(rend_data->onion_address));
+      rend_client_refetch_v2_renddesc(rend_data);
+      return 0;
+    }
+
     /* Look up if we have client authorization configured for this hidden
      * service.  If we do, associate it with the rend_data. */
     rend_service_authorization_t *client_auth =
@@ -1532,22 +1554,13 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,
       rend_data->auth_type = client_auth->auth_type;
     }
 
-    /* Now, we either launch an attempt to connect to the hidden service,
-     * or we launch an attempt to look up its descriptor, depending on
-     * whether we had the descriptor. */
-    if (rend_cache_lookup_result == 0) {
-      base_conn->state = AP_CONN_STATE_RENDDESC_WAIT;
-      log_info(LD_REND, "Unknown descriptor %s. Fetching.",
-               safe_str_client(rend_data->onion_address));
-      rend_client_refetch_v2_renddesc(rend_data);
-    } else { /* rend_cache_lookup_result > 0 */
-      base_conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
-      log_info(LD_REND, "Descriptor is here. Great.");
-      if (connection_ap_handshake_attach_circuit(conn) < 0) {
-        if (!base_conn->marked_for_close)
-          connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
-        return -1;
-      }
+    /* We have the descriptor so launch a connection to the HS. */
+    base_conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
+    log_info(LD_REND, "Descriptor is here. Great.");
+    if (connection_ap_handshake_attach_circuit(conn) < 0) {
+      if (!base_conn->marked_for_close)
+        connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH);
+      return -1;
     }
     return 0;
   }