[tor-commits] [tor/maint-0.3.2] hs-v3: Attempt descriptor refetch when dirinfo changes
nickm at torproject.org
nickm at torproject.org
Tue Oct 31 16:19:47 UTC 2017
commit 5dbcd48f0ee2f57557f6bcce6ee3ec11a76727e4
Author: David Goulet <dgoulet at torproject.org>
Date: Wed Oct 4 16:22:49 2017 -0400
hs-v3: Attempt descriptor refetch when dirinfo changes
When the directory information changes, callback to the HS client subsystem so
it can check if any pending SOCKS connections are waiting for a descriptor. If
yes, attempt a refetch for those.
Fixes #23762
Signed-off-by: David Goulet <dgoulet at torproject.org>
---
changes/bug23762 | 4 +++
src/or/connection_edge.c | 9 +++----
src/or/hs_client.c | 59 ++++++++++++++++++++++++++++++++++++++++++
src/or/hs_client.h | 1 +
src/or/nodelist.c | 2 ++
src/test/test_channelpadding.c | 12 ++++++---
src/test/test_entryconn.c | 5 ++--
7 files changed, 81 insertions(+), 11 deletions(-)
diff --git a/changes/bug23762 b/changes/bug23762
new file mode 100644
index 000000000..bf35e5ec1
--- /dev/null
+++ b/changes/bug23762
@@ -0,0 +1,4 @@
+ o Minor bugfixes (hidden service v3):
+ - Properly retry HSv3 descriptor fetches in the case where we were initially
+ missing required directory information. Closes ticket #23762; bugfix on
+ 0.3.2.1-alpha.
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index 77dac62b0..b9d8eeaff 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -1571,10 +1571,10 @@ connection_ap_handle_onion(entry_connection_t *conn,
int ret = hs_client_refetch_hsdesc(&edge_conn->hs_ident->identity_pk);
switch (ret) {
case HS_CLIENT_FETCH_MISSING_INFO:
- /* By going to the end, the connection is put in waiting for a circuit
- * state which means that it will be retried and consider as a pending
- * connection. */
- goto end;
+ /* Keeping the connection in descriptor wait state is fine because
+ * once we get enough dirinfo or a new live consensus, the HS client
+ * subsystem is notified and every connection in that state will
+ * trigger a fetch for the service key. */
case HS_CLIENT_FETCH_LAUNCHED:
case HS_CLIENT_FETCH_PENDING:
case HS_CLIENT_FETCH_HAVE_DESC:
@@ -1591,7 +1591,6 @@ connection_ap_handle_onion(entry_connection_t *conn,
/* We have the descriptor! So launch a connection to the HS. */
log_info(LD_REND, "Descriptor is here. Great.");
- end:
base_conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
/* We'll try to attach it at the next event loop, or whenever
* we call connection_ap_attach_pending() */
diff --git a/src/or/hs_client.c b/src/or/hs_client.c
index d66e56352..18b76e8b3 100644
--- a/src/or/hs_client.c
+++ b/src/or/hs_client.c
@@ -233,6 +233,55 @@ close_all_socks_conns_waiting_for_desc(const ed25519_public_key_t *identity_pk,
smartlist_free(conns);
}
+/* Find all pending SOCKS connection waiting for a descriptor and retry them
+ * all. This is called when the directory information changed. */
+static void
+retry_all_socks_conn_waiting_for_desc(void)
+{
+ smartlist_t *conns =
+ connection_list_by_type_state(CONN_TYPE_AP, AP_CONN_STATE_RENDDESC_WAIT);
+
+ SMARTLIST_FOREACH_BEGIN(conns, connection_t *, base_conn) {
+ hs_client_fetch_status_t status;
+ const edge_connection_t *edge_conn =
+ ENTRY_TO_EDGE_CONN(TO_ENTRY_CONN(base_conn));
+
+ /* Ignore non HS or non v3 connection. */
+ if (edge_conn->hs_ident == NULL) {
+ continue;
+ }
+ /* In this loop, we will possibly try to fetch a descriptor for the
+ * pending connections because we just got more directory information.
+ * However, the refetch process can cleanup all SOCKS request so the same
+ * service if an internal error happens. Thus, we can end up with closed
+ * connections in our list. */
+ if (base_conn->marked_for_close) {
+ continue;
+ }
+
+ /* XXX: There is an optimization we could do which is that for a service
+ * key, we could check if we can fetch and remember that decision. */
+
+ /* Order a refetch in case it works this time. */
+ status = hs_client_refetch_hsdesc(&edge_conn->hs_ident->identity_pk);
+ if (BUG(status == HS_CLIENT_FETCH_HAVE_DESC)) {
+ /* This case is unique because it can NOT happen in theory. Once we get
+ * a new descriptor, the HS client subsystem is notified immediately and
+ * the connections waiting for it are handled which means the state will
+ * change from renddesc wait state. Log this and continue to next
+ * connection. */
+ continue;
+ }
+ /* In the case of an error, either all SOCKS connections have been
+ * closed or we are still missing directory information. Leave the
+ * connection in renddesc wait state so when we get more info, we'll be
+ * able to try it again. */
+ } SMARTLIST_FOREACH_END(base_conn);
+
+ /* We don't have ownership of those objects. */
+ 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
@@ -1529,3 +1578,13 @@ hs_client_purge_state(void)
log_info(LD_REND, "Hidden service client state has been purged.");
}
+/* Called when our directory information has changed. */
+void
+hs_client_dir_info_changed(void)
+{
+ /* We have possibly reached the minimum directory information or new
+ * consensus so retry all pending SOCKS connection in
+ * AP_CONN_STATE_RENDDESC_WAIT state in order to fetch the descriptor. */
+ retry_all_socks_conn_waiting_for_desc();
+}
+
diff --git a/src/or/hs_client.h b/src/or/hs_client.h
index 164accc0e..2523568ad 100644
--- a/src/or/hs_client.h
+++ b/src/or/hs_client.h
@@ -41,6 +41,7 @@ int hs_client_decode_descriptor(
int hs_client_any_intro_points_usable(const ed25519_public_key_t *service_pk,
const hs_descriptor_t *desc);
int hs_client_refetch_hsdesc(const ed25519_public_key_t *identity_pk);
+void hs_client_dir_info_changed(void);
int hs_client_send_introduce1(origin_circuit_t *intro_circ,
origin_circuit_t *rend_circ);
diff --git a/src/or/nodelist.c b/src/or/nodelist.c
index eae74e18b..fd5a8f0c0 100644
--- a/src/or/nodelist.c
+++ b/src/or/nodelist.c
@@ -48,6 +48,7 @@
#include "entrynodes.h"
#include "geoip.h"
#include "hs_common.h"
+#include "hs_client.h"
#include "main.h"
#include "microdesc.h"
#include "networkstatus.h"
@@ -1973,6 +1974,7 @@ router_dir_info_changed(void)
need_to_update_have_min_dir_info = 1;
rend_hsdir_routers_changed();
hs_service_dir_info_changed();
+ hs_client_dir_info_changed();
}
/** Return a string describing what we're missing before we have enough
diff --git a/src/test/test_channelpadding.c b/src/test/test_channelpadding.c
index d54c9cc52..d5713688a 100644
--- a/src/test/test_channelpadding.c
+++ b/src/test/test_channelpadding.c
@@ -193,7 +193,8 @@ static void
setup_mock_network(void)
{
routerstatus_t *relay;
- connection_array = smartlist_new();
+ if (!connection_array)
+ connection_array = smartlist_new();
relay1_relay2 = (channel_t*)new_fake_channeltls(2);
relay1_relay2->write_cell = mock_channel_write_cell_relay1;
@@ -280,7 +281,8 @@ test_channelpadding_timers(void *arg)
tor_libevent_postfork();
- connection_array = smartlist_new();
+ if (!connection_array)
+ connection_array = smartlist_new();
monotime_init();
monotime_enable_test_mocking();
@@ -570,7 +572,8 @@ test_channelpadding_consensus(void *arg)
monotime_coarse_set_mock_time_nsec(1);
timers_initialize();
- connection_array = smartlist_new();
+ if (!connection_array)
+ connection_array = smartlist_new();
chan = (channel_t*)new_fake_channeltls(0);
channel_timestamp_active(chan);
@@ -928,7 +931,8 @@ test_channelpadding_decide_to_pad_channel(void *arg)
*/
channel_t *chan;
int64_t new_time;
- connection_array = smartlist_new();
+ if (!connection_array)
+ connection_array = smartlist_new();
(void)arg;
tor_libevent_postfork();
diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c
index 86c0c3dca..c29b1a712 100644
--- a/src/test/test_entryconn.c
+++ b/src/test/test_entryconn.c
@@ -790,8 +790,9 @@ test_entryconn_rewrite_onion_v3(void *arg)
retval = connection_ap_handshake_rewrite_and_attach(conn, NULL, NULL);
tt_int_op(retval, OP_EQ, 0);
- /* Check connection state after rewrite */
- tt_int_op(ENTRY_TO_CONN(conn)->state, OP_EQ, AP_CONN_STATE_CIRCUIT_WAIT);
+ /* Check connection state after rewrite. It should be in waiting for
+ * descriptor state. */
+ tt_int_op(ENTRY_TO_CONN(conn)->state, OP_EQ, AP_CONN_STATE_RENDDESC_WAIT);
/* check that the address got rewritten */
tt_str_op(conn->socks_request->address, OP_EQ,
"25njqamcweflpvkl73j4szahhihoc4xt3ktcgjnpaingr5yhkenl5sid");
More information about the tor-commits
mailing list