summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNick Mathewson <nickm@torproject.org>2013-02-01 17:03:12 -0500
committerNick Mathewson <nickm@torproject.org>2013-02-01 17:03:12 -0500
commit4074ff440e20c6549d8589a4f214791d610e3deb (patch)
tree55bdbf66da9024c617b72da9c478891274c671b8
parentec90ed4f6d1155d28fde91097e76f574195583c6 (diff)
parent996db755c237227a5df279b94acfa12aad9cce55 (diff)
downloadtor-4074ff440e20c6549d8589a4f214791d610e3deb.tar.gz
tor-4074ff440e20c6549d8589a4f214791d610e3deb.zip
Merge branch 'ticket8081_squashed'
-rw-r--r--doc/tor.1.txt13
-rw-r--r--src/or/circuitbuild.c270
-rw-r--r--src/or/circuitbuild.h1
-rw-r--r--src/or/circuitstats.c10
-rw-r--r--src/or/circuituse.c8
-rw-r--r--src/or/config.c35
-rw-r--r--src/or/connection_edge.c25
-rw-r--r--src/or/networkstatus.c17
-rw-r--r--src/or/networkstatus.h1
-rw-r--r--src/or/or.h42
-rw-r--r--src/or/routerlist.c2
-rw-r--r--src/or/routerparse.c2
-rw-r--r--src/or/statefile.c1
13 files changed, 284 insertions, 143 deletions
diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index 5d4b51c185..bdc78eac64 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -1238,11 +1238,7 @@ The following options are useful only for clients (that is, if
**PathBiasDropGuards** __NUM__ +
-**PathBiasScaleThreshold** __NUM__ +
-
-**PathBiasMultFactor** __NUM__ +
-
-**PathBiasScaleFactor** __NUM__::
+**PathBiasScaleThreshold** __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
- PathBiasMultFactor/PathBiasScaleFactor, so that new observations don't get
- swamped by old ones. +
+ circuits through a guard, we scale our observations by 0.5 (governed by
+ the consensus) so that new observations don't get 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__ +
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index e3a9d59c0e..5a5a3afea7 100644
--- a/src/or/circuitbuild.c
+++ b/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 int pathbias_check_close_rate(entry_guard_t *guard);
+static void pathbias_measure_use_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
- * of circuit counts for our path bias window.
+ * Compute the path bias scaling ratio from the consensus
+ * parameters pb_multfactor/pb_scalefactor.
*
- * Note that our use of doubles for the path bias state
- * file means that powers of 2 work best here.
+ * Returns a value in (0, 1.0] which we multiply our pathbias
+ * counts with to scale them down.
*/
-static int
-pathbias_get_scale_factor(const or_options_t *options)
-{
-#define DFLT_PATH_BIAS_SCALE_FACTOR 2
- if (options->PathBiasScaleFactor >= 1)
- return options->PathBiasScaleFactor;
- else
- return networkstatus_get_param(NULL, "pb_scalefactor",
- DFLT_PATH_BIAS_SCALE_FACTOR, 1, INT32_MAX);
-}
-
-/**
- * The mult factor is the numerator for our scaling
- * of circuit counts for our path bias window. It
- * allows us to scale by fractions.
- */
-static int
-pathbias_get_mult_factor(const or_options_t *options)
+static double
+pathbias_get_scale_ratio(const or_options_t *options)
{
-#define DFLT_PATH_BIAS_MULT_FACTOR 1
- 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 scale factor is the denominator for our scaling
+ * of circuit counts for our path bias window.
+ *
+ * Note that our use of doubles for the path bias state
+ * file means that powers of 2 work best here.
+ */
+ int denominator = networkstatus_get_param(NULL, "pb_scalefactor",
+ 2, 2, INT32_MAX);
+ (void) options;
+ /**
+ * The mult factor is the numerator for our scaling
+ * of circuit counts for our path bias window. It
+ * allows us to scale by fractions.
+ */
+ return networkstatus_get_param(NULL, "pb_multfactor",
+ 1, 1, denominator)/((double)denominator);
}
/** 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
- * used.
+ * Check the circuit's path state is appropriate and mark it as
+ * successfully used. Used for path bias usage accounting.
*
* We don't actually increment the guard's counters until
- * pathbias_check_close().
- *
- * Used for path bias usage accounting.
+ * pathbias_check_close(), because the circuit can still transition
+ * back to PATH_STATE_USE_ATTEMPTED if a stream fails later (this
+ * is done so we can probe the circuit for liveness at close).
*/
void
pathbias_mark_use_success(origin_circuit_t *circ)
@@ -1639,6 +1679,31 @@ pathbias_mark_use_success(origin_circuit_t *circ)
}
/**
+ * 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
-pathbias_check_use_rate(entry_guard_t *guard)
+static void
+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
-pathbias_check_close_rate(entry_guard_t *guard)
+static void
+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);
- const int mult_factor = pathbias_get_mult_factor(options);
+ double scale_ratio = pathbias_get_scale_ratio(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_successes *= mult_factor;
- guard->timeouts *= mult_factor;
- guard->successful_circuits_closed *= mult_factor;
- guard->collapsed_circuits *= mult_factor;
- guard->unusable_circuits *= mult_factor;
-
- 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 *= scale_ratio;
+ guard->circ_successes *= scale_ratio;
+ guard->timeouts *= scale_ratio;
+ guard->successful_circuits_closed *= scale_ratio;
+ guard->collapsed_circuits *= scale_ratio;
+ guard->unusable_circuits *= scale_ratio;
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",
diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h
index d03a7c5323..3ca8d1531d 100644
--- a/src/or/circuitbuild.h
+++ b/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
diff --git a/src/or/circuitstats.c b/src/or/circuitstats.c
index 625083c9b8..73e34d9ed7 100644
--- a/src/or/circuitstats.c
+++ b/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.
*
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 48a774352e..cfd41be792 100644
--- a/src/or/circuituse.c
+++ b/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,
- * because the rend point could be malicious.
+ * successfully built but unused closed circuit. We don't
+ * 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 */
diff --git a/src/or/config.c b/src/or/config.c
index 74b635e850..2ba9c76729 100644
--- a/src/or/config.c
+++ b/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"),
- V(PathBiasMultFactor, INT, "-1"),
+ OBSOLETE("PathBiasScaleFactor"),
+ 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);
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 9e2c15d2ca..b4fa3e6fe2 100644
--- a/src/or/connection_edge.c
+++ b/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 its state back if it succeeded so that we do so upon close. */
- 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;
- }
+ /* Roll back path bias use state so that we probe the circuit
+ * if nothing else succeeds on it */
+ pathbias_mark_use_rollback(circ);
if (conn->pending_optimistic_data) {
generic_buffer_set_to_copy(&conn->sending_optimistic_data,
diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c
index b45d402ba7..71ac054f88 100644
--- a/src/or/networkstatus.c
+++ b/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);
diff --git a/src/or/networkstatus.h b/src/or/networkstatus.h
index d6d6a036cd..b437c5ec2f 100644
--- a/src/or/networkstatus.h
+++ b/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
diff --git a/src/or/or.h b/src/or/or.h
index df933c353a..11158ff6f8 100644
--- a/src/or/or.h
+++ b/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
- * overcounting due to unknown cases of circuit reuse. See Bug #6475.
+ * on the path bias accounting state for a circuit.
+ *
+ * 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
- * stream probes (#5707), we must *not* set this for those probes
- * (or any other automatic streams) because the adversary could
- * just tag at a later point.
+ * If any streams detatch/fail from this circuit, the code transitions
+ * the circuit back to PATH_STATE_USE_ATTEMPTED to ensure we probe. See
+ * pathbias_mark_use_rollback() for that.
*/
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;
/** @} */
/**
diff --git a/src/or/routerlist.c b/src/or/routerlist.c
index 3034f91f02..8f19947600 100644
--- a/src/or/routerlist.c
+++ b/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);
diff --git a/src/or/routerparse.c b/src/or/routerparse.c
index eaf015c0bc..2a3de12c35 100644
--- a/src/or/routerparse.c
+++ b/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);
diff --git a/src/or/statefile.c b/src/or/statefile.c
index 53b6817ef7..bcb7b07417 100644
--- a/src/or/statefile.c
+++ b/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),