From 343f7aa05967df43c3f7e5b392b66e21c08b7cb1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 15 Feb 2013 17:24:13 -0500 Subject: [PATCH 1/4] Make the guard lifetime configurable and adjustable via the consensus Fixes 8240. (Don't actually increase the default guard lifetime. It seems likely to break too many things if done precipitiously.) --- changes/ticket8240 | 4 ++++ doc/tor.1.txt | 6 ++++++ src/common/util.h | 11 +++++++++++ src/or/circuitbuild.c | 36 ++++++++++++++++++++++++++++++++---- src/or/config.c | 1 + src/or/or.h | 3 +++ 6 files changed, 57 insertions(+), 4 deletions(-) create mode 100644 changes/ticket8240 diff --git a/changes/ticket8240 b/changes/ticket8240 new file mode 100644 index 0000000000..91e6f8c14a --- /dev/null +++ b/changes/ticket8240 @@ -0,0 +1,4 @@ + o Major security fixes: + - Make the default guard lifetime controllable via a new + GuardLifetime torrc option and a GuardLifetime consensus + parameter. Start of a fix for bug 8240; bugfix on 0.1.1.11-alpha. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 773fccf536..5cf5a718c0 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -959,6 +959,12 @@ The following options are useful only for clients (that is, if If UseEntryGuards is set to 1, we will try to pick a total of NUM routers as long-term entries for our circuits. (Default: 3) +**HeartbeatPeriod** __N__ **days**|**weeks**|**months**:: + If nonzero, and UseEntryGuards is set, minimum time to keep a guard before + picking a new one. If zero, we use the GuardLifetime parameter from the + consensus directory. No value here may be less than 2 months or greater + than 5 years; out-of-range values are clamped. (Default: 0) + **SafeSocks** **0**|**1**:: When this option is enabled, Tor will reject application connections that use unsafe variants of the socks protocol -- ones that only provide an IP diff --git a/src/common/util.h b/src/common/util.h index 8977d273c5..4642e40584 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -173,6 +173,17 @@ int n_bits_set_u8(uint8_t v); * overflow. */ #define CEIL_DIV(a,b) (((a)+(b)-1)/(b)) +/* Return v if it's between min and max. Otherwise + * return min if v is smaller than min, or max if + * b is larger than max. + * + * Requires that min is no more than max. May evaluate any of + * its arguments more than once! */ +#define CLAMP(min,v,max) \ + ( ((v) < (min)) ? (min) : \ + ((v) > (max)) ? (max) : \ + (v) ) + /* String manipulation */ /** Allowable characters in a hexadecimal string. */ diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index f8521c5cff..f07d428829 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4203,6 +4203,9 @@ control_event_guard_deferred(void) #endif } +/** Largest amount that we'll backdate chosen_on_date */ +#define CHOSEN_ON_DATE_SLOP (30*86400) + /** Add a new (preferably stable and fast) router to our * entry_guards list. Return a pointer to the router if we succeed, * or NULL if we can't find any more suitable entries. @@ -4241,7 +4244,7 @@ add_an_entry_guard(const node_t *chosen, int reset_status, int prepend) * don't all select them on the same day, and b) avoid leaving a * precise timestamp in the state file about when we first picked * this guard. For details, see the Jan 2010 or-dev thread. */ - entry->chosen_on_date = time(NULL) - crypto_rand_int(3600*24*30); + entry->chosen_on_date = time(NULL) - crypto_rand_int(CHOSEN_ON_DATE_SLOP); entry->chosen_by_version = tor_strdup(VERSION); if (prepend) smartlist_insert(entry_guards, 0, entry); @@ -4285,15 +4288,40 @@ entry_guard_free(entry_guard_t *e) tor_free(e); } +/** + * Return the minimum lifetime of working entry guard, in seconds, + * as given in the consensus networkstatus. + */ +static int32_t +guards_get_lifetime(void) +{ + const or_options_t *options = get_options(); +#define DFLT_GUARD_LIFETIME (86400 * 60) /* Two months. */ +#define MIN_GUARD_LIFETIME (86400 * 60) /* Two months. */ +#define MAX_GUARD_LIFETIME (86400 * 1826) /* Five years. */ + + if (options->GuardLifetime >= 1) { + return CLAMP(MIN_GUARD_LIFETIME, + options->GuardLifetime, + MAX_GUARD_LIFETIME) + CHOSEN_ON_DATE_SLOP; + } + + return networkstatus_get_param(NULL, "GuardLifetime", + DFLT_GUARD_LIFETIME, + MIN_GUARD_LIFETIME, + MAX_GUARD_LIFETIME) + CHOSEN_ON_DATE_SLOP; +} + /** Remove any entry guard which was selected by an unknown version of Tor, * or which was selected by a version of Tor that's known to select - * entry guards badly, or which was selected more 2 months ago. */ + * entry guards badly, or which was selected a long time ago */ /* XXXX The "obsolete guards" and "chosen long ago guards" things should * probably be different functions. */ static int remove_obsolete_entry_guards(time_t now) { int changed = 0, i; + int32_t guard_lifetime = guards_get_lifetime(); for (i = 0; i < smartlist_len(entry_guards); ++i) { entry_guard_t *entry = smartlist_get(entry_guards, i); @@ -4324,8 +4352,8 @@ remove_obsolete_entry_guards(time_t now) } tor_free(tor_ver); } - if (!version_is_bad && entry->chosen_on_date + 3600*24*60 < now) { - /* It's been 2 months since the date listed in our state file. */ + if (!version_is_bad && entry->chosen_on_date + guard_lifetime < now) { + /* It's been too long since the date listed in our state file. */ msg = "was selected several months ago"; date_is_bad = 1; } diff --git a/src/or/config.c b/src/or/config.c index 90a5dfbda1..6ccd65a57a 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -302,6 +302,7 @@ static config_var_t _option_vars[] = { #endif OBSOLETE("GiveGuardFlagTo_CVE_2011_2768_VulnerableRelays"), OBSOLETE("Group"), + V(GuardLifetime, INTERVAL, "0 minutes"), V(HardwareAccel, BOOL, "0"), V(HeartbeatPeriod, INTERVAL, "6 hours"), V(AccelName, STRING, NULL), diff --git a/src/or/or.h b/src/or/or.h index 51c23d305d..b54834de32 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3605,6 +3605,9 @@ typedef struct { int PathBiasScaleFactor; /** @} */ + /** How long (seconds) do we keep a guard before picking a new one? */ + int GuardLifetime; + } or_options_t; /** Persistent state for an onion router, as saved to disk. */ From aa040619d5d584d3dc1639b656f8ab6a619abe56 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 11 Mar 2013 21:25:37 -0400 Subject: [PATCH 2/4] Document the GuardLifetime option --- doc/tor.1.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 5cf5a718c0..5639ad26d4 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -959,7 +959,7 @@ The following options are useful only for clients (that is, if If UseEntryGuards is set to 1, we will try to pick a total of NUM routers as long-term entries for our circuits. (Default: 3) -**HeartbeatPeriod** __N__ **days**|**weeks**|**months**:: +**GuardLifetime** __N__ **days**|**weeks**|**months**:: If nonzero, and UseEntryGuards is set, minimum time to keep a guard before picking a new one. If zero, we use the GuardLifetime parameter from the consensus directory. No value here may be less than 2 months or greater From cf734a08f60f141ce2c21c703a403aeb74017b1f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 11 Mar 2013 21:35:50 -0400 Subject: [PATCH 3/4] Add support for days of the week to intervals --- src/or/config.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/or/config.c b/src/or/config.c index 6ccd65a57a..adbd367d12 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -6534,6 +6534,8 @@ static struct unit_table_t time_units[] = { { "days", 24*60*60 }, { "week", 7*24*60*60 }, { "weeks", 7*24*60*60 }, + { "months", 2629728, }, /* about 30.437 days */ + { "months", 2629728, }, { NULL, 0 }, }; From 18752bca5b57c11b6d843db671e1886ed0624848 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 11 Mar 2013 22:16:25 -0400 Subject: [PATCH 4/4] Drop the minimum guard lifetime back down to one month Mike believes that raising the default to 2 months with no way to lower it may create horrible load-balancing issues. --- doc/tor.1.txt | 2 +- src/or/circuitbuild.c | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 5639ad26d4..0c13a5c7d6 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -962,7 +962,7 @@ The following options are useful only for clients (that is, if **GuardLifetime** __N__ **days**|**weeks**|**months**:: If nonzero, and UseEntryGuards is set, minimum time to keep a guard before picking a new one. If zero, we use the GuardLifetime parameter from the - consensus directory. No value here may be less than 2 months or greater + consensus directory. No value here may be less than 1 month or greater than 5 years; out-of-range values are clamped. (Default: 0) **SafeSocks** **0**|**1**:: diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index f07d428829..d3a29fd0e3 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -4290,13 +4290,15 @@ entry_guard_free(entry_guard_t *e) /** * Return the minimum lifetime of working entry guard, in seconds, - * as given in the consensus networkstatus. + * as given in the consensus networkstatus. (Plus CHOSEN_ON_DATE_SLOP, + * so that we can do the chosen_on_date randomization while achieving the + * desired minimum lifetime.) */ static int32_t guards_get_lifetime(void) { const or_options_t *options = get_options(); -#define DFLT_GUARD_LIFETIME (86400 * 60) /* Two months. */ +#define DFLT_GUARD_LIFETIME (86400 * 30) /* One month. */ #define MIN_GUARD_LIFETIME (86400 * 60) /* Two months. */ #define MAX_GUARD_LIFETIME (86400 * 1826) /* Five years. */