Commit bb548134 authored by Mike Perry's avatar Mike Perry
Browse files

Update with code review changes from Nick.

parent 19299669
......@@ -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;
......
......@@ -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
......@@ -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"),
......
......@@ -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,
......
......@@ -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
......
......@@ -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(). */
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment