commit 011307dd5fa608739456b98d259b013286320b91 Author: Neel Chauhan <neel@neelc.org> Date: Thu Apr 11 15:20:31 2019 -0400 Make repeated/rate limited HSFETCH queries fail with QUERY_RATE_LIMITED --- changes/bug28269 | 7 +++++++ src/feature/hs/hs_client.c | 2 +- src/feature/hs/hs_common.c | 20 ++++++++++++++++---- src/feature/hs/hs_common.h | 2 +- src/feature/rend/rendclient.c | 9 ++++++--- src/test/test_hs.c | 10 ++++++++++ 6 files changed, 41 insertions(+), 9 deletions(-) diff --git a/changes/bug28269 b/changes/bug28269 new file mode 100644 index 000000000..bdfe9e1aa --- /dev/null +++ b/changes/bug28269 @@ -0,0 +1,7 @@ + o Minor bugfixes (onion services): + - If we are launching repeated HSFETCH queries and are rate-limited, + we introduce a new controller response QUERY_RATE_LIMITED instead + of QUERY_NO_HSDIR, while keeping the latter for when onion service + directories are missing a descriptor. Previously, we returned + QUERY_NO_HSDIR for both cases. Fixes bug 28269; bugfix on + 0.3.1.1-alpha. Patch by Neel Chauhan diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index c34271efc..b4b9f0a94 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -434,7 +434,7 @@ pick_hsdir_v3(const ed25519_public_key_t *onion_identity_pk) /* Pick an HSDir from the responsible ones. The ownership of * responsible_hsdirs is given to this function so no need to free it. */ - hsdir_rs = hs_pick_hsdir(responsible_hsdirs, base64_blinded_pubkey); + hsdir_rs = hs_pick_hsdir(responsible_hsdirs, base64_blinded_pubkey, NULL); return hsdir_rs; } diff --git a/src/feature/hs/hs_common.c b/src/feature/hs/hs_common.c index b2227432d..cffec2b87 100644 --- a/src/feature/hs/hs_common.c +++ b/src/feature/hs/hs_common.c @@ -1589,20 +1589,25 @@ hs_purge_last_hid_serv_requests(void) /** Given the list of responsible HSDirs in <b>responsible_dirs</b>, pick the * one that we should use to fetch a descriptor right now. Take into account * previous failed attempts at fetching this descriptor from HSDirs using the - * string identifier <b>req_key_str</b>. + * string identifier <b>req_key_str</b>. We return whether we are rate limited + * into *<b>is_rate_limited</b> if it is not NULL. * * Steals ownership of <b>responsible_dirs</b>. * * Return the routerstatus of the chosen HSDir if successful, otherwise return * NULL if no HSDirs are worth trying right now. */ routerstatus_t * -hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str) +hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str, + int *is_rate_limited) { smartlist_t *usable_responsible_dirs = smartlist_new(); const or_options_t *options = get_options(); routerstatus_t *hs_dir; time_t now = time(NULL); int excluded_some; + int rate_limited; + int rate_limited_count = 0; + int responsible_dirs_count = smartlist_len(responsible_dirs); tor_assert(req_key_str); @@ -1622,6 +1627,7 @@ hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str) if (last + hs_hsdir_requery_period(options) >= now || !node || !node_has_preferred_descriptor(node, 0)) { SMARTLIST_DEL_CURRENT(responsible_dirs, dir); + rate_limited_count++; continue; } if (!routerset_contains_node(options->ExcludeNodes, node)) { @@ -1629,6 +1635,7 @@ hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str) } } SMARTLIST_FOREACH_END(dir); + rate_limited = rate_limited_count == responsible_dirs_count; excluded_some = smartlist_len(usable_responsible_dirs) < smartlist_len(responsible_dirs); @@ -1640,9 +1647,10 @@ hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str) smartlist_free(responsible_dirs); smartlist_free(usable_responsible_dirs); if (!hs_dir) { + const char *warn_str = (rate_limited) ? "we are rate limited." : + "we requested them all recently without success"; log_info(LD_REND, "Could not pick one of the responsible hidden " - "service directories, because we requested them all " - "recently without success."); + "service directories, because %s.", warn_str); if (options->StrictNodes && excluded_some) { log_warn(LD_REND, "Could not pick a hidden service directory for the " "requested hidden service: they are all either down or " @@ -1654,6 +1662,10 @@ hs_pick_hsdir(smartlist_t *responsible_dirs, const char *req_key_str) hs_lookup_last_hid_serv_request(hs_dir, req_key_str, now, 1); } + if (is_rate_limited != NULL) { + *is_rate_limited = rate_limited; + } + return hs_dir; } diff --git a/src/feature/hs/hs_common.h b/src/feature/hs/hs_common.h index abf39fa43..f96fc8beb 100644 --- a/src/feature/hs/hs_common.h +++ b/src/feature/hs/hs_common.h @@ -241,7 +241,7 @@ void hs_get_responsible_hsdirs(const struct ed25519_public_key_t *blinded_pk, int use_second_hsdir_index, int for_fetching, smartlist_t *responsible_dirs); routerstatus_t *hs_pick_hsdir(smartlist_t *responsible_dirs, - const char *req_key_str); + const char *req_key_str, int *is_rate_limited); time_t hs_hsdir_requery_period(const or_options_t *options); time_t hs_lookup_last_hid_serv_request(routerstatus_t *hs_dir, diff --git a/src/feature/rend/rendclient.c b/src/feature/rend/rendclient.c index 5a8b23454..9863fc1c1 100644 --- a/src/feature/rend/rendclient.c +++ b/src/feature/rend/rendclient.c @@ -469,16 +469,19 @@ directory_get_from_hs_dir(const char *desc_id, /* Automatically pick an hs dir if none given. */ if (!rs_hsdir) { + int rate_limited; + /* Determine responsible dirs. Even if we can't get all we want, work with * the ones we have. If it's empty, we'll notice in hs_pick_hsdir(). */ smartlist_t *responsible_dirs = smartlist_new(); hid_serv_get_responsible_directories(responsible_dirs, desc_id); - hs_dir = hs_pick_hsdir(responsible_dirs, desc_id_base32); + hs_dir = hs_pick_hsdir(responsible_dirs, desc_id_base32, &rate_limited); if (!hs_dir) { /* No suitable hs dir can be found, stop right now. */ - control_event_hsv2_descriptor_failed(rend_query, NULL, - "QUERY_NO_HSDIR"); + const char *query_response = (rate_limited) ? "QUERY_RATE_LIMITED" : + "QUERY_NO_HSDIR"; + control_event_hsv2_descriptor_failed(rend_query, NULL, query_response); control_event_hs_descriptor_content(rend_data_get_address(rend_query), desc_id_base32, NULL, NULL); return 0; diff --git a/src/test/test_hs.c b/src/test/test_hs.c index aeb338747..5d3327c77 100644 --- a/src/test/test_hs.c +++ b/src/test/test_hs.c @@ -323,6 +323,16 @@ test_hs_desc_event(void *arg) tt_str_op(received_msg,OP_EQ, expected_msg); tor_free(received_msg); + /* test HSDir rate limited */ + rend_query.auth_type = REND_NO_AUTH; + control_event_hsv2_descriptor_failed(&rend_query.base_, NULL, + "QUERY_RATE_LIMITED"); + expected_msg = "650 HS_DESC FAILED "STR_HS_ADDR" NO_AUTH " \ + "UNKNOWN REASON=QUERY_RATE_LIMITED\r\n"; + tt_assert(received_msg); + tt_str_op(received_msg,OP_EQ, expected_msg); + tor_free(received_msg); + /* Test invalid content with no HSDir fingerprint. */ char *exp_msg; control_event_hs_descriptor_content(rend_query.onion_address,