From 6f6881c4324f35d44b997591939de7e847cca7a3 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 20 Apr 2015 15:51:06 -0400 Subject: [PATCH 1/4] Use a random count of INTRODUCE2 for IP rotation An introduction point is currently rotated when the amount of INTRODUCE2 cells reached a fixed value of 16384. This makes it pretty easy for an attacker to inflate that number and observe when the IP rotates which leaks the popularity of the HS (amount of client that passed through the IP). This commit makes it a random count between the current value of 16384 and two times that. Fixes #15745 Signed-off-by: David Goulet --- changes/bug15745 | 7 +++++++ src/or/or.h | 19 +++++++++++++------ src/or/rendservice.c | 17 +++++++++++------ 3 files changed, 31 insertions(+), 12 deletions(-) create mode 100644 changes/bug15745 diff --git a/changes/bug15745 b/changes/bug15745 new file mode 100644 index 0000000000..6e4bfa4e31 --- /dev/null +++ b/changes/bug15745 @@ -0,0 +1,7 @@ + o Minor feature (HS popularity countermeasure): + - To avoid leaking HS popularity, don't cycle the introduction point + when we've handled a fixed number of INTRODUCE2 cells but instead + cycle it when a random value of introductions is reached thus making + it more difficult for an attacker to find out the amount of clients + that has passed through the introduction point for a specific HS. + Closes ticket 15745. diff --git a/src/or/or.h b/src/or/or.h index d548aeabb6..4fd6d1d9f6 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4805,12 +4805,13 @@ typedef struct rend_encoded_v2_service_descriptor_t { * introduction point. See also rend_intro_point_t.unreachable_count. */ #define MAX_INTRO_POINT_REACHABILITY_FAILURES 5 -/** The maximum number of distinct INTRODUCE2 cells which a hidden - * service's introduction point will receive before it begins to - * expire. - * - * XXX023 Is this number at all sane? */ -#define INTRO_POINT_LIFETIME_INTRODUCTIONS 16384 +/** The minimum and maximum number of distinct INTRODUCE2 cells which a + * hidden service's introduction point will receive before it begins to + * expire. */ +#define INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS 16384 +/* Double the minimum value so the interval is [min, min * 2]. */ +#define INTRO_POINT_MAX_LIFETIME_INTRODUCTIONS \ + (INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS * 2) /** The minimum number of seconds that an introduction point will last * before expiring due to old age. (If it receives @@ -4864,6 +4865,12 @@ typedef struct rend_intro_point_t { */ int accepted_introduce2_count; + /** (Service side only) Number of maximum INTRODUCE2 cells that this IP + * will accept. This is a random value between + * INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS and + * INTRO_POINT_MAX_LIFETIME_INTRODUCTIONS. */ + unsigned int max_introductions; + /** (Service side only) The time at which this intro point was first * published, or -1 if this intro point has not yet been * published. */ diff --git a/src/or/rendservice.c b/src/or/rendservice.c index c1c0c46d17..cf0352cd3e 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1158,16 +1158,17 @@ rend_service_note_removing_intro_point(rend_service_t *service, /* This intro point was never used. Don't change * n_intro_points_wanted. */ } else { + /* We want to increase the number of introduction points service * operates if intro was heavily used, or decrease the number of * intro points if intro was lightly used. * * We consider an intro point's target 'usage' to be - * INTRO_POINT_LIFETIME_INTRODUCTIONS introductions in + * maximum of INTRODUCE2 cells divided by * INTRO_POINT_LIFETIME_MIN_SECONDS seconds. To calculate intro's - * fraction of target usage, we divide the fraction of - * _LIFETIME_INTRODUCTIONS introductions that it has handled by - * the fraction of _LIFETIME_MIN_SECONDS for which it existed. + * fraction of target usage, we divide the amount of INTRODUCE2 cells + * that it has handled by the fraction of _LIFETIME_MIN_SECONDS for + * which it existed. * * Then we multiply that fraction of desired usage by a fudge * factor of 1.5, to decide how many new introduction points @@ -1189,7 +1190,7 @@ rend_service_note_removing_intro_point(rend_service_t *service, intro_point_accepted_intro_count(intro) / (double)(now - intro->time_published); const double intro_point_target_usage = - INTRO_POINT_LIFETIME_INTRODUCTIONS / + intro->max_introductions / (double)INTRO_POINT_LIFETIME_MIN_SECONDS; const double fractional_n_intro_points_wanted_to_replace_this_one = (1.5 * (intro_point_usage / intro_point_target_usage)); @@ -3123,7 +3124,7 @@ intro_point_should_expire_now(rend_intro_point_t *intro, } if (intro_point_accepted_intro_count(intro) >= - INTRO_POINT_LIFETIME_INTRODUCTIONS) { + intro->max_introductions) { /* This intro point has been used too many times. Expire it now. */ return 1; } @@ -3335,6 +3336,10 @@ rend_services_introduce(void) intro->time_published = -1; intro->time_to_expire = -1; intro->time_expiring = -1; + intro->max_introductions = + INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS + + crypto_rand_int(INTRO_POINT_MAX_LIFETIME_INTRODUCTIONS - + INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS); smartlist_add(service->intro_nodes, intro); log_info(LD_REND, "Picked router %s as an intro point for %s.", safe_str_client(node_describe(node)), From 3f413184728c1d7b441b8e54585c43220665218c Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 21 Apr 2015 10:17:12 -0400 Subject: [PATCH 2/4] Add crypto_rand_int_range() and use it Incidently, this fixes a bug where the maximum value was never used when only using crypto_rand_int(). For instance this example below in rendservice.c never gets to INTRO_POINT_LIFETIME_MAX_SECONDS. int intro_point_lifetime_seconds = INTRO_POINT_LIFETIME_MIN_SECONDS + crypto_rand_int(INTRO_POINT_LIFETIME_MAX_SECONDS - INTRO_POINT_LIFETIME_MIN_SECONDS); Signed-off-by: David Goulet --- src/common/crypto.c | 21 ++++++++++++++++++++- src/common/crypto.h | 1 + src/common/tortls.c | 3 ++- src/or/entrynodes.c | 6 ++++-- src/or/main.c | 2 +- src/or/rendservice.c | 10 ++++------ src/or/router.c | 4 +++- 7 files changed, 35 insertions(+), 12 deletions(-) diff --git a/src/common/crypto.c b/src/common/crypto.c index f05be2e8bf..1c4eda90fd 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -2317,6 +2317,25 @@ crypto_rand_int(unsigned int max) } } +/** Return a pseudorandom integer, chosen uniformly from the values between + * min and max inclusive. + * + * min MUST be between 0 and max - 1. + * max MUST be bigger than min and <= to INT_MAX. + */ +int +crypto_rand_int_range(unsigned int min, unsigned int max) +{ + tor_assert(min <= max); + tor_assert(max <= INT_MAX); + + /* The overflow is avoided here because crypto_rand_int() returns a value + * between 0 and (max - min - 1) with max being <= INT_MAX and min <= max. + * This is why we add 1 to the maximum value so we can actually get max as + * a return value. */ + return min + crypto_rand_int(max - min + 1); +} + /** Return a pseudorandom 64-bit integer, chosen uniformly from the values * between 0 and max-1. */ uint64_t @@ -2379,7 +2398,7 @@ crypto_random_hostname(int min_rand_len, int max_rand_len, const char *prefix, if (min_rand_len > max_rand_len) min_rand_len = max_rand_len; - randlen = min_rand_len + crypto_rand_int(max_rand_len - min_rand_len + 1); + randlen = crypto_rand_int_range(min_rand_len, max_rand_len); prefixlen = strlen(prefix); resultlen = prefixlen + strlen(suffix) + randlen + 16; diff --git a/src/common/crypto.h b/src/common/crypto.h index b9c26a4551..3de3c7ea28 100644 --- a/src/common/crypto.h +++ b/src/common/crypto.h @@ -254,6 +254,7 @@ int crypto_seed_rng(int startup); MOCK_DECL(int,crypto_rand,(char *to, size_t n)); int crypto_strongest_rand(uint8_t *out, size_t out_len); int crypto_rand_int(unsigned int max); +int crypto_rand_int_range(unsigned int min, unsigned int max); uint64_t crypto_rand_uint64(uint64_t max); double crypto_rand_double(void); struct tor_weak_rng_t; diff --git a/src/common/tortls.c b/src/common/tortls.c index 32106eb2df..7809c1adaa 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -659,7 +659,8 @@ tor_tls_create_certificate(crypto_pk_t *rsa, * than having it start right now. Don't choose quite uniformly, since * then we might pick a time where we're about to expire. Lastly, be * sure to start on a day boundary. */ - start_time = time(NULL) - crypto_rand_int(cert_lifetime) + 2*24*3600; + time_t now = time(NULL); + start_time = crypto_rand_int_range(now - cert_lifetime, now) + 2*24*3600; start_time -= start_time % (24*3600); tor_assert(rsa); diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index 9663f34002..9f07d5ad69 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -440,7 +440,8 @@ 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); + time_t now = time(NULL); + entry->chosen_on_date = crypto_rand_int_range(now - 3600*24*30, now); entry->chosen_by_version = tor_strdup(VERSION); /* Are we picking this guard because all of our current guards are @@ -1439,8 +1440,9 @@ entry_guards_parse_state(or_state_t *state, int set, char **msg) } } else { if (state_version) { + time_t now = time(NULL); + e->chosen_on_date = crypto_rand_int_range(now - 3600*24*30, now); e->chosen_by_version = tor_strdup(state_version); - e->chosen_on_date = time(NULL) - crypto_rand_int(3600*24*30); } } if (e->path_bias_disabled && !e->bad_since) diff --git a/src/or/main.c b/src/or/main.c index 39c0f5c89e..b9009db151 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -1623,7 +1623,7 @@ run_scheduled_events(time_t now) time_to.check_for_correct_dns < now && ! router_my_exit_policy_is_reject_star()) { if (!time_to.check_for_correct_dns) { - time_to.check_for_correct_dns = now + 60 + crypto_rand_int(120); + time_to.check_for_correct_dns = crypto_rand_int_range(now, now + 120) + 60; } else { dns_launch_correctness_checks(); time_to.check_for_correct_dns = now + 12*3600 + diff --git a/src/or/rendservice.c b/src/or/rendservice.c index cf0352cd3e..4b2331ffca 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -3133,9 +3133,8 @@ intro_point_should_expire_now(rend_intro_point_t *intro, /* This intro point has been published, but we haven't picked an * expiration time for it. Pick one now. */ int intro_point_lifetime_seconds = - INTRO_POINT_LIFETIME_MIN_SECONDS + - crypto_rand_int(INTRO_POINT_LIFETIME_MAX_SECONDS - - INTRO_POINT_LIFETIME_MIN_SECONDS); + crypto_rand_int_range(INTRO_POINT_LIFETIME_MIN_SECONDS, + INTRO_POINT_LIFETIME_MAX_SECONDS); /* Start the expiration timer now, rather than when the intro * point was first published. There shouldn't be much of a time @@ -3337,9 +3336,8 @@ rend_services_introduce(void) intro->time_to_expire = -1; intro->time_expiring = -1; intro->max_introductions = - INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS + - crypto_rand_int(INTRO_POINT_MAX_LIFETIME_INTRODUCTIONS - - INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS); + crypto_rand_int_range(INTRO_POINT_MIN_LIFETIME_INTRODUCTIONS, + INTRO_POINT_MAX_LIFETIME_INTRODUCTIONS); smartlist_add(service->intro_nodes, intro); log_info(LD_REND, "Picked router %s as an intro point for %s.", safe_str_client(node_describe(node)), diff --git a/src/or/router.c b/src/or/router.c index b8bfd3cf6f..afe533ff9a 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -683,7 +683,9 @@ router_initialize_tls_context(void) if (!lifetime) { /* we should guess a good ssl cert lifetime */ /* choose between 5 and 365 days, and round to the day */ - lifetime = 5*24*3600 + crypto_rand_int(361*24*3600); + unsigned int five_days = 5*24*3600; + unsigned int one_year = 365*24*3600; + lifetime = crypto_rand_int_range(five_days, one_year); lifetime -= lifetime % (24*3600); if (crypto_rand_int(2)) { From 6bf31543dcda169aa1840fd7e0a0e0ff8a5f9640 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 21 Apr 2015 11:30:21 -0400 Subject: [PATCH 3/4] Make the crypto_rand_int_range return value right-exclusive. --- src/common/crypto.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/common/crypto.c b/src/common/crypto.c index 1c4eda90fd..24706ccb96 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -2317,23 +2317,23 @@ crypto_rand_int(unsigned int max) } } -/** Return a pseudorandom integer, chosen uniformly from the values between - * min and max inclusive. +/** Return a pseudorandom integer, chosen uniformly from the values i + * such that min <= i < max. * - * min MUST be between 0 and max - 1. - * max MUST be bigger than min and <= to INT_MAX. + * min MUST be in range [0, max). + * max MUST be in range (min, INT_MAX]. */ int crypto_rand_int_range(unsigned int min, unsigned int max) { - tor_assert(min <= max); + tor_assert(min < max); tor_assert(max <= INT_MAX); /* The overflow is avoided here because crypto_rand_int() returns a value * between 0 and (max - min - 1) with max being <= INT_MAX and min <= max. * This is why we add 1 to the maximum value so we can actually get max as * a return value. */ - return min + crypto_rand_int(max - min + 1); + return min + crypto_rand_int(max - min); } /** Return a pseudorandom 64-bit integer, chosen uniformly from the values @@ -2398,7 +2398,7 @@ crypto_random_hostname(int min_rand_len, int max_rand_len, const char *prefix, if (min_rand_len > max_rand_len) min_rand_len = max_rand_len; - randlen = crypto_rand_int_range(min_rand_len, max_rand_len); + randlen = crypto_rand_int_range(min_rand_len, max_rand_len+1); prefixlen = strlen(prefix); resultlen = prefixlen + strlen(suffix) + randlen + 16; From e48ad353a31f3ee376f7914563d960916ad6aecd Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 21 Apr 2015 11:41:31 -0400 Subject: [PATCH 4/4] Add test for random-int-in-range --- src/test/test_crypto.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index e9fb8bf084..9158390db5 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -105,6 +105,30 @@ test_crypto_rng(void *arg) ; } +static void +test_crypto_rng_range(void *arg) +{ + int got_smallest = 0, got_largest = 0; + int i; + + (void)arg; + for (i = 0; i < 1000; ++i) { + int x = crypto_rand_int_range(5,9); + tt_int_op(x, OP_GE, 5); + tt_int_op(x, OP_LT, 9); + if (x == 5) + got_smallest = 1; + if (x == 8) + got_largest = 1; + } + + /* These fail with probability 1/10^603. */ + tt_assert(got_smallest); + tt_assert(got_largest); + done: + ; +} + /** Run unit tests for our AES functionality */ static void test_crypto_aes(void *arg) @@ -1605,6 +1629,7 @@ test_crypto_siphash(void *arg) struct testcase_t crypto_tests[] = { CRYPTO_LEGACY(formats), CRYPTO_LEGACY(rng), + { "rng_range", test_crypto_rng_range, 0, NULL, NULL }, { "aes_AES", test_crypto_aes, TT_FORK, &passthrough_setup, (void*)"aes" }, { "aes_EVP", test_crypto_aes, TT_FORK, &passthrough_setup, (void*)"evp" }, CRYPTO_LEGACY(sha),