[tor-commits] [tor/master] Update with code review changes from Nick.
nickm at torproject.org
nickm at torproject.org
Wed Dec 26 04:34:54 UTC 2012
commit bb548134cd4fd7b4b330cea15111ff257859bba8
Author: Mike Perry <mikeperry-git at 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(). */
More information about the tor-commits
mailing list