commit bb548134cd4fd7b4b330cea15111ff257859bba8 Author: Mike Perry mikeperry-git@fscked.org Date: Wed Oct 31 18:49:49 2012 -0700

Update with code review changes from Nick. --- src/or/circuitbuild.c | 113 ++++++++++++++++++++++++++++++++---------------- src/or/circuitbuild.h | 3 +- src/or/config.c | 3 +- src/or/entrynodes.c | 2 +- src/or/entrynodes.h | 4 +- src/or/or.h | 4 +- 6 files changed, 83 insertions(+), 46 deletions(-)

diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index 4e9a974..feb8e9c 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -994,6 +994,7 @@ pathbias_get_min_circs(const or_options_t *options) 5, INT32_MAX); }

+/** The circuit success rate below which we issue a notice */ static double pathbias_get_notice_rate(const or_options_t *options) { @@ -1006,6 +1007,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 pathbias_get_warn_rate(const or_options_t *options) { @@ -1018,18 +1020,26 @@ pathbias_get_warn_rate(const or_options_t *options) }

/* XXXX024 I'd like to have this be static again, but entrynodes.c needs it. */ +/** + * The extreme rate is the rate at which we would drop the guard, + * if pb_dropguard is also set. Otherwise we just warn. + */ double -pathbias_get_crit_rate(const or_options_t *options) +pathbias_get_extreme_rate(const or_options_t *options) { -#define DFLT_PATH_BIAS_CRIT_PCT 30 - if (options->PathBiasCritRate >= 0.0) - return options->PathBiasCritRate; +#define DFLT_PATH_BIAS_EXTREME_PCT 30 + if (options->PathBiasExtremeRate >= 0.0) + return options->PathBiasExtremeRate; else - return networkstatus_get_param(NULL, "pb_critpct", - DFLT_PATH_BIAS_CRIT_PCT, 0, 100)/100.0; + return networkstatus_get_param(NULL, "pb_extremepct", + DFLT_PATH_BIAS_EXTREME_PCT, 0, 100)/100.0; }

/* XXXX024 I'd like to have this be static again, but entrynodes.c needs it. */ +/** + * If 1, we actually disable use of guards that fall below + * the extreme_pct. + */ int pathbias_get_dropguards(const or_options_t *options) { @@ -1041,6 +1051,12 @@ pathbias_get_dropguards(const or_options_t *options) DFLT_PATH_BIAS_DROP_GUARDS, 0, 100)/100.0; }

+/** + * This is the number of circuits at which we scale our + * 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_get_scale_threshold(const or_options_t *options) { @@ -1053,6 +1069,12 @@ pathbias_get_scale_threshold(const or_options_t *options) INT32_MAX); }

+/** + * 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. + */ static int pathbias_get_scale_factor(const or_options_t *options) { @@ -1064,6 +1086,11 @@ pathbias_get_scale_factor(const or_options_t *options) DFLT_PATH_BIAS_SCALE_FACTOR, 1, INT32_MAX); }

+/** + * The mult factor is the numerator for our scaling + * of circuit counts for our path bias window. It + * allows us to scale by fractions. + */ static int pathbias_get_mult_factor(const or_options_t *options) { @@ -1076,6 +1103,9 @@ pathbias_get_mult_factor(const or_options_t *options) pathbias_get_scale_factor(options)-1); }

+/** + * Convert a Guard's path state to string. + */ static const char * pathbias_state_to_string(path_state_t state) { @@ -1262,7 +1292,7 @@ pathbias_count_success(origin_circuit_t *circ) circ->path_state = PATH_STATE_SUCCEEDED; guard->circuit_successes++;

- log_info(LD_PROTOCOL, "Got success count %u/%u for guard %s=%s", + log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", guard->circuit_successes, guard->first_hops, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); } else { @@ -1353,26 +1383,29 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard) * rate and disable the feature entirely. If refactoring, don't * change to <= */ if (guard->circuit_successes/((double)guard->first_hops) - < pathbias_get_crit_rate(options) - && !guard->path_bias_crited) { - guard->path_bias_crited = 1; - + < pathbias_get_extreme_rate(options)) { + /* Dropping is currently disabled by default. */ if (pathbias_get_dropguards(options)) { - /* This message is currently disabled by default. */ - log_warn(LD_PROTOCOL, + if (!guard->path_bias_disabled) { + log_warn(LD_CIRC, "Your Guard %s=%s is failing an extremely large amount of " - "circuits. Tor has disabled use of this guard. Success " + "circuits. To avoid potential route manipluation attacks, " + "Tor has disabled use of this guard. Success " "counts are %d/%d, with %d timeouts. For reference, your " "timeout cutoff is %ld.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), guard->circuit_successes, guard->first_hops, guard->timeouts, (long)circ_times.close_ms/1000); - guard->path_bias_disabled = 1; - guard->bad_since = approx_time(); - } else { - log_warn(LD_PROTOCOL, + guard->path_bias_disabled = 1; + guard->bad_since = approx_time(); + } + } 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. Success counts are %d/%d, with %d timeouts. " + "circuits. This could indicate a route manipulation attack, " + "extreme network overload, or a bug. " + "Success counts are %d/%d, with %d timeouts. " "For reference, your timeout cutoff is %ld.", guard->nickname, hex_str(guard->identity, DIGEST_LEN), guard->circuit_successes, guard->first_hops, guard->timeouts, @@ -1380,10 +1413,10 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard) } return -1; } else if (guard->circuit_successes/((double)guard->first_hops) - < pathbias_get_warn_rate(options) - && !guard->path_bias_warned) { - guard->path_bias_warned = 1; - log_warn(LD_PROTOCOL, + < 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 " "overloaded, but it could also mean an attack against " @@ -1393,18 +1426,20 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard) guard->nickname, hex_str(guard->identity, DIGEST_LEN), guard->circuit_successes, guard->first_hops, guard->timeouts, (long)circ_times.close_ms/1000); + } } else if (guard->circuit_successes/((double)guard->first_hops) - < pathbias_get_notice_rate(options) - && !guard->path_bias_noticed) { - guard->path_bias_noticed = 1; - log_notice(LD_PROTOCOL, - "Your Guard %s=%s is failing more circuits than usual. Most " - "likely this means the Tor network is overloaded. Success " - "counts are %d/%d, with %d timeouts. For reference, your " - "timeout cutoff is %ld.", - guard->nickname, hex_str(guard->identity, DIGEST_LEN), - guard->circuit_successes, guard->first_hops, guard->timeouts, - (long)circ_times.close_ms/1000); + < 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 %d/%d, with %d timeouts. For " + "reference, your timeout cutoff is %ld.", + guard->nickname, hex_str(guard->identity, DIGEST_LEN), + guard->circuit_successes, guard->first_hops, + guard->timeouts, (long)circ_times.close_ms/1000); + } } }

@@ -1412,24 +1447,26 @@ entry_guard_inc_first_hop_count(entry_guard_t *guard) if (guard->first_hops > (unsigned)pathbias_get_scale_threshold(options)) { const int scale_factor = pathbias_get_scale_factor(options); const int mult_factor = pathbias_get_mult_factor(options); - /* For now, only scale if there will be no rounding error... - * XXX024: We want to switch to a real moving average for 0.2.4. */ + /* Only scale if there will be no rounding error for our scaling + * factors */ if (((mult_factor*guard->first_hops) % scale_factor) == 0 && ((mult_factor*guard->circuit_successes) % scale_factor) == 0) { - log_info(LD_PROTOCOL, + log_info(LD_CIRC, "Scaling pathbias counts to (%u/%u)*(%d/%d) for guard %s=%s", guard->circuit_successes, guard->first_hops, mult_factor, scale_factor, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); guard->first_hops *= mult_factor; guard->circuit_successes *= mult_factor; + guard->timeouts *= mult_factor;

guard->first_hops /= scale_factor; guard->circuit_successes /= scale_factor; + guard->timeouts /= scale_factor; } } guard->first_hops++; - log_info(LD_PROTOCOL, "Got success count %u/%u for guard %s=%s", + log_info(LD_CIRC, "Got success count %u/%u for guard %s=%s", guard->circuit_successes, guard->first_hops, guard->nickname, hex_str(guard->identity, DIGEST_LEN)); return 0; diff --git a/src/or/circuitbuild.h b/src/or/circuitbuild.h index 52f9f79..3338c9e 100644 --- a/src/or/circuitbuild.h +++ b/src/or/circuitbuild.h @@ -53,8 +53,9 @@ 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_crit_rate(const or_options_t *options); +double pathbias_get_extreme_rate(const or_options_t *options); int pathbias_get_dropguards(const or_options_t *options); +void pathbias_count_timeout(origin_circuit_t *circ);

#endif

diff --git a/src/or/config.c b/src/or/config.c index fffe14c..fb95642 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -314,10 +314,11 @@ static config_var_t option_vars_[] = { VPORT(ORPort, LINELIST, NULL), V(OutboundBindAddress, LINELIST, NULL),

+ OBSOLETE("PathBiasDisableRate"), V(PathBiasCircThreshold, INT, "-1"), V(PathBiasNoticeRate, DOUBLE, "-1"), V(PathBiasWarnRate, DOUBLE, "-1"), - V(PathBiasCritRate, DOUBLE, "-1"), + V(PathBiasExtremeRate, DOUBLE, "-1"), V(PathBiasScaleThreshold, INT, "-1"), V(PathBiasScaleFactor, INT, "-1"), V(PathBiasMultFactor, INT, "-1"), diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index faf5269..317a088 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -1049,7 +1049,7 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) * rate and disable the feature entirely. If refactoring, don't * change to <= */ if ((node->circuit_successes/((double)node->first_hops) - < pathbias_get_crit_rate(options)) && + < 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 dfabacb..f087f90 100644 --- a/src/or/entrynodes.h +++ b/src/or/entrynodes.h @@ -35,8 +35,8 @@ typedef struct entry_guard_t { * bias for this node already? */ unsigned int path_bias_warned : 1; /**< Did we alert the user about path bias * for this node already? */ - unsigned int path_bias_crited : 1; /**< Did we alert the user about path bias - * for this node already? */ + unsigned int path_bias_extreme : 1; /**< Did we alert the user about path + * bias for this node already? */ unsigned int path_bias_disabled : 1; /**< Have we disabled this node because * of path bias issues? */ time_t bad_since; /**< 0 if this guard is currently usable, or the time at diff --git a/src/or/or.h b/src/or/or.h index cef5201..a5e96b2 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3785,7 +3785,7 @@ typedef struct { int PathBiasCircThreshold; double PathBiasNoticeRate; double PathBiasWarnRate; - double PathBiasCritRate; + double PathBiasExtremeRate; int PathBiasDropGuards; int PathBiasScaleThreshold; int PathBiasScaleFactor; @@ -4070,8 +4070,6 @@ typedef struct { double close_ms; } circuit_build_times_t;

-void pathbias_count_timeout(origin_circuit_t *circ); - /********************************* config.c ***************************/

/** An error from options_trial_assign() or options_init_from_string(). */