Merge remote-tracking branch 'rransom-tor/bug3335-v2'

Conflicts:
	src/or/connection_edge.c
	src/or/rendclient.c

changes/bug1297b

@@ -0,0 +1,8 @@
+  o Minor bugfixes:
+
+    - When one of a hidden service's introduction points times out,
+      consider trying it again during the next attempt to connect to
+      the HS.  Previously, we would not try it again unless a newly
+      fetched descriptor contained it.  Required by fixes for bugs
+      1297 and 3825.
+

changes/bug3335

@@ -0,0 +1,11 @@
+  o Major bugfixes:
+
+    - When an attempt to connect to a hidden service ends, consider
+      refetching its hidden service descriptors from each of the HSDir
+      relays responsible for them immediately.  Previously, we would
+      not consider refetching the service's descriptors from each
+      HSDir for 15 minutes after the last fetch; this behaviour was
+      inconvenient if the hidden service was not running during the
+      first attempt, for example.  Bugfix on 0.2.0.18-alpha; fixes bug
+      3335.
+

changes/bug3825a

@@ -0,0 +1,8 @@
+  o Major bugfixes:
+
+    - When one of a hidden service's introduction points appears to be
+      unreachable, stop trying it.  Previously, we would keep trying
+      to build circuits to the introduction point until we lost the
+      descriptor, usually because the user gave up and restarted Tor.
+      Partly fixes bug 3825.
+

src/or/circuitlist.c

@@ -1128,9 +1128,11 @@ circuit_expire_all_dirty_circs(void)
  *   - If circ isn't open yet: call circuit_build_failed() if we're
  *     the origin, and in either case call circuit_rep_hist_note_result()
  *     to note stats.
- *   - If purpose is C_INTRODUCE_ACK_WAIT, remove the intro point we
- *     just tried from our list of intro points for that service
- *     descriptor.
+ *   - If purpose is C_INTRODUCE_ACK_WAIT, report the intro point
+ *     failure we just had to the hidden service client module.
+ *   - If purpose is C_INTRODUCING and <b>reason</b> isn't TIMEOUT,
+ *     report to the hidden service client module that the intro point
+ *     we just tried may be unreachable.
  *   - Send appropriate destroys and edge_destroys for conns and
  *     streams attached to circ.
  *   - If circ->rend_splice is set (we are the midpoint of a joined
@@ -1199,16 +1201,33 @@ _circuit_mark_for_close(circuit_t *circ, int reason, int line,
   }
   if (circ->purpose == CIRCUIT_PURPOSE_C_INTRODUCE_ACK_WAIT) {
     origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
+    int timed_out = (reason == END_STREAM_REASON_TIMEOUT);
     tor_assert(circ->state == CIRCUIT_STATE_OPEN);
     tor_assert(ocirc->build_state->chosen_exit);
     tor_assert(ocirc->rend_data);
     /* treat this like getting a nack from it */
-    log_info(LD_REND, "Failed intro circ %s to %s (awaiting ack). "
-           "Removing from descriptor.",
+    log_info(LD_REND, "Failed intro circ %s to %s (awaiting ack). %s",
            safe_str_client(ocirc->rend_data->onion_address),
+           safe_str_client(build_state_get_exit_nickname(ocirc->build_state)),
+           timed_out ? "Recording timeout." : "Removing from descriptor.");
+    rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
+                                           ocirc->rend_data,
+                                           timed_out ?
+                                           INTRO_POINT_FAILURE_TIMEOUT :
+                                           INTRO_POINT_FAILURE_GENERIC);
+  } else if (circ->purpose == CIRCUIT_PURPOSE_C_INTRODUCING &&
+             reason != END_STREAM_REASON_TIMEOUT) {
+    origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
+    tor_assert(ocirc->build_state->chosen_exit);
+    tor_assert(ocirc->rend_data);
+    log_info(LD_REND, "Failed intro circ %s to %s "
+             "(building circuit to intro point). "
+             "Marking intro point as possibly unreachable.",
+             safe_str_client(ocirc->rend_data->onion_address),
            safe_str_client(build_state_get_exit_nickname(ocirc->build_state)));
-    rend_client_remove_intro_point(ocirc->build_state->chosen_exit,
-                                   ocirc->rend_data);
+    rend_client_report_intro_point_failure(ocirc->build_state->chosen_exit,
+                                           ocirc->rend_data,
+                                           INTRO_POINT_FAILURE_UNREACHABLE);
   }
   if (circ->n_conn) {
     circuit_clear_cell_queue(circ, circ->n_conn);

src/or/circuituse.c

@@ -1589,6 +1589,13 @@ link_apconn_to_circ(entry_connection_t *apconn, origin_circuit_t *circ,
   /* assert_connection_ok(conn, time(NULL)); */
   circ->p_streams = ENTRY_TO_EDGE_CONN(apconn);
 
+  if (connection_edge_is_rendezvous_stream(apconn)) {
+    /* We are attaching a stream to a rendezvous circuit.  That means
+     * that an attempt to connect to a hidden service just
+     * succeeded.  Tell rendclient.c. */
+   rend_client_note_connection_attempt_ended(apconn->rend_data->onion_address);
+  }
+
   if (cpath) { /* we were given one; use it */
     tor_assert(cpath_is_on_circuit(circ, cpath));
   } else {

src/or/connection_edge.c

@@ -71,6 +71,18 @@ _connection_mark_unattached_ap(entry_connection_t *conn, int endreason,
   tor_assert(base_conn->type == CONN_TYPE_AP);
   ENTRY_TO_EDGE_CONN(conn)->edge_has_sent_end = 1; /* no circ yet */
 
+  /* If this is a rendezvous stream and it is failing without ever
+   * being attached to a circuit, assume that an attempt to connect to
+   * the destination hidden service has just ended.
+   *
+   * XXX023 This condition doesn't limit to only streams failing
+   * without ever being attached.  That sloppiness should be harmless,
+   * but we should fix it someday anyway. */
+  if ((conn->on_circuit != NULL || conn->edge_has_sent_end) &&
+      connection_edge_is_rendezvous_stream(conn)) {
+    rend_client_note_connection_attempt_ended(conn->rend_data->onion_address);
+  }
+
   if (base_conn->marked_for_close) {
     /* This call will warn as appropriate. */
     _connection_mark_for_close(base_conn, line, file);

src/or/or.h

@@ -3902,6 +3902,11 @@ typedef struct rend_encoded_v2_service_descriptor_t {
   char *desc_str; /**< Descriptor string. */
 } rend_encoded_v2_service_descriptor_t;
 
+/** The maximum number of non-circuit-build-timeout failures a hidden
+ * service client will tolerate while trying to build a circuit to an
+ * introduction point.  See also rend_intro_point_t.unreachable_count. */
+#define MAX_INTRO_POINT_REACHABILITY_FAILURES 5
+
 /** Introduction point information.  Used both in rend_service_t (on
  * the service side) and in rend_service_descriptor_t (on both the
  * client and service side). */
@@ -3909,6 +3914,18 @@ typedef struct rend_intro_point_t {
   extend_info_t *extend_info; /**< Extend info of this introduction point. */
   crypto_pk_env_t *intro_key; /**< Introduction key that replaces the service
                                * key, if this descriptor is V2. */
+
+  /** (Client side only) Flag indicating that a timeout has occurred
+   * after sending an INTRODUCE cell to this intro point.  After a
+   * timeout, an intro point should not be tried again during the same
+   * hidden service connection attempt, but it may be tried again
+   * during a future connection attempt. */
+  unsigned int timed_out : 1;
+
+  /** (Client side only) The number of times we have failed to build a
+   * circuit to this intro point for some reason other than our
+   * circuit-build timeout.  See also MAX_INTRO_POINT_REACHABILITY_FAILURES. */
+  unsigned int unreachable_count : 3;
 } rend_intro_point_t;
 
 /** Information used to connect to a hidden service.  Used on both the

src/or/rendclient.c

@@ -371,8 +371,9 @@ rend_client_introduction_acked(origin_circuit_t *circ,
     log_info(LD_REND, "Got nack for %s from %s...",
         safe_str_client(circ->rend_data->onion_address),
         safe_str_client(extend_info_describe(circ->build_state->chosen_exit)));
-    if (rend_client_remove_intro_point(circ->build_state->chosen_exit,
-                                       circ->rend_data) > 0) {
+    if (rend_client_report_intro_point_failure(circ->build_state->chosen_exit,
+                                               circ->rend_data,
+                                               INTRO_POINT_FAILURE_GENERIC)>0){
       /* There are introduction points left. Re-extend the circuit to
        * another intro point and try again. */
       int result = rend_client_reextend_intro_circuit(circ);
@@ -389,9 +390,12 @@ rend_client_introduction_acked(origin_circuit_t *circ,
 #define REND_HID_SERV_DIR_REQUERY_PERIOD (15 * 60)
 
 /** Contains the last request times to hidden service directories for
- * certain queries; keys are strings consisting of base32-encoded
- * hidden service directory identities and base32-encoded descriptor IDs;
- * values are pointers to timestamps of the last requests. */
+ * certain queries; each key is a string consisting of the
+ * concatenation of a base32-encoded HS directory identity digest, a
+ * base32-encoded HS descriptor ID, and a hidden service address
+ * (without the ".onion" part); each value is a pointer to a time_t
+ * holding the time of the last request for that descriptor ID to that
+ * HS directory. */
 static strmap_t *last_hid_serv_requests_ = NULL;
 
 /** Returns last_hid_serv_requests_, initializing it to a new strmap if
@@ -404,23 +408,34 @@ get_last_hid_serv_requests(void)
   return last_hid_serv_requests_;
 }
 
+#define LAST_HID_SERV_REQUEST_KEY_LEN (REND_DESC_ID_V2_LEN_BASE32 + \
+                                       REND_DESC_ID_V2_LEN_BASE32 + \
+                                       REND_SERVICE_ID_LEN_BASE32)
+
 /** Look up the last request time to hidden service directory <b>hs_dir</b>
- * for descriptor ID <b>desc_id_base32</b>. If <b>set</b> is non-zero,
+ * for descriptor ID <b>desc_id_base32</b> for the service specified in
+ * <b>rend_query</b>. If <b>set</b> is non-zero,
  * assign the current time <b>now</b> and return that. Otherwise, return
  * the most recent request time, or 0 if no such request has been sent
  * before. */
 static time_t
 lookup_last_hid_serv_request(routerstatus_t *hs_dir,
-                             const char *desc_id_base32, time_t now, int set)
+                             const char *desc_id_base32,
+                             const rend_data_t *rend_query,
+                             time_t now, int set)
 {
   char hsdir_id_base32[REND_DESC_ID_V2_LEN_BASE32 + 1];
-  char hsdir_desc_comb_id[2 * REND_DESC_ID_V2_LEN_BASE32 + 1];
+  char hsdir_desc_comb_id[LAST_HID_SERV_REQUEST_KEY_LEN + 1];
   time_t *last_request_ptr;
   strmap_t *last_hid_serv_requests = get_last_hid_serv_requests();
   base32_encode(hsdir_id_base32, sizeof(hsdir_id_base32),
                 hs_dir->identity_digest, DIGEST_LEN);
-  tor_snprintf(hsdir_desc_comb_id, sizeof(hsdir_desc_comb_id), "%s%s",
-               hsdir_id_base32, desc_id_base32);
+  tor_snprintf(hsdir_desc_comb_id, sizeof(hsdir_desc_comb_id), "%s%s%s",
+               hsdir_id_base32,
+               desc_id_base32,
+               rend_query->onion_address);
+  /* XXX023 tor_assert(strlen(hsdir_desc_comb_id) ==
+                       LAST_HID_SERV_REQUEST_KEY_LEN); */
   if (set) {
     time_t *oldptr;
     last_request_ptr = tor_malloc_zero(sizeof(time_t));
@@ -459,6 +474,33 @@ directory_clean_last_hid_serv_requests(time_t now)
   }
 }
 
+/** Remove all requests related to the hidden service named
+ * <b>onion_address</b> from the history of times of requests to
+ * hidden service directories. */
+static void
+purge_hid_serv_from_last_hid_serv_requests(const char *onion_address)
+{
+  strmap_iter_t *iter;
+  strmap_t *last_hid_serv_requests = get_last_hid_serv_requests();
+  /* XXX023 tor_assert(strlen(onion_address) == REND_SERVICE_ID_LEN_BASE32); */
+  for (iter = strmap_iter_init(last_hid_serv_requests);
+       !strmap_iter_done(iter); ) {
+    const char *key;
+    void *val;
+    strmap_iter_get(iter, &key, &val);
+    /* XXX023 tor_assert(strlen(key) == LAST_HID_SERV_REQUEST_KEY_LEN); */
+    if (tor_memeq(key + LAST_HID_SERV_REQUEST_KEY_LEN -
+                  REND_SERVICE_ID_LEN_BASE32,
+                  onion_address,
+                  REND_SERVICE_ID_LEN_BASE32)) {
+      iter = strmap_iter_next_rmv(last_hid_serv_requests, iter);
+      tor_free(val);
+    } else {
+      iter = strmap_iter_next(last_hid_serv_requests, iter);
+    }
+  }
+}
+
 /** Purge the history of request times to hidden service directories,
  * so that future lookups of an HS descriptor will not fail because we
  * accessed all of the HSDir relays responsible for the descriptor
@@ -480,12 +522,11 @@ rend_client_purge_last_hid_serv_requests(void)
 }
 
 /** Determine the responsible hidden service directories for <b>desc_id</b>
- * and fetch the descriptor belonging to that ID from one of them. Only
- * send a request to hidden service directories that we did not try within
- * the last REND_HID_SERV_DIR_REQUERY_PERIOD seconds; on success, return 1,
+ * and fetch the descriptor with that ID from one of them. Only
+ * send a request to a hidden service directory that we have not yet tried
+ * during this attempt to connect to this hidden service; on success, return 1,
  * in the case that no hidden service directory is left to ask for the
- * descriptor, return 0, and in case of a failure -1. <b>query</b> is only
- * passed for pretty log statements. */
+ * descriptor, return 0, and in case of a failure -1.  */
 static int
 directory_get_from_hs_dir(const char *desc_id, const rend_data_t *rend_query)
 {
@@ -510,11 +551,12 @@ directory_get_from_hs_dir(const char *desc_id, const rend_data_t *rend_query)
   directory_clean_last_hid_serv_requests(now);
 
   SMARTLIST_FOREACH(responsible_dirs, routerstatus_t *, dir, {
-      time_t last = lookup_last_hid_serv_request(dir, desc_id_base32, 0, 0);
+      time_t last = lookup_last_hid_serv_request(
+                            dir, desc_id_base32, rend_query, 0, 0);
       const node_t *node = node_get_by_id(dir->identity_digest);
       if (last + REND_HID_SERV_DIR_REQUERY_PERIOD >= now ||
           !node || !node_has_descriptor(node))
-        SMARTLIST_DEL_CURRENT(responsible_dirs, dir);
+      SMARTLIST_DEL_CURRENT(responsible_dirs, dir);
   });
 
   hs_dir = smartlist_choose(responsible_dirs);
@@ -526,9 +568,9 @@ directory_get_from_hs_dir(const char *desc_id, const rend_data_t *rend_query)
     return 0;
   }
 
-  /* Remember, that we are requesting a descriptor from this hidden service
+  /* Remember that we are requesting a descriptor from this hidden service
    * directory now. */
-  lookup_last_hid_serv_request(hs_dir, desc_id_base32, now, 1);
+  lookup_last_hid_serv_request(hs_dir, desc_id_base32, rend_query, now, 1);
 
   /* Encode descriptor cookie for logging purposes. */
   if (rend_query->auth_type != REND_NO_AUTH) {
@@ -582,10 +624,11 @@ rend_client_refetch_v2_renddesc(const rend_data_t *rend_query)
         "service descriptor, but are not fetching service descriptors.");
     return;
   }
-  /* Before fetching, check if we already have the descriptor here. */
-  if (rend_cache_lookup_entry(rend_query->onion_address, -1, &e) > 0) {
+  /* Before fetching, check if we already have a usable descriptor here. */
+  if (rend_cache_lookup_entry(rend_query->onion_address, -1, &e) > 0 &&
+      rend_client_any_intro_points_usable(e)) {
     log_info(LD_REND, "We would fetch a v2 rendezvous descriptor, but we "
-                      "already have that descriptor here. Not fetching.");
+                      "already have a usable descriptor here. Not fetching.");
     return;
   }
   log_debug(LD_REND, "Fetching v2 rendezvous descriptor for service %s",
@@ -655,16 +698,31 @@ rend_client_cancel_descriptor_fetches(void)
   } SMARTLIST_FOREACH_END(conn);
 }
 
-/** Remove failed_intro from ent. If ent now has no intro points, or
- * service is unrecognized, then launch a new renddesc fetch.
-
+/** Mark <b>failed_intro</b> as a failed introduction point for the
+ * hidden service specified by <b>rend_query</b>. If the HS now has no
+ * usable intro points, or we do not have an HS descriptor for it,
+ * then launch a new renddesc fetch.
  *
- * Return -1 if error, 0 if no intro points remain or service
+ * If <b>failure_type</b> is INTRO_POINT_FAILURE_GENERIC, remove the
+ * intro point from (our parsed copy of) the HS descriptor.
+ *
+ * If <b>failure_type</b> is INTRO_POINT_FAILURE_TIMEOUT, mark the
+ * intro point as 'timed out'; it will not be retried until the
+ * current hidden service connection attempt has ended or it has
+ * appeared in a newly fetched rendezvous descriptor.
+ *
+ * If <b>failure_type</b> is INTRO_POINT_FAILURE_UNREACHABLE,
+ * increment the intro point's reachability-failure count; if it has
+ * now failed MAX_INTRO_POINT_REACHABILITY_FAILURES or more times,
+ * remove the intro point from (our parsed copy of) the HS descriptor.
+ *
+ * Return -1 if error, 0 if no usable intro points remain or service
  * unrecognized, 1 if recognized and some intro points remain.
  */
 int
-rend_client_remove_intro_point(extend_info_t *failed_intro,
-                               const rend_data_t *rend_query)
+rend_client_report_intro_point_failure(extend_info_t *failed_intro,
+                                       const rend_data_t *rend_query,
+                                       unsigned int failure_type)
 {
   int i, r;
   rend_cache_entry_t *ent;
@@ -687,8 +745,34 @@ rend_client_remove_intro_point(extend_info_t *failed_intro,
     rend_intro_point_t *intro = smartlist_get(ent->parsed->intro_nodes, i);
     if (tor_memeq(failed_intro->identity_digest,
                 intro->extend_info->identity_digest, DIGEST_LEN)) {
-      rend_intro_point_free(intro);
-      smartlist_del(ent->parsed->intro_nodes, i);
+      switch (failure_type) {
+      default:
+        log_warn(LD_BUG, "Unknown failure type %u. Removing intro point.",
+                 failure_type);
+        tor_fragile_assert();
+        /* fall through */
+      case INTRO_POINT_FAILURE_GENERIC:
+        rend_intro_point_free(intro);
+        smartlist_del(ent->parsed->intro_nodes, i);
+        break;
+      case INTRO_POINT_FAILURE_TIMEOUT:
+        intro->timed_out = 1;
+        break;
+      case INTRO_POINT_FAILURE_UNREACHABLE:
+        ++(intro->unreachable_count);
+        {
+          int zap_intro_point =
+            intro->unreachable_count >= MAX_INTRO_POINT_REACHABILITY_FAILURES;
+          log_info(LD_REND, "Failed to reach this intro point %u times.%s",
+                   intro->unreachable_count,
+                   zap_intro_point ? " Removing from descriptor.": "");
+          if (zap_intro_point) {
+            rend_intro_point_free(intro);
+            smartlist_del(ent->parsed->intro_nodes, i);
+          }
+        }
+        break;
+      }
       break;
     }
   }
@@ -867,10 +951,36 @@ rend_client_desc_trynow(const char *query)
                  "unavailable (try again later).",
                  safe_str_client(query));
       connection_mark_unattached_ap(conn, END_STREAM_REASON_RESOLVEFAILED);
+      rend_client_note_connection_attempt_ended(query);
     }
   } SMARTLIST_FOREACH_END(base_conn);
 }
 
+/** Clear temporary state used only during an attempt to connect to
+ * the hidden service named <b>onion_address</b>.  Called when a
+ * connection attempt has ended; may be called occasionally at other
+ * times, and should be reasonably harmless. */
+void
+rend_client_note_connection_attempt_ended(const char *onion_address)
+{
+  rend_cache_entry_t *cache_entry = NULL;
+  rend_cache_lookup_entry(onion_address, -1, &cache_entry);
+
+  log_info(LD_REND, "Connection attempt for %s has ended; "
+           "cleaning up temporary state.",
+           safe_str_client(onion_address));
+
+  /* Clear the timed_out flag on all remaining intro points for this HS. */
+  if (cache_entry != NULL) {
+    SMARTLIST_FOREACH(cache_entry->parsed->intro_nodes,
+                      rend_intro_point_t *, ip,
+                      ip->timed_out = 0; );
+  }
+
+  /* Remove the HS's entries in last_hid_serv_requests. */
+  purge_hid_serv_from_last_hid_serv_requests(onion_address);
+}
+
 /** Return a newly allocated extend_info_t* for a randomly chosen introduction
  * point for the named hidden service.  Return NULL if all introduction points
  * have been tried and failed.
@@ -919,6 +1029,13 @@ rend_client_get_random_intro_impl(const rend_cache_entry_t *entry,
   usable_nodes = smartlist_create();
   smartlist_add_all(usable_nodes, entry->parsed->intro_nodes);
 
+  /* 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);
+                    });
+
  again:
   if (smartlist_len(usable_nodes) == 0) {
     if (n_excluded && get_options()->StrictNodes && warnings) {

src/or/rendclient.h

@@ -22,8 +22,15 @@ int rend_client_introduction_acked(origin_circuit_t *circ,
 void rend_client_refetch_v2_renddesc(const rend_data_t *rend_query);
 void rend_client_cancel_descriptor_fetches(void);
 void rend_client_purge_last_hid_serv_requests(void);
-int rend_client_remove_intro_point(extend_info_t *failed_intro,
-                                   const rend_data_t *rend_query);
+
+#define INTRO_POINT_FAILURE_GENERIC 0
+#define INTRO_POINT_FAILURE_TIMEOUT 1
+#define INTRO_POINT_FAILURE_UNREACHABLE 2
+
+int rend_client_report_intro_point_failure(extend_info_t *failed_intro,
+                                           const rend_data_t *rend_query,
+                                           unsigned int failure_type);
+
 int rend_client_rendezvous_acked(origin_circuit_t *circ,
                                  const uint8_t *request,
                                  size_t request_len);
@@ -32,6 +39,8 @@ int rend_client_receive_rendezvous(origin_circuit_t *circ,
                                    size_t request_len);
 void rend_client_desc_trynow(const char *query);
 
+void rend_client_note_connection_attempt_ended(const char *onion_address);
+
 extend_info_t *rend_client_get_random_intro(const rend_data_t *rend_query);
 int rend_client_any_intro_points_usable(const rend_cache_entry_t *entry);