Merge remote-tracking branch 'public/bug6174' into maint-0.2.4

changes/bug6174

@@ -0,0 +1,6 @@
+  o Major bugfixes:
+    - When we mark a circuit as unusable for new circuits, have it
+      continue to be unusable for new circuits even if MaxCircuitDirtiness
+      is increased too much at the wrong time, or the system clock jumped
+      backwards. Fix for bug 6174; bugfix on 0.0.2pre26.
+     

src/or/circuitbuild.c

@@ -901,7 +901,7 @@ circuit_note_clock_jumped(int seconds_elapsed)
   control_event_client_status(severity, "CIRCUIT_NOT_ESTABLISHED REASON=%s",
                               "CLOCK_JUMPED");
   circuit_mark_all_unused_circs();
-  circuit_expire_all_dirty_circs();
+  circuit_mark_all_dirty_circs_as_unusable();
 }
 
 /** Take the 'extend' <b>cell</b>, pull out addr/port plus the onion

src/or/circuitlist.c

@@ -1207,6 +1207,7 @@ circuit_find_to_cannibalize(uint8_t purpose, extend_info_t *info,
       if ((!need_uptime || circ->build_state->need_uptime) &&
           (!need_capacity || circ->build_state->need_capacity) &&
           (internal == circ->build_state->is_internal) &&
+          !circ->unusable_for_new_conns &&
           circ->remaining_relay_early_cells &&
           circ->build_state->desired_path_len == DEFAULT_ROUTE_LEN &&
           !circ->build_state->onehop_tunnel &&
@@ -1302,20 +1303,17 @@ circuit_mark_all_unused_circs(void)
  * This is useful for letting the user change pseudonyms, so new
  * streams will not be linkable to old streams.
  */
-/* XXX024 this is a bad name for what this function does */
 void
-circuit_expire_all_dirty_circs(void)
+circuit_mark_all_dirty_circs_as_unusable(void)
 {
   circuit_t *circ;
-  const or_options_t *options = get_options();
 
   for (circ=global_circuitlist; circ; circ = circ->next) {
     if (CIRCUIT_IS_ORIGIN(circ) &&
         !circ->marked_for_close &&
-        circ->timestamp_dirty)
-      /* XXXX024 This is a screwed-up way to say "This is too dirty
-       * for new circuits. */
-      circ->timestamp_dirty -= options->MaxCircuitDirtiness;
+        circ->timestamp_dirty) {
+      mark_circuit_unusable_for_new_conns(TO_ORIGIN_CIRCUIT(circ));
+    }
   }
 }
 

src/or/circuitlist.h

@@ -46,7 +46,7 @@ or_circuit_t *circuit_get_intro_point(const char *digest);
 origin_circuit_t *circuit_find_to_cannibalize(uint8_t purpose,
                                               extend_info_t *info, int flags);
 void circuit_mark_all_unused_circs(void);
-void circuit_expire_all_dirty_circs(void);
+void circuit_mark_all_dirty_circs_as_unusable(void);
 void circuit_mark_for_close_(circuit_t *circ, int reason,
                              int line, const char *file);
 int circuit_get_cpath_len(origin_circuit_t *circ);

src/or/circuituse.c

@@ -85,10 +85,14 @@ circuit_is_acceptable(const origin_circuit_t *origin_circ,
   }
 
   if (purpose == CIRCUIT_PURPOSE_C_GENERAL ||
-      purpose == CIRCUIT_PURPOSE_C_REND_JOINED)
+      purpose == CIRCUIT_PURPOSE_C_REND_JOINED) {
     if (circ->timestamp_dirty &&
        circ->timestamp_dirty+get_options()->MaxCircuitDirtiness <= now)
       return 0;
+  }
+
+  if (origin_circ->unusable_for_new_conns)
+    return 0;
 
   /* decide if this circ is suitable for this conn */
 
@@ -799,9 +803,12 @@ circuit_stream_is_being_handled(entry_connection_t *conn,
         circ->purpose == CIRCUIT_PURPOSE_C_GENERAL &&
         (!circ->timestamp_dirty ||
          circ->timestamp_dirty + get_options()->MaxCircuitDirtiness > now)) {
-      cpath_build_state_t *build_state = TO_ORIGIN_CIRCUIT(circ)->build_state;
+      origin_circuit_t *origin_circ = TO_ORIGIN_CIRCUIT(circ);
+      cpath_build_state_t *build_state = origin_circ->build_state;
       if (build_state->is_internal || build_state->onehop_tunnel)
         continue;
+      if (!origin_circ->unusable_for_new_conns)
+        continue;
 
       exitnode = build_state_get_exit_node(build_state);
       if (exitnode && (!need_uptime || build_state->need_uptime)) {
@@ -843,6 +850,7 @@ circuit_predict_and_launch_new(void)
   /* First, count how many of each type of circuit we have already. */
   for (circ=global_circuitlist;circ;circ = circ->next) {
     cpath_build_state_t *build_state;
+    origin_circuit_t *origin_circ;
     if (!CIRCUIT_IS_ORIGIN(circ))
       continue;
     if (circ->marked_for_close)
@@ -851,7 +859,10 @@ circuit_predict_and_launch_new(void)
       continue; /* only count clean circs */
     if (circ->purpose != CIRCUIT_PURPOSE_C_GENERAL)
       continue; /* only pay attention to general-purpose circs */
-    build_state = TO_ORIGIN_CIRCUIT(circ)->build_state;
+    origin_circ = TO_ORIGIN_CIRCUIT(circ);
+    if (origin_circ->unusable_for_new_conns)
+      continue;
+    build_state = origin_circ->build_state;
     if (build_state->onehop_tunnel)
       continue;
     num++;
@@ -2275,3 +2286,22 @@ circuit_change_purpose(circuit_t *circ, uint8_t new_purpose)
   }
 }
 
+/** Mark <b>circ</b> so that no more connections can be attached to it. */
+void
+mark_circuit_unusable_for_new_conns(origin_circuit_t *circ)
+{
+  const or_options_t *options = get_options();
+  tor_assert(circ);
+
+  /* XXXX025 This is a kludge; we're only keeping it around in case there's
+   * something that doesn't check unusable_for_new_conns, and to avoid
+   * deeper refactoring of our expiration logic. */
+  if (! circ->base_.timestamp_dirty)
+    circ->base_.timestamp_dirty = approx_time();
+  if (options->MaxCircuitDirtiness >= circ->base_.timestamp_dirty)
+    circ->base_.timestamp_dirty = 1; /* prevent underflow */
+  else
+    circ->base_.timestamp_dirty -= options->MaxCircuitDirtiness;
+
+  circ->unusable_for_new_conns = 1;
+}

src/or/circuituse.h

@@ -55,6 +55,7 @@ void circuit_change_purpose(circuit_t *circ, uint8_t new_purpose);
 
 int hostname_in_track_host_exits(const or_options_t *options,
                                  const char *address);
+void mark_circuit_unusable_for_new_conns(origin_circuit_t *circ);
 
 #endif
 

src/or/config.c

@@ -1503,7 +1503,7 @@ options_act(const or_options_t *old_options)
                "preferred or excluded node lists. "
                "Abandoning previous circuits.");
       circuit_mark_all_unused_circs();
-      circuit_expire_all_dirty_circs();
+      circuit_mark_all_dirty_circs_as_unusable();
       revise_trackexithosts = 1;
     }
 

src/or/connection_edge.c

@@ -674,12 +674,10 @@ connection_ap_expire_beginning(void)
     /* un-mark it as ending, since we're going to reuse it */
     conn->edge_has_sent_end = 0;
     conn->end_reason = 0;
-    /* kludge to make us not try this circuit again, yet to allow
-     * current streams on it to survive if they can: make it
-     * unattractive to use for new streams */
-    /* XXXX024 this is a kludgy way to do this. */
-    tor_assert(circ->timestamp_dirty);
-    circ->timestamp_dirty -= options->MaxCircuitDirtiness;
+    /* make us not try this circuit again, but allow
+     * current streams on it to survive if they can */
+    mark_circuit_unusable_for_new_conns(TO_ORIGIN_CIRCUIT(circ));
+
     /* give our stream another 'cutoff' seconds to try */
     conn->base_.timestamp_lastread += cutoff;
     if (entry_conn->num_socks_retries < 250) /* avoid overflow */
@@ -1806,9 +1804,7 @@ connection_ap_handshake_send_begin(entry_connection_t *ap_conn)
     connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL);
 
     /* Mark this circuit "unusable for new streams". */
-    /* XXXX024 this is a kludgy way to do this. */
-    tor_assert(circ->base_.timestamp_dirty);
-    circ->base_.timestamp_dirty -= get_options()->MaxCircuitDirtiness;
+    mark_circuit_unusable_for_new_conns(circ);
     return -1;
   }
 
@@ -1899,9 +1895,7 @@ connection_ap_handshake_send_resolve(entry_connection_t *ap_conn)
     connection_mark_unattached_ap(ap_conn, END_STREAM_REASON_INTERNAL);
 
     /* Mark this circuit "unusable for new streams". */
-    /* XXXX024 this is a kludgy way to do this. */
-    tor_assert(circ->base_.timestamp_dirty);
-    circ->base_.timestamp_dirty -= get_options()->MaxCircuitDirtiness;
+    mark_circuit_unusable_for_new_conns(circ);
     return -1;
   }
 

src/or/main.c

@@ -1125,7 +1125,7 @@ signewnym_impl(time_t now)
     return;
   }
 
-  circuit_expire_all_dirty_circs();
+  circuit_mark_all_dirty_circs_as_unusable();
   addressmap_clear_transient();
   rend_client_purge_state();
   time_of_last_signewnym = now;
@@ -1844,7 +1844,7 @@ do_hup(void)
   /* Rotate away from the old dirty circuits. This has to be done
    * after we've read the new options, but before we start using
    * circuits for directory fetches. */
-  circuit_expire_all_dirty_circs();
+  circuit_mark_all_dirty_circs_as_unusable();
 
   /* retry appropriate downloads */
   router_reset_status_download_failures();

src/or/or.h

@@ -2945,6 +2945,10 @@ typedef struct origin_circuit_t {
    */
   ENUM_BF(path_state_t) path_state : 3;
 
+  /* If this flag is set, we should not consider attaching any more
+   * connections to this circuit. */
+  unsigned int unusable_for_new_conns : 1;
+
   /**
    * Tristate variable to guard against pathbias miscounting
    * due to circuit purpose transitions changing the decision

src/or/relay.c

@@ -17,6 +17,7 @@
 #include "channel.h"
 #include "circuitbuild.h"
 #include "circuitlist.h"
+#include "circuituse.h"
 #include "config.h"
 #include "connection.h"
 #include "connection_edge.h"
@@ -851,9 +852,7 @@ connection_ap_process_end_not_open(
           /* We haven't retried too many times; reattach the connection. */
           circuit_log_path(LOG_INFO,LD_APP,circ);
           /* Mark this circuit "unusable for new streams". */
-          /* XXXX024 this is a kludgy way to do this. */
-          tor_assert(circ->base_.timestamp_dirty);
-          circ->base_.timestamp_dirty -= get_options()->MaxCircuitDirtiness;
+          mark_circuit_unusable_for_new_conns(circ);
 
           if (conn->chosen_exit_optional) {
             /* stop wanting a specific exit */