From c60148c7f52e76397b79faaddf1bdb6204c4d170 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Wed, 5 Oct 2011 15:43:02 -0700 Subject: Record the time at which each intro point was first published --- src/or/or.h | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src/or/or.h') diff --git a/src/or/or.h b/src/or/or.h index f884c12ecc..cf241a6442 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3480,6 +3480,11 @@ typedef struct rend_intro_point_t { * circuit to this intro point for some reason other than our * circuit-build timeout. See also MAX_INTRO_POINT_REACHABILITY_FAILURES. */ unsigned int unreachable_count : 3; + + /** (Service side only) The time at which this intro point was first + * published, or -1 if this intro point has not yet been + * published. */ + time_t time_published; } rend_intro_point_t; /** Information used to connect to a hidden service. Used on both the -- cgit v1.2.3-54-g00ecf From 6f035cb2b450e8779bff50d6ed83e4822a49f0fe Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Wed, 5 Oct 2011 23:52:14 -0700 Subject: Record the number of INTRODUCE2 cells each intro point has received --- src/or/or.h | 4 ++++ src/or/rendservice.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) (limited to 'src/or/or.h') diff --git a/src/or/or.h b/src/or/or.h index cf241a6442..215bde245b 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3481,6 +3481,10 @@ typedef struct rend_intro_point_t { * circuit-build timeout. See also MAX_INTRO_POINT_REACHABILITY_FAILURES. */ unsigned int unreachable_count : 3; + /** (Service side only) The number of INTRODUCE2 cells this intro + * point's circuit has received. */ + unsigned int introduction_count : 24; + /** (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 d21fdcc4bf..a341dd672b 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -25,6 +25,7 @@ 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); /** Represents the mapping from a virtual port of a rendezvous service to * a real port on some IP. @@ -899,6 +900,7 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, char buf[RELAY_PAYLOAD_SIZE]; char keys[DIGEST_LEN+CPATH_KEY_MATERIAL_LEN]; /* Holds KH, Df, Db, Kf, Kb */ rend_service_t *service; + rend_intro_point_t *intro_point; int r, i, v3_shift = 0; size_t len, keylen; crypto_dh_env_t *dh = NULL; @@ -971,6 +973,14 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, return -1; } + intro_point = find_intro_point(circuit); + if (intro_point == NULL) { + log_warn(LD_BUG, "Internal error: Got an INTRODUCE2 cell on an intro circ " + "(for service %s) with no corresponding rend_intro_point_t.", + escaped(serviceid)); + return -1; + } + if (!service->accepted_intros) service->accepted_intros = digestmap_new(); @@ -993,6 +1003,13 @@ rend_service_introduce(origin_circuit_t *circuit, const uint8_t *request, 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); + } + /* Next N bytes is encrypted with service key */ note_crypto_pk_op(REND_SERVER); r = crypto_pk_private_hybrid_decrypt( @@ -1647,6 +1664,35 @@ find_intro_circuit(rend_intro_point_t *intro, const char *pk_digest) return NULL; } +/** Return a pointer to the rend_intro_point_t corresponding to the + * service-side introduction circuit circ. */ +static rend_intro_point_t * +find_intro_point(origin_circuit_t *circ) +{ + const char *serviceid; + rend_service_t *service = NULL; + + tor_assert(TO_CIRCUIT(circ)->purpose == CIRCUIT_PURPOSE_S_ESTABLISH_INTRO || + TO_CIRCUIT(circ)->purpose == CIRCUIT_PURPOSE_S_INTRO); + tor_assert(circ->rend_data); + serviceid = circ->rend_data->onion_address; + + SMARTLIST_FOREACH(rend_service_list, rend_service_t *, s, + if (tor_memeq(s->service_id, serviceid, REND_SERVICE_ID_LEN_BASE32)) { + service = s; + break; + }); + + if (service == NULL) return NULL; + + SMARTLIST_FOREACH(service->intro_nodes, rend_intro_point_t *, intro_point, + if (crypto_pk_cmp_keys(intro_point->intro_key, circ->intro_key) == 0) { + return intro_point; + }); + + return NULL; +} + /** Determine the responsible hidden service directories for the * rend_encoded_v2_service_descriptor_t's in descs and upload them; * service_id and seconds_valid are only passed for logging -- cgit v1.2.3-54-g00ecf From 3f6a2d3e2a83d60f287c485c4f444220792b0a66 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Thu, 13 Oct 2011 08:48:31 -0700 Subject: Record which intro points were listed in the last HS desc --- src/or/or.h | 4 ++++ src/or/rendservice.c | 7 +++++++ 2 files changed, 11 insertions(+) (limited to 'src/or/or.h') diff --git a/src/or/or.h b/src/or/or.h index 215bde245b..e1bd25c738 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3481,6 +3481,10 @@ typedef struct rend_intro_point_t { * circuit-build timeout. See also MAX_INTRO_POINT_REACHABILITY_FAILURES. */ unsigned int unreachable_count : 3; + /** (Service side only) Flag indicating that this intro point was + * 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; diff --git a/src/or/rendservice.c b/src/or/rendservice.c index a341dd672b..96ab7a67aa 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -544,10 +544,17 @@ rend_service_update_descriptor(rend_service_t *service) for (i = 0; i < smartlist_len(service->intro_nodes); ++i) { rend_intro_point_t *intro_svc = smartlist_get(service->intro_nodes, i); rend_intro_point_t *intro_desc; + + /* This intro point won't be listed in the descriptor... */ + intro_svc->listed_in_last_desc = 0; + circ = find_intro_circuit(intro_svc, service->pk_digest); if (!circ || circ->_base.purpose != CIRCUIT_PURPOSE_S_INTRO) continue; + /* ...unless this intro point is listed in the descriptor. */ + intro_svc->listed_in_last_desc = 1; + /* We have an entirely established intro circuit. Publish it in * our descriptor. */ intro_desc = tor_malloc_zero(sizeof(rend_intro_point_t)); -- cgit v1.2.3-54-g00ecf From 00885652db8146d992bcf96315a45e7820688145 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sat, 15 Oct 2011 15:40:28 -0700 Subject: 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(-) (limited to 'src/or/or.h') diff --git a/src/or/or.h b/src/or/or.h index e1bd25c738..9e4cd89cf1 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 30f5df706f..fcbdff0b92 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)), -- cgit v1.2.3-54-g00ecf From 1eba4f0cc370f576537edc3461899b87e71ea107 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Fri, 28 Oct 2011 18:35:55 -0700 Subject: 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(-) create mode 100644 changes/intro-point-expiration (limited to 'src/or/or.h') diff --git a/changes/intro-point-expiration b/changes/intro-point-expiration new file mode 100644 index 0000000000..3de33c188e --- /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 9e4cd89cf1..9c81d0e134 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 fcbdff0b92..ee34edfa6e 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.", -- cgit v1.2.3-54-g00ecf From 1a52a947c557ac04ee96addff404dc50cf5c26eb Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sat, 29 Oct 2011 17:02:53 -0700 Subject: 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(-) create mode 100644 changes/per-intro-point-replay-cache (limited to 'src/or/or.h') diff --git a/changes/per-intro-point-replay-cache b/changes/per-intro-point-replay-cache new file mode 100644 index 0000000000..f63e428e32 --- /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 9c81d0e134..b53220fcba 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 c5bf88163d..0a478c1147 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 ee34edfa6e..413d4f670a 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; } -- cgit v1.2.3-54-g00ecf From 272dd90b5c9998130f65edd65df1c066dc4599aa Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sun, 30 Oct 2011 02:13:46 -0700 Subject: 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(-) create mode 100644 changes/bug3460 (limited to 'src/or/or.h') diff --git a/changes/bug3460 b/changes/bug3460 new file mode 100644 index 0000000000..4fbca01aec --- /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 b53220fcba..8455dc83cb 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 413d4f670a..46806171e9 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. */ -- cgit v1.2.3-54-g00ecf From 60ed98e184c4db70ed1cbac02c891e9bd2d8141a Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sun, 30 Oct 2011 04:41:16 -0700 Subject: Reduce lifetime of DH public key replay-detection cache elements --- changes/reduce-hs-intro-dh-key-replay-cache-lifetime | 9 +++++++++ src/or/or.h | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 changes/reduce-hs-intro-dh-key-replay-cache-lifetime (limited to 'src/or/or.h') 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 0000000000..5ae3785fcb --- /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 8455dc83cb..2e4811d839 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 { -- cgit v1.2.3-54-g00ecf From 256bcb4755ce1296772aadd0e4a461c8c603d3b6 Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sun, 27 Nov 2011 09:26:48 -0800 Subject: 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(-) (limited to 'src/or/or.h') diff --git a/src/or/or.h b/src/or/or.h index 2e4811d839..dcaea39d17 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 0a478c1147..2ca001d984 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 46806171e9..2aa02f9e05 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; -- cgit v1.2.3-54-g00ecf From a2791f43f595d06dbaeff7c4ea0bcecc5c04e2de Mon Sep 17 00:00:00 2001 From: Robert Ransom Date: Sun, 27 Nov 2011 09:30:16 -0800 Subject: Correct documentation comments for fields formerly named accepted_intros --- src/or/or.h | 4 ++-- src/or/rendservice.c | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) (limited to 'src/or/or.h') diff --git a/src/or/or.h b/src/or/or.h index dcaea39d17..3510ab0421 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3508,8 +3508,8 @@ typedef struct rend_intro_point_t { /** (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. */ + * value is a pointer to the time_t at which the cell was received. + * This digestmap is used to prevent replay attacks. */ digestmap_t *accepted_intro_rsa_parts; /** (Service side only) The time at which this intro point was first diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 2aa02f9e05..2c54f3059d 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -89,8 +89,12 @@ typedef struct rend_service_t { * up-to-date. */ time_t next_upload_time; /**< Scheduled next hidden service descriptor * upload time. */ - /** Map from digests of Diffie-Hellman values INTRODUCE2 to time_t of when - * they were received; used to prevent replays. */ + /** Map from digests of Diffie-Hellman values INTRODUCE2 to time_t + * of when they were received. Clients may send INTRODUCE1 cells + * for the same rendezvous point through two or more different + * introduction points; when they do, this digestmap keeps us from + * launching multiple simultaneous attempts to connect to the same + * rend point. */ digestmap_t *accepted_intro_dh_parts; /** Time at which we last removed expired values from * accepted_intro_dh_parts. */ -- cgit v1.2.3-54-g00ecf