nihilist/tor
1
0
Fork 0
mirror of https://gitlab.torproject.org/tpo/core/tor.git synced 2024-11-11 05:33:47 +01:00
Code Issues Packages Projects Releases Wiki Activity

Changes from Nick's code review 'part 1'

Browse Source 
I think this is actually his third code review of this branch so far.
This commit is contained in:
Mike Perry 2012-12-18 12:39:03 -08:00
parent ccaeef22e1
commit b0fc18c37e
5 changed files with 59 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

90
src/or/circuitbuild.c
View File

@ -1015,7 +1015,7 @@ pathbias_get_notice_rate(const or_options_t *options)
/* XXXX024 I'd like to have this be static again, but entrynodes.c needs it. */
/** The circuit success rate below which we issue a warn */
double
static double
pathbias_get_warn_rate(const or_options_t *options)
{
#define DFLT_PATH_BIAS_WARN_PCT 50
@ -1055,7 +1055,7 @@ pathbias_get_dropguards(const or_options_t *options)
return options->PathBiasDropGuards;
else
return networkstatus_get_param(NULL, "pb_dropguards",
DFLT_PATH_BIAS_DROP_GUARDS, 0, 100)/100.0;
DFLT_PATH_BIAS_DROP_GUARDS, 0, 1);
}
/**
@ -1078,9 +1078,10 @@ pathbias_get_scale_threshold(const or_options_t *options)
/**
* The scale factor is the denominator for our scaling
* of circuit counts for our path bias window. Note that
* we must be careful of the values we use here, as the
* code only scales in the event of no integer truncation.
* of circuit counts for our path bias window.
*
* Note that our use of doubles for the path bias state
* file means that powers of 2 work best here.
*/
static int
pathbias_get_scale_factor(const or_options_t *options)
@ -1301,7 +1302,7 @@ pathbias_count_circ_attempt(origin_circuit_t *circ)
} else {
if ((rate_msg = rate_limit_log(&circ_attempt_notice_limit,
approx_time()))) {
log_info(LD_BUG,
log_info(LD_CIRC,
"Unopened circuit has no known guard. "
"Circuit is a %s currently %s.%s",
circuit_purpose_to_string(circ->base_.purpose),
@ -1378,7 +1379,7 @@ pathbias_count_build_success(origin_circuit_t *circ)
} else if (circ->base_.purpose != CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT) {
if ((rate_msg = rate_limit_log(&success_notice_limit,
approx_time()))) {
log_info(LD_BUG,
log_info(LD_CIRC,
"Completed circuit has no known guard. "
"Circuit is a %s currently %s.%s",
circuit_purpose_to_string(circ->base_.purpose),
@ -1490,7 +1491,7 @@ pathbias_count_successful_close(origin_circuit_t *circ)
/* In rare cases, CIRCUIT_PURPOSE_TESTING can get converted to
* CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT and have no guards here.
* No need to log that case. */
log_info(LD_BUG,
log_info(LD_CIRC,
"Successfully closed circuit has no known guard. "
"Circuit is a %s currently %s",
circuit_purpose_to_string(circ->base_.purpose),
@ -1526,7 +1527,7 @@ pathbias_count_collapse(origin_circuit_t *circ)
/* In rare cases, CIRCUIT_PURPOSE_TESTING can get converted to
* CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT and have no guards here.
* No need to log that case. */
log_info(LD_BUG,
log_info(LD_CIRC,
"Destroyed circuit has no known guard. "
"Circuit is a %s currently %s",
circuit_purpose_to_string(circ->base_.purpose),
@ -1554,7 +1555,7 @@ pathbias_count_unusable(origin_circuit_t *circ)
/* In rare cases, CIRCUIT_PURPOSE_TESTING can get converted to
* CIRCUIT_PURPOSE_C_MEASURE_TIMEOUT and have no guards here.
* No need to log that case. */
log_info(LD_BUG,
log_info(LD_CIRC,
"Stream-failing circuit has no known guard. "
"Circuit is a %s currently %s",
circuit_purpose_to_string(circ->base_.purpose),
@ -1620,10 +1621,9 @@ pathbias_get_closed_count(entry_guard_t *guard)
continue;
if (ocirc->path_state >= PATH_STATE_BUILD_SUCCEEDED &&
(memcmp(guard->identity,
fast_memeq(guard->identity,
ocirc->cpath->extend_info->identity_digest,
DIGEST_LEN)
== 0)) {
DIGEST_LEN)) {
open_circuits++;
}
}
@ -1670,15 +1670,17 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard)
"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 %d/%d. %d circuits completed, %d "
"were unusable, %d collapsed, and %d timed out. For "
"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),
(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);
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));
guard->path_bias_disabled = 1;
guard->bad_since = approx_time();
return -1;
@ -1689,15 +1691,17 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard)
"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 %d/%d. %d circuits completed, %d "
"were unusable, %d collapsed, and %d timed out. For "
"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),
(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);
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));
}
} else if (pathbias_get_success_count(guard)/((double)guard->circ_attempts)
< pathbias_get_warn_rate(options)) {
@ -1708,15 +1712,17 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard)
"circuits. Most likely this means the Tor network is "
"overloaded, but it could also mean an attack against "
"you or the potentially the guard itself. "
"Success counts are %d/%d. %d circuits completed, %d "
"were unusable, %d collapsed, and %d timed out. For "
"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),
(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);
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));
}
} else if (pathbias_get_success_count(guard)/((double)guard->circ_attempts)
< pathbias_get_notice_rate(options)) {
@ -1725,15 +1731,17 @@ entry_guard_inc_circ_attempt_count(entry_guard_t *guard)
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 %d/%d. %d circuits completed, %d "
"were unusable, %d collapsed, and %d timed out. For "
"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),
(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);
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));
}
}
}

4
src/or/config.c
View File

@ -322,8 +322,8 @@ static config_var_t option_vars_[] = {
V(PathBiasScaleThreshold, INT, "-1"),
V(PathBiasScaleFactor, INT, "-1"),
V(PathBiasMultFactor, INT, "-1"),
V(PathBiasDropGuards, BOOL, "0"),
V(PathBiasUseCloseCounts, BOOL, "1"),
V(PathBiasDropGuards, AUTOBOOL, "0"),
V(PathBiasUseCloseCounts, AUTOBOOL, "1"),
OBSOLETE("PathlenCoinWeight"),
V(PerConnBWBurst, MEMUNIT, "0"),

2
src/or/connection_edge.c
View File

@ -2189,7 +2189,7 @@ connection_ap_handshake_socks_reply(entry_connection_t *conn, char *reply,
// DNS remaps can trigger this. So can failed hidden service
// lookups.
log_info(LD_BUG,
"No origin circuit for successful SOCKS stream %ld. Reason: "
"No origin circuit for successful SOCKS stream %lu. Reason: "
"%d", ENTRY_TO_CONN(conn)->global_identifier, endreason);
} else {
TO_ORIGIN_CIRCUIT(conn->edge_.on_circuit)->path_state

2
src/or/entrynodes.c
View File

@ -1194,7 +1194,7 @@ entry_guards_update_state(or_state_t *state)
d, e->chosen_by_version, t);
next = &(line->next);
}
if (e->circ_attempts) {
if (e->circ_attempts > 0) {
*next = line = tor_malloc_zero(sizeof(config_line_t));
line->key = tor_strdup("EntryGuardPathBias");
/* In the long run: circuit_success ~= successful_circuit_close +

12
src/or/relay.c
View File

@ -698,17 +698,17 @@ connection_ap_process_end_not_open(
reason == END_STREAM_REASON_INTERNAL ||
reason == END_STREAM_REASON_DESTROY) {
/* All three of these reasons could mean a failed tag
* hit the exit and it shat itself. Do not probe.
* hit the exit and it complained. Do not probe.
* Fail the circuit. */
circ->path_state = PATH_STATE_USE_FAILED;
return -END_CIRC_REASON_TORPROTOCOL;
} else {
/* 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?
// Or more accurately: is it better than nothing? Can the attack
// be done offline?
* it wasn't due to tagging.
*
* 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;
}
}