Fix issues arrising with the dual failure test

3 files changed, 141 insertions, 87 deletions

diff --git a/src/or/or.h b/src/or/or.h
index cfbaba469..b79a8b07f 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4799,6 +4799,10 @@ typedef struct rend_intro_point_t {
    * included in the last HS descriptor we generated. */
   unsigned int listed_in_last_desc : 1;
 
+  /** (Service side only) Flag indicating that this intro point was
+   * included in the last HS descriptor we generated. */
+  unsigned int update_desc_on_connection : 1;
+
   /** (Service side only) Flag indicating that
    * rend_service_note_removing_intro_point has been called for this
    * intro point. */
diff --git a/src/or/rendservice.c b/src/or/rendservice.c
index b6d0fd38b..f654c955a 100644
--- a/src/or/rendservice.c
+++ b/src/or/rendservice.c
@@ -80,7 +80,7 @@ typedef struct rend_service_port_config_t {
 #define INTRO_CIRC_RETRY_PERIOD (60*5)
 /** Don't try to build more than this many circuits before giving up
  * for a while.*/
-#define MAX_INTRO_CIRCS_PER_PERIOD 10
+#define MAX_INTRO_CIRCS_PER_PERIOD 20
 /** How many times will a hidden service operator attempt to connect to
  * a requested rendezvous point before giving up? */
 #define MAX_REND_FAILURES 30
@@ -111,6 +111,8 @@ typedef struct rend_service_t {
                                 * fetched, 0 if it has not been */
   smartlist_t *intro_nodes; /**< List of rend_intro_point_t's we have,
                              * or are trying to establish. */
+  smartlist_t *failed_intro_nodes; /**< List of rend_intro_point_t's we have,
+                             * or are trying to establish. */
   time_t intro_period_started; /**< Start of the current period to build
                                 * introduction points. */
   int n_intro_circuits_launched; /**< Count of intro circuits we have
@@ -230,6 +232,7 @@ rend_add_service(rend_service_t *service)
   rend_service_port_config_t *p;
 
   service->intro_nodes = smartlist_new();
+  service->failed_intro_nodes = smartlist_new();
 
   if (service->auth_type != REND_NO_AUTH &&
       smartlist_len(service->clients) == 0) {
@@ -2308,11 +2311,6 @@ rend_service_launch_establish_intro(rend_service_t *service,
 {
   origin_circuit_t *launched;
 
-  log_info(LD_REND,
-           "Launching circuit to introduction point %s for service %s",
-           safe_str_client(extend_info_describe(intro->extend_info)),
-           service->service_id);
-
   rep_hist_note_used_internal(time(NULL), 1, 0);
 
   ++service->n_intro_circuits_launched;
@@ -2327,13 +2325,13 @@ rend_service_launch_establish_intro(rend_service_t *service,
     return -1;
   } else {
     log_info(LD_REND,
-             "Launched circuit n_circ_id %u global_identifier %u to introduction point %s for service %s",
+             "Launched circuit n_circ_id %u global_identifier %u to introduction point %s",
              (unsigned)launched->base_.n_circ_id,
              launched->global_identifier,
-             safe_str_client(extend_info_describe(intro->extend_info)),
-             service->service_id);
+             safe_str_client(extend_info_describe(intro->extend_info)));
   }
 
+  // TODO: This should never happen
   if (tor_memneq(intro->extend_info->identity_digest,
       launched->build_state->chosen_exit->identity_digest, DIGEST_LEN)) {
     char cann[HEX_DIGEST_LEN+1], orig[HEX_DIGEST_LEN+1];
@@ -2552,7 +2550,7 @@ rend_service_intro_established(origin_circuit_t *circuit,
              (unsigned)circuit->base_.n_circ_id);
     goto err;
   }
-  service->desc_is_dirty = time(NULL);
+
   circuit_change_purpose(TO_CIRCUIT(circuit), CIRCUIT_PURPOSE_S_INTRO);
 
   base32_encode(serviceid, REND_SERVICE_ID_LEN_BASE32 + 1,
@@ -2561,6 +2559,17 @@ rend_service_intro_established(origin_circuit_t *circuit,
            "Received INTRO_ESTABLISHED cell on circuit %u for service %s",
            (unsigned)circuit->base_.n_circ_id, serviceid);
 
+  rend_intro_point_t *intro = find_intro_point(circuit);
+
+  if (intro->update_desc_on_connection) {
+    service->desc_is_dirty = time(NULL);
+    log_info(LD_REND, "Invalidating descriptor for INTRO_ESTABLISHED cell on circuit %u",
+           (unsigned)circuit->base_.n_circ_id);
+  } else {
+    log_info(LD_REND, "Skiping descriptor for INTRO_ESTABLISHED cell on circuit %u",
+           (unsigned)circuit->base_.n_circ_id);
+  }
+
   /* Getting a valid INTRODUCE_ESTABLISHED means we've successfully
    * used the circ */
   pathbias_mark_use_success(circuit);
@@ -3021,6 +3030,8 @@ intro_point_should_expire_now(rend_intro_point_t *intro,
 void
 rend_services_introduce(void)
 {
+  log_info(LD_REND, "start of rend_services_introduce");
+
   int i,j,r;
   const node_t *node;
   rend_service_t *service;
@@ -3028,15 +3039,17 @@ rend_services_introduce(void)
   int intro_point_set_changed, prev_intro_nodes;
   unsigned int n_intro_points_unexpired;
   unsigned int n_intro_points_to_open;
-  smartlist_t *intro_nodes;
+  //smartlist_t *intro_nodes;
   time_t now;
   // const or_options_t *options = get_options(); unused
 
-  intro_nodes = smartlist_new();
+  //intro_nodes = smartlist_new();
   now = time(NULL);
 
+  int establish_intros = 1;
+
   for (i=0; i < smartlist_len(rend_service_list); ++i) {
-    smartlist_clear(intro_nodes);
+    //smartlist_clear(intro_nodes);
     service = smartlist_get(rend_service_list, i);
 
     tor_assert(service);
@@ -3058,13 +3071,20 @@ rend_services_introduce(void)
                MAX_INTRO_CIRCS_PER_PERIOD) {
       /* We have failed too many times in this period; wait for the next
        * one before we try again. */
+      log_info(LD_REND, "skipping as %d >= %d",
+          service->n_intro_circuits_launched, MAX_INTRO_CIRCS_PER_PERIOD);
       continue;
     }
 
+    log_info(LD_REND, "starting to check the introduction points");
+
     /* Find out which introduction points we have in progress for this
        service. */
     SMARTLIST_FOREACH_BEGIN(service->intro_nodes, rend_intro_point_t *,
                             intro) {
+      log_info(LD_REND, "Looking at %s",
+               safe_str_client(extend_info_describe(intro->extend_info)));
+
       origin_circuit_t *intro_circ =
         find_intro_circuit(intro, service->pk_digest);
 
@@ -3082,64 +3102,9 @@ rend_services_introduce(void)
         /* We don't need to set intro_point_set_changed here, because
          * this intro point wouldn't have been published in a current
          * descriptor anyway. */
-        continue;
-      }
 
-      node = node_get_by_id(intro->extend_info->identity_digest);
-      if (!node || !intro_circ) {
-        /* The introduction point could still be up, so attempt to reconnect
-         * before discounting it */
-
-        if (intro->unreachable_count >= 2) {
-          int removing_this_intro_point_changes_the_intro_point_set = 1;
-
-          log_info(LD_REND, "Giving up on %s as intro point for %s"
-                   " (circuit disappeared).",
-                   safe_str_client(extend_info_describe(intro->extend_info)),
-                   safe_str_client(service->service_id));
-
-          rend_service_note_removing_intro_point(service, intro);
-
-          if (intro->time_expiring != -1) {
-            log_info(LD_REND, "We were already expiring the intro point; "
-                     "no need to mark the HS descriptor as dirty over this.");
-            removing_this_intro_point_changes_the_intro_point_set = 0;
-          } else if (intro->listed_in_last_desc) {
-            log_info(LD_REND, "The intro point we are giving up on was "
-                     "included in the last published descriptor. "
-                     "Marking current descriptor as dirty.");
-            service->desc_is_dirty = now;
-          }
-
-          rend_intro_point_free(intro);
-          intro = NULL; /* SMARTLIST_DEL_CURRENT takes a name, not a value. */
-          SMARTLIST_DEL_CURRENT(service->intro_nodes, intro);
-          if (removing_this_intro_point_changes_the_intro_point_set)
-            intro_point_set_changed = 1;
-        } else {
-          if (intro->unreachable_count == 0) {
-            log_info(LD_REND, "Circuit to %s has disapeared",
-                     safe_str_client(extend_info_describe(intro->extend_info)));
-          }
-
-          intro->unreachable_count++;
-          log_info(LD_REND, "incremented unreachable_count for %s to %d)",
-                   safe_str_client(extend_info_describe(intro->extend_info)),
-                   intro->unreachable_count);
-
-          log_info(LD_REND, "launching another circuit to %s",
-                   safe_str_client(extend_info_describe(intro->extend_info)));
-
-          r = rend_service_launch_establish_intro(service, intro);
-          if (r<0) {
-            log_warn(LD_REND, "Error launching circuit to node %s for service %s.",
-                     safe_str_client(extend_info_describe(intro->extend_info)),
-                     safe_str_client(service->service_id));
-          }
-        }
-      }
+      } else if (intro != NULL && intro_point_should_expire_now(intro, now)) {
 
-      if (intro != NULL && intro_point_should_expire_now(intro, now)) {
         log_info(LD_REND, "Expiring %s as intro point for %s.",
                  safe_str_client(extend_info_describe(intro->extend_info)),
                  safe_str_client(service->service_id));
@@ -3159,20 +3124,88 @@ rend_services_introduce(void)
         intro->time_expiring = now;
 
         intro_point_set_changed = 1;
-      }
+      } else {
+
+        node = node_get_by_id(intro->extend_info->identity_digest);
+        if (!node || !intro_circ) {
+          /* The introduction point could still be up, so attempt to reconnect
+           * before discounting it */
+
+          if (intro->unreachable_count >= 2) {
+            int removing_this_intro_point_changes_the_intro_point_set = 1;
+
+            log_info(LD_REND, "Giving up on %s as intro point for %s"
+                     " (circuit disappeared).",
+                     safe_str_client(extend_info_describe(intro->extend_info)),
+                     safe_str_client(service->service_id));
+
+            rend_service_note_removing_intro_point(service, intro);
+
+            if (intro->time_expiring != -1) {
+              log_info(LD_REND, "We were already expiring the intro point; "
+                       "no need to mark the HS descriptor as dirty over this.");
+              removing_this_intro_point_changes_the_intro_point_set = 0;
+            } else if (intro->listed_in_last_desc) {
+              log_info(LD_REND, "The intro point we are giving up on was "
+                       "included in the last published descriptor. "
+                       "Marking current descriptor as dirty.");
+              service->desc_is_dirty = now;
+            }
+
+            SMARTLIST_DEL_CURRENT(service->intro_nodes, intro);
+
+            smartlist_add(service->failed_intro_nodes, (void*)node);
+
+            //rend_intro_point_free(intro);
+            //intro = NULL; /* SMARTLIST_DEL_CURRENT takes a name, not a value. */
+            if (removing_this_intro_point_changes_the_intro_point_set)
+              intro_point_set_changed = 1;
+          } else {
+            // wait until there are no uncertian introduction points before
+            // establishing new ones
+            establish_intros = 0;
+
+            if (intro->unreachable_count == 0) {
+              log_info(LD_REND, "Circuit to %s has disapeared",
+                       safe_str_client(extend_info_describe(intro->extend_info)));
+            }
+
+            intro->unreachable_count++;
+            log_info(LD_REND, "incremented unreachable_count for %s to %d)",
+                     safe_str_client(extend_info_describe(intro->extend_info)),
+                     intro->unreachable_count);
 
-      if (intro != NULL && intro->time_expiring == -1)
-        ++n_intro_points_unexpired;
+            intro->update_desc_on_connection = 0;
 
-      if (node)
-        smartlist_add(intro_nodes, (void*)node);
+            r = rend_service_launch_establish_intro(service, intro);
+            if (r<0) {
+              log_warn(LD_REND, "Error launching circuit to node %s",
+                       safe_str_client(extend_info_describe(intro->extend_info)));
+            }
+          }
+        } else {
+
+          if (intro != NULL && intro->time_expiring == -1)
+            ++n_intro_points_unexpired;
+        }
+      }
+
+      //if (node)
+      //  smartlist_add(intro_nodes, (void*)node);
     } SMARTLIST_FOREACH_END(intro);
 
+    log_info(LD_REND, "finished checking the introduction points");
+
     if (!intro_point_set_changed &&
         (n_intro_points_unexpired >= service->n_intro_points_wanted)) {
       continue;
     }
 
+    if (!establish_intros) {
+      log_info(LD_REND, "skipping establishing new introduction points");
+      continue;
+    }
+
     /* Remember how many introduction circuits we started with.
      *
      * prev_intro_nodes serves a different purpose than
@@ -3186,8 +3219,6 @@ rend_services_introduce(void)
     log_info(LD_REND, "n_intro_points_unexpired %i", n_intro_points_unexpired);
     log_info(LD_REND, "prev_intro_nodes %i", prev_intro_nodes);
 
-    int establish_intros = 1;
-
     if (n_intro_points_unexpired == 0) {
       log_info(LD_REND, "Currently no introduction points");
 
@@ -3310,22 +3341,24 @@ rend_services_introduce(void)
 
       if (n_intro_points_to_open != 0) {
 
-        log_info(LD_REND,
-            "query %s didn't have valid rend desc in cache. "
-            "Connecting to random introduction points",
-            safe_str_client(service->service_id));
-
         smartlist_t *potential_introduction_points = smartlist_new();
 
+        smartlist_t *intros_to_ignore = smartlist_new();
+        smartlist_add_all(intros_to_ignore, service->intro_nodes);
+        smartlist_add_all(intros_to_ignore, service->failed_intro_nodes);
+
         if (hid_serv_get_introduction_points(potential_introduction_points,
               n_intro_points_to_open,
-              intro_nodes,
+              intros_to_ignore, // ignore the current introduction points
               service->service_id) < 0) {
           log_warn(LD_REND, "Could not find any new introduction points");
           smartlist_free(potential_introduction_points);
+          smartlist_free(intros_to_ignore);
           return;
         }
 
+        smartlist_free(intros_to_ignore);
+
         const node_t *node;
         for (j = 0; j < smartlist_len(potential_introduction_points); j++) {
           log_info(LD_REND,
@@ -3334,7 +3367,7 @@ rend_services_introduce(void)
           node = smartlist_get(potential_introduction_points, j);
 
           intro_point_set_changed = 1;
-          smartlist_add(intro_nodes, (void*)node);
+          //smartlist_add(intro_nodes, (void*)node);
           intro = tor_malloc_zero(sizeof(rend_intro_point_t));
           intro->extend_info = extend_info_from_node(node, 0);
           intro->intro_key = crypto_pk_new();
@@ -3357,6 +3390,10 @@ rend_services_introduce(void)
     /* Establish new introduction points. */
     for (j=prev_intro_nodes; j < smartlist_len(service->intro_nodes); ++j) {
       intro = smartlist_get(service->intro_nodes, j);
+
+      // invalidate desc as this is a new introduction point
+      intro->update_desc_on_connection = 1;
+
       r = rend_service_launch_establish_intro(service, intro);
       if (r<0) {
         log_warn(LD_REND, "Error launching circuit to node %s for service %s.",
@@ -3365,7 +3402,7 @@ rend_services_introduce(void)
       }
     }
   }
-  smartlist_free(intro_nodes);
+  //smartlist_free(intro_nodes);
 }
 
 /** Regenerate and upload rendezvous service descriptors for all
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 6382248e4..7f8151b59 100644
--- a/src/or/routerlist.c
+++ b/src/or/routerlist.c
@@ -5055,26 +5055,39 @@ hid_serv_get_introduction_points(smartlist_t *introduction_points,
   int start, found, i;
 
   networkstatus_t *c = networkstatus_get_latest_consensus();
+
   if (!c || !smartlist_len(c->routerstatus_list)) {
     log_warn(LD_REND, "We don't have a consensus, so determine introduction "
              "points");
     return -1;
   }
+
   tor_assert(id);
+
   start = networkstatus_vote_find_entry_idx(c, id, &found);
-  if (start == smartlist_len(c->routerstatus_list)) start = 0;
+
+  if (start == smartlist_len(c->routerstatus_list))
+    start = 0;
+
   i = start;
+
   do {
     routerstatus_t *r = smartlist_get(c->routerstatus_list, i);
 
     node_t *node = node_get_by_id(r->identity_digest);
 
-    if (!excludedsmartlist || !smartlist_contains(excludedsmartlist, node)) {
+    log_info(LD_REND, "considering node %s",
+                      safe_str_client(node_describe(node)));
+
+    if ((!excludedsmartlist || !smartlist_contains(excludedsmartlist, node)) &&
+        node->is_stable &&
+        (node->ri || (node->rs && node->md))) {
+
       smartlist_add(introduction_points, node);
       if (--number <= 0)
         return 0;
     } else {
-      log_warn(LD_REND, "Ignoring node %s ",
+      log_info(LD_REND, "ignoring node %s ",
                         safe_str_client(node_describe(node)));
     }