- tor-commits - lists.torproject.org

[tor/master] rip out hid_serv_responsible_for_desc_id()
by nickm＠torproject.org 21 Mar '16

21 Mar '16

commit e167910fce2b83d9de3a252cdf02cabdafced14b Author: Roger Dingledine <arma(a)torproject.org> Date: Fri Mar 11 10:40:31 2016 -0500 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(-) diff --git a/changes/ticket18332 b/changes/ticket18332 new file mode 100644 index 0000000..6fb8118 --- /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 cb8c14b..6c22503 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 79509cb..f1fb0d9 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 fe45299..b3ae790 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 954830d..67bd840 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 eaa8671..9898e87 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(); }

1 0

[tor/master] rip out hid_serv_acting_as_directory()
by nickm＠torproject.org 21 Mar '16

21 Mar '16

commit 53902963383e1babfccb8a4ffc7ed4e8accf2214 Author: Roger Dingledine <arma(a)torproject.org> Date: Fri Mar 11 10:45:03 2016 -0500 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 e4feda4..961e334 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 6c22503..dad4b2b 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 867270f..3eecd3b 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 f1fb0d9..79aba80 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 b3ae790..bab53fc 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 67bd840..c75b2b6 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 9898e87..3ccfe5f 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");

1 0

[tor/master] rip out rend_id_is_in_interval()
by nickm＠torproject.org 21 Mar '16

21 Mar '16

commit dc500c8cb4dc7643230d160d1b458a0acdadd242 Author: Roger Dingledine <arma(a)torproject.org> Date: Fri Mar 11 10:46:21 2016 -0500 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 79aba80..438fbc4 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 bab53fc..c520c06 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);

1 0

[tor/master] simplify rend_cache_store_status_t back to a boolean
by nickm＠torproject.org 21 Mar '16

21 Mar '16

commit edeba3d472e303a9a3263582d1e4c0fbcb5a28b8 Author: Roger Dingledine <arma(a)torproject.org> Date: Mon Mar 7 19:29:05 2016 -0500 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 | 20 +++++---------- src/test/test_dir_handle_get.c | 2 +- src/test/test_rendcache.c | 58 +++++++++++++++++++++--------------------- 5 files changed, 58 insertions(+), 72 deletions(-) diff --git a/src/or/directory.c b/src/or/directory.c index 961e334..6f271b2 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 dad4b2b..7c58442 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 3eecd3b..0e8b918 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 c75b2b6..d74aeb6 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 3ccfe5f..d1b5264 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);

1 0

[tor/master] simplify the #18332 changes file based on feedback on ticket
by nickm＠torproject.org 21 Mar '16

21 Mar '16

commit 413b0c0ddba289da2cd90f394940f5f58a92a1a6 Author: Roger Dingledine <arma(a)torproject.org> Date: Fri Mar 11 13:28:51 2016 -0500 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 6fb8118..9a8a6bd 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.

1 0

[tor/master] Bridges now refuse "rendezvous2" publish attempts
by nickm＠torproject.org 21 Mar '16

21 Mar '16

commit e28448a23e06e4bc997e1fdcca5af04a9d20598c Author: Roger Dingledine <arma(a)torproject.org> Date: Wed Mar 16 16:46:14 2016 -0400 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 9a8a6bd..d19c617 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 6f271b2..39fffc7 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 7c58442..8b2d3ce 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)

1 0

[tor/master] Merge remote-tracking branch 'arma/ticket18332-try3'
by nickm＠torproject.org 21 Mar '16

21 Mar '16

commit ddd30f966a4b186d42650e3b321da8352a19b5f5 Merge: 70024ea e28448a Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 21 10:41:23 2016 -0400 Merge remote-tracking branch 'arma/ticket18332-try3' changes/ticket18332 | 10 ++++ src/or/directory.c | 35 +++++------ src/or/rendcache.c | 49 ++++++---------- src/or/rendcache.h | 21 +++---- src/or/rendcommon.c | 69 ---------------------- src/or/rendcommon.h | 3 - src/test/test_dir_handle_get.c | 16 +---- src/test/test_rendcache.c | 129 +++++++++-------------------------------- 8 files changed, 78 insertions(+), 254 deletions(-) diff --cc src/or/rendcommon.h index 7d81976,c520c06..d67552e --- a/src/or/rendcommon.h +++ b/src/or/rendcommon.h @@@ -58,9 -57,7 +57,7 @@@ void rend_get_descriptor_id_bytes(char const char *service_id, const char *secret_id_part); int hid_serv_get_responsible_directories(smartlist_t *responsible_dirs, - const char *id); + 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,

1 0

[tor/master] Scrub service name in introduction circuit warning
by nickm＠torproject.org 21 Mar '16

21 Mar '16

commit 2c057c2833ff2ab0779cbd1e5f3747be0a232126 Author: John Brooks <special(a)torproject.org> Date: Mon Mar 21 19:22:14 2016 +0700 Scrub service name in introduction circuit warning Fixes bug 18600. --- src/or/rendservice.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/rendservice.c b/src/or/rendservice.c index 09b20e3..39e5831 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -2717,7 +2717,7 @@ rend_service_intro_has_opened(origin_circuit_t *circuit) circuit->rend_data->rend_pk_digest); if (!service) { log_warn(LD_REND, "Unrecognized service ID %s on introduction circuit %u.", - serviceid, (unsigned)circuit->base_.n_circ_id); + safe_str_client(serviceid), (unsigned)circuit->base_.n_circ_id); reason = END_CIRC_REASON_NOSUCHSERVICE; goto err; }

1 0

[tor/master] Add new channel/queue_incoming unit tests; modify channel unit tests for new clarified handling of alloc/free responsibility for queued incoming cells
by nickm＠torproject.org 21 Mar '16

21 Mar '16

commit 1cdc7fddb2ed10d72f4e65e15d1af4d803a1acdb Author: Andrea Shepard <andrea(a)torproject.org> Date: Tue Mar 15 03:28:04 2016 +0000 Add new channel/queue_incoming unit tests; modify channel unit tests for new clarified handling of alloc/free responsibility for queued incoming cells --- src/test/test_channel.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 116 insertions(+), 5 deletions(-) diff --git a/src/test/test_channel.c b/src/test/test_channel.c index e11ac3f..b705ee5 100644 --- a/src/test/test_channel.c +++ b/src/test/test_channel.c @@ -25,7 +25,9 @@ extern uint64_t estimated_total_queue_size; static int test_chan_accept_cells = 0; static int test_chan_fixed_cells_recved = 0; +static cell_t * test_chan_last_seen_fixed_cell_ptr = NULL; static int test_chan_var_cells_recved = 0; +static var_cell_t * test_chan_last_seen_var_cell_ptr = NULL; static int test_cells_written = 0; static int test_destroy_not_pending_calls = 0; static int test_doesnt_want_writes_count = 0; @@ -70,6 +72,7 @@ static void test_channel_flushmux(void *arg); static void test_channel_incoming(void *arg); static void test_channel_lifecycle(void *arg); static void test_channel_multi(void *arg); +static void test_channel_queue_incoming(void *arg); static void test_channel_queue_size(void *arg); static void test_channel_write(void *arg); @@ -179,7 +182,7 @@ chan_test_cell_handler(channel_t *ch, tt_assert(ch); tt_assert(cell); - tor_free(cell); + test_chan_last_seen_fixed_cell_ptr = cell; ++test_chan_fixed_cells_recved; done: @@ -214,7 +217,7 @@ chan_test_var_cell_handler(channel_t *ch, tt_assert(ch); tt_assert(var_cell); - tor_free(var_cell); + test_chan_last_seen_var_cell_ptr = var_cell; ++test_chan_var_cells_recved; done: @@ -608,7 +611,7 @@ test_channel_dumpstats(void *arg) make_fake_cell(cell); old_count = test_chan_fixed_cells_recved; channel_queue_cell(ch, cell); - cell = NULL; + tor_free(cell); tt_int_op(test_chan_fixed_cells_recved, ==, old_count + 1); tt_assert(ch->n_bytes_recved > 0); tt_assert(ch->n_cells_recved > 0); @@ -819,7 +822,7 @@ test_channel_incoming(void *arg) make_fake_cell(cell); old_count = test_chan_fixed_cells_recved; channel_queue_cell(ch, cell); - cell = NULL; + tor_free(cell); tt_int_op(test_chan_fixed_cells_recved, ==, old_count + 1); /* Receive a variable-size cell */ @@ -827,7 +830,7 @@ test_channel_incoming(void *arg) make_fake_var_cell(var_cell); old_count = test_chan_var_cells_recved; channel_queue_var_cell(ch, var_cell); - var_cell = NULL; + tor_free(cell); tt_int_op(test_chan_var_cells_recved, ==, old_count + 1); /* Close it */ @@ -1423,6 +1426,113 @@ test_channel_queue_impossible(void *arg) } static void +test_channel_queue_incoming(void *arg) +{ + channel_t *ch = NULL; + cell_t *cell = NULL; + var_cell_t *var_cell = NULL; + int old_fixed_count, old_var_count; + + (void)arg; + + /* Mock these for duration of the test */ + MOCK(scheduler_channel_doesnt_want_writes, + scheduler_channel_doesnt_want_writes_mock); + MOCK(scheduler_release_channel, + scheduler_release_channel_mock); + + /* Accept cells to lower layer */ + test_chan_accept_cells = 1; + /* Use default overhead factor */ + test_overhead_estimate = 1.0f; + + ch = new_fake_channel(); + tt_assert(ch); + /* Start it off in OPENING */ + ch->state = CHANNEL_STATE_OPENING; + /* We'll need a cmux */ + ch->cmux = circuitmux_alloc(); + + /* Test cell handler getters */ + tt_ptr_op(channel_get_cell_handler(ch), ==, NULL); + tt_ptr_op(channel_get_var_cell_handler(ch), ==, NULL); + + /* Try to register it */ + channel_register(ch); + tt_assert(ch->registered); + + /* Open it */ + channel_change_state(ch, CHANNEL_STATE_OPEN); + tt_int_op(ch->state, ==, CHANNEL_STATE_OPEN); + + /* Assert that the incoming queue is empty */ + tt_assert(TOR_SIMPLEQ_EMPTY(&(ch->incoming_queue))); + + /* Queue an incoming fixed-length cell */ + cell = tor_malloc_zero(sizeof(cell_t)); + make_fake_cell(cell); + channel_queue_cell(ch, cell); + + /* Assert that the incoming queue has one entry */ + tt_int_op(chan_cell_queue_len(&(ch->incoming_queue)), ==, 1); + + /* Queue an incoming var cell */ + var_cell = tor_malloc_zero(sizeof(var_cell_t) + CELL_PAYLOAD_SIZE); + make_fake_var_cell(var_cell); + channel_queue_var_cell(ch, var_cell); + + /* Assert that the incoming queue has two entries */ + tt_int_op(chan_cell_queue_len(&(ch->incoming_queue)), ==, 2); + + /* + * Install cell handlers; this will drain the queue, so save the old + * cell counters first + */ + old_fixed_count = test_chan_fixed_cells_recved; + old_var_count = test_chan_var_cells_recved; + channel_set_cell_handlers(ch, + chan_test_cell_handler, + chan_test_var_cell_handler); + tt_ptr_op(channel_get_cell_handler(ch), ==, chan_test_cell_handler); + tt_ptr_op(channel_get_var_cell_handler(ch), ==, chan_test_var_cell_handler); + + /* Assert cells were received */ + tt_int_op(test_chan_fixed_cells_recved, ==, old_fixed_count + 1); + tt_int_op(test_chan_var_cells_recved, ==, old_var_count + 1); + + /* + * Assert that the pointers are different from the cells we allocated; + * when queueing cells with no incoming cell handlers installed, the + * channel layer should copy them to a new buffer, and free them after + * delivery. These pointers will have already been freed by the time + * we get here, so don't dereference them. + */ + tt_ptr_op(test_chan_last_seen_fixed_cell_ptr, !=, cell); + tt_ptr_op(test_chan_last_seen_var_cell_ptr, !=, var_cell); + + /* Assert queue is now empty */ + tt_assert(TOR_SIMPLEQ_EMPTY(&(ch->incoming_queue))); + + /* Close it; this contains an assertion that the incoming queue is empty */ + channel_mark_for_close(ch); + tt_int_op(ch->state, ==, CHANNEL_STATE_CLOSING); + chan_test_finish_close(ch); + tt_int_op(ch->state, ==, CHANNEL_STATE_CLOSED); + channel_run_cleanup(); + ch = NULL; + + done: + free_fake_channel(ch); + tor_free(cell); + tor_free(var_cell); + + UNMOCK(scheduler_channel_doesnt_want_writes); + UNMOCK(scheduler_release_channel); + + return; +} + +static void test_channel_queue_size(void *arg) { channel_t *ch = NULL; @@ -1666,6 +1776,7 @@ struct testcase_t channel_tests[] = { { "lifecycle_2", test_channel_lifecycle_2, TT_FORK, NULL, NULL }, { "multi", test_channel_multi, TT_FORK, NULL, NULL }, { "queue_impossible", test_channel_queue_impossible, TT_FORK, NULL, NULL }, + { "queue_incoming", test_channel_queue_incoming, TT_FORK, NULL, NULL }, { "queue_size", test_channel_queue_size, TT_FORK, NULL, NULL }, { "write", test_channel_write, TT_FORK, NULL, NULL }, END_OF_TESTCASES

1 0

[tor/master] Make sure channel_t queues its own copy of incoming cells
by nickm＠torproject.org 21 Mar '16

21 Mar '16

commit bd87d37a861c541afbeb660b4d8dd62df14d5b45 Author: Andrea Shepard <andrea(a)torproject.org> Date: Tue Mar 15 07:40:19 2016 +0000 Make sure channel_t queues its own copy of incoming cells --- src/or/channel.c | 38 ++++++++++++++++++++++++++++++++++---- src/or/channeltls.c | 11 +++++++++++ src/or/connection_or.c | 35 +++++++++++++++++++++++++++++++++++ src/or/connection_or.h | 1 + 4 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/or/channel.c b/src/or/channel.c index 21522a5..62a21be 100644 --- a/src/or/channel.c +++ b/src/or/channel.c @@ -2652,6 +2652,11 @@ channel_process_cells(channel_t *chan) /* * Process cells until we're done or find one we have no current handler * for. + * + * We must free the cells here after calling the handler, since custody + * of the buffer was given to the channel layer when they were queued; + * see comments on memory management in channel_queue_cell() and in + * channel_queue_var_cell() below. */ while (NULL != (q = TOR_SIMPLEQ_FIRST(&chan->incoming_queue))) { tor_assert(q); @@ -2669,6 +2674,7 @@ channel_process_cells(channel_t *chan) q->u.fixed.cell, chan, U64_PRINTF_ARG(chan->global_identifier)); chan->cell_handler(chan, q->u.fixed.cell); + tor_free(q->u.fixed.cell); tor_free(q); } else if (q->type == CELL_QUEUE_VAR && chan->var_cell_handler) { @@ -2681,6 +2687,7 @@ channel_process_cells(channel_t *chan) q->u.var.var_cell, chan, U64_PRINTF_ARG(chan->global_identifier)); chan->var_cell_handler(chan, q->u.var.var_cell); + tor_free(q->u.var.var_cell); tor_free(q); } else { /* Can't handle this one */ @@ -2701,6 +2708,7 @@ channel_queue_cell(channel_t *chan, cell_t *cell) { int need_to_queue = 0; cell_queue_entry_t *q; + cell_t *cell_copy = NULL; tor_assert(chan); tor_assert(cell); @@ -2728,8 +2736,19 @@ channel_queue_cell(channel_t *chan, cell_t *cell) U64_PRINTF_ARG(chan->global_identifier)); chan->cell_handler(chan, cell); } else { - /* Otherwise queue it and then process the queue if possible. */ - q = cell_queue_entry_new_fixed(cell); + /* + * Otherwise queue it and then process the queue if possible. + * + * We queue a copy, not the original pointer - it might have been on the + * stack in connection_or_process_cells_from_inbuf() (or another caller + * if we ever have a subclass other than channel_tls_t), or be freed + * there after we return. This is the uncommon case; the non-copying + * fast path occurs in the if (!need_to_queue) case above when the + * upper layer has installed cell handlers. + */ + cell_copy = tor_malloc_zero(sizeof(cell_t)); + memcpy(cell_copy, cell, sizeof(cell_t)); + q = cell_queue_entry_new_fixed(cell_copy); log_debug(LD_CHANNEL, "Queueing incoming cell_t %p for channel %p " "(global ID " U64_FORMAT ")", @@ -2755,6 +2774,7 @@ channel_queue_var_cell(channel_t *chan, var_cell_t *var_cell) { int need_to_queue = 0; cell_queue_entry_t *q; + var_cell_t *cell_copy = NULL; tor_assert(chan); tor_assert(var_cell); @@ -2783,8 +2803,18 @@ channel_queue_var_cell(channel_t *chan, var_cell_t *var_cell) U64_PRINTF_ARG(chan->global_identifier)); chan->var_cell_handler(chan, var_cell); } else { - /* Otherwise queue it and then process the queue if possible. */ - q = cell_queue_entry_new_var(var_cell); + /* + * Otherwise queue it and then process the queue if possible. + * + * We queue a copy, not the original pointer - it might have been on the + * stack in connection_or_process_cells_from_inbuf() (or another caller + * if we ever have a subclass other than channel_tls_t), or be freed + * there after we return. This is the uncommon case; the non-copying + * fast path occurs in the if (!need_to_queue) case above when the + * upper layer has installed cell handlers. + */ + cell_copy = var_cell_copy(var_cell); + q = cell_queue_entry_new_var(cell_copy); log_debug(LD_CHANNEL, "Queueing incoming var_cell_t %p for channel %p " "(global ID " U64_FORMAT ")", diff --git a/src/or/channeltls.c b/src/or/channeltls.c index c90f569..2a84514 100644 --- a/src/or/channeltls.c +++ b/src/or/channeltls.c @@ -1009,6 +1009,11 @@ channel_tls_time_process_cell(cell_t *cell, channel_tls_t *chan, int *time, * for cell types specific to the handshake for this transport protocol and * handles them, and queues all other cells to the channel_t layer, which * eventually will hand them off to command.c. + * + * The channel layer itself decides whether the cell should be queued or + * can be handed off immediately to the upper-layer code. It is responsible + * for copying in the case that it queues; we merely pass pointers through + * which we get from connection_or_process_cells_from_inbuf(). */ void @@ -1106,6 +1111,12 @@ channel_tls_handle_cell(cell_t *cell, or_connection_t *conn) * related and live below the channel_t layer, so no variable-length * cells ever get delivered in the current implementation, but I've left * the mechanism in place for future use. + * + * If we were handing them off to the upper layer, the channel_t queueing + * code would be responsible for memory management, and we'd just be passing + * pointers through from connection_or_process_cells_from_inbuf(). That + * caller always frees them after this function returns, so this function + * should never free var_cell. */ void diff --git a/src/or/connection_or.c b/src/or/connection_or.c index a967c93..9944494 100644 --- a/src/or/connection_or.c +++ b/src/or/connection_or.c @@ -488,6 +488,28 @@ var_cell_new(uint16_t payload_len) return cell; } +/** + * Copy a var_cell_t + */ + +var_cell_t * +var_cell_copy(const var_cell_t *src) +{ + var_cell_t *copy = NULL; + size_t size = 0; + + if (src != NULL) { + size = STRUCT_OFFSET(var_cell_t, payload) + src->payload_len; + copy = tor_malloc_zero(size); + copy->payload_len = src->payload_len; + copy->command = src->command; + copy->circ_id = src->circ_id; + memcpy(copy->payload, src->payload, copy->payload_len); + } + + return copy; +} + /** Release all space held by cell. */ void var_cell_free(var_cell_t *cell) @@ -2060,6 +2082,19 @@ connection_or_process_cells_from_inbuf(or_connection_t *conn) { var_cell_t *var_cell; + /* + * Note on memory management for incoming cells: below the channel layer, + * we shouldn't need to consider its internal queueing/copying logic. It + * is safe to pass cells to it on the stack or on the heap, but in the + * latter case we must be sure we free them later. + * + * The incoming cell queue code in channel.c will (in the common case) + * decide it can pass them to the upper layer immediately, in which case + * those functions may run directly on the cell pointers we pass here, or + * it may decide to queue them, in which case it will allocate its own + * buffer and copy the cell. + */ + while (1) { log_debug(LD_OR, TOR_SOCKET_T_FORMAT": starting, inbuf_datalen %d " diff --git a/src/or/connection_or.h b/src/or/connection_or.h index 3877fd5..0bd8567 100644 --- a/src/or/connection_or.h +++ b/src/or/connection_or.h @@ -97,6 +97,7 @@ void cell_pack(packed_cell_t *dest, const cell_t *src, int wide_circ_ids); int var_cell_pack_header(const var_cell_t *cell, char *hdr_out, int wide_circ_ids); var_cell_t *var_cell_new(uint16_t payload_len); +var_cell_t *var_cell_copy(const var_cell_t *src); void var_cell_free(var_cell_t *cell); /** DOCDOC */

1 0