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) +static double +pathbias_get_scale_ratio(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) -{ -#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) @@ -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 -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 * ns whose name is weight_name. If ns is NULL, try * loading the latest consensus ourselves. Return default_val 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),