summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--changes/bug236537
-rw-r--r--src/or/connection_edge.c1
-rw-r--r--src/or/hs_client.c161
-rw-r--r--src/or/hs_client.h5
-rw-r--r--src/test/test_hs_client.c131
5 files changed, 296 insertions, 9 deletions
diff --git a/changes/bug23653 b/changes/bug23653
new file mode 100644
index 0000000000..81760cbb82
--- /dev/null
+++ b/changes/bug23653
@@ -0,0 +1,7 @@
+ o Minor bugfixes (hidden service client):
+ - When getting multiple SOCKS request for the same .onion address, don't
+ trigger multiple descriptor fetches.
+ - When the descriptor fetch fails with an internal error, no more HSDir to
+ query or we aren't allowed to fetch (FetchHidServDescriptors 0), close
+ all pending SOCKS request for that .onion. Fixes bug 23653; bugfix on
+ 0.3.2.1-alpha.
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 9098cb6908..77dac62b09 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -1576,6 +1576,7 @@ connection_ap_handle_onion(entry_connection_t *conn,
* connection. */
goto end;
case HS_CLIENT_FETCH_LAUNCHED:
+ case HS_CLIENT_FETCH_PENDING:
case HS_CLIENT_FETCH_HAVE_DESC:
return 0;
case HS_CLIENT_FETCH_ERROR:
diff --git a/src/or/hs_client.c b/src/or/hs_client.c
index 942e332f0c..93a913b34c 100644
--- a/src/or/hs_client.c
+++ b/src/or/hs_client.c
@@ -32,6 +32,58 @@
#include "hs_ntor.h"
#include "circuitbuild.h"
#include "networkstatus.h"
+#include "reasons.h"
+
+/* Return a human-readable string for the client fetch status code. */
+static const char *
+fetch_status_to_string(hs_client_fetch_status_t status)
+{
+ switch (status) {
+ case HS_CLIENT_FETCH_ERROR:
+ return "Internal error";
+ case HS_CLIENT_FETCH_LAUNCHED:
+ return "Descriptor fetch launched";
+ case HS_CLIENT_FETCH_HAVE_DESC:
+ return "Already have descriptor";
+ case HS_CLIENT_FETCH_NO_HSDIRS:
+ return "No more HSDir available to query";
+ case HS_CLIENT_FETCH_NOT_ALLOWED:
+ return "Fetching descriptors is not allowed";
+ case HS_CLIENT_FETCH_MISSING_INFO:
+ return "Missing directory information";
+ case HS_CLIENT_FETCH_PENDING:
+ return "Pending descriptor fetch";
+ default:
+ return "(Unknown client fetch status code)";
+ }
+}
+
+/* Return true iff tor should close the SOCKS request(s) for the descriptor
+ * fetch that ended up with this given status code. */
+static int
+fetch_status_should_close_socks(hs_client_fetch_status_t status)
+{
+ switch (status) {
+ case HS_CLIENT_FETCH_NO_HSDIRS:
+ /* No more HSDir to query, we can't complete the SOCKS request(s). */
+ case HS_CLIENT_FETCH_ERROR:
+ /* The fetch triggered an internal error. */
+ case HS_CLIENT_FETCH_NOT_ALLOWED:
+ /* Client is not allowed to fetch (FetchHidServDescriptors 0). */
+ goto close;
+ case HS_CLIENT_FETCH_MISSING_INFO:
+ case HS_CLIENT_FETCH_HAVE_DESC:
+ case HS_CLIENT_FETCH_PENDING:
+ case HS_CLIENT_FETCH_LAUNCHED:
+ /* The rest doesn't require tor to close the SOCKS request(s). */
+ goto no_close;
+ }
+
+ no_close:
+ return 0;
+ close:
+ return 1;
+}
/* Cancel all descriptor fetches currently in progress. */
static void
@@ -100,7 +152,7 @@ purge_hid_serv_request(const ed25519_public_key_t *identity_pk)
* from the previous time period. That is fine because they will expire at
* some point and we don't care about those anymore. */
hs_build_blinded_pubkey(identity_pk, NULL, 0,
- hs_get_time_period_num(approx_time()), &blinded_pk);
+ hs_get_time_period_num(0), &blinded_pk);
if (BUG(ed25519_public_to_base64(base64_blinded_pk, &blinded_pk) < 0)) {
return;
}
@@ -108,6 +160,79 @@ purge_hid_serv_request(const ed25519_public_key_t *identity_pk)
hs_purge_hid_serv_from_last_hid_serv_requests(base64_blinded_pk);
}
+/* Return true iff there is at least one pending directory descriptor request
+ * for the service identity_pk. */
+static int
+directory_request_is_pending(const ed25519_public_key_t *identity_pk)
+{
+ int ret = 0;
+ smartlist_t *conns =
+ connection_list_by_type_purpose(CONN_TYPE_DIR, DIR_PURPOSE_FETCH_HSDESC);
+
+ SMARTLIST_FOREACH_BEGIN(conns, connection_t *, conn) {
+ const hs_ident_dir_conn_t *ident = TO_DIR_CONN(conn)->hs_ident;
+ if (BUG(ident == NULL)) {
+ /* A directory connection fetching a service descriptor can't have an
+ * empty hidden service identifier. */
+ continue;
+ }
+ if (!ed25519_pubkey_eq(identity_pk, &ident->identity_pk)) {
+ continue;
+ }
+ ret = 1;
+ break;
+ } SMARTLIST_FOREACH_END(conn);
+
+ /* No ownership of the objects in this list. */
+ smartlist_free(conns);
+ return ret;
+}
+
+/* We failed to fetch a descriptor for the service with <b>identity_pk</b>
+ * because of <b>status</b>. Find all pending SOCKS connections for this
+ * service that are waiting on the descriptor and close them with
+ * <b>reason</b>. */
+static void
+close_all_socks_conns_waiting_for_desc(const ed25519_public_key_t *identity_pk,
+ hs_client_fetch_status_t status,
+ int reason)
+{
+ unsigned int count = 0;
+ time_t now = approx_time();
+ smartlist_t *conns =
+ connection_list_by_type_state(CONN_TYPE_AP, AP_CONN_STATE_RENDDESC_WAIT);
+
+ SMARTLIST_FOREACH_BEGIN(conns, connection_t *, base_conn) {
+ entry_connection_t *entry_conn = TO_ENTRY_CONN(base_conn);
+ const edge_connection_t *edge_conn = ENTRY_TO_EDGE_CONN(entry_conn);
+
+ /* Only consider the entry connections that matches the service for which
+ * we tried to get the descriptor */
+ if (!edge_conn->hs_ident ||
+ !ed25519_pubkey_eq(identity_pk,
+ &edge_conn->hs_ident->identity_pk)) {
+ continue;
+ }
+ assert_connection_ok(base_conn, now);
+ /* Unattach the entry connection which will close for the reason. */
+ connection_mark_unattached_ap(entry_conn, reason);
+ count++;
+ } SMARTLIST_FOREACH_END(base_conn);
+
+ if (count > 0) {
+ char onion_address[HS_SERVICE_ADDR_LEN_BASE32 + 1];
+ hs_build_address(identity_pk, HS_VERSION_THREE, onion_address);
+ log_notice(LD_REND, "Closed %u streams for service %s.onion "
+ "for reason %s. Fetch status: %s.",
+ count, safe_str_client(onion_address),
+ stream_end_reason_to_string(reason),
+ fetch_status_to_string(status));
+ }
+
+ /* No ownership of the object(s) in this list. */
+ smartlist_free(conns);
+}
+
/* A v3 HS circuit successfully connected to the hidden service. Update the
* stream state at <b>hs_conn_ident</b> appropriately. */
static void
@@ -131,9 +256,9 @@ note_connection_attempt_succeeded(const hs_ident_edge_conn_t *hs_conn_ident)
}
/* Given the pubkey of a hidden service in <b>onion_identity_pk</b>, fetch its
- * descriptor by launching a dir connection to <b>hsdir</b>. Return 1 on
- * success or -1 on error. */
-static int
+ * descriptor by launching a dir connection to <b>hsdir</b>. Return a
+ * hs_client_fetch_status_t status code depending on how it went. */
+static hs_client_fetch_status_t
directory_launch_v3_desc_fetch(const ed25519_public_key_t *onion_identity_pk,
const routerstatus_t *hsdir)
{
@@ -224,10 +349,10 @@ pick_hsdir_v3(const ed25519_public_key_t *onion_identity_pk)
/** Fetch a v3 descriptor using the given <b>onion_identity_pk</b>.
*
- * On success, 1 is returned. If no hidden service is left to ask, return 0.
- * On error, -1 is returned. */
-static int
-fetch_v3_desc(const ed25519_public_key_t *onion_identity_pk)
+ * On success, HS_CLIENT_FETCH_LAUNCHED is returned. Otherwise, an error from
+ * hs_client_fetch_status_t is returned. */
+MOCK_IMPL(STATIC hs_client_fetch_status_t,
+fetch_v3_desc, (const ed25519_public_key_t *onion_identity_pk))
{
routerstatus_t *hsdir_rs =NULL;
@@ -1009,6 +1134,8 @@ hs_client_any_intro_points_usable(const ed25519_public_key_t *service_pk,
int
hs_client_refetch_hsdesc(const ed25519_public_key_t *identity_pk)
{
+ int ret;
+
tor_assert(identity_pk);
/* Are we configured to fetch descriptors? */
@@ -1046,7 +1173,23 @@ hs_client_refetch_hsdesc(const ed25519_public_key_t *identity_pk)
}
}
- return fetch_v3_desc(identity_pk);
+ /* Don't try to refetch while we have a pending request for it. */
+ if (directory_request_is_pending(identity_pk)) {
+ log_info(LD_REND, "Already a pending directory request. Waiting on it.");
+ return HS_CLIENT_FETCH_PENDING;
+ }
+
+ /* Try to fetch the desc and if we encounter an unrecoverable error, mark the
+ * desc as unavailable for now. */
+ ret = fetch_v3_desc(identity_pk);
+ if (fetch_status_should_close_socks(ret)) {
+ close_all_socks_conns_waiting_for_desc(identity_pk, ret,
+ END_STREAM_REASON_RESOLVEFAILED);
+ /* Remove HSDir fetch attempts so that we can retry later if the user
+ * wants us to regardless of if we closed any connections. */
+ purge_hid_serv_request(identity_pk);
+ }
+ return ret;
}
/* This is called when we are trying to attach an AP connection to these
diff --git a/src/or/hs_client.h b/src/or/hs_client.h
index 08ab7736b6..164accc0e6 100644
--- a/src/or/hs_client.h
+++ b/src/or/hs_client.h
@@ -27,6 +27,8 @@ typedef enum {
HS_CLIENT_FETCH_NOT_ALLOWED = 3,
/* We are missing information to be able to launch a request. */
HS_CLIENT_FETCH_MISSING_INFO = 4,
+ /* There is a pending fetch for the requested service. */
+ HS_CLIENT_FETCH_PENDING = 5,
} hs_client_fetch_status_t;
void hs_client_note_connection_attempt_succeeded(
@@ -80,6 +82,9 @@ desc_intro_point_to_extend_info(const hs_desc_intro_point_t *ip);
STATIC int handle_rendezvous2(origin_circuit_t *circ, const uint8_t *payload,
size_t payload_len);
+MOCK_DECL(STATIC hs_client_fetch_status_t,
+ fetch_v3_desc, (const ed25519_public_key_t *onion_identity_pk));
+
#endif /* defined(HS_CLIENT_PRIVATE) */
#endif /* !defined(TOR_HS_CLIENT_H) */
diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c
index 38878d6ed5..750920fac0 100644
--- a/src/test/test_hs_client.c
+++ b/src/test/test_hs_client.c
@@ -23,6 +23,8 @@
#include "config.h"
#include "crypto.h"
#include "channeltls.h"
+#include "main.h"
+#include "nodelist.h"
#include "routerset.h"
#include "hs_circuit.h"
@@ -44,6 +46,14 @@ mock_connection_ap_handshake_send_begin(entry_connection_t *ap_conn)
static networkstatus_t mock_ns;
+/* Always return NULL. */
+static networkstatus_t *
+mock_networkstatus_get_live_consensus_false(time_t now)
+{
+ (void) now;
+ return NULL;
+}
+
static networkstatus_t *
mock_networkstatus_get_live_consensus(time_t now)
{
@@ -456,6 +466,125 @@ test_client_pick_intro(void *arg)
hs_descriptor_free(desc);
}
+static int
+mock_router_have_minimum_dir_info_false(void)
+{
+ return 0;
+}
+static int
+mock_router_have_minimum_dir_info_true(void)
+{
+ return 1;
+}
+
+static hs_client_fetch_status_t
+mock_fetch_v3_desc_error(const ed25519_public_key_t *key)
+{
+ (void) key;
+ return HS_CLIENT_FETCH_ERROR;
+}
+
+static void
+mock_connection_mark_unattached_ap_(entry_connection_t *conn, int endreason,
+ int line, const char *file)
+{
+ (void) line;
+ (void) file;
+ conn->edge_.end_reason = endreason;
+}
+
+static void
+test_descriptor_fetch(void *arg)
+{
+ int ret;
+ entry_connection_t *ec = NULL;
+ ed25519_public_key_t service_pk;
+ ed25519_secret_key_t service_sk;
+
+ (void) arg;
+
+ hs_init();
+ memset(&service_sk, 'A', sizeof(service_sk));
+ ret = ed25519_public_key_generate(&service_pk, &service_sk);
+ tt_int_op(ret, OP_EQ, 0);
+
+ /* Initialize this so get_voting_interval() doesn't freak out. */
+ ret = parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC",
+ &mock_ns.valid_after);
+ tt_int_op(ret, OP_EQ, 0);
+ ret = parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC",
+ &mock_ns.fresh_until);
+ tt_int_op(ret, OP_EQ, 0);
+
+ ec = entry_connection_new(CONN_TYPE_AP, AF_INET);
+ tt_assert(ec);
+ ENTRY_TO_EDGE_CONN(ec)->hs_ident = hs_ident_edge_conn_new(&service_pk);
+ tt_assert(ENTRY_TO_EDGE_CONN(ec)->hs_ident);
+ TO_CONN(ENTRY_TO_EDGE_CONN(ec))->state = AP_CONN_STATE_RENDDESC_WAIT;
+ smartlist_add(get_connection_array(), &ec->edge_.base_);
+
+ /* 1. FetchHidServDescriptors is false so we shouldn't be able to fetch. */
+ get_options_mutable()->FetchHidServDescriptors = 0;
+ ret = hs_client_refetch_hsdesc(&service_pk);
+ tt_int_op(ret, OP_EQ, HS_CLIENT_FETCH_NOT_ALLOWED);
+ get_options_mutable()->FetchHidServDescriptors = 1;
+
+ /* 2. We don't have a live consensus. */
+ MOCK(networkstatus_get_live_consensus,
+ mock_networkstatus_get_live_consensus_false);
+ ret = hs_client_refetch_hsdesc(&service_pk);
+ UNMOCK(networkstatus_get_live_consensus);
+ tt_int_op(ret, OP_EQ, HS_CLIENT_FETCH_MISSING_INFO);
+
+ /* From now on, return a live consensus. */
+ MOCK(networkstatus_get_live_consensus,
+ mock_networkstatus_get_live_consensus);
+
+ /* 3. Not enough dir information. */
+ MOCK(router_have_minimum_dir_info,
+ mock_router_have_minimum_dir_info_false);
+ ret = hs_client_refetch_hsdesc(&service_pk);
+ UNMOCK(router_have_minimum_dir_info);
+ tt_int_op(ret, OP_EQ, HS_CLIENT_FETCH_MISSING_INFO);
+
+ /* From now on, we do have enough directory information. */
+ MOCK(router_have_minimum_dir_info,
+ mock_router_have_minimum_dir_info_true);
+
+ /* 4. We do have a pending directory request. */
+ {
+ dir_connection_t *dir_conn = dir_connection_new(AF_INET);
+ dir_conn->hs_ident = tor_malloc_zero(sizeof(hs_ident_dir_conn_t));
+ TO_CONN(dir_conn)->purpose = DIR_PURPOSE_FETCH_HSDESC;
+ ed25519_pubkey_copy(&dir_conn->hs_ident->identity_pk, &service_pk);
+ smartlist_add(get_connection_array(), TO_CONN(dir_conn));
+ ret = hs_client_refetch_hsdesc(&service_pk);
+ smartlist_remove(get_connection_array(), TO_CONN(dir_conn));
+ connection_free_(TO_CONN(dir_conn));
+ tt_int_op(ret, OP_EQ, HS_CLIENT_FETCH_PENDING);
+ }
+
+ /* 5. We'll trigger an error on the fetch_desc_v3 and force to close all
+ * pending SOCKS request. */
+ MOCK(router_have_minimum_dir_info,
+ mock_router_have_minimum_dir_info_true);
+ MOCK(fetch_v3_desc, mock_fetch_v3_desc_error);
+ MOCK(connection_mark_unattached_ap_,
+ mock_connection_mark_unattached_ap_);
+ ret = hs_client_refetch_hsdesc(&service_pk);
+ UNMOCK(fetch_v3_desc);
+ UNMOCK(connection_mark_unattached_ap_);
+ tt_int_op(ret, OP_EQ, HS_CLIENT_FETCH_ERROR);
+ /* The close waiting for descriptor function has been called. */
+ tt_int_op(ec->edge_.end_reason, OP_EQ, END_STREAM_REASON_RESOLVEFAILED);
+
+ done:
+ connection_free_(ENTRY_TO_CONN(ec));
+ UNMOCK(networkstatus_get_live_consensus);
+ UNMOCK(router_have_minimum_dir_info);
+ hs_free_all();
+}
+
struct testcase_t hs_client_tests[] = {
{ "e2e_rend_circuit_setup_legacy", test_e2e_rend_circuit_setup_legacy,
TT_FORK, NULL, NULL },
@@ -463,6 +592,8 @@ struct testcase_t hs_client_tests[] = {
TT_FORK, NULL, NULL },
{ "client_pick_intro", test_client_pick_intro,
TT_FORK, NULL, NULL },
+ { "descriptor_fetch", test_descriptor_fetch,
+ TT_FORK, NULL, NULL },
END_OF_TESTCASES
};
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion