November 2011 - tor-commits - lists.torproject.org

[tor/master] Use a more meaningful variable name
by nickm＠torproject.org 30 Nov '11

30 Nov '11

commit 290b4dc3d62d0a593babcc1801aa94df40e83640 Author: Robert Ransom <rransom.8774(a)gmail.com> Date: Sat Oct 15 12:16:27 2011 -0700 Use a more meaningful variable name --- src/or/rendservice.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index ce7295e..ec58d60 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1912,7 +1912,7 @@ rend_services_introduce(void) routerinfo_t *router; rend_service_t *service; rend_intro_point_t *intro; - int changed, prev_intro_nodes; + int intro_point_set_changed, prev_intro_nodes; unsigned int n_intro_points_to_open; smartlist_t *intro_routers; time_t now; @@ -1926,7 +1926,7 @@ rend_services_introduce(void) service = smartlist_get(rend_service_list, i); tor_assert(service); - changed = 0; + intro_point_set_changed = 0; if (now > service->intro_period_started+INTRO_CIRC_RETRY_PERIOD) { /* One period has elapsed; we can try building circuits again. */ service->intro_period_started = now; @@ -1955,7 +1955,7 @@ rend_services_introduce(void) } rend_intro_point_free(intro); smartlist_del(service->intro_nodes,j--); - changed = 1; + intro_point_set_changed = 1; } if (router) smartlist_add(intro_routers, router); @@ -1964,8 +1964,9 @@ rend_services_introduce(void) /* We have enough intro points, and the intro points we thought we had were * all connected. */ - if (!changed && (smartlist_len(service->intro_nodes) >= - (int)service->n_intro_points_wanted)) { /*XXX023 remove cast*/ + if (!intro_point_set_changed && + (smartlist_len(service->intro_nodes) >= + (int)service->n_intro_points_wanted)) { /*XXX023 remove cast*/ /* We have all our intro points! Start a fresh period and reset the * circuit count. */ service->intro_period_started = now; @@ -2001,7 +2002,7 @@ rend_services_introduce(void) n_intro_points_to_open); break; } - changed = 1; + intro_point_set_changed = 1; smartlist_add(intro_routers, router); intro = tor_malloc_zero(sizeof(rend_intro_point_t)); intro->extend_info = extend_info_from_router(router); @@ -2015,7 +2016,7 @@ rend_services_introduce(void) } /* If there's no need to launch new circuits, stop here. */ - if (!changed) + if (!intro_point_set_changed) continue; /* Establish new introduction points. */

1 0

[tor/master] Correct bogus comments
by nickm＠torproject.org 30 Nov '11

30 Nov '11

commit e46d56a9b4458370cb8de0c92e10688402749845 Author: Robert Ransom <rransom.8774(a)gmail.com> Date: Sat Oct 15 12:18:27 2011 -0700 Correct bogus comments The behaviour of rend_services_introduce here is likely as bogus as the comments were. --- src/or/rendservice.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index ec58d60..e89d689 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1961,14 +1961,15 @@ rend_services_introduce(void) smartlist_add(intro_routers, router); } - /* We have enough intro points, and the intro points we thought we had were - * all connected. - */ if (!intro_point_set_changed && (smartlist_len(service->intro_nodes) >= (int)service->n_intro_points_wanted)) { /*XXX023 remove cast*/ - /* We have all our intro points! Start a fresh period and reset the - * circuit count. */ + /* We have enough intro circuits in progress, and none of our + * intro circuits have died since the last call to + * rend_services_introduce! Start a fresh period and reset the + * circuit count. + * + * XXXX WTF? */ service->intro_period_started = now; service->n_intro_circuits_launched = 0; continue;

1 0

[tor/master] Use SMARTLIST_FOREACH_BEGIN and _END, not a for loop
by nickm＠torproject.org 30 Nov '11

30 Nov '11

commit ae9a831294bd9bc6129b447ea3e7e24ab210794c Author: Robert Ransom <rransom.8774(a)gmail.com> Date: Sat Oct 15 14:22:56 2011 -0700 Use SMARTLIST_FOREACH_BEGIN and _END, not a for loop --- src/or/rendservice.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index e89d689..30f5df7 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1940,8 +1940,7 @@ rend_services_introduce(void) /* Find out which introduction points we have in progress for this service. */ - for (j=0; j < smartlist_len(service->intro_nodes); ++j) { - intro = smartlist_get(service->intro_nodes, j); + SMARTLIST_FOREACH_BEGIN(service->intro_nodes, rend_intro_point_t *, intro){ router = router_get_by_digest(intro->extend_info->identity_digest); if (!router || !find_intro_circuit(intro, service->pk_digest)) { log_info(LD_REND,"Giving up on %s as intro point for %s.", @@ -1954,12 +1953,12 @@ rend_services_introduce(void) service->desc_is_dirty = now; } rend_intro_point_free(intro); - smartlist_del(service->intro_nodes,j--); + SMARTLIST_DEL_CURRENT(service->intro_nodes, intro); intro_point_set_changed = 1; } if (router) smartlist_add(intro_routers, router); - } + } SMARTLIST_FOREACH_END(intro); if (!intro_point_set_changed && (smartlist_len(service->intro_nodes) >=

1 0

[tor/master] Move the real INTRODUCE2 replay-detection cache into rend_intro_point_t
by nickm＠torproject.org 30 Nov '11

30 Nov '11

commit 1a52a947c557ac04ee96addff404dc50cf5c26eb Author: Robert Ransom <rransom.8774(a)gmail.com> Date: Sat Oct 29 17:02:53 2011 -0700 Move the real INTRODUCE2 replay-detection cache into rend_intro_point_t --- changes/per-intro-point-replay-cache | 7 +++++++ src/or/or.h | 9 ++++++--- src/or/rendcommon.c | 5 +++++ src/or/rendservice.c | 22 ++++++++-------------- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/changes/per-intro-point-replay-cache b/changes/per-intro-point-replay-cache new file mode 100644 index 0000000..f63e428 --- /dev/null +++ b/changes/per-intro-point-replay-cache @@ -0,0 +1,7 @@ + o Minor features: + + - Move the replay-detection cache for the RSA-encrypted parts of + INTRODUCE2 cells to the introduction point data structures. + Previously, we would use one replay-detection cache per hidden + service. Required by fix for bug 3460. + diff --git a/src/or/or.h b/src/or/or.h index 9c81d0e..b53220f 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3505,9 +3505,12 @@ typedef struct rend_intro_point_t { * included in the last HS descriptor we generated. */ unsigned int listed_in_last_desc : 1; - /** (Service side only) The number of INTRODUCE2 cells this intro - * point's circuit has received. */ - unsigned int introduction_count : 24; + /** (Service side only) A digestmap recording the INTRODUCE2 cells + * this intro point's circuit has received. Each key is the digest + * of the RSA-encrypted part of a received INTRODUCE2 cell; each + * value is a pointer to the time_t at which the cell was + * received. */ + digestmap_t *accepted_intros; /** (Service side only) The time at which this intro point was first * published, or -1 if this intro point has not yet been diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index c5bf881..0a478c1 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -440,6 +440,11 @@ rend_intro_point_free(rend_intro_point_t *intro) extend_info_free(intro->extend_info); crypto_free_pk_env(intro->intro_key); + + if (intro->accepted_intros != NULL) { + digestmap_free(intro->accepted_intros, _tor_free); + } + tor_free(intro); } diff --git a/src/or/rendservice.c b/src/or/rendservice.c index ee34edf..413d4f6 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1005,14 +1005,14 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, if (!service->accepted_intros) service->accepted_intros = digestmap_new(); + if (!intro_point->accepted_intros) + intro_point->accepted_intros = digestmap_new(); + { char pkpart_digest[DIGEST_LEN]; - /* Check for replay of PK-encrypted portion. It is slightly naughty to - use the same digestmap to check for this and for g^x replays, but - collisions are tremendously unlikely. - */ + /* Check for replay of PK-encrypted portion. */ crypto_digest(pkpart_digest, (char*)request+DIGEST_LEN, keylen); - access_time = digestmap_get(service->accepted_intros, pkpart_digest); + access_time = digestmap_get(intro_point->accepted_intros, pkpart_digest); if (access_time != NULL) { log_warn(LD_REND, "Possible replay detected! We received an " "INTRODUCE2 cell with same PK-encrypted part %d seconds ago. " @@ -1021,14 +1021,7 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, } access_time = tor_malloc(sizeof(time_t)); *access_time = now; - digestmap_set(service->accepted_intros, pkpart_digest, access_time); - } - - /* Record that we've received another INTRODUCE2 cell through this - * intro point. */ - ++(intro_point->introduction_count); - if (intro_point->introduction_count == 0) { - --(intro_point->introduction_count); + digestmap_set(intro_point->accepted_intros, pkpart_digest, access_time); } /* Next N bytes is encrypted with service key */ @@ -1935,7 +1928,8 @@ intro_point_should_expire_now(rend_intro_point_t *intro, return 1; } - if (intro->introduction_count >= INTRO_POINT_LIFETIME_INTRODUCTIONS) { + if (digestmap_size(intro->accepted_intros) >= + INTRO_POINT_LIFETIME_INTRODUCTIONS) { /* This intro point has been used too many times. Expire it now. */ return 1; }

1 0

[tor/master] Allow intro points to expire somewhat gracefully
by nickm＠torproject.org 30 Nov '11

30 Nov '11

commit 00885652db8146d992bcf96315a45e7820688145 Author: Robert Ransom <rransom.8774(a)gmail.com> Date: Sat Oct 15 15:40:28 2011 -0700 Allow intro points to expire somewhat gracefully The Right Way to expire an intro point is to establish a new one to replace it, publish a new descriptor that doesn't list any expiring intro points, and *then*, once our upload attempts for the new descriptor have ended (whether in success or failure), close the expiring intro points. Unfortunately, we can't find out when the new descriptor has actually been uploaded, so we'll have to settle for a five-minute timer. There should be no significant behaviour changes due to this commit (only a log-message change or two), despite the rather massive overhaul, so this commit doesn't include a changes/ file. (The commit that teaches intro_point_should_expire_now to return non-zero gets a changes/ file, though.) --- src/or/or.h | 10 ++++ src/or/rendservice.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 117 insertions(+), 8 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index e1bd25c..9e4cd89 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3493,6 +3493,16 @@ typedef struct rend_intro_point_t { * published, or -1 if this intro point has not yet been * published. */ time_t time_published; + + /** (Service side only) The time at which we decided that this intro + * point should start expiring, or -1 if this intro point is not yet + * expiring. + * + * This field also serves as a flag to indicate that we have decided + * to expire this intro point, in case intro_point_should_expire_now + * flaps (perhaps due to a clock jump; perhaps due to other + * weirdness, or even a (present or future) bug). */ + time_t time_expiring; } rend_intro_point_t; /** Information used to connect to a hidden service. Used on both the diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 30f5df7..fcbdff0 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -27,6 +27,9 @@ static origin_circuit_t *find_intro_circuit(rend_intro_point_t *intro, const char *pk_digest); static rend_intro_point_t *find_intro_point(origin_circuit_t *circ); +static int intro_point_should_expire_now(rend_intro_point_t *intro, + time_t now); + /** Represents the mapping from a virtual port of a rendezvous service to * a real port on some IP. */ @@ -53,6 +56,10 @@ typedef struct rend_service_port_config_t { * rendezvous point before giving up? */ #define MAX_REND_TIMEOUT 30 +/** How many seconds should we wait for new HS descriptors to reach + * our clients before we close an expiring intro point? */ +#define INTRO_POINT_EXPIRATION_GRACE_PERIOD 5*60 + /** Represents a single hidden service running at this OP. */ typedef struct rend_service_t { /* Fields specified in config file */ @@ -548,9 +555,16 @@ rend_service_update_descriptor(rend_service_t *service) /* This intro point won't be listed in the descriptor... */ intro_svc->listed_in_last_desc = 0; + if (intro_svc->time_expiring != -1) { + /* This intro point is expiring. Don't list it. */ + continue; + } + circ = find_intro_circuit(intro_svc, service->pk_digest); - if (!circ || circ->_base.purpose != CIRCUIT_PURPOSE_S_INTRO) + if (!circ || circ->_base.purpose != CIRCUIT_PURPOSE_S_INTRO) { + /* This intro point's circuit isn't finished yet. Don't list it. */ continue; + } /* ...unless this intro point is listed in the descriptor. */ intro_svc->listed_in_last_desc = 1; @@ -1901,6 +1915,20 @@ upload_service_descriptor(rend_service_t *service) service->desc_is_dirty = 0; } +/** Return non-zero iff intro should 'expire' now (i.e. we + * should stop publishing it in new descriptors and eventually close + * it). + * + * XXXX This is a dummy function for now. It will actually do + * something in a later commit. */ +static int +intro_point_should_expire_now(rend_intro_point_t *intro, + time_t now) +{ + (void)intro; (void)now; + return 0; +} + /** For every service, check how many intro points it currently has, and: * - Pick new intro points as necessary. * - Launch circuits to any new intro points. @@ -1913,6 +1941,7 @@ rend_services_introduce(void) rend_service_t *service; rend_intro_point_t *intro; int intro_point_set_changed, prev_intro_nodes; + unsigned int n_intro_points_unexpired; unsigned int n_intro_points_to_open; smartlist_t *intro_routers; time_t now; @@ -1926,7 +1955,16 @@ rend_services_introduce(void) service = smartlist_get(rend_service_list, i); tor_assert(service); + + /* intro_point_set_changed becomes non-zero iff the set of intro + * points to be published in service's descriptor has changed. */ intro_point_set_changed = 0; + + /* n_intro_points_unexpired collects the number of non-expiring + * intro points we have, so that we know how many new intro + * circuits we need to launch for this service. */ + n_intro_points_unexpired = 0; + if (now > service->intro_period_started+INTRO_CIRC_RETRY_PERIOD) { /* One period has elapsed; we can try building circuits again. */ service->intro_period_started = now; @@ -1941,28 +1979,78 @@ rend_services_introduce(void) /* Find out which introduction points we have in progress for this service. */ SMARTLIST_FOREACH_BEGIN(service->intro_nodes, rend_intro_point_t *, intro){ + origin_circuit_t *intro_circ = + find_intro_circuit(intro, service->pk_digest); + + if (intro->time_expiring + INTRO_POINT_EXPIRATION_GRACE_PERIOD > now) { + /* This intro point has completely expired. Remove it, and + * mark the circuit for close if it's still alive. */ + if (intro_circ != NULL) { + circuit_mark_for_close(TO_CIRCUIT(intro_circ), + END_CIRC_REASON_FINISHED); + } + rend_intro_point_free(intro); + intro = NULL; /* SMARTLIST_DEL_CURRENT takes a name, not a value. */ + SMARTLIST_DEL_CURRENT(service->intro_nodes, intro); + /* We don't need to set intro_point_set_changed here, because + * this intro point wouldn't have been published in a current + * descriptor anyway. */ + continue; + } + router = router_get_by_digest(intro->extend_info->identity_digest); - if (!router || !find_intro_circuit(intro, service->pk_digest)) { - log_info(LD_REND,"Giving up on %s as intro point for %s.", + if (!router || !intro_circ) { + int removing_this_intro_point_changes_the_intro_point_set = 1; + log_info(LD_REND, "Giving up on %s as intro point for %s" + " (circuit disappeared).", safe_str_client(extend_info_describe(intro->extend_info)), safe_str_client(service->service_id)); - if (intro->listed_in_last_desc) { + if (intro->time_expiring != -1) { + log_info(LD_REND, "We were already expiring the intro point; " + "no need to mark the HS descriptor as dirty over this."); + removing_this_intro_point_changes_the_intro_point_set = 0; + } else if (intro->listed_in_last_desc) { log_info(LD_REND, "The intro point we are giving up on was " "included in the last published descriptor. " "Marking current descriptor as dirty."); service->desc_is_dirty = now; } rend_intro_point_free(intro); + intro = NULL; /* SMARTLIST_DEL_CURRENT takes a name, not a value. */ SMARTLIST_DEL_CURRENT(service->intro_nodes, intro); + if (removing_this_intro_point_changes_the_intro_point_set) + intro_point_set_changed = 1; + } + + if (intro != NULL && intro_point_should_expire_now(intro, now)) { + log_info(LD_REND, "Expiring %s as intro point for %s.", + safe_str_client(extend_info_describe(intro->extend_info)), + safe_str_client(service->service_id)); + + /* The polite (and generally Right) way to expire an intro + * point is to establish a new one to replace it, publish a + * new descriptor that doesn't list any expiring intro points, + * and *then*, once our upload attempts for the new descriptor + * have ended (whether in success or failure), close the + * expiring intro points. + * + * Unfortunately, we can't find out when the new descriptor + * has actually been uploaded, so we'll have to settle for a + * five-minute timer. Start it. XXX023 This sucks. */ + intro->time_expiring = now; + intro_point_set_changed = 1; } + + if (intro != NULL && intro->time_expiring == -1) + ++n_intro_points_unexpired; + if (router) smartlist_add(intro_routers, router); } SMARTLIST_FOREACH_END(intro); if (!intro_point_set_changed && - (smartlist_len(service->intro_nodes) >= - (int)service->n_intro_points_wanted)) { /*XXX023 remove cast*/ + (n_intro_points_unexpired >= service->n_intro_points_wanted)) { /* We have enough intro circuits in progress, and none of our * intro circuits have died since the last call to * rend_services_introduce! Start a fresh period and reset the @@ -1974,8 +2062,16 @@ rend_services_introduce(void) continue; } - /* Remember how many introduction circuits we started with. */ + /* Remember how many introduction circuits we started with. + * + * prev_intro_nodes serves a different purpose than + * n_intro_points_unexpired -- this variable tells us where our + * previously-created intro points end and our new ones begin in + * the intro-point list, so we don't have to launch the circuits + * at the same time as we create the intro points they correspond + * to. XXXX This is daft. */ prev_intro_nodes = smartlist_len(service->intro_nodes); + /* We have enough directory information to start establishing our * intro points. We want to end up with n_intro_points_wanted * intro points, but if we're just starting, we launch two extra @@ -1988,7 +2084,9 @@ rend_services_introduce(void) * in progress" loop. */ n_intro_points_to_open = (service->n_intro_points_wanted + (prev_intro_nodes == 0 ? 2 : 0)); - for (j=prev_intro_nodes; j < (int)n_intro_points_to_open; ++j) { /* XXXX remove cast */ + for (j = (int)n_intro_points_unexpired; + j < (int)n_intro_points_to_open; + ++j) { /* XXXX remove casts */ router_crn_flags_t flags = CRN_NEED_UPTIME; if (get_options()->_AllowInvalid & ALLOW_INVALID_INTRODUCTION) flags |= CRN_ALLOW_INVALID; @@ -2009,6 +2107,7 @@ rend_services_introduce(void) intro->intro_key = crypto_new_pk_env(); tor_assert(!crypto_pk_generate_key(intro->intro_key)); intro->time_published = -1; + intro->time_expiring = -1; smartlist_add(service->intro_nodes, intro); log_info(LD_REND, "Picked router %s as an intro point for %s.", safe_str_client(router_describe(router)),

1 0

[tor/master] Reduce lifetime of DH public key replay-detection cache elements
by nickm＠torproject.org 30 Nov '11

30 Nov '11

commit 60ed98e184c4db70ed1cbac02c891e9bd2d8141a Author: Robert Ransom <rransom.8774(a)gmail.com> Date: Sun Oct 30 04:41:16 2011 -0700 Reduce lifetime of DH public key replay-detection cache elements --- .../reduce-hs-intro-dh-key-replay-cache-lifetime | 9 +++++++++ src/or/or.h | 2 +- 2 files changed, 10 insertions(+), 1 deletions(-) diff --git a/changes/reduce-hs-intro-dh-key-replay-cache-lifetime b/changes/reduce-hs-intro-dh-key-replay-cache-lifetime new file mode 100644 index 0000000..5ae3785 --- /dev/null +++ b/changes/reduce-hs-intro-dh-key-replay-cache-lifetime @@ -0,0 +1,9 @@ + o Minor features: + + - Reduce the lifetime of elements of hidden services' + Diffie-Hellman public key replay-detection cache from 60 minutes + to 5 minutes. This replay-detection cache is now used only to + detect multiple INTRODUCE2 cells specifying the same rendezvous + point, so we don't launch multiple simultaneous attempts to + connect to it. + diff --git a/src/or/or.h b/src/or/or.h index 8455dc8..2e4811d 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -759,7 +759,7 @@ typedef struct rend_data_t { /** Time interval for tracking replays of DH public keys received in * INTRODUCE2 cells. Used only to avoid launching multiple * simultaneous attempts to connect to the same rendezvous point. */ -#define REND_REPLAY_TIME_INTERVAL (60 * 60) +#define REND_REPLAY_TIME_INTERVAL (5 * 60) /** Used to indicate which way a cell is going on a circuit. */ typedef enum {

1 0

[tor/master] Make introduction points expire
by nickm＠torproject.org 30 Nov '11

30 Nov '11

commit 1eba4f0cc370f576537edc3461899b87e71ea107 Author: Robert Ransom <rransom.8774(a)gmail.com> Date: Fri Oct 28 18:35:55 2011 -0700 Make introduction points expire --- changes/intro-point-expiration | 5 ++++ src/or/or.h | 25 ++++++++++++++++++++++ src/or/rendservice.c | 44 ++++++++++++++++++++++++++++++++++----- 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/changes/intro-point-expiration b/changes/intro-point-expiration new file mode 100644 index 0000000..3de33c1 --- /dev/null +++ b/changes/intro-point-expiration @@ -0,0 +1,5 @@ + o Minor features: + + - Expire old or over-used hidden service introduction points. + Required by fix for bug 3460. + diff --git a/src/or/or.h b/src/or/or.h index 9e4cd89..9c81d0e 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3461,6 +3461,26 @@ 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 number of seconds that an introduction point will last + * before expiring due to old age. (If it receives + * INTRO_POINT_LIFETIME_INTRODUCTIONS INTRODUCE2 cells, it may expire + * sooner.) + * + * XXX023 Should this be configurable? */ +#define INTRO_POINT_LIFETIME_MIN_SECONDS 18*60*60 +/** The maximum number of seconds that an introduction point will last + * before expiring due to old age. + * + * XXX023 Should this be configurable? */ +#define INTRO_POINT_LIFETIME_MAX_SECONDS 24*60*60 + /** Introduction point information. Used both in rend_service_t (on * the service side) and in rend_service_descriptor_t (on both the * client and service side). */ @@ -3494,6 +3514,11 @@ typedef struct rend_intro_point_t { * published. */ time_t time_published; + /** (Service side only) The time at which this intro point should + * (start to) expire, or -1 if we haven't decided when this intro + * point should expire. */ + time_t time_to_expire; + /** (Service side only) The time at which we decided that this intro * point should start expiring, or -1 if this intro point is not yet * expiring. diff --git a/src/or/rendservice.c b/src/or/rendservice.c index fcbdff0..ee34edf 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1917,16 +1917,47 @@ upload_service_descriptor(rend_service_t *service) /** Return non-zero iff intro should 'expire' now (i.e. we * should stop publishing it in new descriptors and eventually close - * it). - * - * XXXX This is a dummy function for now. It will actually do - * something in a later commit. */ + * it). */ static int intro_point_should_expire_now(rend_intro_point_t *intro, time_t now) { - (void)intro; (void)now; - return 0; + tor_assert(intro != NULL); + + if (intro->time_published == -1) { + /* Don't expire an intro point if we haven't even published it yet. */ + return 0; + } + + if (intro->time_expiring != -1) { + /* We've already started expiring this intro point. *Don't* let + * this function's result 'flap'. */ + return 1; + } + + if (intro->introduction_count >= INTRO_POINT_LIFETIME_INTRODUCTIONS) { + /* This intro point has been used too many times. Expire it now. */ + return 1; + } + + if (intro->time_to_expire == -1) { + /* 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); + + /* Start the expiration timer now, rather than when the intro + * point was first published. There shouldn't be much of a time + * difference. */ + intro->time_to_expire = now + intro_point_lifetime_seconds; + + return 0; + } + + /* This intro point has a time to expire set already. Use it. */ + return (now >= intro->time_to_expire); } /** For every service, check how many intro points it currently has, and: @@ -2107,6 +2138,7 @@ rend_services_introduce(void) intro->intro_key = crypto_new_pk_env(); tor_assert(!crypto_pk_generate_key(intro->intro_key)); intro->time_published = -1; + intro->time_to_expire = -1; intro->time_expiring = -1; smartlist_add(service->intro_nodes, intro); log_info(LD_REND, "Picked router %s as an intro point for %s.",

1 0

[tor/master] Ignore timestamps of INTRODUCE2 cells
by nickm＠torproject.org 30 Nov '11

30 Nov '11

commit 272dd90b5c9998130f65edd65df1c066dc4599aa Author: Robert Ransom <rransom.8774(a)gmail.com> Date: Sun Oct 30 02:13:46 2011 -0700 Ignore timestamps of INTRODUCE2 cells --- changes/bug3460 | 11 +++++++++++ src/or/or.h | 6 +++--- src/or/rendservice.c | 12 +----------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/changes/bug3460 b/changes/bug3460 new file mode 100644 index 0000000..4fbca01 --- /dev/null +++ b/changes/bug3460 @@ -0,0 +1,11 @@ + o Major bugfixes: + + - Ignore the timestamps of INTRODUCE2 cells received by a hidden + service. Previously, hidden services would check that the + timestamp was within 30 minutes of their system clock, so that + services could keep only INTRODUCE2 cells they had received in + the last hour in their replay-detection cache. Bugfix on + 0.2.1.6-alpha, when the v3 intro-point protocol (the first one + which sent a timestamp field in the INTRODUCE2 cell) was + introduced; fixes bug 3460. + diff --git a/src/or/or.h b/src/or/or.h index b53220f..8455dc8 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -756,9 +756,9 @@ typedef struct rend_data_t { char rend_cookie[REND_COOKIE_LEN]; } rend_data_t; -/** Time interval for tracking possible replays of INTRODUCE2 cells. - * Incoming cells with timestamps half of this interval in the past or - * future are dropped immediately. */ +/** Time interval for tracking replays of DH public keys received in + * INTRODUCE2 cells. Used only to avoid launching multiple + * simultaneous attempts to connect to the same rendezvous point. */ #define REND_REPLAY_TIME_INTERVAL (60 * 60) /** Used to indicate which way a cell is going on a circuit. */ diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 413d4f6..4680617 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -1037,7 +1037,6 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, len = r; if (*buf == 3) { /* Version 3 INTRODUCE2 cell. */ - time_t ts = 0; v3_shift = 1; auth_type = buf[1]; switch (auth_type) { @@ -1059,17 +1058,8 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, log_info(LD_REND, "Unknown authorization type '%d'", auth_type); } - /* Check timestamp. */ - ts = ntohl(get_uint32(buf+1+v3_shift)); + /* Skip the timestamp field. We no longer use it. */ v3_shift += 4; - if ((now - ts) < -1 * REND_REPLAY_TIME_INTERVAL / 2 || - (now - ts) > REND_REPLAY_TIME_INTERVAL / 2) { - /* This is far more likely to mean that a client's clock is - * skewed than that a replay attack is in progress. */ - log_info(LD_REND, "INTRODUCE2 cell is too %s. Discarding.", - (now - ts) < 0 ? "old" : "new"); - return -1; - } } if (*buf == 2 || *buf == 3) { /* Version 2 INTRODUCE2 cell. */

1 0

[tor/master] Rename accepted_intros fields
by nickm＠torproject.org 30 Nov '11

30 Nov '11

commit 256bcb4755ce1296772aadd0e4a461c8c603d3b6 Author: Robert Ransom <rransom.8774(a)gmail.com> Date: Sun Nov 27 09:26:48 2011 -0800 Rename accepted_intros fields --- src/or/or.h | 2 +- src/or/rendcommon.c | 4 ++-- src/or/rendservice.c | 45 ++++++++++++++++++++++++++------------------- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/or/or.h b/src/or/or.h index 2e4811d..dcaea39 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3510,7 +3510,7 @@ typedef struct rend_intro_point_t { * of the RSA-encrypted part of a received INTRODUCE2 cell; each * value is a pointer to the time_t at which the cell was * received. */ - digestmap_t *accepted_intros; + digestmap_t *accepted_intro_rsa_parts; /** (Service side only) The time at which this intro point was first * published, or -1 if this intro point has not yet been diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index 0a478c1..2ca001d 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -441,8 +441,8 @@ rend_intro_point_free(rend_intro_point_t *intro) extend_info_free(intro->extend_info); crypto_free_pk_env(intro->intro_key); - if (intro->accepted_intros != NULL) { - digestmap_free(intro->accepted_intros, _tor_free); + if (intro->accepted_intro_rsa_parts != NULL) { + digestmap_free(intro->accepted_intro_rsa_parts, _tor_free); } tor_free(intro); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 4680617..2aa02f9 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -91,9 +91,10 @@ typedef struct rend_service_t { * upload time. */ /** Map from digests of Diffie-Hellman values INTRODUCE2 to time_t of when * they were received; used to prevent replays. */ - digestmap_t *accepted_intros; - /** Time at which we last removed expired values from accepted_intros. */ - time_t last_cleaned_accepted_intros; + digestmap_t *accepted_intro_dh_parts; + /** Time at which we last removed expired values from + * accepted_intro_dh_parts. */ + time_t last_cleaned_accepted_intro_dh_parts; } rend_service_t; /** A list of rend_service_t's for services run on this OP. @@ -153,7 +154,7 @@ rend_service_free(rend_service_t *service) rend_authorized_client_free(c);); smartlist_free(service->clients); } - digestmap_free(service->accepted_intros, _tor_free); + digestmap_free(service->accepted_intro_dh_parts, _tor_free); tor_free(service); } @@ -888,15 +889,16 @@ rend_check_authorization(rend_service_t *service, /** Remove elements from service's replay cache that are old enough to * be noticed by timestamp checking. */ static void -clean_accepted_intros(rend_service_t *service, time_t now) +clean_accepted_intro_dh_parts(rend_service_t *service, time_t now) { const time_t cutoff = now - REND_REPLAY_TIME_INTERVAL; - service->last_cleaned_accepted_intros = now; - if (!service->accepted_intros) + service->last_cleaned_accepted_intro_dh_parts = now; + if (!service->accepted_intro_dh_parts) return; - DIGESTMAP_FOREACH_MODIFY(service->accepted_intros, digest, time_t *, t) { + DIGESTMAP_FOREACH_MODIFY(service->accepted_intro_dh_parts, digest, + time_t *, t) { if (*t < cutoff) { tor_free(t); MAP_DEL_CURRENT(digest); @@ -1002,17 +1004,18 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, return -1; } - if (!service->accepted_intros) - service->accepted_intros = digestmap_new(); + if (!service->accepted_intro_dh_parts) + service->accepted_intro_dh_parts = digestmap_new(); - if (!intro_point->accepted_intros) - intro_point->accepted_intros = digestmap_new(); + if (!intro_point->accepted_intro_rsa_parts) + intro_point->accepted_intro_rsa_parts = digestmap_new(); { char pkpart_digest[DIGEST_LEN]; /* Check for replay of PK-encrypted portion. */ crypto_digest(pkpart_digest, (char*)request+DIGEST_LEN, keylen); - access_time = digestmap_get(intro_point->accepted_intros, pkpart_digest); + access_time = digestmap_get(intro_point->accepted_intro_rsa_parts, + pkpart_digest); if (access_time != NULL) { log_warn(LD_REND, "Possible replay detected! We received an " "INTRODUCE2 cell with same PK-encrypted part %d seconds ago. " @@ -1021,7 +1024,8 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, } access_time = tor_malloc(sizeof(time_t)); *access_time = now; - digestmap_set(intro_point->accepted_intros, pkpart_digest, access_time); + digestmap_set(intro_point->accepted_intro_rsa_parts, + pkpart_digest, access_time); } /* Next N bytes is encrypted with service key */ @@ -1158,7 +1162,8 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, /* Check whether there is a past request with the same Diffie-Hellman, * part 1. */ - access_time = digestmap_get(service->accepted_intros, diffie_hellman_hash); + access_time = digestmap_get(service->accepted_intro_dh_parts, + diffie_hellman_hash); if (access_time != NULL) { /* A Tor client will send a new INTRODUCE1 cell with the same rend * cookie and DH public key as its previous one if its intro circ @@ -1180,9 +1185,11 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, * one hour. */ access_time = tor_malloc(sizeof(time_t)); *access_time = now; - digestmap_set(service->accepted_intros, diffie_hellman_hash, access_time); - if (service->last_cleaned_accepted_intros + REND_REPLAY_TIME_INTERVAL < now) - clean_accepted_intros(service, now); + digestmap_set(service->accepted_intro_dh_parts, + diffie_hellman_hash, access_time); + if (service->last_cleaned_accepted_intro_dh_parts + REND_REPLAY_TIME_INTERVAL + < now) + clean_accepted_intro_dh_parts(service, now); /* If the service performs client authorization, check included auth data. */ if (service->clients) { @@ -1918,7 +1925,7 @@ intro_point_should_expire_now(rend_intro_point_t *intro, return 1; } - if (digestmap_size(intro->accepted_intros) >= + if (digestmap_size(intro->accepted_intro_rsa_parts) >= INTRO_POINT_LIFETIME_INTRODUCTIONS) { /* This intro point has been used too many times. Expire it now. */ return 1;

1 0

[tor/master] Merge remote-tracking branch 'rransom-tor/bug3460-v4'
by nickm＠torproject.org 30 Nov '11

30 Nov '11

commit 628b735fe39e13cc37afb567b32d4b006da51c89 Merge: 441ab6c a2791f4 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Nov 29 20:56:39 2011 -0500 Merge remote-tracking branch 'rransom-tor/bug3460-v4' Conflicts: src/or/rendservice.c changes/bug3460 | 11 + changes/intro-point-expiration | 5 + changes/per-intro-point-replay-cache | 7 + .../reduce-hs-intro-dh-key-replay-cache-lifetime | 9 + src/or/or.h | 59 +++- src/or/rendcommon.c | 5 + src/or/rendservice.c | 348 +++++++++++++++----- 7 files changed, 363 insertions(+), 81 deletions(-) diff --cc src/or/rendservice.c index e0c1a8c,2c54f30..0ded538 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@@ -1412,8 -1452,9 +1458,9 @@@ rend_service_intro_has_opened(origin_ci /* If we already have enough introduction circuits for this service, * redefine this one as a general circuit or close it, depending. */ - if (count_established_intro_points(serviceid) > NUM_INTRO_POINTS) { + if (count_established_intro_points(serviceid) > + (int)service->n_intro_points_wanted) { /* XXX023 remove cast */ - or_options_t *options = get_options(); + const or_options_t *options = get_options(); if (options->ExcludeNodes) { /* XXXX in some future version, we can test whether the transition is allowed or not given the actual nodes in the circuit. But for now, @@@ -1863,15 -1963,17 +1985,17 @@@ voi rend_services_introduce(void) { int i,j,r; - routerinfo_t *router; + const node_t *node; rend_service_t *service; rend_intro_point_t *intro; - int changed, prev_intro_nodes; + int intro_point_set_changed, prev_intro_nodes; + unsigned int n_intro_points_unexpired; + unsigned int n_intro_points_to_open; - smartlist_t *intro_routers; + smartlist_t *intro_nodes; time_t now; - or_options_t *options = get_options(); + const or_options_t *options = get_options(); - intro_routers = smartlist_create(); + intro_nodes = smartlist_create(); now = time(NULL); for (i=0; i < smartlist_len(rend_service_list); ++i) { @@@ -1893,39 -2004,85 +2026,85 @@@ /* Find out which introduction points we have in progress for this service. */ - for (j=0; j < smartlist_len(service->intro_nodes); ++j) { - intro = smartlist_get(service->intro_nodes, j); + SMARTLIST_FOREACH_BEGIN(service->intro_nodes, rend_intro_point_t *, intro){ + origin_circuit_t *intro_circ = + find_intro_circuit(intro, service->pk_digest); + + if (intro->time_expiring + INTRO_POINT_EXPIRATION_GRACE_PERIOD > now) { + /* This intro point has completely expired. Remove it, and + * mark the circuit for close if it's still alive. */ + if (intro_circ != NULL) { + circuit_mark_for_close(TO_CIRCUIT(intro_circ), + END_CIRC_REASON_FINISHED); + } + rend_intro_point_free(intro); + intro = NULL; /* SMARTLIST_DEL_CURRENT takes a name, not a value. */ + SMARTLIST_DEL_CURRENT(service->intro_nodes, intro); + /* We don't need to set intro_point_set_changed here, because + * this intro point wouldn't have been published in a current + * descriptor anyway. */ + continue; + } + - router = router_get_by_digest(intro->extend_info->identity_digest); - if (!router || !intro_circ) { + node = node_get_by_id(intro->extend_info->identity_digest); - if (!node || !find_intro_circuit(intro, service->pk_digest)) { - log_info(LD_REND,"Giving up on %s as intro point for %s.", ++ if (!node || !intro_circ) { + int removing_this_intro_point_changes_the_intro_point_set = 1; + log_info(LD_REND, "Giving up on %s as intro point for %s" + " (circuit disappeared).", safe_str_client(extend_info_describe(intro->extend_info)), safe_str_client(service->service_id)); - if (service->desc) { - SMARTLIST_FOREACH(service->desc->intro_nodes, rend_intro_point_t *, - dintro, { - if (tor_memeq(dintro->extend_info->identity_digest, - intro->extend_info->identity_digest, DIGEST_LEN)) { - log_info(LD_REND, "The intro point we are giving up on was " - "included in the last published descriptor. " - "Marking current descriptor as dirty."); - service->desc_is_dirty = now; - } - }); + if (intro->time_expiring != -1) { + log_info(LD_REND, "We were already expiring the intro point; " + "no need to mark the HS descriptor as dirty over this."); + removing_this_intro_point_changes_the_intro_point_set = 0; + } else if (intro->listed_in_last_desc) { + log_info(LD_REND, "The intro point we are giving up on was " + "included in the last published descriptor. " + "Marking current descriptor as dirty."); + service->desc_is_dirty = now; } rend_intro_point_free(intro); - smartlist_del(service->intro_nodes,j--); - changed = 1; + intro = NULL; /* SMARTLIST_DEL_CURRENT takes a name, not a value. */ + SMARTLIST_DEL_CURRENT(service->intro_nodes, intro); + if (removing_this_intro_point_changes_the_intro_point_set) + intro_point_set_changed = 1; } + + if (intro != NULL && intro_point_should_expire_now(intro, now)) { + log_info(LD_REND, "Expiring %s as intro point for %s.", + safe_str_client(extend_info_describe(intro->extend_info)), + safe_str_client(service->service_id)); + + /* The polite (and generally Right) way to expire an intro + * point is to establish a new one to replace it, publish a + * new descriptor that doesn't list any expiring intro points, + * and *then*, once our upload attempts for the new descriptor + * have ended (whether in success or failure), close the + * expiring intro points. + * + * Unfortunately, we can't find out when the new descriptor + * has actually been uploaded, so we'll have to settle for a + * five-minute timer. Start it. XXX023 This sucks. */ + intro->time_expiring = now; + + intro_point_set_changed = 1; + } + + if (intro != NULL && intro->time_expiring == -1) + ++n_intro_points_unexpired; + - if (router) - smartlist_add(intro_routers, router); + if (node) + smartlist_add(intro_nodes, (void*)node); - } - - /* We have enough intro points, and the intro points we thought we had were - * all connected. - */ - if (!changed && smartlist_len(service->intro_nodes) >= NUM_INTRO_POINTS) { - /* We have all our intro points! Start a fresh period and reset the - * circuit count. */ + } SMARTLIST_FOREACH_END(intro); + + if (!intro_point_set_changed && + (n_intro_points_unexpired >= service->n_intro_points_wanted)) { + /* We have enough intro circuits in progress, and none of our + * intro circuits have died since the last call to + * rend_services_introduce! Start a fresh period and reset the + * circuit count. + * + * XXXX WTF? */ service->intro_period_started = now; service->n_intro_circuits_launched = 0; continue; @@@ -1943,29 -2108,36 +2130,36 @@@ * we'll drop them from the list of intro points next time we * go through the above "find out which introduction points we have * in progress" loop. */ - #define NUM_INTRO_POINTS_INIT (NUM_INTRO_POINTS + 2) - for (j=prev_intro_nodes; j < (prev_intro_nodes == 0 ? - NUM_INTRO_POINTS_INIT : NUM_INTRO_POINTS); ++j) { + n_intro_points_to_open = (service->n_intro_points_wanted + + (prev_intro_nodes == 0 ? 2 : 0)); + for (j = (int)n_intro_points_unexpired; + j < (int)n_intro_points_to_open; + ++j) { /* XXXX remove casts */ - router_crn_flags_t flags = CRN_NEED_UPTIME; + router_crn_flags_t flags = CRN_NEED_UPTIME|CRN_NEED_DESC; if (get_options()->_AllowInvalid & ALLOW_INVALID_INTRODUCTION) flags |= CRN_ALLOW_INVALID; - router = router_choose_random_node(intro_routers, - options->ExcludeNodes, flags); - if (!router) { + node = router_choose_random_node(intro_nodes, + options->ExcludeNodes, flags); + if (!node) { log_warn(LD_REND, - "Could only establish %d introduction points for %s.", - smartlist_len(service->intro_nodes), service->service_id); + "Could only establish %d introduction points for %s; " + "wanted %u.", + smartlist_len(service->intro_nodes), service->service_id, + n_intro_points_to_open); break; } - changed = 1; + intro_point_set_changed = 1; - smartlist_add(intro_routers, router); + smartlist_add(intro_nodes, (void*)node); intro = tor_malloc_zero(sizeof(rend_intro_point_t)); - intro->extend_info = extend_info_from_router(router); + intro->extend_info = extend_info_from_node(node); intro->intro_key = crypto_new_pk_env(); tor_assert(!crypto_pk_generate_key(intro->intro_key)); + intro->time_published = -1; + intro->time_to_expire = -1; + intro->time_expiring = -1; smartlist_add(service->intro_nodes, intro); log_info(LD_REND, "Picked router %s as an intro point for %s.", - safe_str_client(router_describe(router)), + safe_str_client(node_describe(node)), safe_str_client(service->service_id)); }

1 0