1 files changed, 120 insertions, 69 deletions

diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c
index ebe49f09a5..a5747fe170 100644
--- a/src/feature/hs/hs_common.c
+++ b/src/feature/hs/hs_common.c
@@ -926,7 +926,8 @@ hs_parse_address(const char *address, ed25519_public_key_t *key_out,
   }
 
   /* Decode address so we can extract needed fields. */
-  if (base32_decode(decoded, sizeof(decoded), address, strlen(address)) < 0) {
+  if (base32_decode(decoded, sizeof(decoded), address, strlen(address))
+      != sizeof(decoded)) {
     log_warn(LD_REND, "Service address %s can't be decoded.",
              escaped_safe_str(address));
     goto invalid;
@@ -940,7 +941,7 @@ hs_parse_address(const char *address, ed25519_public_key_t *key_out,
   return -1;
 }
 
-/* Validate a given onion address. The length, the base32 decoding and
+/* Validate a given onion address. The length, the base32 decoding, and
  * checksum are validated. Return 1 if valid else 0. */
 int
 hs_address_is_valid(const char *address)
@@ -955,7 +956,7 @@ hs_address_is_valid(const char *address)
     goto invalid;
   }
 
-  /* Get the checksum it's suppose to be and compare it with what we have
+  /* Get the checksum it's supposed to be and compare it with what we have
    * encoded in the address. */
   build_hs_checksum(&service_pubkey, version, target_checksum);
   if (tor_memcmp(checksum, target_checksum, sizeof(checksum))) {
@@ -983,7 +984,7 @@ hs_address_is_valid(const char *address)
  * The returned address is base32 encoded and put in addr_out. The caller MUST
  * make sure the addr_out is at least HS_SERVICE_ADDR_LEN_BASE32 + 1 long.
  *
- * Format is as follow:
+ * Format is as follows:
  *     base32(PUBKEY || CHECKSUM || VERSION)
  *     CHECKSUM = H(".onion checksum" || PUBKEY || VERSION)
  * */
@@ -1009,24 +1010,6 @@ hs_build_address(const ed25519_public_key_t *key, uint8_t version,
   tor_assert(hs_address_is_valid(addr_out));
 }
 
-/* Return a newly allocated copy of lspec. */
-link_specifier_t *
-hs_link_specifier_dup(const link_specifier_t *lspec)
-{
-  link_specifier_t *result = link_specifier_new();
-  memcpy(result, lspec, sizeof(*result));
-  /* The unrecognized field is a dynamic array so make sure to copy its
-   * content and not the pointer. */
-  link_specifier_setlen_un_unrecognized(
-                  result, link_specifier_getlen_un_unrecognized(lspec));
-  if (link_specifier_getlen_un_unrecognized(result)) {
-    memcpy(link_specifier_getarray_un_unrecognized(result),
-           link_specifier_getconstarray_un_unrecognized(lspec),
-           link_specifier_getlen_un_unrecognized(result));
-  }
-  return result;
-}
-
 /* From a given ed25519 public key pk and an optional secret, compute a
  * blinded public key and put it in blinded_pk_out. This is only useful to
  * the client side because the client only has access to the identity public
@@ -1042,7 +1025,7 @@ hs_build_blinded_pubkey(const ed25519_public_key_t *pk,
 
   tor_assert(pk);
   tor_assert(blinded_pk_out);
-  tor_assert(!tor_mem_is_zero((char *) pk, ED25519_PUBKEY_LEN));
+  tor_assert(!fast_mem_is_zero((char *) pk, ED25519_PUBKEY_LEN));
 
   build_blinded_key_param(pk, secret, secret_len,
                           time_period_num, get_time_period_length(), param);
@@ -1067,8 +1050,8 @@ hs_build_blinded_keypair(const ed25519_keypair_t *kp,
   tor_assert(kp);
   tor_assert(blinded_kp_out);
   /* Extra safety. A zeroed key is bad. */
-  tor_assert(!tor_mem_is_zero((char *) &kp->pubkey, ED25519_PUBKEY_LEN));
-  tor_assert(!tor_mem_is_zero((char *) &kp->seckey, ED25519_SECKEY_LEN));
+  tor_assert(!fast_mem_is_zero((char *) &kp->pubkey, ED25519_PUBKEY_LEN));
+  tor_assert(!fast_mem_is_zero((char *) &kp->seckey, ED25519_SECKEY_LEN));
 
   build_blinded_key_param(&kp->pubkey, secret, secret_len,
                           time_period_num, get_time_period_length(), param);
@@ -1300,15 +1283,15 @@ node_has_hsdir_index(const node_t *node)
 
   /* At this point, since the node has a desc, this node must also have an
    * hsdir index. If not, something went wrong, so BUG out. */
-  if (BUG(tor_mem_is_zero((const char*)node->hsdir_index.fetch,
+  if (BUG(fast_mem_is_zero((const char*)node->hsdir_index.fetch,
                           DIGEST256_LEN))) {
     return 0;
   }
-  if (BUG(tor_mem_is_zero((const char*)node->hsdir_index.store_first,
+  if (BUG(fast_mem_is_zero((const char*)node->hsdir_index.store_first,
                           DIGEST256_LEN))) {
     return 0;
   }
-  if (BUG(tor_mem_is_zero((const char*)node->hsdir_index.store_second,
+  if (BUG(fast_mem_is_zero((const char*)node->hsdir_index.store_second,
                           DIGEST256_LEN))) {
     return 0;
   }
@@ -1606,20 +1589,25 @@ hs_purge_last_hid_serv_requests(void)
 /** Given the list of responsible HSDirs in <b>responsible_dirs</b>, pick the
  *  one that we should use to fetch a descriptor right now. Take into account
  *  previous failed attempts at fetching this descriptor from HSDirs using the
- *  string identifier <b>req_key_str</b>.
+ *  string identifier <b>req_key_str</b>. We return whether we are rate limited
+ *  into *<b>is_rate_limited_out</b> if it is not NULL.
  *
  *  Steals ownership of <b>responsible_dirs</b>.
  *
  *  Return the routerstatus of the chosen HSDir if successful, otherwise return
  *  NULL if no HSDirs are worth trying right now. */
 routerstatus_t *
-hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str)
+hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str,
+              bool *is_rate_limited_out)
 {
   smartlist_t *usable_responsible_dirs = smartlist_new();
   const or_options_t *options = get_options();
   routerstatus_t *hs_dir;
   time_t now = time(NULL);
   int excluded_some;
+  bool rate_limited = false;
+  int rate_limited_count = 0;
+  int responsible_dirs_count = smartlist_len(responsible_dirs);
 
   tor_assert(req_key_str);
 
@@ -1639,6 +1627,7 @@ hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str)
     if (last + hs_hsdir_requery_period(options) >= now ||
         !node || !node_has_preferred_descriptor(node, 0)) {
       SMARTLIST_DEL_CURRENT(responsible_dirs, dir);
+      rate_limited_count++;
       continue;
     }
     if (!routerset_contains_node(options->ExcludeNodes, node)) {
@@ -1646,6 +1635,10 @@ hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str)
     }
   } SMARTLIST_FOREACH_END(dir);
 
+  if (rate_limited_count > 0 || responsible_dirs_count > 0) {
+    rate_limited = rate_limited_count == responsible_dirs_count;
+  }
+
   excluded_some =
     smartlist_len(usable_responsible_dirs) < smartlist_len(responsible_dirs);
 
@@ -1657,9 +1650,10 @@ hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str)
   smartlist_free(responsible_dirs);
   smartlist_free(usable_responsible_dirs);
   if (!hs_dir) {
+    const char *warn_str = (rate_limited) ? "we are rate limited." :
+                              "we requested them all recently without success";
     log_info(LD_REND, "Could not pick one of the responsible hidden "
-                      "service directories, because we requested them all "
-                      "recently without success.");
+                      "service directories, because %s.", warn_str);
     if (options->StrictNodes && excluded_some) {
       log_warn(LD_REND, "Could not pick a hidden service directory for the "
                "requested hidden service: they are all either down or "
@@ -1671,17 +1665,23 @@ hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str)
     hs_lookup_last_hid_serv_request(hs_dir, req_key_str, now, 1);
   }
 
+  if (is_rate_limited_out != NULL) {
+    *is_rate_limited_out = rate_limited;
+  }
+
   return hs_dir;
 }
 
-/* From a list of link specifier, an onion key and if we are requesting a
- * direct connection (ex: single onion service), return a newly allocated
- * extend_info_t object. This function always returns an extend info with
- * an IPv4 address, or NULL.
+/* Given a list of link specifiers lspecs, a curve 25519 onion_key, and
+ * a direct connection boolean direct_conn (true for single onion services),
+ * return a newly allocated extend_info_t object.
+ *
+ * This function always returns an extend info with a valid IP address and
+ * ORPort, or NULL. If direct_conn is false, the IP address is always IPv4.
  *
  * It performs the following checks:
- *  if either IPv4 or legacy ID is missing, return NULL.
- *  if direct_conn, and we can't reach the IPv4 address, return NULL.
+ *  if there is no usable IP address, or legacy ID is missing, return NULL.
+ *  if direct_conn, and we can't reach any IP address, return NULL.
  */
 extend_info_t *
 hs_get_extend_info_from_lspecs(const smartlist_t *lspecs,
@@ -1690,21 +1690,40 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs,
 {
   int have_v4 = 0, have_legacy_id = 0, have_ed25519_id = 0;
   char legacy_id[DIGEST_LEN] = {0};
-  uint16_t port_v4 = 0;
-  tor_addr_t addr_v4;
   ed25519_public_key_t ed25519_pk;
   extend_info_t *info = NULL;
+  tor_addr_port_t ap;
 
-  tor_assert(lspecs);
+  tor_addr_make_null(&ap.addr, AF_UNSPEC);
+  ap.port = 0;
+
+  if (lspecs == NULL) {
+    log_warn(LD_BUG, "Specified link specifiers is null");
+    goto done;
+  }
+
+  if (onion_key == NULL) {
+    log_warn(LD_BUG, "Specified onion key is null");
+    goto done;
+  }
+
+  if (smartlist_len(lspecs) == 0) {
+    log_fn(LOG_PROTOCOL_WARN, LD_REND, "Empty link specifier list.");
+    /* Return NULL. */
+    goto done;
+  }
 
   SMARTLIST_FOREACH_BEGIN(lspecs, const link_specifier_t *, ls) {
     switch (link_specifier_get_ls_type(ls)) {
     case LS_IPV4:
-      /* Skip if we already seen a v4. */
-      if (have_v4) continue;
-      tor_addr_from_ipv4h(&addr_v4,
+      /* Skip if we already seen a v4. If direct_conn is true, we skip this
+       * block because fascist_firewall_choose_address_ls() will set ap. If
+       * direct_conn is false, set ap to the first IPv4 address and port in
+       * the link specifiers.*/
+      if (have_v4 || direct_conn) continue;
+      tor_addr_from_ipv4h(&ap.addr,
                           link_specifier_get_un_ipv4_addr(ls));
-      port_v4 = link_specifier_get_un_ipv4_port(ls);
+      ap.port = link_specifier_get_un_ipv4_port(ls);
       have_v4 = 1;
       break;
     case LS_LEGACY_ID:
@@ -1728,45 +1747,38 @@ hs_get_extend_info_from_lspecs(const smartlist_t *lspecs,
     }
   } SMARTLIST_FOREACH_END(ls);
 
-  /* Legacy ID is mandatory, and we require IPv4. */
-  if (!have_v4 || !have_legacy_id) {
+  /* Choose a preferred address first, but fall back to an allowed address. */
+  if (direct_conn)
+    fascist_firewall_choose_address_ls(lspecs, 0, &ap);
+
+  /* Legacy ID is mandatory, and we require an IP address. */
+  if (!tor_addr_port_is_valid_ap(&ap, 0)) {
+    /* If we're missing the IP address, log a warning and return NULL. */
+    log_info(LD_NET, "Unreachable or invalid IP address in link state");
     goto done;
   }
-
-  /* We know we have IPv4, because we just checked. */
-  if (!direct_conn) {
-    /* All clients can extend to any IPv4 via a 3-hop path. */
-    goto validate;
-  } else if (direct_conn &&
-             fascist_firewall_allows_address_addr(&addr_v4, port_v4,
-                                                  FIREWALL_OR_CONNECTION,
-                                                  0, 0)) {
-    /* Direct connection and we can reach it in IPv4 so go for it. */
-    goto validate;
-
-    /* We will add support for falling back to a 3-hop path in a later
-     * release. */
-  } else {
-    /* If we can't reach IPv4, return NULL. */
+  if (!have_legacy_id) {
+    /* If we're missing the legacy ID, log a warning and return NULL. */
+    log_warn(LD_PROTOCOL, "Missing Legacy ID in link state");
     goto done;
   }
 
-  /* We will add support for IPv6 in a later release. */
+  /* We will add support for falling back to a 3-hop path in a later
+   * release. */
 
- validate:
   /* We'll validate now that the address we've picked isn't a private one. If
-   * it is, are we allowing to extend to private address? */
-  if (!extend_info_addr_is_allowed(&addr_v4)) {
+   * it is, are we allowed to extend to private addresses? */
+  if (!extend_info_addr_is_allowed(&ap.addr)) {
     log_fn(LOG_PROTOCOL_WARN, LD_REND,
            "Requested address is private and we are not allowed to extend to "
-           "it: %s:%u", fmt_addr(&addr_v4), port_v4);
+           "it: %s:%u", fmt_addr(&ap.addr), ap.port);
     goto done;
   }
 
   /* We do have everything for which we think we can connect successfully. */
   info = extend_info_new(NULL, legacy_id,
                          (have_ed25519_id) ? &ed25519_pk : NULL, NULL,
-                         onion_key, &addr_v4, port_v4);
+                         onion_key, &ap.addr, ap.port);
  done:
   return info;
 }
@@ -1827,3 +1839,42 @@ hs_inc_rdv_stream_counter(origin_circuit_t *circ)
     tor_assert_nonfatal_unreached();
   }
 }
+
+/* Return a newly allocated link specifier object that is a copy of dst. */
+link_specifier_t *
+link_specifier_dup(const link_specifier_t *src)
+{
+  link_specifier_t *dup = NULL;
+  uint8_t *buf = NULL;
+
+  if (BUG(!src)) {
+    goto err;
+  }
+
+  ssize_t encoded_len_alloc = link_specifier_encoded_len(src);
+  if (BUG(encoded_len_alloc < 0)) {
+    goto err;
+  }
+
+  buf = tor_malloc_zero(encoded_len_alloc);
+  ssize_t encoded_len_data = link_specifier_encode(buf,
+                                                   encoded_len_alloc,
+                                                   src);
+  if (BUG(encoded_len_data < 0)) {
+    goto err;
+  }
+
+  ssize_t parsed_len = link_specifier_parse(&dup, buf, encoded_len_alloc);
+  if (BUG(parsed_len < 0)) {
+    goto err;
+  }
+
+  goto done;
+
+ err:
+  dup = NULL;
+
+ done:
+  tor_free(buf);
+  return dup;
+}