diff options
| -rw-r--r-- | changes/bug7802 | 11 | ||||
| -rw-r--r-- | doc/tor.1.txt | 31 | ||||
| -rw-r--r-- | src/or/circuitbuild.c | 618 | ||||
| -rw-r--r-- | src/or/circuitbuild.h | 3 | ||||
| -rw-r--r-- | src/or/circuituse.c | 30 | ||||
| -rw-r--r-- | src/or/config.c | 7 | ||||
| -rw-r--r-- | src/or/connection_edge.c | 28 | ||||
| -rw-r--r-- | src/or/entrynodes.c | 48 | ||||
| -rw-r--r-- | src/or/entrynodes.h | 7 | ||||
| -rw-r--r-- | src/or/or.h | 32 | ||||
| -rw-r--r-- | src/or/relay.c | 2 | ||||
| -rw-r--r-- | src/or/rendclient.c | 9 | ||||
| -rw-r--r-- | src/or/rendservice.c | 16 |
13 files changed, 691 insertions, 151 deletions
diff --git a/changes/bug7802 b/changes/bug7802 new file mode 100644 index 000000000..9bc0f36fe --- /dev/null +++ b/changes/bug7802 @@ -0,0 +1,11 @@ + o Minor features: + - Path Use Bias: Perform separate accounting for successful circuit use. + Separate statistics on stream attempt versus success rates are kept + for each guard. Configurable thresholds are provided to determine + when to emit log messages or disable use of guards that fail too + many stream attempts. + o Minor bugfixes: + - Remove a source of rounding error during path bias count scaling. + - Don't count cannibalized circuits as used for path bias until we + actually try to use them. + - Fix circuit_package_relay_cell warning message about n_chan==NULL. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index d107fa915..432774c30 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1242,16 +1242,11 @@ The following options are useful only for clients (that is, if **PathBiasMultFactor** __NUM__ + -**PathBiasScaleFactor** __NUM__ + - -**PathBiasUseCloseCounts** __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 - fraction of circuits through that guard fail to get built. If - PathBiasUseCloseCounts is set to 1 (the default), usage-based accounting is - performed, and circuits that fail to carry streams are also counted as - failures. + + fraction of circuits through that guard fail to get built. + The PathBiasCircThreshold option controls how many circuits we need to build through a guard before we make these checks. The PathBiasNoticeRate, @@ -1270,6 +1265,28 @@ The following options are useful only for clients (that is, if If no defaults are available there, these options default to 150, .70, .50, .30, 0, 300, 1, and 2 respectively. +**PathBiasUseThreshold** __NUM__ + + +**PathBiasNoticeUseRate** __NUM__ + + +**PathBiasExtremeUseRate** __NUM__ + + +**PathBiasScaleUseThreshold** __NUM__:: + Similar to the above options, these options override the default behavior + of Tor's (**currently experimental**) path use bias detection algorithm. + + + Where as the path bias parameters govern thresholds for successfully + building circuits, these four path use bias parameters govern thresholds + only for circuit usage. Circuits which receive no stream usage + are not counted by this detection algorithm. A used circuit is considered + successful if it is capable of carrying streams or otherwise receiving + well-formed responses to RELAY cells. + + + 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 20, .90, + .70, and 100, respectively. + **ClientUseIPv6** **0**|**1**:: If this option is set to 1, Tor might connect to entry nodes over IPv6. Note that clients configured with an IPv6 address in a diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index b98624386..e3a9d59c0 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -67,7 +67,9 @@ static int entry_guard_inc_circ_attempt_count(entry_guard_t *guard); 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_unusable(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); /** This function tries to get a channel to the specified endpoint, * and then calls command_setup_channel() to give it the right @@ -821,9 +823,6 @@ circuit_send_next_onion_skin(origin_circuit_t *circ) /* We're done with measurement circuits here. Just close them */ if (circ->base_.purpose == CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) { - /* If a measurement circ ever gets back to us, consider it - * succeeded for path bias */ - circ->path_state = PATH_STATE_USE_SUCCEEDED; circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_FINISHED); } return 0; @@ -1210,19 +1209,65 @@ pathbias_get_mult_factor(const or_options_t *options) pathbias_get_scale_factor(options)); } +/** The minimum number of circuit usage attempts before we start + * thinking about warning about path use bias and dropping guards */ +static int +pathbias_get_min_use(const or_options_t *options) +{ +#define DFLT_PATH_BIAS_MIN_USE 20 + if (options->PathBiasUseThreshold >= 3) + return options->PathBiasUseThreshold; + else + return networkstatus_get_param(NULL, "pb_minuse", + DFLT_PATH_BIAS_MIN_USE, + 3, INT32_MAX); +} + +/** The circuit use success rate below which we issue a notice */ +static double +pathbias_get_notice_use_rate(const or_options_t *options) +{ +#define DFLT_PATH_BIAS_NOTICE_USE_PCT 90 + if (options->PathBiasNoticeUseRate >= 0.0) + return options->PathBiasNoticeUseRate; + else + return networkstatus_get_param(NULL, "pb_noticeusepct", + DFLT_PATH_BIAS_NOTICE_USE_PCT, + 0, 100)/100.0; +} + +/** + * The extreme use rate is the rate at which we would drop the guard, + * if pb_dropguard is also set. Otherwise we just warn. + */ +double +pathbias_get_extreme_use_rate(const or_options_t *options) +{ +#define DFLT_PATH_BIAS_EXTREME_USE_PCT 70 + if (options->PathBiasExtremeUseRate >= 0.0) + return options->PathBiasExtremeUseRate; + else + return networkstatus_get_param(NULL, "pb_extremeusepct", + DFLT_PATH_BIAS_EXTREME_USE_PCT, + 0, 100)/100.0; +} + /** - * If this parameter is set to a true value (default), we use the - * successful_circuits_closed. Otherwise, we use the success_count. + * This is the number of circuits at which we scale our + * use counts by mult_factor/scale_factor. Note, this count is + * not exact, as we only perform the scaling in the event + * of no integer truncation. */ static int -pathbias_use_close_counts(const or_options_t *options) +pathbias_get_scale_use_threshold(const or_options_t *options) { -#define DFLT_PATH_BIAS_USE_CLOSE_COUNTS 1 - if (options->PathBiasUseCloseCounts >= 0) - return options->PathBiasUseCloseCounts; +#define DFLT_PATH_BIAS_SCALE_USE_THRESHOLD 100 + if (options->PathBiasScaleUseThreshold >= 10) + return options->PathBiasScaleUseThreshold; else - return networkstatus_get_param(NULL, "pb_useclosecounts", - DFLT_PATH_BIAS_USE_CLOSE_COUNTS, 0, 1); + return networkstatus_get_param(NULL, "pb_scaleuse", + DFLT_PATH_BIAS_SCALE_USE_THRESHOLD, + 10, INT32_MAX); } /** @@ -1238,10 +1283,14 @@ pathbias_state_to_string(path_state_t state) return "build attempted"; case PATH_STATE_BUILD_SUCCEEDED: return "build succeeded"; + case PATH_STATE_USE_ATTEMPTED: + return "use attempted"; case PATH_STATE_USE_SUCCEEDED: return "use succeeded"; case PATH_STATE_USE_FAILED: return "use failed"; + case PATH_STATE_ALREADY_COUNTED: + return "already counted"; } return "unknown"; @@ -1338,7 +1387,7 @@ pathbias_should_count(origin_circuit_t *circ) * Also check for several potential error cases for bug #6475. */ static int -pathbias_count_circ_attempt(origin_circuit_t *circ) +pathbias_count_build_attempt(origin_circuit_t *circ) { #define CIRC_ATTEMPT_NOTICE_INTERVAL (600) static ratelim_t circ_attempt_notice_limit = @@ -1505,6 +1554,130 @@ pathbias_count_build_success(origin_circuit_t *circ) } /** + * Record an attempt to use a circuit. Changes the circuit's + * path state and update its guard's usage counter. + * + * Used for path bias usage accounting. + */ +void +pathbias_count_use_attempt(origin_circuit_t *circ) +{ + entry_guard_t *guard; + + if (!pathbias_should_count(circ)) { + return; + } + + if (circ->path_state < PATH_STATE_BUILD_SUCCEEDED) { + log_notice(LD_BUG, + "Used circuit is in strange path state %s. " + "Circuit is a %s currently %s.", + pathbias_state_to_string(circ->path_state), + circuit_purpose_to_string(circ->base_.purpose), + circuit_state_to_string(circ->base_.state)); + } else if (circ->path_state < PATH_STATE_USE_ATTEMPTED) { + guard = entry_guard_get_by_id_digest( + circ->cpath->extend_info->identity_digest); + if (guard) { + pathbias_check_use_rate(guard); + guard->use_attempts++; + + log_debug(LD_CIRC, + "Marked circuit %d (%f/%f) as used for guard %s=%s.", + circ->global_identifier, + guard->use_successes, guard->use_attempts, + guard->nickname, hex_str(guard->identity, DIGEST_LEN)); + } + + circ->path_state = PATH_STATE_USE_ATTEMPTED; + } else { + /* Harmless but educational log message */ + log_info(LD_CIRC, + "Used circuit %d is already in path state %s. " + "Circuit is a %s currently %s.", + circ->global_identifier, + pathbias_state_to_string(circ->path_state), + circuit_purpose_to_string(circ->base_.purpose), + circuit_state_to_string(circ->base_.state)); + } + + return; +} + +/** + * Check the circuit's path stat is appropriate and it as successfully + * used. + * + * We don't actually increment the guard's counters until + * pathbias_check_close(). + * + * Used for path bias usage accounting. + */ +void +pathbias_mark_use_success(origin_circuit_t *circ) +{ + if (!pathbias_should_count(circ)) { + return; + } + + if (circ->path_state < PATH_STATE_USE_ATTEMPTED) { + log_notice(LD_BUG, + "Used circuit %d is in strange path state %s. " + "Circuit is a %s currently %s.", + circ->global_identifier, + pathbias_state_to_string(circ->path_state), + circuit_purpose_to_string(circ->base_.purpose), + circuit_state_to_string(circ->base_.state)); + + pathbias_count_use_attempt(circ); + } + + /* We don't do any accounting at the guard until actual circuit close */ + circ->path_state = PATH_STATE_USE_SUCCEEDED; + + return; +} + +/** + * Actually count a circuit success towards a guard's usage counters + * if the path state is appropriate. + */ +static void +pathbias_count_use_success(origin_circuit_t *circ) +{ + entry_guard_t *guard; + + if (!pathbias_should_count(circ)) { + return; + } + + if (circ->path_state != PATH_STATE_USE_SUCCEEDED) { + log_notice(LD_BUG, + "Successfully used circuit %d is in strange path state %s. " + "Circuit is a %s currently %s.", + circ->global_identifier, + pathbias_state_to_string(circ->path_state), + circuit_purpose_to_string(circ->base_.purpose), + circuit_state_to_string(circ->base_.state)); + } else { + guard = entry_guard_get_by_id_digest( + circ->cpath->extend_info->identity_digest); + if (guard) { + guard->use_successes++; + + log_debug(LD_CIRC, + "Marked circuit %d (%f/%f) as used successfully for guard " + "%s=%s.", + circ->global_identifier, guard->use_successes, + guard->use_attempts, guard->nickname, + hex_str(guard->identity, DIGEST_LEN)); + } + } + + return; +} + +/** * Send a probe down a circuit that the client attempted to use, * but for which the stream timed out/failed. The probe is a * RELAY_BEGIN cell with a 0.a.b.c destination address, which @@ -1554,6 +1727,16 @@ pathbias_send_usable_probe(circuit_t *circ) return -1; } + /* Can't probe if the channel isn't open */ + if (circ->n_chan == NULL || + (circ->n_chan->state != CHANNEL_STATE_OPEN + && circ->n_chan->state != CHANNEL_STATE_MAINT)) { + log_info(LD_CIRC, + "Skipping pathbias probe for circuit %d: Channel is not open.", + ocirc->global_identifier); + return -1; + } + circuit_change_purpose(circ, CIRCUIT_PURPOSE_PATH_BIAS_TESTING); /* Update timestamp for when circuit_expire_building() should kill us */ @@ -1648,7 +1831,7 @@ pathbias_check_probe_response(circuit_t *circ, const cell_t *cell) /* Check nonce */ if (ipv4_host == ocirc->pathbias_probe_nonce) { - ocirc->path_state = PATH_STATE_USE_SUCCEEDED; + pathbias_mark_use_success(ocirc); circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED); log_info(LD_CIRC, "Got valid path bias probe back for circ %d, stream %d.", @@ -1691,24 +1874,11 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) return 0; } - if (ocirc->path_state == PATH_STATE_BUILD_SUCCEEDED) { - if (circ->timestamp_dirty) { - if (pathbias_send_usable_probe(circ) == 0) - return -1; - else - pathbias_count_unusable(ocirc); - - /* Any circuit where there were attempted streams but no successful - * streams could be bias */ - log_info(LD_CIRC, - "Circuit %d closed without successful use for reason %d. " - "Circuit purpose %d currently %d,%s. Len %d.", - ocirc->global_identifier, - reason, circ->purpose, ocirc->has_opened, - circuit_state_to_string(circ->state), - ocirc->build_state->desired_path_len); - - } else { + switch (ocirc->path_state) { + /* If the circuit was closed after building, but before use, we need + * to ensure we were the ones who tried to close it (and not a remote + * actor). */ + case PATH_STATE_BUILD_SUCCEEDED: if (reason & END_CIRC_REASON_FLAG_REMOTE) { /* Remote circ close reasons on an unused circuit all could be bias */ log_info(LD_CIRC, @@ -1739,11 +1909,41 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) } else { pathbias_count_successful_close(ocirc); } - } - } else if (ocirc->path_state == PATH_STATE_USE_SUCCEEDED) { - pathbias_count_successful_close(ocirc); + break; + + /* If we tried to use a circuit but failed, we should probe it to ensure + * it has not been tampered with. */ + case PATH_STATE_USE_ATTEMPTED: + /* XXX: Only probe and/or count failure if the network is live? + * What about clock jumps/suspends? */ + if (pathbias_send_usable_probe(circ) == 0) + return -1; + else + pathbias_count_use_failed(ocirc); + + /* Any circuit where there were attempted streams but no successful + * streams could be bias */ + log_info(LD_CIRC, + "Circuit %d closed without successful use for reason %d. " + "Circuit purpose %d currently %d,%s. Len %d.", + ocirc->global_identifier, + reason, circ->purpose, ocirc->has_opened, + circuit_state_to_string(circ->state), + ocirc->build_state->desired_path_len); + break; + + case PATH_STATE_USE_SUCCEEDED: + pathbias_count_successful_close(ocirc); + pathbias_count_use_success(ocirc); + break; + + default: + // Other states are uninteresting. No stats to count. + break; } + ocirc->path_state = PATH_STATE_ALREADY_COUNTED; + return 0; } @@ -1792,6 +1992,7 @@ static void pathbias_count_collapse(origin_circuit_t *circ) { entry_guard_t *guard = NULL; + if (!pathbias_should_count(circ)) { return; } @@ -1816,8 +2017,13 @@ pathbias_count_collapse(origin_circuit_t *circ) } } +/** + * Count a known failed circuit (because we could not probe it). + * + * This counter is informational. + */ static void -pathbias_count_unusable(origin_circuit_t *circ) +pathbias_count_use_failed(origin_circuit_t *circ) { entry_guard_t *guard = NULL; if (!pathbias_should_count(circ)) { @@ -1879,20 +2085,20 @@ pathbias_count_timeout(origin_circuit_t *circ) } /** - * Return the number of circuits counted as successfully closed for - * this guard. - * - * Also add in the currently open circuits to give them the benefit - * of the doubt. + * Helper function to count all of the currently opened circuits + * for a guard that are in a given path state range. The state + * range is inclusive on both ends. */ -double -pathbias_get_closed_count(entry_guard_t *guard) +static int +pathbias_count_circs_in_states(entry_guard_t *guard, + path_state_t from, + path_state_t to) { - circuit_t *circ; + circuit_t *circ = global_circuitlist; int open_circuits = 0; - /* Count currently open circuits. Give them the benefit of the doubt. */ - for (circ = global_circuitlist; circ; circ = circ->next) { + /* Count currently open circuits. Give them the benefit of the doubt */ + for ( ; circ; circ = circ->next) { origin_circuit_t *ocirc = NULL; if (!CIRCUIT_IS_ORIGIN(circ) || /* didn't originate here */ circ->marked_for_close) /* already counted */ @@ -1903,63 +2109,214 @@ pathbias_get_closed_count(entry_guard_t *guard) if (!ocirc->cpath || !ocirc->cpath->extend_info) continue; - if (ocirc->path_state >= PATH_STATE_BUILD_SUCCEEDED && + if (ocirc->path_state >= from && + ocirc->path_state <= to && + pathbias_should_count(ocirc) && fast_memeq(guard->identity, - ocirc->cpath->extend_info->identity_digest, - DIGEST_LEN)) { + ocirc->cpath->extend_info->identity_digest, + DIGEST_LEN)) { + log_debug(LD_CIRC, "Found opened circuit %d in path_state %s", + ocirc->global_identifier, + pathbias_state_to_string(ocirc->path_state)); open_circuits++; } } - return guard->successful_circuits_closed + open_circuits; + return open_circuits; } /** - * This function checks the consensus parameters to decide - * if it should return guard->circ_successes or - * guard->successful_circuits_closed. + * Return the number of circuits counted as successfully closed for + * this guard. + * + * Also add in the currently open circuits to give them the benefit + * of the doubt. */ double -pathbias_get_success_count(entry_guard_t *guard) +pathbias_get_close_success_count(entry_guard_t *guard) { - if (pathbias_use_close_counts(get_options())) { - return pathbias_get_closed_count(guard); - } else { - return guard->circ_successes; - } + return guard->successful_circuits_closed + + pathbias_count_circs_in_states(guard, + PATH_STATE_BUILD_SUCCEEDED, + PATH_STATE_USE_SUCCEEDED); } -/** Increment the number of times we successfully extended a circuit to - * <b>guard</b>, first checking if the failure rate is high enough that - * we should eliminate the guard. Return -1 if the guard looks no good; - * return 0 if the guard looks fine. +/** + * Return the number of circuits counted as successfully used + * this guard. + * + * Also add in the currently open circuits that we are attempting + * to use to give them the benefit of the doubt. + */ +double +pathbias_get_use_success_count(entry_guard_t *guard) +{ + return guard->use_successes + + pathbias_count_circs_in_states(guard, + PATH_STATE_USE_ATTEMPTED, + PATH_STATE_USE_SUCCEEDED); +} + +/** + * Check the path bias use rate against our consensus parameter limits. + * + * Emits a log message if the use success rates are too low. + * + * 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 -entry_guard_inc_circ_attempt_count(entry_guard_t *guard) +pathbias_check_use_rate(entry_guard_t *guard) { const or_options_t *options = get_options(); - entry_guards_changed(); + if (guard->use_attempts > pathbias_get_min_use(options)) { + /* Note: We rely on the < comparison here to allow us to set a 0 + * rate and disable the feature entirely. If refactoring, don't + * change to <= */ + if (pathbias_get_use_success_count(guard)/guard->use_attempts + < pathbias_get_extreme_use_rate(options)) { + /* Dropping is currently disabled by default. */ + if (pathbias_get_dropguards(options)) { + if (!guard->path_bias_disabled) { + log_warn(LD_CIRC, + "Your Guard %s=%s is failing to carry an extremely large " + "amount of stream on its circuits. " + "To avoid potential route manipluation attacks, Tor has " + "disabled use of this guard. " + "Use counts are %ld/%ld. Success counts are %ld/%ld. " + "%ld circuits completed, %ld were unusable, %ld collapsed, " + "and %ld timed out. " + "For reference, your timeout cutoff is %ld seconds.", + guard->nickname, hex_str(guard->identity, DIGEST_LEN), + tor_lround(pathbias_get_use_success_count(guard)), + tor_lround(guard->use_attempts), + tor_lround(pathbias_get_close_success_count(guard)), + tor_lround(guard->circ_attempts), + tor_lround(guard->circ_successes), + tor_lround(guard->unusable_circuits), + tor_lround(guard->collapsed_circuits), + tor_lround(guard->timeouts), + tor_lround(circ_times.close_ms/1000)); + guard->path_bias_disabled = 1; + guard->bad_since = approx_time(); + return -1; + } + } else if (!guard->path_bias_extreme) { + guard->path_bias_extreme = 1; + log_warn(LD_CIRC, + "Your Guard %s=%s is failing to carry an extremely large " + "amount of streams on its circuits. " + "This could indicate a route manipulation attack, network " + "overload, bad local network connectivity, or a bug. " + "Use counts are %ld/%ld. Success counts are %ld/%ld. " + "%ld circuits completed, %ld were unusable, %ld collapsed, " + "and %ld timed out. " + "For reference, your timeout cutoff is %ld seconds.", + guard->nickname, hex_str(guard->identity, DIGEST_LEN), + tor_lround(pathbias_get_use_success_count(guard)), + tor_lround(guard->use_attempts), + tor_lround(pathbias_get_close_success_count(guard)), + tor_lround(guard->circ_attempts), + tor_lround(guard->circ_successes), + tor_lround(guard->unusable_circuits), + tor_lround(guard->collapsed_circuits), + tor_lround(guard->timeouts), + tor_lround(circ_times.close_ms/1000)); + } + } else if (pathbias_get_use_success_count(guard)/guard->use_attempts + < pathbias_get_notice_use_rate(options)) { + if (!guard->path_bias_noticed) { + guard->path_bias_noticed = 1; + log_notice(LD_CIRC, + "Your Guard %s=%s is failing to carry more streams on its " + "circuits than usual. " + "Most likely this means the Tor network is overloaded " + "or your network connection is poor. " + "Use counts are %ld/%ld. Success counts are %ld/%ld. " + "%ld circuits completed, %ld were unusable, %ld collapsed, " + "and %ld timed out. " + "For reference, your timeout cutoff is %ld seconds.", + guard->nickname, hex_str(guard->identity, DIGEST_LEN), + tor_lround(pathbias_get_use_success_count(guard)), + tor_lround(guard->use_attempts), + tor_lround(pathbias_get_close_success_count(guard)), + tor_lround(guard->circ_attempts), + tor_lround(guard->circ_successes), + tor_lround(guard->unusable_circuits), + tor_lround(guard->collapsed_circuits), + tor_lround(guard->timeouts), + tor_lround(circ_times.close_ms/1000)); + } + } + } + + /* 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; +} + +/** + * Check the path bias circuit close status rates against our consensus + * parameter limits. + * + * Emits a log message if the use success rates are too low. + * + * 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_close_rate(entry_guard_t *guard) +{ + const or_options_t *options = get_options(); if (guard->circ_attempts > pathbias_get_min_circs(options)) { /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't * change to <= */ - if (pathbias_get_success_count(guard)/guard->circ_attempts + if (pathbias_get_close_success_count(guard)/guard->circ_attempts < pathbias_get_extreme_rate(options)) { /* Dropping is currently disabled by default. */ if (pathbias_get_dropguards(options)) { if (!guard->path_bias_disabled) { log_warn(LD_CIRC, - "Your Guard %s=%s is failing an extremely large amount of " - "circuits. To avoid potential route manipulation attacks, " - "Tor has disabled use of this guard. " - "Success counts are %ld/%ld. %ld circuits completed, %ld " - "were unusable, %ld collapsed, and %ld timed out. For " - "reference, your timeout cutoff is %ld seconds.", + "Your Guard %s=%s is failing an extremely large " + "amount of circuits. " + "To avoid potential route manipluation attacks, Tor has " + "disabled use of this guard. " + "Success counts are %ld/%ld. Use counts are %ld/%ld. " + "%ld circuits completed, %ld were unusable, %ld collapsed, " + "and %ld timed out. " + "For reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - tor_lround(pathbias_get_closed_count(guard)), + tor_lround(pathbias_get_close_success_count(guard)), tor_lround(guard->circ_attempts), + tor_lround(pathbias_get_use_success_count(guard)), + tor_lround(guard->use_attempts), tor_lround(guard->circ_successes), tor_lround(guard->unusable_circuits), tor_lround(guard->collapsed_circuits), @@ -1972,60 +2329,72 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) } else if (!guard->path_bias_extreme) { guard->path_bias_extreme = 1; log_warn(LD_CIRC, - "Your Guard %s=%s is failing an extremely large amount of " - "circuits. This could indicate a route manipulation attack, " + "Your Guard %s=%s is failing an extremely large " + "amount of circuits. " + "This could indicate a route manipulation attack, " "extreme network overload, or a bug. " - "Success counts are %ld/%ld. %ld circuits completed, %ld " - "were unusable, %ld collapsed, and %ld timed out. For " - "reference, your timeout cutoff is %ld seconds.", + "Success counts are %ld/%ld. Use counts are %ld/%ld. " + "%ld circuits completed, %ld were unusable, %ld collapsed, " + "and %ld timed out. " + "For reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - tor_lround(pathbias_get_closed_count(guard)), + tor_lround(pathbias_get_close_success_count(guard)), tor_lround(guard->circ_attempts), + tor_lround(pathbias_get_use_success_count(guard)), + tor_lround(guard->use_attempts), tor_lround(guard->circ_successes), tor_lround(guard->unusable_circuits), tor_lround(guard->collapsed_circuits), tor_lround(guard->timeouts), tor_lround(circ_times.close_ms/1000)); } - } else if (pathbias_get_success_count(guard)/((double)guard->circ_attempts) - < pathbias_get_warn_rate(options)) { + } else if (pathbias_get_close_success_count(guard)/guard->circ_attempts + < pathbias_get_warn_rate(options)) { if (!guard->path_bias_warned) { guard->path_bias_warned = 1; log_warn(LD_CIRC, - "Your Guard %s=%s is failing a very large amount of " - "circuits. Most likely this means the Tor network is " + "Your Guard %s=%s is failing a very large " + "amount of circuits. " + "Most likely this means the Tor network is " "overloaded, but it could also mean an attack against " - "you or potentially the guard itself. " - "Success counts are %ld/%ld. %ld circuits completed, %ld " - "were unusable, %ld collapsed, and %ld timed out. For " - "reference, your timeout cutoff is %ld seconds.", + "you or the 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. " + "For reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - tor_lround(pathbias_get_closed_count(guard)), + tor_lround(pathbias_get_close_success_count(guard)), tor_lround(guard->circ_attempts), + tor_lround(pathbias_get_use_success_count(guard)), + tor_lround(guard->use_attempts), tor_lround(guard->circ_successes), tor_lround(guard->unusable_circuits), tor_lround(guard->collapsed_circuits), tor_lround(guard->timeouts), tor_lround(circ_times.close_ms/1000)); } - } else if (pathbias_get_success_count(guard)/((double)guard->circ_attempts) + } else if (pathbias_get_close_success_count(guard)/guard->circ_attempts < pathbias_get_notice_rate(options)) { if (!guard->path_bias_noticed) { guard->path_bias_noticed = 1; log_notice(LD_CIRC, - "Your Guard %s=%s is failing more circuits than usual. " - "Most likely this means the Tor network is overloaded. " - "Success counts are %ld/%ld. %ld circuits completed, %ld " - "were unusable, %ld collapsed, and %ld timed out. For " - "reference, your timeout cutoff is %ld seconds.", - guard->nickname, hex_str(guard->identity, DIGEST_LEN), - tor_lround(pathbias_get_closed_count(guard)), - tor_lround(guard->circ_attempts), - tor_lround(guard->circ_successes), - tor_lround(guard->unusable_circuits), - tor_lround(guard->collapsed_circuits), - tor_lround(guard->timeouts), - tor_lround(circ_times.close_ms/1000)); + "Your Guard %s=%s is failing more circuits than " + "usual. " + "Most likely this means the Tor network is overloaded. " + "Success counts are %ld/%ld. Use counts are %ld/%ld. " + "%ld circuits completed, %ld were unusable, %ld collapsed, " + "and %ld timed out. " + "For reference, your timeout cutoff is %ld seconds.", + guard->nickname, hex_str(guard->identity, DIGEST_LEN), + tor_lround(pathbias_get_close_success_count(guard)), + tor_lround(guard->circ_attempts), + tor_lround(pathbias_get_use_success_count(guard)), + tor_lround(guard->use_attempts), + tor_lround(guard->circ_successes), + tor_lround(guard->unusable_circuits), + tor_lround(guard->collapsed_circuits), + tor_lround(guard->timeouts), + tor_lround(circ_times.close_ms/1000)); } } } @@ -2034,11 +2403,13 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) 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); - log_info(LD_CIRC, - "Scaling pathbias counts to (%f/%f)*(%d/%d) for guard %s=%s", - guard->circ_successes, guard->circ_attempts, - mult_factor, scale_factor, guard->nickname, - hex_str(guard->identity, DIGEST_LEN)); + 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, + PATH_STATE_BUILD_SUCCEEDED, + PATH_STATE_USE_FAILED); + guard->circ_attempts -= opened_attempts; + guard->circ_successes -= opened_built; guard->circ_attempts *= mult_factor; guard->circ_successes *= mult_factor; @@ -2053,8 +2424,35 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) 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; + + log_info(LD_CIRC, + "Scaled pathbias counts to (%f,%f)/%f (%d/%d open) for guard " + "%s=%s", + guard->circ_successes, guard->successful_circuits_closed, + guard->circ_attempts, opened_built, opened_attempts, + guard->nickname, hex_str(guard->identity, DIGEST_LEN)); } + + return 0; +} + +/** Increment the number of times we successfully extended a circuit to + * 'guard', first checking if the failure rate is high enough that we should + * eliminate the guard. Return -1 if the guard looks no good; return 0 if the + * guard looks fine. */ +static int +entry_guard_inc_circ_attempt_count(entry_guard_t *guard) +{ + entry_guards_changed(); + + if (pathbias_check_close_rate(guard) < 0) + return -1; + guard->circ_attempts++; + log_info(LD_CIRC, "Got success count %f/%f for guard %s=%s", guard->circ_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); @@ -2078,7 +2476,7 @@ circuit_finish_handshake(origin_circuit_t *circ, crypt_path_t *hop; int rv; - if ((rv = pathbias_count_circ_attempt(circ)) < 0) + if ((rv = pathbias_count_build_attempt(circ)) < 0) return rv; if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) { @@ -2761,9 +3159,7 @@ circuit_extend_to_new_exit(origin_circuit_t *circ, extend_info_t *exit) return -1; } - /* Set timestamp_dirty, so we can check it for path use bias */ - if (!circ->base_.timestamp_dirty) - circ->base_.timestamp_dirty = time(NULL); + // XXX: Should cannibalized circuits be dirty or not? Not easy to say.. return 0; } diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index e6cf802f2..d03a7c532 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -58,10 +58,13 @@ const char *build_state_get_exit_nickname(cpath_build_state_t *state); const node_t *choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state); double pathbias_get_extreme_rate(const or_options_t *options); +double pathbias_get_extreme_use_rate(const or_options_t *options); int pathbias_get_dropguards(const or_options_t *options); void pathbias_count_timeout(origin_circuit_t *circ); 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); #endif diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 83734c9d6..48a774352 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -668,18 +668,6 @@ circuit_expire_building(void) circuit_build_times_set_timeout(&circ_times); } } - - if (TO_ORIGIN_CIRCUIT(victim)->has_opened && - victim->purpose != CIRCUIT_PURPOSE_PATH_BIAS_TESTING) { - /* For path bias: we want to let these guys live for a while - * so we get a chance to test them. */ - log_info(LD_CIRC, - "Allowing cannibalized circuit %d time to finish building " - "as a pathbias testing circ.", - TO_ORIGIN_CIRCUIT(victim)->global_identifier); - circuit_change_purpose(victim, CIRCUIT_PURPOSE_PATH_BIAS_TESTING); - continue; /* It now should have a longer timeout next time */ - } } /* If this is a hidden service client circuit which is far enough @@ -1090,7 +1078,10 @@ circuit_expire_old_circuits_clientside(void) "purpose %d)", circ->n_circ_id, (long)(now.tv_sec - circ->timestamp_dirty), circ->purpose); - circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED); + /* Don't do this magic for testing circuits. Their death is governed + * by circuit_expire_building */ + if (circ->purpose != CIRCUIT_PURPOSE_PATH_BIAS_TESTING) + circuit_mark_for_close(circ, END_CIRC_REASON_FINISHED); } else if (!circ->timestamp_dirty && circ->state == CIRCUIT_STATE_OPEN) { if (timercmp(&circ->timestamp_began, &cutoff, <)) { if (circ->purpose == CIRCUIT_PURPOSE_C_GENERAL || @@ -1517,7 +1508,7 @@ circuit_launch_by_extend_info(uint8_t purpose, * If we decide to probe the initial portion of these circs, * (up to the adversaries final hop), we need to remove this. */ - circ->path_state = PATH_STATE_USE_SUCCEEDED; + /* This must be called before the purpose change */ pathbias_check_close(circ, END_CIRC_REASON_FINISHED); } @@ -2037,6 +2028,8 @@ connection_ap_handshake_attach_chosen_circuit(entry_connection_t *conn, if (!circ->base_.timestamp_dirty) circ->base_.timestamp_dirty = time(NULL); + pathbias_count_use_attempt(circ); + link_apconn_to_circ(conn, circ, cpath); tor_assert(conn->socks_request); if (conn->socks_request->command == SOCKS_COMMAND_CONNECT) { @@ -2163,6 +2156,11 @@ connection_ap_handshake_attach_circuit(entry_connection_t *conn) * feasibility, at this point. */ rendcirc->base_.timestamp_dirty = time(NULL); + + /* We've also attempted to use them. If they fail, we need to + * probe them for path bias */ + pathbias_count_use_attempt(rendcirc); + link_apconn_to_circ(conn, rendcirc, NULL); if (connection_ap_handshake_send_begin(conn) < 0) return 0; /* already marked, let them fade away */ @@ -2214,6 +2212,10 @@ connection_ap_handshake_attach_circuit(entry_connection_t *conn) case 0: /* success */ rendcirc->base_.timestamp_dirty = time(NULL); introcirc->base_.timestamp_dirty = time(NULL); + + pathbias_count_use_attempt(introcirc); + pathbias_count_use_attempt(rendcirc); + assert_circuit_ok(TO_CIRCUIT(rendcirc)); assert_circuit_ok(TO_CIRCUIT(introcirc)); return 0; diff --git a/src/or/config.c b/src/or/config.c index f61d50d97..0e9c0fd6f 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -324,7 +324,12 @@ static config_var_t option_vars_[] = { V(PathBiasScaleFactor, INT, "-1"), V(PathBiasMultFactor, INT, "-1"), V(PathBiasDropGuards, AUTOBOOL, "0"), - V(PathBiasUseCloseCounts, AUTOBOOL, "1"), + OBSOLETE("PathBiasUseCloseCounts"), + + V(PathBiasUseThreshold, INT, "-1"), + V(PathBiasNoticeUseRate, DOUBLE, "-1"), + V(PathBiasExtremeUseRate, DOUBLE, "-1"), + V(PathBiasScaleUseThreshold, INT, "-1"), OBSOLETE("PathlenCoinWeight"), V(PerConnBWBurst, MEMUNIT, "0"), diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 870ded98c..9e2c15d2c 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -37,6 +37,7 @@ #include "router.h" #include "routerlist.h" #include "routerset.h" +#include "circuitbuild.h" #ifdef HAVE_LINUX_TYPES_H #include <linux/types.h> @@ -636,6 +637,16 @@ 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'.", @@ -805,6 +816,15 @@ 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; + } + if (conn->pending_optimistic_data) { generic_buffer_set_to_copy(&conn->sending_optimistic_data, conn->pending_optimistic_data); @@ -2205,8 +2225,10 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply, U64_PRINTF_ARG(ENTRY_TO_CONN(conn)->global_identifier), endreason); } else { - TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->path_state - = PATH_STATE_USE_SUCCEEDED; + // XXX: Hrmm. It looks like optimistic data can't go through this + // codepath, but someone should probably test it and make sure. + // We don't want to mark optimistically opened streams as successful. + pathbias_mark_use_success(TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)); } } @@ -2480,7 +2502,7 @@ connection_exit_begin_conn(cell_t *cell, circuit_t *circ) connection_exit_connect(n_stream); /* For path bias: This circuit was used successfully */ - origin_circ->path_state = PATH_STATE_USE_SUCCEEDED; + pathbias_mark_use_success(origin_circ); tor_free(address); return 0; diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 3e5837164..4ca56cbac 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1098,6 +1098,40 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) continue; } digestmap_set(added_by, d, tor_strdup(line->value+HEX_DIGEST_LEN+1)); + } else if (!strcasecmp(line->key, "EntryGuardPathUseBias")) { + const or_options_t *options = get_options(); + double use_cnt, success_cnt; + + if (!node) { + *msg = tor_strdup("Unable to parse entry nodes: " + "EntryGuardPathUseBias without EntryGuard"); + break; + } + + if (tor_sscanf(line->value, "%lf %lf", + &use_cnt, &success_cnt) != 2) { + log_info(LD_GENERAL, "Malformed path use bias line for node %s", + node->nickname); + continue; + } + + node->use_attempts = use_cnt; + node->use_successes = success_cnt; + + log_info(LD_GENERAL, "Read %f/%f path use bias for node %s", + node->use_successes, node->use_attempts, node->nickname); + + /* Note: We rely on the < comparison here to allow us to set a 0 + * rate and disable the feature entirely. If refactoring, don't + * change to <= */ + if (pathbias_get_use_success_count(node)/node->use_attempts + < pathbias_get_extreme_use_rate(options) && + pathbias_get_dropguards(options)) { + node->path_bias_disabled = 1; + log_info(LD_GENERAL, + "Path use bias is too high (%f/%f); disabling node %s", + node->circ_successes, node->circ_attempts, node->nickname); + } } else if (!strcasecmp(line->key, "EntryGuardPathBias")) { const or_options_t *options = get_options(); double hop_cnt, success_cnt, timeouts, collapsed, successful_closed, @@ -1144,7 +1178,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) /* Note: We rely on the < comparison here to allow us to set a 0 * rate and disable the feature entirely. If refactoring, don't * change to <= */ - if (pathbias_get_success_count(node)/node->circ_attempts + if (pathbias_get_close_success_count(node)/node->circ_attempts < pathbias_get_extreme_rate(options) && pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; @@ -1282,10 +1316,20 @@ entry_guards_update_state(or_state_t *state) * unusable_circuits */ tor_asprintf(&line->value, "%f %f %f %f %f %f", e->circ_attempts, e->circ_successes, - pathbias_get_closed_count(e), e->collapsed_circuits, + pathbias_get_close_success_count(e), + e->collapsed_circuits, e->unusable_circuits, e->timeouts); next = &(line->next); } + if (e->use_attempts > 0) { + *next = line = tor_malloc_zero(sizeof(config_line_t)); + line->key = tor_strdup("EntryGuardPathUseBias"); + + tor_asprintf(&line->value, "%f %f", + e->use_attempts, + pathbias_get_use_success_count(e)); + next = &(line->next); + } } SMARTLIST_FOREACH_END(e); if (!get_options()->AvoidDiskWrites) diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 1e52ba28e..e6c973c95 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -61,6 +61,9 @@ typedef struct entry_guard_t { * attempted, but none succeeded. */ double timeouts; /**< Number of 'right-censored' circuit timeouts for this * guard. */ + double use_attempts; /**< Number of circuits we tried to use with streams */ + double use_successes; /**< Number of successfully used circuits using + * this guard as first hop. */ } entry_guard_t; entry_guard_t *entry_guard_get_by_id_digest(const char *digest); @@ -113,8 +116,8 @@ int find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, int validate_pluggable_transports_config(void); -double pathbias_get_closed_count(entry_guard_t *gaurd); -double pathbias_get_success_count(entry_guard_t *guard); +double pathbias_get_close_success_count(entry_guard_t *guard); +double pathbias_get_use_success_count(entry_guard_t *guard); #endif diff --git a/src/or/or.h b/src/or/or.h index 251512dd3..cfcddba3b 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2839,6 +2839,15 @@ typedef enum { PATH_STATE_BUILD_ATTEMPTED = 1, /** This circuit has been completely built */ PATH_STATE_BUILD_SUCCEEDED = 2, + /** Did we try to attach any SOCKS streams or hidserv introductions to + * 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. + */ + PATH_STATE_USE_ATTEMPTED = 3, /** Did any SOCKS streams or hidserv introductions actually succeed on * this circuit? * @@ -2847,13 +2856,20 @@ typedef enum { * (or any other automatic streams) because the adversary could * just tag at a later point. */ - PATH_STATE_USE_SUCCEEDED = 3, + PATH_STATE_USE_SUCCEEDED = 4, /** * This is a special state to indicate that we got a corrupted * relay cell on a circuit and we don't intend to probe it. */ - PATH_STATE_USE_FAILED = 4, + PATH_STATE_USE_FAILED = 5, + + /** + * This is a special state to indicate that we already counted + * the circuit. Used to guard against potential state machine + * violations. + */ + PATH_STATE_ALREADY_COUNTED = 6, } path_state_t; /** An origin_circuit_t holds data necessary to build and use a circuit. @@ -2998,7 +3014,6 @@ typedef struct origin_circuit_t { * ISO_STREAM. */ uint64_t associated_isolated_stream_global_id; /**@}*/ - } origin_circuit_t; struct onion_queue_t; @@ -3913,7 +3928,16 @@ typedef struct { int PathBiasScaleThreshold; int PathBiasScaleFactor; int PathBiasMultFactor; - int PathBiasUseCloseCounts; + /** @} */ + + /** + * Parameters for path-bias use detection + * @{ + */ + int PathBiasUseThreshold; + double PathBiasNoticeUseRate; + double PathBiasExtremeUseRate; + int PathBiasScaleUseThreshold; /** @} */ int IPv6Exit; /**< Do we support exiting to IPv6 addresses? */ diff --git a/src/or/relay.c b/src/or/relay.c index bb3a83544..f711eae92 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -730,7 +730,7 @@ connection_ap_process_end_not_open( * We rely on recognized+digest being strong enough to make * tags unlikely to allow us to get tagged, yet 'recognized' * reason codes here. */ - circ->path_state = PATH_STATE_USE_SUCCEEDED; + pathbias_mark_use_success(circ); } } diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 32623c3f4..61e3b917e 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -71,6 +71,9 @@ rend_client_send_establish_rendezvous(origin_circuit_t *circ) * and the rend cookie also means we've used the circ. */ circ->base_.timestamp_dirty = time(NULL); + /* We've attempted to use this circuit. Probe it if we fail */ + pathbias_count_use_attempt(circ); + if (relay_send_command_from_edge(0, TO_CIRCUIT(circ), RELAY_COMMAND_ESTABLISH_RENDEZVOUS, circ->rend_data->rend_cookie, @@ -316,6 +319,8 @@ rend_client_send_introduction(origin_circuit_t *introcirc, * state. */ introcirc->base_.timestamp_dirty = time(NULL); + pathbias_count_use_attempt(introcirc); + goto cleanup; perm_err: @@ -395,7 +400,7 @@ rend_client_introduction_acked(origin_circuit_t *circ, /* For path bias: This circuit was used successfully. Valid * nacks and acks count. */ - circ->path_state = PATH_STATE_USE_SUCCEEDED; + pathbias_mark_use_success(circ); if (request_len == 0) { /* It's an ACK; the introduction point relayed our introduction request. */ @@ -902,7 +907,7 @@ rend_client_rendezvous_acked(origin_circuit_t *circ, const uint8_t *request, * Waiting any longer opens us up to attacks from Bob. He could induce * Alice to attempt to connect to his hidden service and never reply * to her rend requests */ - circ->path_state = PATH_STATE_USE_SUCCEEDED; + pathbias_mark_use_success(circ); /* XXXX This is a pretty brute-force approach. It'd be better to * attach only the connections that are waiting on this circuit, rather diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 6ffa4f8f9..f115d8bfc 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1384,9 +1384,6 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, goto err; memcpy(cpath->rend_circ_nonce, keys, DIGEST_LEN); - /* For path bias: This intro circuit was used successfully */ - circuit->path_state = PATH_STATE_USE_SUCCEEDED; - goto done; log_error: @@ -2511,6 +2508,9 @@ rend_service_intro_has_opened(origin_circuit_t *circuit) goto err; } + /* We've attempted to use this circuit */ + pathbias_count_use_attempt(circuit); + goto done; err: @@ -2558,6 +2558,10 @@ rend_service_intro_established(origin_circuit_t *circuit, "Received INTRO_ESTABLISHED cell on circuit %d for service %s", circuit->base_.n_circ_id, serviceid); + /* Getting a valid INTRODUCE_ESTABLISHED means we've successfully + * used the circ */ + pathbias_mark_use_success(circuit); + return 0; err: circuit_mark_for_close(TO_CIRCUIT(circuit), END_CIRC_REASON_TORPROTOCOL); @@ -2589,6 +2593,9 @@ rend_service_rendezvous_has_opened(origin_circuit_t *circuit) if (!circuit->base_.timestamp_dirty) circuit->base_.timestamp_dirty = time(NULL); + /* This may be redundant */ + pathbias_count_use_attempt(circuit); + hop = circuit->build_state->service_pending_final_cpath_ref->cpath; base16_encode(hexcookie,9,circuit->rend_data->rend_cookie,4); @@ -3061,7 +3068,8 @@ rend_services_introduce(void) if (intro->time_expiring + INTRO_POINT_EXPIRATION_GRACE_PERIOD > now) { /* This intro point has completely expired. Remove it, and * mark the circuit for close if it's still alive. */ - if (intro_circ != NULL) { + if (intro_circ != NULL && + intro_circ->base_.purpose != CIRCUIT_PURPOSE_PATH_BIAS_TESTING) { circuit_mark_for_close(TO_CIRCUIT(intro_circ), END_CIRC_REASON_FINISHED); } |