[tor-commits] [tor/master] Make repeated/rate limited HSFETCH queries fail with QUERY_RATE_LIMITED
nickm at torproject.org
nickm at torproject.org
Tue Apr 23 18:14:23 UTC 2019
commit 011307dd5fa608739456b98d259b013286320b91
Author: Neel Chauhan <neel at 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,
More information about the tor-commits
mailing list