From e167910fce2b83d9de3a252cdf02cabdafced14b Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Fri, 11 Mar 2016 10:40:31 -0500 Subject: [PATCH 1/6] rip out hid_serv_responsible_for_desc_id() This simplifies relay behavior, because the relay offers the hsdir functionality independent of whether the directory authorities have decided this relay is suitable for clients to use yet. Implements ticket 18332. --- changes/ticket18332 | 7 ++++ src/or/rendcache.c | 11 +----- src/or/rendcommon.c | 26 -------------- src/or/rendcommon.h | 1 - src/test/test_dir_handle_get.c | 12 ------- src/test/test_rendcache.c | 66 ---------------------------------- 6 files changed, 8 insertions(+), 115 deletions(-) create mode 100644 changes/ticket18332 diff --git a/changes/ticket18332 b/changes/ticket18332 new file mode 100644 index 0000000000..6fb811827f --- /dev/null +++ b/changes/ticket18332 @@ -0,0 +1,7 @@ + o Removed features: + - Streamline relay-side hsdir handling: when relays consider whether + to accept an uploaded hidden service descriptor, they no longer + check whether directory authorities think they deserve the HSDir + flag, or whether they are one of the relays in the network that is + "supposed" to handle that descriptor. Implements ticket 18332. + diff --git a/src/or/rendcache.c b/src/or/rendcache.c index cb8c14b756..6c22503e53 100644 --- a/src/or/rendcache.c +++ b/src/or/rendcache.c @@ -483,8 +483,7 @@ rend_cache_clean_v2_descs_as_dir(time_t now, size_t force_remove) digestmap_iter_get(iter, &key, &val); ent = val; if (ent->parsed->timestamp < cutoff || - ent->last_served < last_served_cutoff || - !hid_serv_responsible_for_desc_id(key)) { + ent->last_served < last_served_cutoff) { char key_base32[REND_DESC_ID_V2_LEN_BASE32 + 1]; base32_encode(key_base32, sizeof(key_base32), key, DIGEST_LEN); log_info(LD_REND, "Removing descriptor with ID '%s' from cache", @@ -657,14 +656,6 @@ rend_cache_store_v2_desc_as_dir(const char *desc) /* For pretty log statements. */ base32_encode(desc_id_base32, sizeof(desc_id_base32), desc_id, DIGEST_LEN); - /* Is desc ID in the range that we are (directly or indirectly) responsible - * for? */ - if (!hid_serv_responsible_for_desc_id(desc_id)) { - log_info(LD_REND, "Service descriptor with desc ID %s is not in " - "interval that we are responsible for.", - safe_str_client(desc_id_base32)); - goto skip; - } /* Is descriptor too old? */ if (parsed->timestamp < now - REND_CACHE_MAX_AGE-REND_CACHE_MAX_SKEW) { log_info(LD_REND, "Service descriptor with desc ID %s is too old.", diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index 79509cbe2f..f1fb0d934c 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -983,29 +983,3 @@ hid_serv_acting_as_directory(void) return 1; } -/** Return true if this node is responsible for storing the descriptor ID - * in query and false otherwise. */ -MOCK_IMPL(int, hid_serv_responsible_for_desc_id, - (const char *query)) -{ - const routerinfo_t *me; - routerstatus_t *last_rs; - const char *my_id, *last_id; - int result; - smartlist_t *responsible; - if (!hid_serv_acting_as_directory()) - return 0; - if (!(me = router_get_my_routerinfo())) - return 0; /* This is redundant, but let's be paranoid. */ - my_id = me->cache_info.identity_digest; - responsible = smartlist_new(); - if (hid_serv_get_responsible_directories(responsible, query) < 0) { - smartlist_free(responsible); - return 0; - } - last_rs = smartlist_get(responsible, smartlist_len(responsible)-1); - last_id = last_rs->identity_digest; - result = rend_id_is_in_interval(my_id, query, last_id); - smartlist_free(responsible); - return result; -} diff --git a/src/or/rendcommon.h b/src/or/rendcommon.h index fe45299598..b3ae790518 100644 --- a/src/or/rendcommon.h +++ b/src/or/rendcommon.h @@ -60,7 +60,6 @@ void rend_get_descriptor_id_bytes(char *descriptor_id_out, int hid_serv_get_responsible_directories(smartlist_t *responsible_dirs, const char *id); int hid_serv_acting_as_directory(void); -MOCK_DECL(int, hid_serv_responsible_for_desc_id, (const char *id)); rend_data_t *rend_data_dup(const rend_data_t *data); rend_data_t *rend_data_client_create(const char *onion_address, diff --git a/src/test/test_dir_handle_get.c b/src/test/test_dir_handle_get.c index 954830dcaf..67bd840baf 100644 --- a/src/test/test_dir_handle_get.c +++ b/src/test/test_dir_handle_get.c @@ -389,10 +389,8 @@ test_dir_handle_get_rendezvous2_not_found(void *data) } NS_DECL(const routerinfo_t *, router_get_my_routerinfo, (void)); -NS_DECL(int, hid_serv_responsible_for_desc_id, (const char *id)); static routerinfo_t *mock_routerinfo; -static int hid_serv_responsible_for_desc_id_response; static const routerinfo_t * NS(router_get_my_routerinfo)(void) @@ -404,13 +402,6 @@ NS(router_get_my_routerinfo)(void) return mock_routerinfo; } -static int -NS(hid_serv_responsible_for_desc_id)(const char *id) -{ - (void)id; - return hid_serv_responsible_for_desc_id_response; -} - static void test_dir_handle_get_rendezvous2_on_encrypted_conn_success(void *data) { @@ -428,10 +419,8 @@ test_dir_handle_get_rendezvous2_on_encrypted_conn_success(void *data) MOCK(connection_write_to_buf_impl_, connection_write_to_buf_mock); NS_MOCK(router_get_my_routerinfo); - NS_MOCK(hid_serv_responsible_for_desc_id); rend_cache_init(); - hid_serv_responsible_for_desc_id_response = 1; /* create a valid rend service descriptor */ #define RECENT_TIME -10 @@ -473,7 +462,6 @@ test_dir_handle_get_rendezvous2_on_encrypted_conn_success(void *data) done: UNMOCK(connection_write_to_buf_impl_); NS_UNMOCK(router_get_my_routerinfo); - NS_UNMOCK(hid_serv_responsible_for_desc_id); tor_free(mock_routerinfo->cache_info.signed_descriptor_body); tor_free(mock_routerinfo); diff --git a/src/test/test_rendcache.c b/src/test/test_rendcache.c index eaa8671af0..9898e87098 100644 --- a/src/test/test_rendcache.c +++ b/src/test/test_rendcache.c @@ -377,10 +377,8 @@ test_rend_cache_store_v2_desc_as_client_with_different_time(void *data) #define NS_SUBMODULE lookup_v2_desc_as_dir NS_DECL(const routerinfo_t *, router_get_my_routerinfo, (void)); -NS_DECL(int, hid_serv_responsible_for_desc_id, (const char *id)); static routerinfo_t *mock_routerinfo; -static int hid_serv_responsible_for_desc_id_response; static const routerinfo_t * NS(router_get_my_routerinfo)(void) @@ -392,13 +390,6 @@ NS(router_get_my_routerinfo)(void) return mock_routerinfo; } -static int -NS(hid_serv_responsible_for_desc_id)(const char *id) -{ - (void)id; - return hid_serv_responsible_for_desc_id_response; -} - static void test_rend_cache_lookup_v2_desc_as_dir(void *data) { @@ -411,7 +402,6 @@ test_rend_cache_lookup_v2_desc_as_dir(void *data) (void)data; NS_MOCK(router_get_my_routerinfo); - NS_MOCK(hid_serv_responsible_for_desc_id); rend_cache_init(); @@ -425,7 +415,6 @@ test_rend_cache_lookup_v2_desc_as_dir(void *data) tt_int_op(ret, OP_EQ, 0); // Test existing descriptor - hid_serv_responsible_for_desc_id_response = 1; generate_desc(RECENT_TIME, &desc_holder, &service_id, 3); rend_cache_store_v2_desc_as_dir(desc_holder->desc_str); base32_encode(desc_id_base32, sizeof(desc_id_base32), desc_holder->desc_id, @@ -436,7 +425,6 @@ test_rend_cache_lookup_v2_desc_as_dir(void *data) done: NS_UNMOCK(router_get_my_routerinfo); - NS_UNMOCK(hid_serv_responsible_for_desc_id); tor_free(mock_routerinfo); rend_cache_free_all(); rend_encoded_v2_service_descriptor_free(desc_holder); @@ -447,7 +435,6 @@ test_rend_cache_lookup_v2_desc_as_dir(void *data) #define NS_SUBMODULE store_v2_desc_as_dir NS_DECL(const routerinfo_t *, router_get_my_routerinfo, (void)); -NS_DECL(int, hid_serv_responsible_for_desc_id, (const char *id)); static const routerinfo_t * NS(router_get_my_routerinfo)(void) @@ -455,13 +442,6 @@ NS(router_get_my_routerinfo)(void) return mock_routerinfo; } -static int -NS(hid_serv_responsible_for_desc_id)(const char *id) -{ - (void)id; - return hid_serv_responsible_for_desc_id_response; -} - static void test_rend_cache_store_v2_desc_as_dir(void *data) { @@ -471,7 +451,6 @@ test_rend_cache_store_v2_desc_as_dir(void *data) char *service_id = NULL; NS_MOCK(router_get_my_routerinfo); - NS_MOCK(hid_serv_responsible_for_desc_id); rend_cache_init(); @@ -482,21 +461,10 @@ test_rend_cache_store_v2_desc_as_dir(void *data) // Test when we can't parse the descriptor mock_routerinfo = tor_malloc(sizeof(routerinfo_t)); - hid_serv_responsible_for_desc_id_response = 1; ret = rend_cache_store_v2_desc_as_dir("unparseable"); tt_int_op(ret, OP_EQ, RCS_BADDESC); - // Test when we are not responsible for an HS - hid_serv_responsible_for_desc_id_response = 0; - generate_desc(RECENT_TIME, &desc_holder, &service_id, 3); - ret = rend_cache_store_v2_desc_as_dir(desc_holder->desc_str); - tt_int_op(ret, OP_EQ, RCS_OKAY); - - rend_encoded_v2_service_descriptor_free(desc_holder); - tor_free(service_id); - // Test when we have an old descriptor - hid_serv_responsible_for_desc_id_response = 1; generate_desc(TIME_IN_THE_PAST, &desc_holder, &service_id, 3); ret = rend_cache_store_v2_desc_as_dir(desc_holder->desc_str); tt_int_op(ret, OP_EQ, RCS_OKAY); @@ -529,7 +497,6 @@ test_rend_cache_store_v2_desc_as_dir(void *data) done: NS_UNMOCK(router_get_my_routerinfo); - NS_UNMOCK(hid_serv_responsible_for_desc_id); rend_encoded_v2_service_descriptor_free(desc_holder); tor_free(service_id); rend_cache_free_all(); @@ -550,7 +517,6 @@ test_rend_cache_store_v2_desc_as_dir_with_different_time(void *data) rend_encoded_v2_service_descriptor_t *desc_holder_older; NS_MOCK(router_get_my_routerinfo); - NS_MOCK(hid_serv_responsible_for_desc_id); rend_cache_init(); @@ -577,7 +543,6 @@ test_rend_cache_store_v2_desc_as_dir_with_different_time(void *data) // Test when we have a newer descriptor stored mock_routerinfo = tor_malloc(sizeof(routerinfo_t)); - hid_serv_responsible_for_desc_id_response = 1; rend_cache_store_v2_desc_as_dir(desc_holder_newer->desc_str); ret = rend_cache_store_v2_desc_as_dir(desc_holder_older->desc_str); tt_int_op(ret, OP_EQ, RCS_OKAY); @@ -590,7 +555,6 @@ test_rend_cache_store_v2_desc_as_dir_with_different_time(void *data) done: NS_UNMOCK(router_get_my_routerinfo); - NS_UNMOCK(hid_serv_responsible_for_desc_id); rend_cache_free_all(); rend_service_descriptor_free(generated); tor_free(service_id); @@ -616,7 +580,6 @@ test_rend_cache_store_v2_desc_as_dir_with_different_content(void *data) rend_encoded_v2_service_descriptor_t *desc_holder_two = NULL; NS_MOCK(router_get_my_routerinfo); - NS_MOCK(hid_serv_responsible_for_desc_id); rend_cache_init(); @@ -645,14 +608,12 @@ test_rend_cache_store_v2_desc_as_dir_with_different_content(void *data) // Test when we have another descriptor stored, with a different descriptor mock_routerinfo = tor_malloc(sizeof(routerinfo_t)); - hid_serv_responsible_for_desc_id_response = 1; rend_cache_store_v2_desc_as_dir(desc_holder_one->desc_str); ret = rend_cache_store_v2_desc_as_dir(desc_holder_two->desc_str); tt_int_op(ret, OP_EQ, RCS_OKAY); done: NS_UNMOCK(router_get_my_routerinfo); - NS_UNMOCK(hid_serv_responsible_for_desc_id); rend_cache_free_all(); rend_service_descriptor_free(generated); tor_free(service_id); @@ -1113,14 +1074,6 @@ test_rend_cache_intro_failure_note(void *data) } #define NS_SUBMODULE clean_v2_descs_as_dir -NS_DECL(int, hid_serv_responsible_for_desc_id, (const char *id)); - -static int -NS(hid_serv_responsible_for_desc_id)(const char *id) -{ - (void)id; - return hid_serv_responsible_for_desc_id_response; -} static void test_rend_cache_clean_v2_descs_as_dir(void *data) @@ -1133,7 +1086,6 @@ test_rend_cache_clean_v2_descs_as_dir(void *data) (void)data; - NS_MOCK(hid_serv_responsible_for_desc_id); rend_cache_init(); // Test running with an empty cache @@ -1149,7 +1101,6 @@ test_rend_cache_clean_v2_descs_as_dir(void *data) e->parsed = desc; digestmap_set(rend_cache_v2_dir, key, e); - hid_serv_responsible_for_desc_id_response = 1; rend_cache_clean_v2_descs_as_dir(now, 0); tt_int_op(digestmap_size(rend_cache_v2_dir), OP_EQ, 1); @@ -1158,20 +1109,6 @@ test_rend_cache_clean_v2_descs_as_dir(void *data) rend_cache_clean_v2_descs_as_dir(now, 0); tt_int_op(digestmap_size(rend_cache_v2_dir), OP_EQ, 0); - // Test with one entry that is not under the responsibility of this - // hidden service - e = tor_malloc_zero(sizeof(rend_cache_entry_t)); - e->last_served = now; - desc = tor_malloc_zero(sizeof(rend_service_descriptor_t)); - desc->timestamp = now; - desc->pk = pk_generate(0); - e->parsed = desc; - digestmap_set(rend_cache_v2_dir, key, e); - - hid_serv_responsible_for_desc_id_response = 0; - rend_cache_clean_v2_descs_as_dir(now, 0); - tt_int_op(digestmap_size(rend_cache_v2_dir), OP_EQ, 0); - // Test with one entry that has an old last served e = tor_malloc_zero(sizeof(rend_cache_entry_t)); e->last_served = now - (REND_CACHE_MAX_AGE + REND_CACHE_MAX_SKEW + 1000); @@ -1181,7 +1118,6 @@ test_rend_cache_clean_v2_descs_as_dir(void *data) e->parsed = desc; digestmap_set(rend_cache_v2_dir, key, e); - hid_serv_responsible_for_desc_id_response = 1; rend_cache_clean_v2_descs_as_dir(now, 0); tt_int_op(digestmap_size(rend_cache_v2_dir), OP_EQ, 0); @@ -1194,12 +1130,10 @@ test_rend_cache_clean_v2_descs_as_dir(void *data) e->parsed = desc; digestmap_set(rend_cache_v2_dir, key, e); - hid_serv_responsible_for_desc_id_response = 1; rend_cache_clean_v2_descs_as_dir(now, 20000); tt_int_op(digestmap_size(rend_cache_v2_dir), OP_EQ, 1); done: - NS_UNMOCK(hid_serv_responsible_for_desc_id); rend_cache_free_all(); } From 53902963383e1babfccb8a4ffc7ed4e8accf2214 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Fri, 11 Mar 2016 10:45:03 -0500 Subject: [PATCH 2/6] rip out hid_serv_acting_as_directory() When we made HidServDirectoryV2 always 1, we removed the situation where a relay could choose not to be an HSDir. Now simplify the rest of the code to reflect this decision. (We have to remove two apparently unrelated free() calls in the unit tests, since they used to free stuff that we created as a side effect of calling router_get_my_routerinfo(), and now we no longer call that.) --- src/or/directory.c | 8 -------- src/or/rendcache.c | 6 ------ src/or/rendcache.h | 1 - src/or/rendcommon.c | 11 ----------- src/or/rendcommon.h | 1 - src/test/test_dir_handle_get.c | 2 -- src/test/test_rendcache.c | 5 ----- 7 files changed, 34 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index e4feda44fc..961e33424b 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2301,7 +2301,6 @@ connection_dir_client_reached_eof(dir_connection_t *conn) conn->requested_resource, conn->rend_data, &entry)) { case RCS_BADDESC: - case RCS_NOTDIR: /* Impossible */ log_warn(LD_REND,"Fetching v2 rendezvous descriptor failed. " "Retrying at another directory."); /* We'll retry when connection_about_to_close_connection() @@ -3428,13 +3427,6 @@ directory_handle_command_post(dir_connection_t *conn, const char *headers, if (connection_dir_is_encrypted(conn) && !strcmpstart(url,"/tor/rendezvous2/publish")) { switch (rend_cache_store_v2_desc_as_dir(body)) { - case RCS_NOTDIR: - log_info(LD_REND, "Rejected v2 rend descriptor (length %d) from %s " - "since we're not currently a hidden service directory.", - (int)body_len, conn->base_.address); - write_http_status_line(conn, 503, "Currently not acting as v2 " - "hidden service directory"); - break; case RCS_BADDESC: log_warn(LD_REND, "Rejected v2 rend descriptor (length %d) from %s.", (int)body_len, conn->base_.address); diff --git a/src/or/rendcache.c b/src/or/rendcache.c index 6c22503e53..dad4b2b5ba 100644 --- a/src/or/rendcache.c +++ b/src/or/rendcache.c @@ -641,12 +641,6 @@ rend_cache_store_v2_desc_as_dir(const char *desc) time_t now = time(NULL); tor_assert(rend_cache_v2_dir); tor_assert(desc); - if (!hid_serv_acting_as_directory()) { - /* Cannot store descs, because we are (currently) not acting as - * hidden service directory. */ - log_info(LD_REND, "Cannot store descs: Not acting as hs dir"); - return RCS_NOTDIR; - } while (rend_parse_v2_service_descriptor(&parsed, desc_id, &intro_content, &intro_size, &encoded_size, &next_desc, current_desc, 1) >= 0) { diff --git a/src/or/rendcache.h b/src/or/rendcache.h index 867270f996..3eecd3b2a0 100644 --- a/src/or/rendcache.h +++ b/src/or/rendcache.h @@ -66,7 +66,6 @@ int rend_cache_lookup_v2_desc_as_service(const char *query, int rend_cache_lookup_v2_desc_as_dir(const char *query, const char **desc); /** Return value from rend_cache_store_v2_desc_as_{dir,client}. */ typedef enum { - RCS_NOTDIR = -2, /**< We're not a directory */ RCS_BADDESC = -1, /**< This descriptor is no good. */ RCS_OKAY = 0 /**< All worked as expected */ } rend_cache_store_status_t; diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index f1fb0d934c..79aba80694 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -972,14 +972,3 @@ hid_serv_get_responsible_directories(smartlist_t *responsible_dirs, return smartlist_len(responsible_dirs) ? 0 : -1; } -/** Return true if this node is currently acting as hidden service - * directory, false otherwise. */ -int -hid_serv_acting_as_directory(void) -{ - const routerinfo_t *me = router_get_my_routerinfo(); - if (!me) - return 0; - return 1; -} - diff --git a/src/or/rendcommon.h b/src/or/rendcommon.h index b3ae790518..bab53fc1f6 100644 --- a/src/or/rendcommon.h +++ b/src/or/rendcommon.h @@ -59,7 +59,6 @@ void rend_get_descriptor_id_bytes(char *descriptor_id_out, const char *secret_id_part); int hid_serv_get_responsible_directories(smartlist_t *responsible_dirs, const char *id); -int hid_serv_acting_as_directory(void); rend_data_t *rend_data_dup(const rend_data_t *data); rend_data_t *rend_data_client_create(const char *onion_address, diff --git a/src/test/test_dir_handle_get.c b/src/test/test_dir_handle_get.c index 67bd840baf..c75b2b6632 100644 --- a/src/test/test_dir_handle_get.c +++ b/src/test/test_dir_handle_get.c @@ -462,8 +462,6 @@ test_dir_handle_get_rendezvous2_on_encrypted_conn_success(void *data) done: UNMOCK(connection_write_to_buf_impl_); NS_UNMOCK(router_get_my_routerinfo); - tor_free(mock_routerinfo->cache_info.signed_descriptor_body); - tor_free(mock_routerinfo); connection_free_(TO_CONN(conn)); tor_free(header); diff --git a/src/test/test_rendcache.c b/src/test/test_rendcache.c index 9898e87098..3ccfe5f9bc 100644 --- a/src/test/test_rendcache.c +++ b/src/test/test_rendcache.c @@ -454,11 +454,6 @@ test_rend_cache_store_v2_desc_as_dir(void *data) rend_cache_init(); - // Test when we are not an HS dir - mock_routerinfo = NULL; - ret = rend_cache_store_v2_desc_as_dir(""); - tt_int_op(ret, OP_EQ, RCS_NOTDIR); - // Test when we can't parse the descriptor mock_routerinfo = tor_malloc(sizeof(routerinfo_t)); ret = rend_cache_store_v2_desc_as_dir("unparseable"); From dc500c8cb4dc7643230d160d1b458a0acdadd242 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Fri, 11 Mar 2016 10:46:21 -0500 Subject: [PATCH 3/6] rip out rend_id_is_in_interval() it was used by hid_serv_responsible_for_desc_id(), which we no longer use. --- src/or/rendcommon.c | 31 ------------------------------- src/or/rendcommon.h | 1 - 2 files changed, 32 deletions(-) diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index 79aba80694..438fbc4d9a 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -688,37 +688,6 @@ rend_get_service_id(crypto_pk_t *pk, char *out) return 0; } -/** Determines whether a is in the interval of b (excluded) and - * c (included) in a circular digest ring; returns 1 if this is the - * case, and 0 otherwise. - */ -int -rend_id_is_in_interval(const char *a, const char *b, const char *c) -{ - int a_b, b_c, c_a; - tor_assert(a); - tor_assert(b); - tor_assert(c); - - /* There are five cases in which a is outside the interval ]b,c]: */ - a_b = tor_memcmp(a,b,DIGEST_LEN); - if (a_b == 0) - return 0; /* 1. a == b (b is excluded) */ - b_c = tor_memcmp(b,c,DIGEST_LEN); - if (b_c == 0) - return 0; /* 2. b == c (interval is empty) */ - else if (a_b <= 0 && b_c < 0) - return 0; /* 3. a b c */ - c_a = tor_memcmp(c,a,DIGEST_LEN); - if (c_a < 0 && a_b <= 0) - return 0; /* 4. c a b */ - else if (b_c < 0 && c_a < 0) - return 0; /* 5. b c a */ - - /* In the other cases (a c b; b a c; c b a), a is inside the interval. */ - return 1; -} - /** Return true iff query is a syntactically valid service ID (as * generated by rend_get_service_id). */ int diff --git a/src/or/rendcommon.h b/src/or/rendcommon.h index bab53fc1f6..c520c0634d 100644 --- a/src/or/rendcommon.h +++ b/src/or/rendcommon.h @@ -53,7 +53,6 @@ int rend_encode_v2_descriptors(smartlist_t *descs_out, int rend_compute_v2_desc_id(char *desc_id_out, const char *service_id, const char *descriptor_cookie, time_t now, uint8_t replica); -int rend_id_is_in_interval(const char *a, const char *b, const char *c); void rend_get_descriptor_id_bytes(char *descriptor_id_out, const char *service_id, const char *secret_id_part); From edeba3d472e303a9a3263582d1e4c0fbcb5a28b8 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Mon, 7 Mar 2016 19:29:05 -0500 Subject: [PATCH 4/6] simplify rend_cache_store_status_t back to a boolean it used to be a tri-state, but now it's just a bi-state, so we can take out all the machinery like the enum. --- src/or/directory.c | 20 ++++-------- src/or/rendcache.c | 30 +++++++++--------- src/or/rendcache.h | 18 ++++------- src/test/test_dir_handle_get.c | 2 +- src/test/test_rendcache.c | 58 +++++++++++++++++----------------- 5 files changed, 57 insertions(+), 71 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 961e33424b..6f271b2b1f 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -2297,10 +2297,8 @@ connection_dir_client_reached_eof(dir_connection_t *conn) { rend_cache_entry_t *entry = NULL; - switch (rend_cache_store_v2_desc_as_client(body, - conn->requested_resource, conn->rend_data, - &entry)) { - case RCS_BADDESC: + if (rend_cache_store_v2_desc_as_client(body, + conn->requested_resource, conn->rend_data, &entry) < 0) { log_warn(LD_REND,"Fetching v2 rendezvous descriptor failed. " "Retrying at another directory."); /* We'll retry when connection_about_to_close_connection() @@ -2308,11 +2306,9 @@ connection_dir_client_reached_eof(dir_connection_t *conn) SEND_HS_DESC_FAILED_EVENT("BAD_DESC"); SEND_HS_DESC_FAILED_CONTENT(); break; - case RCS_OKAY: - default: - { + } else { char service_id[REND_SERVICE_ID_LEN_BASE32 + 1]; - /* Should never be NULL here for an OKAY returned code. */ + /* Should never be NULL here if we found the descriptor. */ tor_assert(entry); rend_get_service_id(entry->parsed->pk, service_id); @@ -2330,7 +2326,6 @@ connection_dir_client_reached_eof(dir_connection_t *conn) rend_client_desc_trynow(service_id); memwipe(service_id, 0, sizeof(service_id)); break; - } } break; } @@ -3426,15 +3421,12 @@ directory_handle_command_post(dir_connection_t *conn, const char *headers, /* Handle v2 rendezvous service publish request. */ if (connection_dir_is_encrypted(conn) && !strcmpstart(url,"/tor/rendezvous2/publish")) { - switch (rend_cache_store_v2_desc_as_dir(body)) { - case RCS_BADDESC: + if (rend_cache_store_v2_desc_as_dir(body) < 0) { log_warn(LD_REND, "Rejected v2 rend descriptor (length %d) from %s.", (int)body_len, conn->base_.address); write_http_status_line(conn, 400, "Invalid v2 service descriptor rejected"); - break; - case RCS_OKAY: - default: + } else { write_http_status_line(conn, 200, "Service descriptor (v2) stored"); log_info(LD_REND, "Handled v2 rendezvous descriptor post: accepted"); } diff --git a/src/or/rendcache.c b/src/or/rendcache.c index dad4b2b5ba..7c5844262b 100644 --- a/src/or/rendcache.c +++ b/src/or/rendcache.c @@ -622,9 +622,9 @@ rend_cache_lookup_v2_desc_as_dir(const char *desc_id, const char **desc) * If we have a newer descriptor with the same ID, ignore this one. * If we have an older descriptor with the same ID, replace it. * - * Return an appropriate rend_cache_store_status_t. + * Return 0 on success, or -1 if we couldn't parse any of them. */ -rend_cache_store_status_t +int rend_cache_store_v2_desc_as_dir(const char *desc) { const or_options_t *options = get_options(); @@ -717,11 +717,11 @@ rend_cache_store_v2_desc_as_dir(const char *desc) } if (!number_parsed) { log_info(LD_REND, "Could not parse any descriptor."); - return RCS_BADDESC; + return -1; } log_info(LD_REND, "Parsed %d and added %d descriptor%s.", number_parsed, number_stored, number_stored != 1 ? "s" : ""); - return RCS_OKAY; + return 0; } /** Parse the v2 service descriptor in desc and store it to the @@ -731,9 +731,9 @@ rend_cache_store_v2_desc_as_dir(const char *desc) * If we have a newer descriptor with the same ID, ignore this one. * If we have an older descriptor with the same ID, replace it. * -* Return an appropriate rend_cache_store_status_t. +* Return 0 on success, or -1 if we couldn't understand the descriptor. */ -rend_cache_store_status_t +int rend_cache_store_v2_desc_as_service(const char *desc) { rend_service_descriptor_t *parsed = NULL; @@ -744,7 +744,7 @@ rend_cache_store_v2_desc_as_service(const char *desc) const char *next_desc; char service_id[REND_SERVICE_ID_LEN_BASE32+1]; rend_cache_entry_t *e; - rend_cache_store_status_t retval = RCS_BADDESC; + int retval = -1; tor_assert(rend_cache_local_service); tor_assert(desc); @@ -787,10 +787,10 @@ rend_cache_store_v2_desc_as_service(const char *desc) rend_cache_increment_allocation(rend_cache_entry_allocation(e)); log_debug(LD_REND,"Successfully stored rend desc '%s', len %d.", safe_str_client(service_id), (int)encoded_size); - return RCS_OKAY; + return 0; okay: - retval = RCS_OKAY; + retval = 0; err: rend_service_descriptor_free(parsed); @@ -811,10 +811,10 @@ rend_cache_store_v2_desc_as_service(const char *desc) * If the descriptor's descriptor ID doesn't match desc_id_base32, * reject it. * - * Return an appropriate rend_cache_store_status_t. If entry is not NULL, - * set it with the cache entry pointer of the descriptor. + * Return 0 on success, or -1 if we rejected the descriptor. + * If entry is not NULL, set it with the cache entry pointer of the descriptor. */ -rend_cache_store_status_t +int rend_cache_store_v2_desc_as_client(const char *desc, const char *desc_id_base32, const rend_data_t *rend_query, @@ -846,7 +846,7 @@ rend_cache_store_v2_desc_as_client(const char *desc, char service_id[REND_SERVICE_ID_LEN_BASE32+1]; char want_desc_id[DIGEST_LEN]; rend_cache_entry_t *e; - rend_cache_store_status_t retval = RCS_BADDESC; + int retval = -1; tor_assert(rend_cache); tor_assert(desc); tor_assert(desc_id_base32); @@ -993,13 +993,13 @@ rend_cache_store_v2_desc_as_client(const char *desc, if (entry) { *entry = e; } - return RCS_OKAY; + return 0; okay: if (entry) { *entry = e; } - retval = RCS_OKAY; + retval = 0; err: rend_service_descriptor_free(parsed); diff --git a/src/or/rendcache.h b/src/or/rendcache.h index 3eecd3b2a0..0e8b918753 100644 --- a/src/or/rendcache.h +++ b/src/or/rendcache.h @@ -64,19 +64,13 @@ int rend_cache_lookup_entry(const char *query, int version, int rend_cache_lookup_v2_desc_as_service(const char *query, rend_cache_entry_t **entry_out); int rend_cache_lookup_v2_desc_as_dir(const char *query, const char **desc); -/** Return value from rend_cache_store_v2_desc_as_{dir,client}. */ -typedef enum { - RCS_BADDESC = -1, /**< This descriptor is no good. */ - RCS_OKAY = 0 /**< All worked as expected */ -} rend_cache_store_status_t; -rend_cache_store_status_t rend_cache_store_v2_desc_as_dir(const char *desc); -rend_cache_store_status_t rend_cache_store_v2_desc_as_service( - const char *desc); -rend_cache_store_status_t rend_cache_store_v2_desc_as_client(const char *desc, - const char *desc_id_base32, - const rend_data_t *rend_query, - rend_cache_entry_t **entry); +int rend_cache_store_v2_desc_as_dir(const char *desc); +int rend_cache_store_v2_desc_as_service(const char *desc); +int rend_cache_store_v2_desc_as_client(const char *desc, + const char *desc_id_base32, + const rend_data_t *rend_query, + rend_cache_entry_t **entry); size_t rend_cache_get_total_allocation(void); void rend_cache_intro_failure_note(rend_intro_point_failure_t failure, diff --git a/src/test/test_dir_handle_get.c b/src/test/test_dir_handle_get.c index c75b2b6632..d74aeb6110 100644 --- a/src/test/test_dir_handle_get.c +++ b/src/test/test_dir_handle_get.c @@ -427,7 +427,7 @@ test_dir_handle_get_rendezvous2_on_encrypted_conn_success(void *data) generate_desc(RECENT_TIME, &desc_holder, &service_id, 3); tt_int_op(rend_cache_store_v2_desc_as_dir(desc_holder->desc_str), - OP_EQ, RCS_OKAY); + OP_EQ, 0); base32_encode(desc_id_base32, sizeof(desc_id_base32), desc_holder->desc_id, DIGEST_LEN); diff --git a/src/test/test_rendcache.c b/src/test/test_rendcache.c index 3ccfe5f9bc..d1b52649b2 100644 --- a/src/test/test_rendcache.c +++ b/src/test/test_rendcache.c @@ -91,7 +91,7 @@ test_rend_cache_lookup_entry(void *data) static void test_rend_cache_store_v2_desc_as_client(void *data) { - rend_cache_store_status_t ret; + int ret; rend_data_t *mock_rend_query; char desc_id_base32[REND_DESC_ID_V2_LEN_BASE32 + 1]; rend_cache_entry_t *entry = NULL; @@ -112,7 +112,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) desc_id_base32, mock_rend_query, &entry); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); tt_assert(entry); tt_int_op(entry->len, OP_EQ, strlen(desc_holder->desc_str)); tt_str_op(entry->desc, OP_EQ, desc_holder->desc_str); @@ -124,7 +124,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) /* desc_holder->desc_str, */ /* "3TOOLONG3TOOLONG3TOOLONG3TOOLONG3TOOLONG3TOOLONG", */ /* &mock_rend_query, NULL); */ - /* tt_int_op(ret, OP_EQ, RCS_BADDESC); */ + /* tt_int_op(ret, OP_EQ, -1); */ // Test bad base32 failure // This causes an assertion failure if we're running with assertions. @@ -132,13 +132,13 @@ test_rend_cache_store_v2_desc_as_client(void *data) #ifdef DISABLE_ASSERTS_IN_UNIT_TESTS ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, "!xqunszqnaolrrfmtzgaki7mxelgvkj", mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_BADDESC); + tt_int_op(ret, OP_EQ, -1); #endif // Test invalid descriptor ret = rend_cache_store_v2_desc_as_client("invalid descriptor", "3xqunszqnaolrrfmtzgaki7mxelgvkje", mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_BADDESC); + tt_int_op(ret, OP_EQ, -1); // TODO: it doesn't seem to be possible to test invalid service ID condition. // that means it is likely not possible to have that condition without @@ -152,7 +152,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_BADDESC); + tt_int_op(ret, OP_EQ, -1); rend_cache_free_all(); rend_data_free(mock_rend_query); @@ -163,7 +163,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_BADDESC); + tt_int_op(ret, OP_EQ, -1); desc_id_base32[0]--; rend_cache_free_all(); @@ -181,7 +181,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_BADDESC); + tt_int_op(ret, OP_EQ, -1); rend_cache_free_all(); // Test too new descriptor (in the future) @@ -198,7 +198,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_BADDESC); + tt_int_op(ret, OP_EQ, -1); rend_cache_free_all(); // Test when a descriptor is already in the cache @@ -217,12 +217,12 @@ test_rend_cache_store_v2_desc_as_client(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, desc_id_base32, mock_rend_query, &entry); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); tt_assert(entry); rend_cache_free_all(); @@ -243,7 +243,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); rend_cache_free_all(); // Test successful run when we have REND_BASIC_AUTH but not cookie @@ -260,7 +260,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); rend_cache_free_all(); @@ -277,7 +277,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_BADDESC); + tt_int_op(ret, OP_EQ, -1); rend_cache_free_all(); // Test when we have too many intro points @@ -293,7 +293,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_BADDESC); + tt_int_op(ret, OP_EQ, -1); done: rend_encoded_v2_service_descriptor_free(desc_holder); @@ -305,7 +305,7 @@ test_rend_cache_store_v2_desc_as_client(void *data) static void test_rend_cache_store_v2_desc_as_client_with_different_time(void *data) { - rend_cache_store_status_t ret; + int ret; rend_data_t *mock_rend_query; char desc_id_base32[REND_DESC_ID_V2_LEN_BASE32 + 1]; rend_service_descriptor_t *generated = NULL; @@ -350,7 +350,7 @@ test_rend_cache_store_v2_desc_as_client_with_different_time(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder_older->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); rend_cache_free_all(); @@ -361,7 +361,7 @@ test_rend_cache_store_v2_desc_as_client_with_different_time(void *data) ret = rend_cache_store_v2_desc_as_client(desc_holder_newer->desc_str, desc_id_base32, mock_rend_query, NULL); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); done: rend_encoded_v2_service_descriptor_free(desc_holder_newer); @@ -446,7 +446,7 @@ static void test_rend_cache_store_v2_desc_as_dir(void *data) { (void)data; - rend_cache_store_status_t ret; + int ret; rend_encoded_v2_service_descriptor_t *desc_holder = NULL; char *service_id = NULL; @@ -457,12 +457,12 @@ test_rend_cache_store_v2_desc_as_dir(void *data) // Test when we can't parse the descriptor mock_routerinfo = tor_malloc(sizeof(routerinfo_t)); ret = rend_cache_store_v2_desc_as_dir("unparseable"); - tt_int_op(ret, OP_EQ, RCS_BADDESC); + tt_int_op(ret, OP_EQ, -1); // Test when we have an old descriptor generate_desc(TIME_IN_THE_PAST, &desc_holder, &service_id, 3); ret = rend_cache_store_v2_desc_as_dir(desc_holder->desc_str); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); rend_encoded_v2_service_descriptor_free(desc_holder); tor_free(service_id); @@ -470,7 +470,7 @@ test_rend_cache_store_v2_desc_as_dir(void *data) // Test when we have a descriptor in the future generate_desc(TIME_IN_THE_FUTURE, &desc_holder, &service_id, 3); ret = rend_cache_store_v2_desc_as_dir(desc_holder->desc_str); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); rend_encoded_v2_service_descriptor_free(desc_holder); tor_free(service_id); @@ -478,7 +478,7 @@ test_rend_cache_store_v2_desc_as_dir(void *data) // Test when two descriptors generate_desc(TIME_IN_THE_FUTURE, &desc_holder, &service_id, 3); ret = rend_cache_store_v2_desc_as_dir(desc_holder->desc_str); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); rend_encoded_v2_service_descriptor_free(desc_holder); tor_free(service_id); @@ -488,7 +488,7 @@ test_rend_cache_store_v2_desc_as_dir(void *data) generate_desc(RECENT_TIME, &desc_holder, &service_id, 3); get_options_mutable()->HiddenServiceStatistics = 1; ret = rend_cache_store_v2_desc_as_dir(desc_holder->desc_str); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); done: NS_UNMOCK(router_get_my_routerinfo); @@ -503,7 +503,7 @@ test_rend_cache_store_v2_desc_as_dir_with_different_time(void *data) { (void)data; - rend_cache_store_status_t ret; + int ret; rend_service_descriptor_t *generated = NULL; smartlist_t *descs = smartlist_new(); time_t t; @@ -540,13 +540,13 @@ test_rend_cache_store_v2_desc_as_dir_with_different_time(void *data) mock_routerinfo = tor_malloc(sizeof(routerinfo_t)); rend_cache_store_v2_desc_as_dir(desc_holder_newer->desc_str); ret = rend_cache_store_v2_desc_as_dir(desc_holder_older->desc_str); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); // Test when we have an old descriptor stored rend_cache_purge(); rend_cache_store_v2_desc_as_dir(desc_holder_older->desc_str); ret = rend_cache_store_v2_desc_as_dir(desc_holder_newer->desc_str); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); done: NS_UNMOCK(router_get_my_routerinfo); @@ -566,7 +566,7 @@ test_rend_cache_store_v2_desc_as_dir_with_different_content(void *data) { (void)data; - rend_cache_store_status_t ret; + int ret; rend_service_descriptor_t *generated = NULL; smartlist_t *descs = smartlist_new(); time_t t; @@ -605,7 +605,7 @@ test_rend_cache_store_v2_desc_as_dir_with_different_content(void *data) mock_routerinfo = tor_malloc(sizeof(routerinfo_t)); rend_cache_store_v2_desc_as_dir(desc_holder_one->desc_str); ret = rend_cache_store_v2_desc_as_dir(desc_holder_two->desc_str); - tt_int_op(ret, OP_EQ, RCS_OKAY); + tt_int_op(ret, OP_EQ, 0); done: NS_UNMOCK(router_get_my_routerinfo); From 413b0c0ddba289da2cd90f394940f5f58a92a1a6 Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Fri, 11 Mar 2016 13:28:51 -0500 Subject: [PATCH 5/6] simplify the #18332 changes file based on feedback on ticket --- changes/ticket18332 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/changes/ticket18332 b/changes/ticket18332 index 6fb811827f..9a8a6bd6ab 100644 --- a/changes/ticket18332 +++ b/changes/ticket18332 @@ -1,7 +1,6 @@ o Removed features: - Streamline relay-side hsdir handling: when relays consider whether to accept an uploaded hidden service descriptor, they no longer - check whether directory authorities think they deserve the HSDir - flag, or whether they are one of the relays in the network that is + check whether they are one of the relays in the network that is "supposed" to handle that descriptor. Implements ticket 18332. From e28448a23e06e4bc997e1fdcca5af04a9d20598c Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Wed, 16 Mar 2016 16:46:14 -0400 Subject: [PATCH 6/6] Bridges now refuse "rendezvous2" publish attempts Suggested during review of ticket 18332. --- changes/ticket18332 | 4 ++++ src/or/directory.c | 7 +++++++ src/or/rendcache.c | 2 ++ 3 files changed, 13 insertions(+) diff --git a/changes/ticket18332 b/changes/ticket18332 index 9a8a6bd6ab..d19c617392 100644 --- a/changes/ticket18332 +++ b/changes/ticket18332 @@ -4,3 +4,7 @@ check whether they are one of the relays in the network that is "supposed" to handle that descriptor. Implements ticket 18332. + o Minor bugfixes: + - Bridges now refuse "rendezvous2" (hidden service descriptor) + publish attempts. Suggested by ticket 18332. + diff --git a/src/or/directory.c b/src/or/directory.c index 6f271b2b1f..39fffc749e 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -3412,6 +3412,13 @@ directory_handle_command_post(dir_connection_t *conn, const char *headers, conn->base_.state = DIR_CONN_STATE_SERVER_WRITING; + if (!public_server_mode(options)) { + log_info(LD_DIR, "Rejected dir post request from %s " + "since we're not a public relay.", conn->base_.address); + write_http_status_line(conn, 503, "Not acting as a public relay"); + goto done; + } + if (parse_http_url(headers, &url) < 0) { write_http_status_line(conn, 400, "Bad request"); return 0; diff --git a/src/or/rendcache.c b/src/or/rendcache.c index 7c5844262b..8b2d3ce6f7 100644 --- a/src/or/rendcache.c +++ b/src/or/rendcache.c @@ -623,6 +623,8 @@ rend_cache_lookup_v2_desc_as_dir(const char *desc_id, const char **desc) * If we have an older descriptor with the same ID, replace it. * * Return 0 on success, or -1 if we couldn't parse any of them. + * + * We should only call this function for public (e.g. non bridge) relays. */ int rend_cache_store_v2_desc_as_dir(const char *desc)