Merge branch 'ticket8081_squashed'

doc/tor.1.txt

@@ -1238,11 +1238,7 @@ The following options are useful only for clients (that is, if
 
 **PathBiasDropGuards** __NUM__ +
 
-**PathBiasScaleThreshold** __NUM__ +
+**PathBiasScaleThreshold** __NUM__::
-
-**PathBiasMultFactor** __NUM__ +
-
-**PathBiasScaleFactor** __NUM__::
     These options override the default behavior of Tor's (**currently
     experimental**) path bias detection algorithm. To try to find broken or
     misbehaving guard nodes, Tor looks for nodes where more than a certain
@@ -1256,14 +1252,13 @@ The following options are useful only for clients (that is, if
     is set to 1, we disable use of that guard. +
  +
     When we have seen more than PathBiasScaleThreshold
-    circuits through a guard, we scale our observations by
+    circuits through a guard, we scale our observations by 0.5 (governed by
-    PathBiasMultFactor/PathBiasScaleFactor, so that new observations don't get
+    the consensus) so that new observations don't get swamped by old ones. +
-    swamped by old ones. +
  +
     By default, or if a negative value is provided for one of these options,
     Tor uses reasonable defaults from the networkstatus consensus document.
     If no defaults are available there, these options default to 150, .70,
-    .50, .30, 0, 300, 1, and 2 respectively.
+    .50, .30, 0, and 300 respectively.
 
 **PathBiasUseThreshold** __NUM__ +
 

src/or/circuitbuild.c

@@ -68,8 +68,9 @@ static void pathbias_count_build_success(origin_circuit_t *circ);
 static void pathbias_count_successful_close(origin_circuit_t *circ);
 static void pathbias_count_collapse(origin_circuit_t *circ);
 static void pathbias_count_use_failed(origin_circuit_t *circ);
-static int pathbias_check_use_rate(entry_guard_t *guard);
+static void pathbias_measure_use_rate(entry_guard_t *guard);
-static int pathbias_check_close_rate(entry_guard_t *guard);
+static void pathbias_measure_close_rate(entry_guard_t *guard);
+static void pathbias_scale_use_rates(entry_guard_t *guard);
 
 /** This function tries to get a channel to the specified endpoint,
  * and then calls command_setup_channel() to give it the right
@@ -1175,38 +1176,32 @@ pathbias_get_scale_threshold(const or_options_t *options)
 }
 
 /**
- * The scale factor is the denominator for our scaling
+ * Compute the path bias scaling ratio from the consensus
- * of circuit counts for our path bias window.
+ * parameters pb_multfactor/pb_scalefactor.
  *
- * Note that our use of doubles for the path bias state
+ * Returns a value in (0, 1.0] which we multiply our pathbias
- * file means that powers of 2 work best here.
+ * counts with to scale them down.
  */
-static int
+static double
-pathbias_get_scale_factor(const or_options_t *options)
+pathbias_get_scale_ratio(const or_options_t *options)
 {
-#define DFLT_PATH_BIAS_SCALE_FACTOR 2
+  /*
-  if (options->PathBiasScaleFactor >= 1)
+   * The scale factor is the denominator for our scaling
-    return options->PathBiasScaleFactor;
+   * of circuit counts for our path bias window.
-  else
+   *
-    return networkstatus_get_param(NULL, "pb_scalefactor",
+   * Note that our use of doubles for the path bias state
-                                DFLT_PATH_BIAS_SCALE_FACTOR, 1, INT32_MAX);
+   * file means that powers of 2 work best here.
-}
+   */
-
+  int denominator = networkstatus_get_param(NULL, "pb_scalefactor",
-/**
+                              2, 2, INT32_MAX);
- * The mult factor is the numerator for our scaling
+  (void) options;
- * of circuit counts for our path bias window. It
+  /**
- * allows us to scale by fractions.
+   * The mult factor is the numerator for our scaling
- */
+   * of circuit counts for our path bias window. It
-static int
+   * allows us to scale by fractions.
-pathbias_get_mult_factor(const or_options_t *options)
+   */
-{
+  return networkstatus_get_param(NULL, "pb_multfactor",
-#define DFLT_PATH_BIAS_MULT_FACTOR 1
+                              1, 1, denominator)/((double)denominator);
-  if (options->PathBiasMultFactor >= 1)
-    return options->PathBiasMultFactor;
-  else
-    return networkstatus_get_param(NULL, "pb_multfactor",
-                                DFLT_PATH_BIAS_MULT_FACTOR, 1,
-                                pathbias_get_scale_factor(options));
 }
 
 /** The minimum number of circuit usage attempts before we start
@@ -1352,6 +1347,24 @@ pathbias_should_count(origin_circuit_t *circ)
           circ->base_.purpose == CIRCUIT_PURPOSE_S_REND_JOINED ||
           (circ->base_.purpose >= CIRCUIT_PURPOSE_C_INTRODUCING &&
            circ->base_.purpose <= CIRCUIT_PURPOSE_C_INTRODUCE_ACKED)) {
+
+    /* Check to see if the shouldcount result has changed due to a
+     * unexpected purpose change that would affect our results.
+     *
+     * The reason we check the path state too here is because for the
+     * cannibalized versions of these purposes, we count them as successful
+     * before their purpose change.
+     */
+    if (circ->pathbias_shouldcount == PATHBIAS_SHOULDCOUNT_COUNTED
+            && circ->path_state != PATH_STATE_ALREADY_COUNTED) {
+      log_info(LD_BUG,
+               "Circuit %d is now being ignored despite being counted "
+               "in the past. Purpose is %s, path state is %s",
+               circ->global_identifier,
+               circuit_purpose_to_string(circ->base_.purpose),
+               pathbias_state_to_string(circ->path_state));
+    }
+    circ->pathbias_shouldcount = PATHBIAS_SHOULDCOUNT_IGNORED;
     return 0;
   }
 
@@ -1374,9 +1387,33 @@ pathbias_should_count(origin_circuit_t *circ)
       }
       tor_fragile_assert();
     }
+
+    /* Check to see if the shouldcount result has changed due to a
+     * unexpected change that would affect our results */
+    if (circ->pathbias_shouldcount == PATHBIAS_SHOULDCOUNT_COUNTED) {
+      log_info(LD_BUG,
+               "One-hop circuit %d is now being ignored despite being counted "
+               "in the past. Purpose is %s, path state is %s",
+               circ->global_identifier,
+               circuit_purpose_to_string(circ->base_.purpose),
+               pathbias_state_to_string(circ->path_state));
+    }
+    circ->pathbias_shouldcount = PATHBIAS_SHOULDCOUNT_IGNORED;
     return 0;
   }
 
+  /* Check to see if the shouldcount result has changed due to a
+   * unexpected purpose change that would affect our results */
+  if (circ->pathbias_shouldcount == PATHBIAS_SHOULDCOUNT_IGNORED) {
+      log_info(LD_BUG,
+              "Circuit %d is now being counted despite being ignored "
+              "in the past. Purpose is %s, path state is %s",
+              circ->global_identifier,
+              circuit_purpose_to_string(circ->base_.purpose),
+              pathbias_state_to_string(circ->path_state));
+  }
+  circ->pathbias_shouldcount = PATHBIAS_SHOULDCOUNT_COUNTED;
+
   return 1;
 }
 
@@ -1497,6 +1534,7 @@ pathbias_count_build_success(origin_circuit_t *circ)
       if (circ->path_state == PATH_STATE_BUILD_ATTEMPTED) {
         circ->path_state = PATH_STATE_BUILD_SUCCEEDED;
         guard->circ_successes++;
+        entry_guards_changed();
 
         log_info(LD_CIRC, "Got success count %f/%f for guard %s=%s",
                  guard->circ_successes, guard->circ_attempts,
@@ -1579,8 +1617,10 @@ pathbias_count_use_attempt(origin_circuit_t *circ)
     guard = entry_guard_get_by_id_digest(
                 circ->cpath->extend_info->identity_digest);
     if (guard) {
-      pathbias_check_use_rate(guard);
+      pathbias_measure_use_rate(guard);
+      pathbias_scale_use_rates(guard);
       guard->use_attempts++;
+      entry_guards_changed();
 
       log_debug(LD_CIRC,
                "Marked circuit %d (%f/%f) as used for guard %s=%s.",
@@ -1605,13 +1645,13 @@ pathbias_count_use_attempt(origin_circuit_t *circ)
 }
 
 /**
- * Check the circuit's path stat is appropriate and it as successfully
+ * Check the circuit's path state is appropriate and mark it as
- * used.
+ * successfully used. Used for path bias usage accounting.
  *
  * We don't actually increment the guard's counters until
- * pathbias_check_close().
+ * pathbias_check_close(), because the circuit can still transition
- *
+ * back to PATH_STATE_USE_ATTEMPTED if a stream fails later (this
- * Used for path bias usage accounting.
+ * is done so we can probe the circuit for liveness at close).
  */
 void
 pathbias_mark_use_success(origin_circuit_t *circ)
@@ -1638,6 +1678,31 @@ pathbias_mark_use_success(origin_circuit_t *circ)
   return;
 }
 
+/**
+ * If a stream ever detatches from a circuit in a retriable way,
+ * we need to mark this circuit as still needing either another
+ * successful stream, or in need of a probe.
+ *
+ * An adversary could let the first stream request succeed (ie the
+ * resolve), but then tag and timeout the remainder (via cell
+ * dropping), forcing them on new circuits.
+ *
+ * Rolling back the state will cause us to probe such circuits, which
+ * should lead to probe failures in the event of such tagging due to
+ * either unrecognized cells coming in while we wait for the probe,
+ * or the cipher state getting out of sync in the case of dropped cells.
+ */
+void
+pathbias_mark_use_rollback(origin_circuit_t *circ)
+{
+  if (circ->path_state == PATH_STATE_USE_SUCCEEDED) {
+    log_info(LD_CIRC,
+             "Rolling back pathbias use state to 'attempted' for detached "
+             "circuit %d", circ->global_identifier);
+    circ->path_state = PATH_STATE_USE_ATTEMPTED;
+  }
+}
+
 /**
  * Actually count a circuit success towards a guard's usage counters
  * if the path state is appropriate.
@@ -1664,6 +1729,7 @@ pathbias_count_use_success(origin_circuit_t *circ)
                 circ->cpath->extend_info->identity_digest);
     if (guard) {
       guard->use_successes++;
+      entry_guards_changed();
 
       log_debug(LD_CIRC,
                 "Marked circuit %d (%f/%f) as used successfully for guard "
@@ -1937,6 +2003,10 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason)
       pathbias_count_use_success(ocirc);
       break;
 
+    case PATH_STATE_USE_FAILED:
+      pathbias_count_use_failed(ocirc);
+      break;
+
     default:
       // Other states are uninteresting. No stats to count.
       break;
@@ -2164,11 +2234,9 @@ pathbias_get_use_success_count(entry_guard_t *guard)
  *
  * If pathbias_get_dropguards() is set, we also disable the use of
  * very failure prone guards.
- *
- * Returns -1 if we decided to disable the guard, 0 otherwise.
  */
-static int
+static void
-pathbias_check_use_rate(entry_guard_t *guard)
+pathbias_measure_use_rate(entry_guard_t *guard)
 {
   const or_options_t *options = get_options();
 
@@ -2202,7 +2270,8 @@ pathbias_check_use_rate(entry_guard_t *guard)
                  tor_lround(circ_times.close_ms/1000));
           guard->path_bias_disabled = 1;
           guard->bad_since = approx_time();
-          return -1;
+          entry_guards_changed();
+          return;
         }
       } else if (!guard->path_bias_extreme) {
         guard->path_bias_extreme = 1;
@@ -2252,30 +2321,6 @@ pathbias_check_use_rate(entry_guard_t *guard)
       }
     }
   }
-
-  /* If we get a ton of circuits, just scale everything down */
-  if (guard->use_attempts > pathbias_get_scale_use_threshold(options)) {
-    const int scale_factor = pathbias_get_scale_factor(options);
-    const int mult_factor = pathbias_get_mult_factor(options);
-    int opened_attempts = pathbias_count_circs_in_states(guard,
-            PATH_STATE_USE_ATTEMPTED, PATH_STATE_USE_SUCCEEDED);
-    guard->use_attempts -= opened_attempts;
-
-    guard->use_attempts *= mult_factor;
-    guard->use_successes *= mult_factor;
-
-    guard->use_attempts /= scale_factor;
-    guard->use_successes /= scale_factor;
-
-    guard->use_attempts += opened_attempts;
-
-    log_info(LD_CIRC,
-             "Scaled pathbias use counts to %f/%f (%d open) for guard %s=%s",
-             guard->use_successes, guard->use_attempts, opened_attempts,
-             guard->nickname, hex_str(guard->identity, DIGEST_LEN));
-  }
-
-  return 0;
 }
 
 /**
@@ -2287,10 +2332,14 @@ pathbias_check_use_rate(entry_guard_t *guard)
  * If pathbias_get_dropguards() is set, we also disable the use of
  * very failure prone guards.
  *
- * Returns -1 if we decided to disable the guard, 0 otherwise.
+ * XXX: This function shares similar log messages and checks to
+ * pathbias_measure_use_rate(). It may be possible to combine them
+ * eventually, especially if we can ever remove the need for 3
+ * levels of closure warns (if the overall circuit failure rate
+ * goes down with ntor).
  */
-static int
+static void
-pathbias_check_close_rate(entry_guard_t *guard)
+pathbias_measure_close_rate(entry_guard_t *guard)
 {
   const or_options_t *options = get_options();
 
@@ -2324,7 +2373,8 @@ pathbias_check_close_rate(entry_guard_t *guard)
                  tor_lround(circ_times.close_ms/1000));
           guard->path_bias_disabled = 1;
           guard->bad_since = approx_time();
-          return -1;
+          entry_guards_changed();
+          return;
         }
       } else if (!guard->path_bias_extreme) {
         guard->path_bias_extreme = 1;
@@ -2357,7 +2407,7 @@ pathbias_check_close_rate(entry_guard_t *guard)
                  "amount of circuits. "
                  "Most likely this means the Tor network is "
                  "overloaded, but it could also mean an attack against "
-                 "you or the potentially the guard itself. "
+                 "you or potentially the guard itself. "
                  "Success counts are %ld/%ld. Use counts are %ld/%ld. "
                  "%ld circuits completed, %ld were unusable, %ld collapsed, "
                  "and %ld timed out. "
@@ -2398,11 +2448,25 @@ pathbias_check_close_rate(entry_guard_t *guard)
       }
     }
   }
+}
+
+/**
+ * This function scales the path bias use rates if we have
+ * more data than the scaling threshold. This allows us to
+ * be more sensitive to recent measurements.
+ *
+ * XXX: The attempt count transfer stuff here might be done
+ * better by keeping separate pending counters that get
+ * transfered at circuit close.
+ */
+static void
+pathbias_scale_close_rates(entry_guard_t *guard)
+{
+  const or_options_t *options = get_options();
 
   /* If we get a ton of circuits, just scale everything down */
   if (guard->circ_attempts > pathbias_get_scale_threshold(options)) {
-    const int scale_factor = pathbias_get_scale_factor(options);
+    double scale_ratio = pathbias_get_scale_ratio(options);
-    const int mult_factor = pathbias_get_mult_factor(options);
     int opened_attempts = pathbias_count_circs_in_states(guard,
             PATH_STATE_BUILD_ATTEMPTED, PATH_STATE_BUILD_ATTEMPTED);
     int opened_built = pathbias_count_circs_in_states(guard,
@@ -2411,23 +2475,18 @@ pathbias_check_close_rate(entry_guard_t *guard)
     guard->circ_attempts -= opened_attempts;
     guard->circ_successes -= opened_built;
 
-    guard->circ_attempts *= mult_factor;
+    guard->circ_attempts *= scale_ratio;
-    guard->circ_successes *= mult_factor;
+    guard->circ_successes *= scale_ratio;
-    guard->timeouts *= mult_factor;
+    guard->timeouts *= scale_ratio;
-    guard->successful_circuits_closed *= mult_factor;
+    guard->successful_circuits_closed *= scale_ratio;
-    guard->collapsed_circuits *= mult_factor;
+    guard->collapsed_circuits *= scale_ratio;
-    guard->unusable_circuits *= mult_factor;
+    guard->unusable_circuits *= scale_ratio;
-
-    guard->circ_attempts /= scale_factor;
-    guard->circ_successes /= scale_factor;
-    guard->timeouts /= scale_factor;
-    guard->successful_circuits_closed /= scale_factor;
-    guard->collapsed_circuits /= scale_factor;
-    guard->unusable_circuits /= scale_factor;
 
     guard->circ_attempts += opened_attempts;
     guard->circ_successes += opened_built;
 
+    entry_guards_changed();
+
     log_info(LD_CIRC,
              "Scaled pathbias counts to (%f,%f)/%f (%d/%d open) for guard "
              "%s=%s",
@@ -2435,8 +2494,40 @@ pathbias_check_close_rate(entry_guard_t *guard)
              guard->circ_attempts, opened_built, opened_attempts,
              guard->nickname, hex_str(guard->identity, DIGEST_LEN));
   }
+}
 
-  return 0;
+/**
+ * This function scales the path bias circuit close rates if we have
+ * more data than the scaling threshold. This allows us to be more
+ * sensitive to recent measurements.
+ *
+ * XXX: The attempt count transfer stuff here might be done
+ * better by keeping separate pending counters that get
+ * transfered at circuit close.
+ */
+void
+pathbias_scale_use_rates(entry_guard_t *guard)
+{
+  const or_options_t *options = get_options();
+
+  /* If we get a ton of circuits, just scale everything down */
+  if (guard->use_attempts > pathbias_get_scale_use_threshold(options)) {
+    double scale_ratio = pathbias_get_scale_ratio(options);
+    int opened_attempts = pathbias_count_circs_in_states(guard,
+            PATH_STATE_USE_ATTEMPTED, PATH_STATE_USE_SUCCEEDED);
+    guard->use_attempts -= opened_attempts;
+
+    guard->use_attempts *= scale_ratio;
+    guard->use_successes *= scale_ratio;
+
+    guard->use_attempts += opened_attempts;
+
+    log_info(LD_CIRC,
+             "Scaled pathbias use counts to %f/%f (%d open) for guard %s=%s",
+             guard->use_successes, guard->use_attempts, opened_attempts,
+             guard->nickname, hex_str(guard->identity, DIGEST_LEN));
+    entry_guards_changed();
+  }
 }
 
 /** Increment the number of times we successfully extended a circuit to
@@ -2448,9 +2539,12 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard)
 {
   entry_guards_changed();
 
-  if (pathbias_check_close_rate(guard) < 0)
+  pathbias_measure_close_rate(guard);
+
+  if (guard->path_bias_disabled)
     return -1;
 
+  pathbias_scale_close_rates(guard);
   guard->circ_attempts++;
 
   log_info(LD_CIRC, "Got success count %f/%f for guard %s=%s",

src/or/circuitbuild.h

@@ -65,6 +65,7 @@ int pathbias_check_close(origin_circuit_t *circ, int reason);
 int pathbias_check_probe_response(circuit_t *circ, const cell_t *cell);
 void pathbias_count_use_attempt(origin_circuit_t *circ);
 void pathbias_mark_use_success(origin_circuit_t *circ);
+void pathbias_mark_use_rollback(origin_circuit_t *circ);
 
 #endif
 

src/or/circuitstats.c

@@ -183,16 +183,6 @@ circuit_build_times_quantile_cutoff(void)
   return num/100.0;
 }
 
-/* DOCDOC circuit_build_times_get_bw_scale */
-int
-circuit_build_times_get_bw_scale(networkstatus_t *ns)
-{
-  return networkstatus_get_param(ns, "bwweightscale",
-                                 BW_WEIGHT_SCALE,
-                                 BW_MIN_WEIGHT_SCALE,
-                                 BW_MAX_WEIGHT_SCALE);
-}
-
 /**
  * Retrieve and bounds-check the cbtclosequantile consensus paramter.
  *

src/or/circuituse.c

@@ -1498,15 +1498,17 @@ circuit_launch_by_extend_info(uint8_t purpose,
            purpose == CIRCUIT_PURPOSE_C_INTRODUCING) &&
           circ->path_state == PATH_STATE_BUILD_SUCCEEDED) {
         /* Path bias: Cannibalized rends pre-emptively count as a
-         * successfully used circ. We don't wait until the extend,
+         * successfully built but unused closed circuit. We don't
-         * because the rend point could be malicious.
+         * wait until the extend (or the close) because the rend
+         * point could be malicious.
          *
          * Same deal goes for client side introductions. Clients
          * can be manipulated to connect repeatedly to them
          * (especially web clients).
          *
          * If we decide to probe the initial portion of these circs,
-         * (up to the adversaries final hop), we need to remove this.
+         * (up to the adversary's final hop), we need to remove this,
+         * or somehow mark the circuit with a special path state.
          */
 
         /* This must be called before the purpose change */

src/or/config.c

@@ -321,8 +321,8 @@ static config_var_t option_vars_[] = {
   V(PathBiasWarnRate,            DOUBLE,   "-1"),
   V(PathBiasExtremeRate,         DOUBLE,   "-1"),
   V(PathBiasScaleThreshold,      INT,      "-1"),
-  V(PathBiasScaleFactor,         INT,      "-1"),
+  OBSOLETE("PathBiasScaleFactor"),
-  V(PathBiasMultFactor,          INT,      "-1"),
+  OBSOLETE("PathBiasMultFactor"),
   V(PathBiasDropGuards,          AUTOBOOL, "0"),
   OBSOLETE("PathBiasUseCloseCounts"),
 
@@ -2649,6 +2649,37 @@ options_validate(or_options_t *old_options, or_options_t *options,
         RECOMMENDED_MIN_CIRCUIT_BUILD_TIMEOUT );
   }
 
+  if (options->PathBiasNoticeRate > 1.0) {
+    tor_asprintf(msg,
+              "PathBiasNoticeRate is too high. "
+              "It must be between 0 and 1.0");
+    return -1;
+  }
+  if (options->PathBiasWarnRate > 1.0) {
+    tor_asprintf(msg,
+              "PathBiasWarnRate is too high. "
+              "It must be between 0 and 1.0");
+    return -1;
+  }
+  if (options->PathBiasExtremeRate > 1.0) {
+    tor_asprintf(msg,
+              "PathBiasExtremeRate is too high. "
+              "It must be between 0 and 1.0");
+    return -1;
+  }
+  if (options->PathBiasNoticeUseRate > 1.0) {
+    tor_asprintf(msg,
+              "PathBiasNoticeUseRate is too high. "
+              "It must be between 0 and 1.0");
+    return -1;
+  }
+  if (options->PathBiasExtremeUseRate > 1.0) {
+    tor_asprintf(msg,
+              "PathBiasExtremeUseRate is too high. "
+              "It must be between 0 and 1.0");
+    return -1;
+  }
+
   if (options->MaxCircuitDirtiness < MIN_MAX_CIRCUIT_DIRTINESS) {
     log_warn(LD_CONFIG, "MaxCircuitDirtiness option is too short; "
              "raising to %d seconds.", MIN_MAX_CIRCUIT_DIRTINESS);

src/or/connection_edge.c

@@ -637,21 +637,15 @@ connection_ap_expire_beginning(void)
     }
     if (circ->purpose == CIRCUIT_PURPOSE_C_REND_JOINED) {
       if (seconds_idle >= options->SocksTimeout) {
-        /* Path bias: We need to probe the circuit to ensure validity.
-         * Roll its state back if it succeeded so that we do so upon close. */
-        if (TO_ORIGIN_CIRCUIT(circ)->path_state == PATH_STATE_USE_SUCCEEDED) {
-          log_info(LD_CIRC,
-                   "Rolling back pathbias use state to 'attempted' for timed "
-                   "out rend circ %d",
-                   TO_ORIGIN_CIRCUIT(circ)->global_identifier);
-          TO_ORIGIN_CIRCUIT(circ)->path_state = PATH_STATE_USE_ATTEMPTED;
-        }
-
         log_fn(severity, LD_REND,
                "Rend stream is %d seconds late. Giving up on address"
                " '%s.onion'.",
                seconds_idle,
                safe_str_client(entry_conn->socks_request->address));
+        /* Roll back path bias use state so that we probe the circuit
+         * if nothing else succeeds on it */
+        pathbias_mark_use_rollback(TO_ORIGIN_CIRCUIT(circ));
+
         connection_edge_end(conn, END_STREAM_REASON_TIMEOUT);
         connection_mark_unattached_ap(entry_conn, END_STREAM_REASON_TIMEOUT);
       }
@@ -816,14 +810,9 @@ connection_ap_detach_retriable(entry_connection_t *conn,
   control_event_stream_status(conn, STREAM_EVENT_FAILED_RETRIABLE, reason);
   ENTRY_TO_CONN(conn)->timestamp_lastread = time(NULL);
 
-  /* Path bias: We need to probe the circuit to ensure validity.
+  /* Roll back path bias use state so that we probe the circuit
-   * Roll its state back if it succeeded so that we do so upon close. */
+   * if nothing else succeeds on it */
-  if (circ->path_state == PATH_STATE_USE_SUCCEEDED) {
+  pathbias_mark_use_rollback(circ);
-    log_info(LD_CIRC,
-             "Rolling back pathbias use state to 'attempted' for detached "
-             "circuit %d", circ->global_identifier);
-    circ->path_state = PATH_STATE_USE_ATTEMPTED;
-  }
 
   if (conn->pending_optimistic_data) {
     generic_buffer_set_to_copy(&conn->sending_optimistic_data,

src/or/networkstatus.c

@@ -2239,6 +2239,21 @@ networkstatus_get_param(const networkstatus_t *ns, const char *param_name,
                                  default_val, min_val, max_val);
 }
 
+/**
+ * Retrieve the consensus parameter that governs the
+ * fixed-point precision of our network balancing 'bandwidth-weights'
+ * (which are themselves integer consensus values). We divide them
+ * by this value and ensure they never exceed this value.
+ */
+int
+networkstatus_get_weight_scale_param(networkstatus_t *ns)
+{
+  return networkstatus_get_param(ns, "bwweightscale",
+                                 BW_WEIGHT_SCALE,
+                                 BW_MIN_WEIGHT_SCALE,
+                                 BW_MAX_WEIGHT_SCALE);
+}
+
 /** Return the value of a integer bw weight parameter from the networkstatus
  * <b>ns</b> whose name is <b>weight_name</b>.  If <b>ns</b> is NULL, try
  * loading the latest consensus ourselves. Return <b>default_val</b> if no
@@ -2255,7 +2270,7 @@ networkstatus_get_bw_weight(networkstatus_t *ns, const char *weight_name,
   if (!ns || !ns->weight_params)
     return default_val;
 
-  max = circuit_build_times_get_bw_scale(ns);
+  max = networkstatus_get_weight_scale_param(ns);
   param = get_net_param_from_list(ns->weight_params, weight_name,
                                   default_val, -1,
                                   BW_MAX_WEIGHT_SCALE);

src/or/networkstatus.h

@@ -112,6 +112,7 @@ int networkstatus_parse_flavor_name(const char *flavname);
 void document_signature_free(document_signature_t *sig);
 document_signature_t *document_signature_dup(const document_signature_t *sig);
 void networkstatus_free_all(void);
+int networkstatus_get_weight_scale_param(networkstatus_t *ns);
 
 #endif
 

src/or/or.h

@@ -2827,8 +2827,18 @@ typedef struct circuit_t {
 
 /**
  * Describes the circuit building process in simplified terms based
- * on the path bias accounting state for a circuit. Created to prevent
+ * on the path bias accounting state for a circuit.
- * overcounting due to unknown cases of circuit reuse. See Bug #6475.
+ *
+ * NOTE: These state values are enumerated in the order for which we
+ * expect circuits to transition through them. If you add states,
+ * you need to preserve this overall ordering. The various pathbias
+ * state transition and accounting functions (pathbias_mark_* and
+ * pathbias_count_*) contain ordinal comparisons to enforce proper
+ * state transitions for corrections.
+ *
+ * This state machine and the associated logic was created to prevent
+ * miscounting due to unknown cases of circuit reuse. See also tickets
+ * #6475 and #7802.
  */
 typedef enum {
     /** This circuit is "new". It has not yet completed a first hop
@@ -2851,10 +2861,9 @@ typedef enum {
     /** Did any SOCKS streams or hidserv introductions actually succeed on
       * this circuit?
       *
-      * Note: If we ever implement end-to-end stream timing through test
+      * If any streams detatch/fail from this circuit, the code transitions
-      * stream probes (#5707), we must *not* set this for those probes
+      * the circuit back to PATH_STATE_USE_ATTEMPTED to ensure we probe. See
-      * (or any other automatic streams) because the adversary could
+      * pathbias_mark_use_rollback() for that.
-      * just tag at a later point.
       */
     PATH_STATE_USE_SUCCEEDED = 4,
 
@@ -2905,10 +2914,25 @@ typedef struct origin_circuit_t {
    * cannibalized circuits. */
   unsigned int has_opened : 1;
 
-  /** Kludge to help us prevent the warn in bug #6475 and eventually
+  /**
-   * debug why we are not seeing first hops in some cases. */
+   * Path bias state machine. Used to ensure integrity of our
+   * circuit building and usage accounting. See path_state_t
+   * for more details.
+   */
   ENUM_BF(path_state_t) path_state : 3;
 
+  /**
+   * Tristate variable to guard against pathbias miscounting
+   * due to circuit purpose transitions changing the decision
+   * of pathbias_should_count(). This variable is informational
+   * only. The current results of pathbias_should_count() are
+   * the official decision for pathbias accounting.
+   */
+  uint8_t pathbias_shouldcount;
+#define PATHBIAS_SHOULDCOUNT_UNDECIDED 0
+#define PATHBIAS_SHOULDCOUNT_IGNORED   1
+#define PATHBIAS_SHOULDCOUNT_COUNTED   2
+
   /** For path probing. Store the temporary probe stream ID
    * for response comparison */
   streamid_t pathbias_probe_id;
@@ -3926,8 +3950,6 @@ typedef struct {
   double PathBiasExtremeRate;
   int PathBiasDropGuards;
   int PathBiasScaleThreshold;
-  int PathBiasScaleFactor;
-  int PathBiasMultFactor;
   /** @} */
 
   /**

src/or/routerlist.c

@@ -1734,7 +1734,7 @@ compute_weighted_bandwidths(const smartlist_t *sl,
     return -1;
   }
 
-  weight_scale = circuit_build_times_get_bw_scale(NULL);
+  weight_scale = networkstatus_get_weight_scale_param(NULL);
 
   if (rule == WEIGHT_FOR_GUARD) {
     Wg = networkstatus_get_bw_weight(NULL, "Wgg", -1);

src/or/routerparse.c

@@ -2255,7 +2255,7 @@ networkstatus_verify_bw_weights(networkstatus_t *ns)
   const char *casename = NULL;
   int valid = 1;
 
-  weight_scale = circuit_build_times_get_bw_scale(ns);
+  weight_scale = networkstatus_get_weight_scale_param(ns);
   Wgg = networkstatus_get_bw_weight(ns, "Wgg", -1);
   Wgm = networkstatus_get_bw_weight(ns, "Wgm", -1);
   Wgd = networkstatus_get_bw_weight(ns, "Wgd", -1);

src/or/statefile.c

@@ -52,6 +52,7 @@ static config_var_t state_vars_[] = {
   VAR("EntryGuardUnlistedSince", LINELIST_S,  EntryGuards,             NULL),
   VAR("EntryGuardAddedBy",       LINELIST_S,  EntryGuards,             NULL),
   VAR("EntryGuardPathBias",      LINELIST_S,  EntryGuards,             NULL),
+  VAR("EntryGuardPathUseBias",   LINELIST_S,  EntryGuards,             NULL),
   V(EntryGuards,                 LINELIST_V,  NULL),
 
   VAR("TransportProxy",               LINELIST_S, TransportProxies, NULL),