December 2012 - tor-commits - lists.torproject.org

[tor/master] Add intro+rend cannibalize param..
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit fbbf894d4db1b1109fedca5f74891642f61370fe Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 20:02:08 2012 -0800 Add intro+rend cannibalize param.. --- src/or/circuituse.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 781e984..381c2b0 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1429,7 +1429,7 @@ circuit_launch_by_extend_info(uint8_t purpose, */ circ->path_state = PATH_STATE_USE_SUCCEEDED; /* This must be called before the purpose change */ - pathbias_check_close(circ); + pathbias_check_close(circ, END_CIRC_REASON_FINISHED); } circuit_change_purpose(TO_CIRCUIT(circ), purpose);

1 0

[tor/master] Convert to doubles for all pathbias state.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 2dbb62f1b571ea57af111f1f660a5149d160c4fb Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 20:53:22 2012 -0800 Convert to doubles for all pathbias state. Let's hope this solves the rounding error issue.. --- src/or/circuitbuild.c | 83 +++++++++++++++++++++++-------------------------- src/or/entrynodes.c | 18 ++++++----- src/or/entrynodes.h | 14 ++++---- 3 files changed, 56 insertions(+), 59 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 04b429b..cbc1af9 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1363,7 +1363,7 @@ pathbias_count_build_success(origin_circuit_t *circ) circ->path_state = PATH_STATE_BUILD_SUCCEEDED; guard->circ_successes++; - log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", + log_info(LD_CIRC, "Got success count %lf/%lf for guard %s=%s", guard->circ_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); } else { @@ -1381,7 +1381,7 @@ pathbias_count_build_success(origin_circuit_t *circ) } if (guard->circ_attempts < guard->circ_successes) { - log_notice(LD_BUG, "Unexpectedly high successes counts (%u/%u) " + log_notice(LD_BUG, "Unexpectedly high successes counts (%lf/%lf) " "for guard %s=%s", guard->circ_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); @@ -1607,7 +1607,7 @@ pathbias_count_timeout(origin_circuit_t *circ) } // XXX: DOCDOC -int +double pathbias_get_closed_count(entry_guard_t *guard) { circuit_t *circ = global_circuitlist; @@ -1642,7 +1642,7 @@ pathbias_get_closed_count(entry_guard_t *guard) * if it should return guard->circ_successes or * guard->successful_circuits_closed. */ -static int +static double pathbias_get_success_count(entry_guard_t *guard) { if (pathbias_use_close_counts(get_options())) { @@ -1663,11 +1663,11 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) entry_guards_changed(); - if (guard->circ_attempts > (unsigned)pathbias_get_min_circs(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)/((double)guard->circ_attempts) + if (pathbias_get_success_count(guard)/guard->circ_attempts < pathbias_get_extreme_rate(options)) { /* Dropping is currently disabled by default. */ if (pathbias_get_dropguards(options)) { @@ -1680,9 +1680,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circ_successes, guard->unusable_circuits, - guard->collapsed_circuits, guard->timeouts, + (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, + (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); guard->path_bias_disabled = 1; guard->bad_since = approx_time(); @@ -1698,9 +1698,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circ_successes, guard->unusable_circuits, - guard->collapsed_circuits, guard->timeouts, + (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, + (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } } else if (pathbias_get_success_count(guard)/((double)guard->circ_attempts) @@ -1716,9 +1716,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circ_successes, guard->unusable_circuits, - guard->collapsed_circuits, guard->timeouts, + (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, + (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } } else if (pathbias_get_success_count(guard)/((double)guard->circ_attempts) @@ -1732,45 +1732,40 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circ_successes, guard->unusable_circuits, - guard->collapsed_circuits, guard->timeouts, + (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, + (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } } } /* If we get a ton of circuits, just scale everything down */ - if (guard->circ_attempts > (unsigned)pathbias_get_scale_threshold(options)) { + 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); - /* Only scale if there will be no rounding error for our scaling - * factors */ - if (((mult_factor*guard->circ_attempts) % scale_factor) == 0 && - ((mult_factor*guard->circ_successes) % scale_factor) == 0) { - log_info(LD_CIRC, - "Scaling pathbias counts to (%u/%u)*(%d/%d) for guard %s=%s", - guard->circ_successes, guard->circ_attempts, mult_factor, - scale_factor, guard->nickname, hex_str(guard->identity, - DIGEST_LEN)); - - 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; - } + log_info(LD_CIRC, + "Scaling pathbias counts to (%lf/%lf)*(%d/%d) for guard %s=%s", + guard->circ_successes, guard->circ_attempts, + mult_factor, scale_factor, guard->nickname, + hex_str(guard->identity, DIGEST_LEN)); + + 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++; - log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", + log_info(LD_CIRC, "Got success count %lf/%lf for guard %s=%s", guard->circ_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); return 0; diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 84764d1..21c09f7 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1021,7 +1021,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) digestmap_set(added_by, d, tor_strdup(line->value+HEX_DIGEST_LEN+1)); } else if (!strcasecmp(line->key, "EntryGuardPathBias")) { const or_options_t *options = get_options(); - unsigned hop_cnt, success_cnt, timeouts, collapsed, successful_closed, + double hop_cnt, success_cnt, timeouts, collapsed, successful_closed, unusable; if (!node) { @@ -1031,20 +1031,22 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) } /* First try 3 params, then 2. */ - // XXX: We want to convert this to floating point before merge /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ - if (tor_sscanf(line->value, "%u %u %u %u %u %u", + if (tor_sscanf(line->value, "%lf %lf %lf %lf %lf %lf", &hop_cnt, &success_cnt, &successful_closed, &collapsed, &unusable, &timeouts) != 6) { - if (tor_sscanf(line->value, "%u %u", &success_cnt, &hop_cnt) != 2) { + int old_success, old_hops; + if (tor_sscanf(line->value, "%u %u", &old_success, &old_hops) != 2) { continue; } log_info(LD_GENERAL, "Reading old-style EntryGuardPathBias %s", escaped(line->value)); - successful_closed = success_cnt; + success_cnt = old_success; + successful_closed = old_success; + hop_cnt = old_hops; timeouts = 0; collapsed = 0; unusable = 0; @@ -1058,7 +1060,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) node->collapsed_circuits = collapsed; node->unusable_circuits = unusable; - log_info(LD_GENERAL, "Read %u/%u path bias for node %s", + log_info(LD_GENERAL, "Read %lf/%lf path bias for node %s", node->circ_successes, node->circ_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 @@ -1068,7 +1070,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, - "Path bias is too high (%u/%u); disabling node %s", + "Path bias is too high (%lf/%lf); disabling node %s", node->circ_successes, node->circ_attempts, node->nickname); } @@ -1198,7 +1200,7 @@ entry_guards_update_state(or_state_t *state) /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ - tor_asprintf(&line->value, "%u %u %u %u %u %u", + tor_asprintf(&line->value, "%lf %lf %lf %lf %lf %lf", e->circ_attempts, e->circ_successes, pathbias_get_closed_count(e), e->collapsed_circuits, e->unusable_circuits, e->timeouts); diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index 0e58802..de8c60c 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -48,17 +48,17 @@ typedef struct entry_guard_t { time_t last_attempted; /**< 0 if we can connect to this guard, or the time * at which we last failed to connect to it. */ - unsigned circ_attempts; /**< Number of circuits this guard has "attempted" */ - unsigned circ_successes; /**< Number of successfully built circuits using + double circ_attempts; /**< Number of circuits this guard has "attempted" */ + double circ_successes; /**< Number of successfully built circuits using * this guard as first hop. */ - unsigned successful_circuits_closed; /**< Number of circuits that carried + double successful_circuits_closed; /**< Number of circuits that carried * streams successfully. */ - unsigned collapsed_circuits; /**< Number of fully built circuits that were + double collapsed_circuits; /**< Number of fully built circuits that were * remotely closed before any streams were * attempted. */ - unsigned unusable_circuits; /**< Number of circuits for which streams were + double unusable_circuits; /**< Number of circuits for which streams were * attempted, but none succeeded. */ - unsigned timeouts; /**< Number of 'right-censored' circuit timeouts for this + double timeouts; /**< Number of 'right-censored' circuit timeouts for this * guard. */ } entry_guard_t; @@ -111,7 +111,7 @@ int find_transport_by_bridge_addrport(const tor_addr_t *addr, uint16_t port, int validate_pluggable_transports_config(void); -int pathbias_get_closed_count(entry_guard_t *gaurd); +double pathbias_get_closed_count(entry_guard_t *gaurd); #endif

1 0

[tor/master] Flag cannibalized circs as used (non-ideal).
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 930fbb2fec2b0c4e56cc4f10f8faec9d0d135274 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 19:18:04 2012 -0800 Flag cannibalized circs as used (non-ideal). Also add some comments. --- src/or/circuitbuild.c | 4 ++++ src/or/circuituse.c | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 1 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index c3a5827..7282d57 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1219,6 +1219,10 @@ pathbias_count_first_hop(origin_circuit_t *circ) return 0; } + // XXX: Technically, we could make this only count from the *second* hop.. + // Until we get per-hop MACs or a lower circ failure rate, this might be + // better from a false positive POV. Should we s/first_hop/circ_attempt/g? + // Then we can control this check from the consensus. if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) { /* Help track down the real cause of bug #6475: */ if (circ->has_opened && circ->path_state != PATH_STATE_DID_FIRST_HOP) { diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 0b799b1..781e984 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1160,6 +1160,17 @@ circuit_has_opened(origin_circuit_t *circ) { control_event_circuit_status(circ, CIRC_EVENT_BUILT, 0); + /* Cannibalized circuits count as used for path bias. + * (PURPOSE_GENERAL circs especially, since they are + * marked dirty and often go unused after preemptive + * building). */ + // XXX: Cannibalized now use RELAY_EARLY, which is visible + // to taggers end-to-end! We really need to probe these instead. + if (circ->has_opened && + circ->build_state->desired_path_len > DEFAULT_ROUTE_LEN) { + circ->path_state = PATH_STATE_USE_SUCCEEDED; + } + /* Remember that this circuit has finished building. Now if we start * it building again later (e.g. by extending it), we will know not * to consider its build time. */ @@ -1411,7 +1422,11 @@ circuit_launch_by_extend_info(uint8_t purpose, * * Same deal goes for client side introductions. Clients * can be manipulated to connect repeatedly to them - * (especially web clients). */ + * (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. + */ circ->path_state = PATH_STATE_USE_SUCCEEDED; /* This must be called before the purpose change */ pathbias_check_close(circ);

1 0

[tor/master] Change from first hop accounting to 2nd hop accounting
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 04866055e8dadc9eb5b09773b3bbdc81e3b4dbbf Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 20:02:41 2012 -0800 Change from first hop accounting to 2nd hop accounting This has several advantages, including more resilience to ambient failure. I still need to rename all the first_hop vars tho.. Saving that for a separate commit. --- src/or/circuitbuild.c | 34 +++++++++++++++++++++++++++++----- 1 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 7282d57..ad52a6c 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1146,6 +1146,34 @@ pathbias_state_to_string(path_state_t state) } /** + * This function decides if a circuit has progressed far enough to count + * as a circuit "attempt". As long as end-to-end tagging is possible, + * we assume the adversary will use it over hop-to-hop failure. Therefore, + * we only need to account bias for the last hop. This should make us + * much more resilient to ambient circuit failure, and also make that + * failure easier to measure (we only need to measure Exit failure rates). + */ +static int +pathbias_is_new_circ_attempt(origin_circuit_t *circ) +{ +#define N2N_TAGGING_IS_POSSIBLE +#ifdef N2N_TAGGING_IS_POSSIBLE + /* cpath is a circular list. We want circs with more than one hop, + * and the second hop must be waiting for keys still (it's just + * about to get them). */ + return circ->cpath->next != circ->cpath && + circ->cpath->next->state == CPATH_STATE_AWAITING_KEYS; +#else + /* If tagging attacks are no longer possible, we probably want to + * count bias from the first hop. However, one could argue that + * timing-based tagging is still more useful than per-hop failure. + * In which case, we'd never want to use this. + */ + return circ->cpath->state == CPATH_STATE_AWAITING_KEYS; +#endif +} + +/** * Decide if the path bias code should count a circuit. * * @returns 1 if we should count it, 0 otherwise. @@ -1219,11 +1247,7 @@ pathbias_count_first_hop(origin_circuit_t *circ) return 0; } - // XXX: Technically, we could make this only count from the *second* hop.. - // Until we get per-hop MACs or a lower circ failure rate, this might be - // better from a false positive POV. Should we s/first_hop/circ_attempt/g? - // Then we can control this check from the consensus. - if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) { + if (pathbias_is_new_circ_attempt(circ)) { /* Help track down the real cause of bug #6475: */ if (circ->has_opened && circ->path_state != PATH_STATE_DID_FIRST_HOP) { if ((rate_msg = rate_limit_log(&first_hop_notice_limit,

1 0

[tor/master] Space fixes.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 4590993ff3d4393caaa1d9d68d04cf0af95c23c7 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 23:47:04 2012 -0800 Space fixes. --- src/or/circuitbuild.c | 38 +++++++++++++++++++++----------------- src/or/circuitbuild.h | 1 + src/or/circuitlist.c | 6 +++--- src/or/circuituse.c | 4 ++-- src/or/connection_edge.c | 6 +++--- src/or/entrynodes.c | 4 ++-- src/or/entrynodes.h | 2 +- src/or/relay.c | 2 +- src/or/rendservice.c | 4 ++-- 9 files changed, 36 insertions(+), 31 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 349063d..a724006 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1199,7 +1199,7 @@ pathbias_should_count(origin_circuit_t *circ) circ->base_.purpose == CIRCUIT_PURPOSE_CONTROLLER || circ->base_.purpose == CIRCUIT_PURPOSE_S_CONNECT_REND || circ->base_.purpose == CIRCUIT_PURPOSE_S_REND_JOINED || - (circ->base_.purpose >= CIRCUIT_PURPOSE_C_INTRODUCING && + (circ->base_.purpose >= CIRCUIT_PURPOSE_C_INTRODUCING && circ->base_.purpose <= CIRCUIT_PURPOSE_C_INTRODUCE_ACKED)) { return 0; } @@ -1350,7 +1350,7 @@ pathbias_count_build_success(origin_circuit_t *circ) return; } - /* Don't count cannibalized/reused circs for path bias + /* Don't count cannibalized/reused circs for path bias * build success.. They get counted under use success */ if (!circ->has_opened) { if (circ->cpath && circ->cpath->extend_info) { @@ -1456,14 +1456,14 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) } else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE) == END_CIRC_REASON_CHANNEL_CLOSED && circ->n_chan && - circ->n_chan->reason_for_closing + circ->n_chan->reason_for_closing != CHANNEL_CLOSE_REQUESTED) { /* If we didn't close the channel ourselves, it could be bias */ /* FIXME: Only count bias if the network is live? * What about clock jumps/suspends? */ log_info(LD_CIRC, - "Circuit %d's channel closed without successful use for reason %d, " - "channel reason %d. Circuit purpose %d currently %s.", + "Circuit %d's channel closed without successful use for reason " + "%d, channel reason %d. Circuit purpose %d currently %s.", ocirc->global_identifier, reason, circ->n_chan->reason_for_closing, circ->purpose, circuit_state_to_string(circ->state)); @@ -1494,7 +1494,7 @@ pathbias_count_successful_close(origin_circuit_t *circ) } if (guard) { - /* In the long run: circuit_success ~= successful_circuit_close + + /* In the long run: circuit_success ~= successful_circuit_close + * circ_failure + stream_failure */ guard->successful_circuits_closed++; entry_guards_changed(); @@ -1511,7 +1511,7 @@ pathbias_count_successful_close(origin_circuit_t *circ) } /** - * Count a circuit that fails after it is built, but before it can + * Count a circuit that fails after it is built, but before it can * carry any traffic. * * This is needed because there are ways to destroy a @@ -1622,7 +1622,7 @@ pathbias_get_closed_count(entry_guard_t *guard) ocirc = TO_ORIGIN_CIRCUIT(circ); - if(!ocirc->cpath || !ocirc->cpath->extend_info) + if (!ocirc->cpath || !ocirc->cpath->extend_info) continue; if (ocirc->path_state >= PATH_STATE_BUILD_SUCCEEDED && @@ -1642,7 +1642,7 @@ pathbias_get_closed_count(entry_guard_t *guard) * if it should return guard->circ_successes or * guard->successful_circuits_closed. */ -double +double pathbias_get_success_count(entry_guard_t *guard) { if (pathbias_use_close_counts(get_options())) { @@ -1680,8 +1680,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, - (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)pathbias_get_closed_count(guard), + (int)guard->circ_attempts, (int)guard->circ_successes, + (int)guard->unusable_circuits, (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); guard->path_bias_disabled = 1; @@ -1698,8 +1699,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, - (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)pathbias_get_closed_count(guard), + (int)guard->circ_attempts, (int)guard->circ_successes, + (int)guard->unusable_circuits, (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } @@ -1716,8 +1718,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, - (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)pathbias_get_closed_count(guard), + (int)guard->circ_attempts, (int)guard->circ_successes, + (int)guard->unusable_circuits, (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } @@ -1732,8 +1735,9 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - (int)pathbias_get_closed_count(guard), (int)guard->circ_attempts, - (int)guard->circ_successes, (int)guard->unusable_circuits, + (int)pathbias_get_closed_count(guard), + (int)guard->circ_attempts, (int)guard->circ_successes, + (int)guard->unusable_circuits, (int)guard->collapsed_circuits, (int)guard->timeouts, (long)circ_times.close_ms/1000); } diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 53c9fe5..8cd61fa 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -59,3 +59,4 @@ void pathbias_count_timeout(origin_circuit_t *circ); void pathbias_check_close(origin_circuit_t *circ, int reason); #endif + diff --git a/src/or/circuitlist.c b/src/or/circuitlist.c index 6fab492..0ee2900 100644 --- a/src/or/circuitlist.c +++ b/src/or/circuitlist.c @@ -1040,9 +1040,9 @@ circuit_unlink_all_from_channel(channel_t *chan, int reason) if (circ->n_chan == chan) { circuit_set_n_circid_chan(circ, 0, NULL); mark = 1; - + /* If we didn't request this closure, pass the remote - * bit to mark_for_close. */ + * bit to mark_for_close. */ if (chan->reason_for_closing != CHANNEL_CLOSE_REQUESTED) reason |= END_CIRC_REASON_FLAG_REMOTE; } @@ -1352,7 +1352,7 @@ circuit_mark_for_close_(circuit_t *circ, int reason, int line, } reason = END_CIRC_REASON_NONE; } - + if (CIRCUIT_IS_ORIGIN(circ)) { pathbias_check_close(TO_ORIGIN_CIRCUIT(circ), reason); diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 381c2b0..d3b4801 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1161,7 +1161,7 @@ circuit_has_opened(origin_circuit_t *circ) control_event_circuit_status(circ, CIRC_EVENT_BUILT, 0); /* Cannibalized circuits count as used for path bias. - * (PURPOSE_GENERAL circs especially, since they are + * (PURPOSE_GENERAL circs especially, since they are * marked dirty and often go unused after preemptive * building). */ // XXX: Cannibalized now use RELAY_EARLY, which is visible @@ -1418,7 +1418,7 @@ circuit_launch_by_extend_info(uint8_t purpose, 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. + * because the rend point could be malicious. * * Same deal goes for client side introductions. Clients * can be manipulated to connect repeatedly to them diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index ca6060c..570ffe4 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2184,13 +2184,13 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply, endreason == END_STREAM_REASON_CONNRESET || endreason == END_STREAM_REASON_NOROUTE || endreason == END_STREAM_REASON_RESOURCELIMIT) { - if(!conn->edge_.on_circuit || + if (!conn->edge_.on_circuit || !CIRCUIT_IS_ORIGIN(conn->edge_.on_circuit)) { // DNS remaps can trigger this. So can failed hidden service // lookups. log_info(LD_BUG, - "(Harmless.) No origin circuit for successful SOCKS stream %ld. " - "Reason: %d", ENTRY_TO_CONN(conn)->global_identifier, endreason); + "No origin circuit for successful SOCKS stream %ld. Reason: " + "%d", ENTRY_TO_CONN(conn)->global_identifier, endreason); } else { TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->path_state = PATH_STATE_USE_SUCCEEDED; diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 96b075a..066dbec 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1031,7 +1031,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) } /* First try 3 params, then 2. */ - /* In the long run: circuit_success ~= successful_circuit_close + + /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ if (tor_sscanf(line->value, "%lf %lf %lf %lf %lf %lf", @@ -1197,7 +1197,7 @@ entry_guards_update_state(or_state_t *state) if (e->circ_attempts) { *next = line = tor_malloc_zero(sizeof(config_line_t)); line->key = tor_strdup("EntryGuardPathBias"); - /* In the long run: circuit_success ~= successful_circuit_close + + /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ tor_asprintf(&line->value, "%lf %lf %lf %lf %lf %lf", diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index b9d0e55..2686a4f 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -56,7 +56,7 @@ typedef struct entry_guard_t { double collapsed_circuits; /**< Number of fully built circuits that were * remotely closed before any streams were * attempted. */ - double unusable_circuits; /**< Number of circuits for which streams were + double unusable_circuits; /**< Number of circuits for which streams were * attempted, but none succeeded. */ double timeouts; /**< Number of 'right-censored' circuit timeouts for this * guard. */ diff --git a/src/or/relay.c b/src/or/relay.c index 1638dae..3ee0c83 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -694,7 +694,7 @@ connection_ap_process_end_not_open( (void) layer_hint; /* unused */ if (rh->length > 0) { - /* Path bias: If we get a valid reason code from the exit, + /* Path bias: If we get a valid reason code from the exit, * it wasn't due to tagging */ // XXX: This relies on recognized+digest being strong enough not // to be spoofable.. Is that a valid assumption? diff --git a/src/or/rendservice.c b/src/or/rendservice.c index fbf14e9..e70f969 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1383,7 +1383,7 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, if (circuit_init_cpath_crypto(cpath,keys+DIGEST_LEN,1)<0) goto err; memcpy(cpath->handshake_digest, keys, DIGEST_LEN); - + /* For path bias: This intro circuit was used successfully */ circuit->path_state = PATH_STATE_USE_SUCCEEDED; @@ -2586,7 +2586,7 @@ rend_service_rendezvous_has_opened(origin_circuit_t *circuit) tor_assert(circuit->rend_data); /* Declare the circuit dirty to avoid reuse, and for path-bias */ - if(!circuit->base_.timestamp_dirty) + if (!circuit->base_.timestamp_dirty) circuit->base_.timestamp_dirty = time(NULL); hop = circuit->build_state->service_pending_final_cpath_ref->cpath;

1 0

[tor/master] Allow any valid 'end' cell to mean a circuit was used successfully.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit 686fc222593fd46ec82d62f0fa62ca02900c1014 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sat Dec 8 16:37:22 2012 -0800 Allow any valid 'end' cell to mean a circuit was used successfully. Also improve some log messages. --- src/or/circuitbuild.c | 9 ++++++--- src/or/connection_edge.c | 17 +++++++++++------ src/or/relay.c | 8 ++++++++ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 9b1236f..c3a5827 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1411,16 +1411,18 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) // state.. Can we use that? Does optimistic data change this? log_info(LD_CIRC, - "Circuit closed without successful use for reason %d. " + "Circuit %d closed without successful use for reason %d. " "Circuit purpose %d currently %s.", + ocirc->global_identifier, reason, circ->purpose, circuit_state_to_string(circ->state)); pathbias_count_unusable(ocirc); } else { if (reason & END_CIRC_REASON_FLAG_REMOTE) { /* Unused remote circ close reasons all could be bias */ log_info(LD_CIRC, - "Circuit remote-closed without successful use for reason %d. " + "Circuit %d remote-closed without successful use for reason %d. " "Circuit purpose %d currently %s.", + ocirc->global_identifier, reason, circ->purpose, circuit_state_to_string(circ->state)); pathbias_count_collapse(ocirc); } else if ((reason & ~END_CIRC_REASON_FLAG_REMOTE) @@ -1432,8 +1434,9 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) /* FIXME: Only count bias if the network is live? * What about clock jumps/suspends? */ log_info(LD_CIRC, - "Circuit's channel closed without successful use for reason %d, " + "Circuit %d's channel closed without successful use for reason %d, " "channel reason %d. Circuit purpose %d currently %s.", + ocirc->global_identifier, reason, circ->n_chan->reason_for_closing, circ->purpose, circuit_state_to_string(circ->state)); pathbias_count_collapse(ocirc); diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 79bb54c..ca6060c 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -2178,14 +2178,19 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply, /* Flag this stream's circuit as having completed a stream successfully * (for path bias) */ - if (status == SOCKS5_SUCCEEDED) { + if (status == SOCKS5_SUCCEEDED || + endreason == END_STREAM_REASON_RESOLVEFAILED || + endreason == END_STREAM_REASON_CONNECTREFUSED || + endreason == END_STREAM_REASON_CONNRESET || + endreason == END_STREAM_REASON_NOROUTE || + endreason == END_STREAM_REASON_RESOURCELIMIT) { if(!conn->edge_.on_circuit || !CIRCUIT_IS_ORIGIN(conn->edge_.on_circuit)) { - // XXX: Weird. We hit this a lot, and yet have no unusable_circs. - // Maybe during addrmaps/resolves? - log_warn(LD_BUG, - "(Harmless.) No origin circuit for successful SOCKS stream. " - "Reason: %d", endreason); + // DNS remaps can trigger this. So can failed hidden service + // lookups. + log_info(LD_BUG, + "(Harmless.) No origin circuit for successful SOCKS stream %ld. " + "Reason: %d", ENTRY_TO_CONN(conn)->global_identifier, endreason); } else { TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->path_state = PATH_STATE_USE_SUCCEEDED; diff --git a/src/or/relay.c b/src/or/relay.c index 7f49299..1638dae 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -693,6 +693,14 @@ connection_ap_process_end_not_open( edge_connection_t *edge_conn = ENTRY_TO_EDGE_CONN(conn); (void) layer_hint; /* unused */ + if (rh->length > 0) { + /* Path bias: If we get a valid reason code from the exit, + * it wasn't due to tagging */ + // XXX: This relies on recognized+digest being strong enough not + // to be spoofable.. Is that a valid assumption? + circ->path_state = PATH_STATE_USE_SUCCEEDED; + } + if (rh->length > 0 && edge_reason_is_retriable(reason) && /* avoid retry if rend */ !connection_edge_is_rendezvous_stream(edge_conn)) {

1 0

[tor/master] Rename first_hop to circ_attempt.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit a90f165b83bc1603873308d7918e99057afdf72a Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 20:18:31 2012 -0800 Rename first_hop to circ_attempt. Since we've generalized what we can count from (first or second hop), we should generalize the variable and constant naming too. --- src/or/circuitbuild.c | 76 ++++++++++++++++++++++++------------------------ src/or/entrynodes.c | 12 ++++---- src/or/entrynodes.h | 2 +- src/or/or.h | 4 +- 4 files changed, 47 insertions(+), 47 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index ad52a6c..42964eb 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -59,7 +59,7 @@ static crypt_path_t *onion_next_hop_in_cpath(crypt_path_t *cpath); static int onion_extend_cpath(origin_circuit_t *circ); static int count_acceptable_nodes(smartlist_t *routers); static int onion_append_hop(crypt_path_t **head_ptr, extend_info_t *choice); -static int entry_guard_inc_first_hop_count(entry_guard_t *guard); +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); @@ -987,7 +987,7 @@ circuit_init_cpath_crypto(crypt_path_t *cpath, const char *key_data, return 0; } -/** The minimum number of first hop completions before we start +/** The minimum number of circuit attempts before we start * thinking about warning about path bias and dropping guards */ static int pathbias_get_min_circs(const or_options_t *options) @@ -1134,8 +1134,8 @@ pathbias_state_to_string(path_state_t state) switch (state) { case PATH_STATE_NEW_CIRC: return "new"; - case PATH_STATE_DID_FIRST_HOP: - return "first hop"; + case PATH_STATE_BUILD_ATTEMPTED: + return "build attempted"; case PATH_STATE_BUILD_SUCCEEDED: return "build succeeded"; case PATH_STATE_USE_SUCCEEDED: @@ -1230,17 +1230,17 @@ pathbias_should_count(origin_circuit_t *circ) } /** - * Check our circuit state to see if this is a successful first hop. - * If so, record it in the current guard's path bias first_hop count. + * Check our circuit state to see if this is a successful circuit attempt. + * If so, record it in the current guard's path bias circ_attempt count. * * Also check for several potential error cases for bug #6475. */ static int -pathbias_count_first_hop(origin_circuit_t *circ) +pathbias_count_circ_attempt(origin_circuit_t *circ) { -#define FIRST_HOP_NOTICE_INTERVAL (600) - static ratelim_t first_hop_notice_limit = - RATELIM_INIT(FIRST_HOP_NOTICE_INTERVAL); +#define CIRC_ATTEMPT_NOTICE_INTERVAL (600) + static ratelim_t circ_attempt_notice_limit = + RATELIM_INIT(CIRC_ATTEMPT_NOTICE_INTERVAL); char *rate_msg = NULL; if (!pathbias_should_count(circ)) { @@ -1249,8 +1249,8 @@ pathbias_count_first_hop(origin_circuit_t *circ) if (pathbias_is_new_circ_attempt(circ)) { /* Help track down the real cause of bug #6475: */ - if (circ->has_opened && circ->path_state != PATH_STATE_DID_FIRST_HOP) { - if ((rate_msg = rate_limit_log(&first_hop_notice_limit, + if (circ->has_opened && circ->path_state != PATH_STATE_BUILD_ATTEMPTED) { + if ((rate_msg = rate_limit_log(&circ_attempt_notice_limit, approx_time()))) { log_info(LD_BUG, "Opened circuit is in strange path state %s. " @@ -1277,14 +1277,14 @@ pathbias_count_first_hop(origin_circuit_t *circ) if (guard) { if (circ->path_state == PATH_STATE_NEW_CIRC) { - circ->path_state = PATH_STATE_DID_FIRST_HOP; + circ->path_state = PATH_STATE_BUILD_ATTEMPTED; - if (entry_guard_inc_first_hop_count(guard) < 0) { + if (entry_guard_inc_circ_attempt_count(guard) < 0) { /* Bogus guard; we already warned. */ return -END_CIRC_REASON_TORPROTOCOL; } } else { - if ((rate_msg = rate_limit_log(&first_hop_notice_limit, + if ((rate_msg = rate_limit_log(&circ_attempt_notice_limit, approx_time()))) { log_info(LD_BUG, "Unopened circuit has strange path state %s. " @@ -1297,7 +1297,7 @@ pathbias_count_first_hop(origin_circuit_t *circ) } } } else { - if ((rate_msg = rate_limit_log(&first_hop_notice_limit, + if ((rate_msg = rate_limit_log(&circ_attempt_notice_limit, approx_time()))) { log_info(LD_BUG, "Unopened circuit has no known guard. " @@ -1312,7 +1312,7 @@ pathbias_count_first_hop(origin_circuit_t *circ) } else { /* Help track down the real cause of bug #6475: */ if (circ->path_state == PATH_STATE_NEW_CIRC) { - if ((rate_msg = rate_limit_log(&first_hop_notice_limit, + if ((rate_msg = rate_limit_log(&circ_attempt_notice_limit, approx_time()))) { log_info(LD_BUG, "A %s circuit is in cpath state %d (opened: %d). " @@ -1359,12 +1359,12 @@ pathbias_count_build_success(origin_circuit_t *circ) } if (guard) { - if (circ->path_state == PATH_STATE_DID_FIRST_HOP) { + if (circ->path_state == PATH_STATE_BUILD_ATTEMPTED) { circ->path_state = PATH_STATE_BUILD_SUCCEEDED; guard->circuit_successes++; log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", - guard->circuit_successes, guard->first_hops, + guard->circuit_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); } else { if ((rate_msg = rate_limit_log(&success_notice_limit, @@ -1380,10 +1380,10 @@ pathbias_count_build_success(origin_circuit_t *circ) } } - if (guard->first_hops < guard->circuit_successes) { + if (guard->circ_attempts < guard->circuit_successes) { log_notice(LD_BUG, "Unexpectedly high circuit_successes (%u/%u) " "for guard %s=%s", - guard->circuit_successes, guard->first_hops, + guard->circuit_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); } /* In rare cases, CIRCUIT_PURPOSE_TESTING can get converted to @@ -1657,17 +1657,17 @@ pathbias_get_success_count(entry_guard_t *guard) * eliminate the guard. Return -1 if the guard looks no good; return 0 if the * guard looks fine. */ static int -entry_guard_inc_first_hop_count(entry_guard_t *guard) +entry_guard_inc_circ_attempt_count(entry_guard_t *guard) { const or_options_t *options = get_options(); entry_guards_changed(); - if (guard->first_hops > (unsigned)pathbias_get_min_circs(options)) { + if (guard->circ_attempts > (unsigned)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)/((double)guard->first_hops) + if (pathbias_get_success_count(guard)/((double)guard->circ_attempts) < pathbias_get_extreme_rate(options)) { /* Dropping is currently disabled by default. */ if (pathbias_get_dropguards(options)) { @@ -1680,7 +1680,7 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->first_hops, + pathbias_get_closed_count(guard), guard->circ_attempts, guard->circuit_successes, guard->unusable_circuits, guard->collapsed_circuits, guard->timeouts, (long)circ_times.close_ms/1000); @@ -1698,12 +1698,12 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->first_hops, + pathbias_get_closed_count(guard), guard->circ_attempts, guard->circuit_successes, guard->unusable_circuits, guard->collapsed_circuits, guard->timeouts, (long)circ_times.close_ms/1000); } - } else if (pathbias_get_success_count(guard)/((double)guard->first_hops) + } else if (pathbias_get_success_count(guard)/((double)guard->circ_attempts) < pathbias_get_warn_rate(options)) { if (!guard->path_bias_warned) { guard->path_bias_warned = 1; @@ -1716,12 +1716,12 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->first_hops, + pathbias_get_closed_count(guard), guard->circ_attempts, guard->circuit_successes, guard->unusable_circuits, guard->collapsed_circuits, guard->timeouts, (long)circ_times.close_ms/1000); } - } else if (pathbias_get_success_count(guard)/((double)guard->first_hops) + } else if (pathbias_get_success_count(guard)/((double)guard->circ_attempts) < pathbias_get_notice_rate(options)) { if (!guard->path_bias_noticed) { guard->path_bias_noticed = 1; @@ -1732,7 +1732,7 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard) "were unusable, %d collapsed, and %d timed out. For " "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), - pathbias_get_closed_count(guard), guard->first_hops, + pathbias_get_closed_count(guard), guard->circ_attempts, guard->circuit_successes, guard->unusable_circuits, guard->collapsed_circuits, guard->timeouts, (long)circ_times.close_ms/1000); @@ -1741,27 +1741,27 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard) } /* If we get a ton of circuits, just scale everything down */ - if (guard->first_hops > (unsigned)pathbias_get_scale_threshold(options)) { + if (guard->circ_attempts > (unsigned)pathbias_get_scale_threshold(options)) { const int scale_factor = pathbias_get_scale_factor(options); const int mult_factor = pathbias_get_mult_factor(options); /* Only scale if there will be no rounding error for our scaling * factors */ - if (((mult_factor*guard->first_hops) % scale_factor) == 0 && + if (((mult_factor*guard->circ_attempts) % scale_factor) == 0 && ((mult_factor*guard->circuit_successes) % scale_factor) == 0) { log_info(LD_CIRC, "Scaling pathbias counts to (%u/%u)*(%d/%d) for guard %s=%s", - guard->circuit_successes, guard->first_hops, mult_factor, + guard->circuit_successes, guard->circ_attempts, mult_factor, scale_factor, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); - guard->first_hops *= mult_factor; + guard->circ_attempts *= mult_factor; guard->circuit_successes *= mult_factor; guard->timeouts *= mult_factor; guard->successful_circuits_closed *= mult_factor; guard->collapsed_circuits *= mult_factor; guard->unusable_circuits *= mult_factor; - guard->first_hops /= scale_factor; + guard->circ_attempts /= scale_factor; guard->circuit_successes /= scale_factor; guard->timeouts /= scale_factor; guard->successful_circuits_closed /= scale_factor; @@ -1769,9 +1769,9 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard) guard->unusable_circuits /= scale_factor; } } - guard->first_hops++; + guard->circ_attempts++; log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", - guard->circuit_successes, guard->first_hops, guard->nickname, + guard->circuit_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); return 0; } @@ -1794,7 +1794,7 @@ circuit_finish_handshake(origin_circuit_t *circ, uint8_t reply_type, crypt_path_t *hop; int rv; - if ((rv = pathbias_count_first_hop(circ)) < 0) + if ((rv = pathbias_count_circ_attempt(circ)) < 0) return rv; if (circ->cpath->state == CPATH_STATE_AWAITING_KEYS) { diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 1e64aaf..14a1e3c 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1050,7 +1050,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) unusable = 0; } - node->first_hops = hop_cnt; + node->circ_attempts = hop_cnt; node->circuit_successes = success_cnt; node->successful_circuits_closed = successful_closed; @@ -1059,17 +1059,17 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) node->unusable_circuits = unusable; log_info(LD_GENERAL, "Read %u/%u path bias for node %s", - node->circuit_successes, node->first_hops, node->nickname); + node->circuit_successes, node->circ_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 ((node->circuit_successes/((double)node->first_hops) + if ((node->circuit_successes/((double)node->circ_attempts) < pathbias_get_extreme_rate(options)) && pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, "Path bias is too high (%u/%u); disabling node %s", - node->circuit_successes, node->first_hops, node->nickname); + node->circuit_successes, node->circ_attempts, node->nickname); } } else { @@ -1192,14 +1192,14 @@ entry_guards_update_state(or_state_t *state) d, e->chosen_by_version, t); next = &(line->next); } - if (e->first_hops) { + if (e->circ_attempts) { *next = line = tor_malloc_zero(sizeof(config_line_t)); line->key = tor_strdup("EntryGuardPathBias"); /* In the long run: circuit_success ~= successful_circuit_close + * collapsed_circuits + * unusable_circuits */ tor_asprintf(&line->value, "%u %u %u %u %u %u", - e->first_hops, e->circuit_successes, + e->circ_attempts, e->circuit_successes, pathbias_get_closed_count(e), e->collapsed_circuits, e->unusable_circuits, e->timeouts); next = &(line->next); diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index c3f7b14..b737dad 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -48,7 +48,7 @@ typedef struct entry_guard_t { time_t last_attempted; /**< 0 if we can connect to this guard, or the time * at which we last failed to connect to it. */ - unsigned first_hops; /**< Number of first hops this guard has completed */ + unsigned circ_attempts; /**< Number of circuits this guard has "attempted" */ unsigned circuit_successes; /**< Number of successfully built circuits using * this guard as first hop. */ unsigned successful_circuits_closed; /**< Number of circuits that carried diff --git a/src/or/or.h b/src/or/or.h index c8ea12f..aaf817d 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2765,9 +2765,9 @@ typedef enum { /** This circuit is "new". It has not yet completed a first hop * or been counted by the path bias code. */ PATH_STATE_NEW_CIRC = 0, - /** This circuit has completed a first hop, and has been counted by + /** This circuit has completed one/two hops, and has been counted by * the path bias logic. */ - PATH_STATE_DID_FIRST_HOP = 1, + PATH_STATE_BUILD_ATTEMPTED = 1, /** This circuit has been completely built */ PATH_STATE_BUILD_SUCCEEDED = 2, /** Did any SOCKS streams or hidserv introductions actually succeed on

1 0

[tor/master] Also shorten circuit_successes to circ_successes.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit ab1fce5c19b64b3f1ba15d6ffa1f0d11d6a959c3 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 20:20:44 2012 -0800 Also shorten circuit_successes to circ_successes. For consistency and great justice. Ok, mostly consistency. --- src/or/circuitbuild.c | 32 ++++++++++++++++---------------- src/or/entrynodes.c | 10 +++++----- src/or/entrynodes.h | 2 +- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 42964eb..04b429b 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1361,10 +1361,10 @@ pathbias_count_build_success(origin_circuit_t *circ) if (guard) { if (circ->path_state == PATH_STATE_BUILD_ATTEMPTED) { circ->path_state = PATH_STATE_BUILD_SUCCEEDED; - guard->circuit_successes++; + guard->circ_successes++; log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", - guard->circuit_successes, guard->circ_attempts, + guard->circ_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); } else { if ((rate_msg = rate_limit_log(&success_notice_limit, @@ -1380,10 +1380,10 @@ pathbias_count_build_success(origin_circuit_t *circ) } } - if (guard->circ_attempts < guard->circuit_successes) { - log_notice(LD_BUG, "Unexpectedly high circuit_successes (%u/%u) " + if (guard->circ_attempts < guard->circ_successes) { + log_notice(LD_BUG, "Unexpectedly high successes counts (%u/%u) " "for guard %s=%s", - guard->circuit_successes, guard->circ_attempts, + guard->circ_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); } /* In rare cases, CIRCUIT_PURPOSE_TESTING can get converted to @@ -1639,7 +1639,7 @@ pathbias_get_closed_count(entry_guard_t *guard) /** * This function checks the consensus parameters to decide - * if it should return guard->circuit_successes or + * if it should return guard->circ_successes or * guard->successful_circuits_closed. */ static int @@ -1648,7 +1648,7 @@ pathbias_get_success_count(entry_guard_t *guard) if (pathbias_use_close_counts(get_options())) { return pathbias_get_closed_count(guard); } else { - return guard->circuit_successes; + return guard->circ_successes; } } @@ -1681,7 +1681,7 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circuit_successes, guard->unusable_circuits, + guard->circ_successes, guard->unusable_circuits, guard->collapsed_circuits, guard->timeouts, (long)circ_times.close_ms/1000); guard->path_bias_disabled = 1; @@ -1699,7 +1699,7 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circuit_successes, guard->unusable_circuits, + guard->circ_successes, guard->unusable_circuits, guard->collapsed_circuits, guard->timeouts, (long)circ_times.close_ms/1000); } @@ -1717,7 +1717,7 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circuit_successes, guard->unusable_circuits, + guard->circ_successes, guard->unusable_circuits, guard->collapsed_circuits, guard->timeouts, (long)circ_times.close_ms/1000); } @@ -1733,7 +1733,7 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) "reference, your timeout cutoff is %ld seconds.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), pathbias_get_closed_count(guard), guard->circ_attempts, - guard->circuit_successes, guard->unusable_circuits, + guard->circ_successes, guard->unusable_circuits, guard->collapsed_circuits, guard->timeouts, (long)circ_times.close_ms/1000); } @@ -1747,22 +1747,22 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) /* Only scale if there will be no rounding error for our scaling * factors */ if (((mult_factor*guard->circ_attempts) % scale_factor) == 0 && - ((mult_factor*guard->circuit_successes) % scale_factor) == 0) { + ((mult_factor*guard->circ_successes) % scale_factor) == 0) { log_info(LD_CIRC, "Scaling pathbias counts to (%u/%u)*(%d/%d) for guard %s=%s", - guard->circuit_successes, guard->circ_attempts, mult_factor, + guard->circ_successes, guard->circ_attempts, mult_factor, scale_factor, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); guard->circ_attempts *= mult_factor; - guard->circuit_successes *= 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->circuit_successes /= scale_factor; + guard->circ_successes /= scale_factor; guard->timeouts /= scale_factor; guard->successful_circuits_closed /= scale_factor; guard->collapsed_circuits /= scale_factor; @@ -1771,7 +1771,7 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard) } guard->circ_attempts++; log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", - guard->circuit_successes, guard->circ_attempts, guard->nickname, + guard->circ_successes, guard->circ_attempts, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); return 0; } diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 14a1e3c..84764d1 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1051,7 +1051,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) } node->circ_attempts = hop_cnt; - node->circuit_successes = success_cnt; + node->circ_successes = success_cnt; node->successful_circuits_closed = successful_closed; node->timeouts = timeouts; @@ -1059,17 +1059,17 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) node->unusable_circuits = unusable; log_info(LD_GENERAL, "Read %u/%u path bias for node %s", - node->circuit_successes, node->circ_attempts, node->nickname); + node->circ_successes, node->circ_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 ((node->circuit_successes/((double)node->circ_attempts) + if ((node->circ_successes/((double)node->circ_attempts) < pathbias_get_extreme_rate(options)) && pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, "Path bias is too high (%u/%u); disabling node %s", - node->circuit_successes, node->circ_attempts, node->nickname); + node->circ_successes, node->circ_attempts, node->nickname); } } else { @@ -1199,7 +1199,7 @@ entry_guards_update_state(or_state_t *state) * collapsed_circuits + * unusable_circuits */ tor_asprintf(&line->value, "%u %u %u %u %u %u", - e->circ_attempts, e->circuit_successes, + e->circ_attempts, e->circ_successes, pathbias_get_closed_count(e), e->collapsed_circuits, e->unusable_circuits, e->timeouts); next = &(line->next); diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index b737dad..0e58802 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -49,7 +49,7 @@ typedef struct entry_guard_t { * at which we last failed to connect to it. */ unsigned circ_attempts; /**< Number of circuits this guard has "attempted" */ - unsigned circuit_successes; /**< Number of successfully built circuits using + unsigned circ_successes; /**< Number of successfully built circuits using * this guard as first hop. */ unsigned successful_circuits_closed; /**< Number of circuits that carried * streams successfully. */

1 0

[tor/master] Fix a rather serious use-count state bug.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit b75880d7b3d02f5c60bf2e215c6e84da4f3e1938 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 20:56:48 2012 -0800 Fix a rather serious use-count state bug. We need to use the success count or the use count depending on the consensus parameter. --- src/or/circuitbuild.c | 2 +- src/or/entrynodes.c | 4 ++-- src/or/entrynodes.h | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index cbc1af9..349063d 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1642,7 +1642,7 @@ pathbias_get_closed_count(entry_guard_t *guard) * if it should return guard->circ_successes or * guard->successful_circuits_closed. */ -static double +double pathbias_get_success_count(entry_guard_t *guard) { if (pathbias_use_close_counts(get_options())) { diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 21c09f7..96b075a 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1065,8 +1065,8 @@ 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 ((node->circ_successes/((double)node->circ_attempts) - < pathbias_get_extreme_rate(options)) && + if (pathbias_get_success_count(node)/node->circ_attempts + < pathbias_get_extreme_rate(options) && pathbias_get_dropguards(options)) { node->path_bias_disabled = 1; log_info(LD_GENERAL, diff --git a/src/or/entrynodes.h b/src/or/entrynodes.h index de8c60c..b9d0e55 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -112,6 +112,7 @@ 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); #endif

1 0

[tor/master] Clean up some XXX comments.
by nickm＠torproject.org 26 Dec '12

26 Dec '12

commit aa16d59ee7abdf7c3309c267052ae265e141e1b3 Author: Mike Perry <mikeperry-git(a)fscked.org> Date: Sun Dec 9 23:50:05 2012 -0800 Clean up some XXX comments. --- src/or/circuitbuild.c | 4 ---- src/or/circuituse.c | 1 + src/or/relay.c | 4 +++- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index a724006..f903bbf 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1434,10 +1434,6 @@ pathbias_check_close(origin_circuit_t *ocirc, int reason) if (circ->timestamp_dirty) { /* Any circuit where there were attempted streams but no successful * streams could be bias */ - // XXX: May open up attacks if the adversary can force connections - // on unresponsive hosts to use new circs. Vidalia displayes a "Retrying" - // state.. Can we use that? Does optimistic data change this? - log_info(LD_CIRC, "Circuit %d closed without successful use for reason %d. " "Circuit purpose %d currently %s.", diff --git a/src/or/circuituse.c b/src/or/circuituse.c index d3b4801..cb44bba 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -1166,6 +1166,7 @@ circuit_has_opened(origin_circuit_t *circ) * building). */ // XXX: Cannibalized now use RELAY_EARLY, which is visible // to taggers end-to-end! We really need to probe these instead. + // Don't forget to remove this check once that's done! if (circ->has_opened && circ->build_state->desired_path_len > DEFAULT_ROUTE_LEN) { circ->path_state = PATH_STATE_USE_SUCCEEDED; diff --git a/src/or/relay.c b/src/or/relay.c index 3ee0c83..6ed4b93 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -697,7 +697,9 @@ connection_ap_process_end_not_open( /* Path bias: If we get a valid reason code from the exit, * it wasn't due to tagging */ // XXX: This relies on recognized+digest being strong enough not - // to be spoofable.. Is that a valid assumption? + // to be spoofable.. Is that a valid assumption? + // Or more accurately: is it better than nothing? Can the attack + // be done offline? circ->path_state = PATH_STATE_USE_SUCCEEDED; }

1 0