[tor-commits] [tor/master] hs-v3: Give a cleanup type to hs_circ_cleanup()
asn at torproject.org
asn at torproject.org
Wed Nov 27 13:36:50 UTC 2019
commit cbc495453cb522db1584525a06d26386f5819e05
Author: David Goulet <dgoulet at torproject.org>
Date: Thu Oct 31 08:06:16 2019 -0400
hs-v3: Give a cleanup type to hs_circ_cleanup()
By centralizing the circuit cleanup type that is: on close, free and
repurpose, some actions on the circuit can not happen for a certain cleanup
type or for all types.
This passes a cleanup type so the HS subsystem (v2 and v3) can take actions
based on the type of cleanup.
For instance, there is slow code that we do not run on a circuit close but
rather only on free.
Part of #32020
Signed-off-by: David Goulet <dgoulet at torproject.org>
---
src/core/or/circuitlist.c | 4 +-
src/core/or/circuituse.c | 2 +-
src/feature/hs/hs_circuit.c | 94 ++++++++++++++++++++++++++++++++++---------
src/feature/hs/hs_circuit.h | 6 ++-
src/feature/hs/hs_client.c | 2 +-
src/feature/hs/hs_client.h | 4 +-
src/feature/rend/rendclient.c | 2 +-
src/feature/rend/rendclient.h | 3 +-
8 files changed, 88 insertions(+), 29 deletions(-)
diff --git a/src/core/or/circuitlist.c b/src/core/or/circuitlist.c
index 3ae482a5b..49a63c50a 100644
--- a/src/core/or/circuitlist.c
+++ b/src/core/or/circuitlist.c
@@ -1137,7 +1137,7 @@ circuit_free_(circuit_t *circ)
* circuit is closed. This is to avoid any code path that free registered
* circuits without closing them before. This needs to be done before the
* hs identifier is freed. */
- hs_circ_cleanup(circ);
+ hs_circ_cleanup_on_free(circ);
if (CIRCUIT_IS_ORIGIN(circ)) {
origin_circuit_t *ocirc = TO_ORIGIN_CIRCUIT(circ);
@@ -2261,7 +2261,7 @@ circuit_mark_for_close_, (circuit_t *circ, int reason, int line,
}
/* Notify the HS subsystem that this circuit is closing. */
- hs_circ_cleanup(circ);
+ hs_circ_cleanup_on_close(circ);
if (circuits_pending_close == NULL)
circuits_pending_close = smartlist_new();
diff --git a/src/core/or/circuituse.c b/src/core/or/circuituse.c
index 8a667024d..ad9841cc3 100644
--- a/src/core/or/circuituse.c
+++ b/src/core/or/circuituse.c
@@ -3124,7 +3124,7 @@ circuit_change_purpose(circuit_t *circ, uint8_t new_purpose)
/* Take specific actions if we are repurposing a hidden service circuit. */
if (circuit_purpose_is_hidden_service(circ->purpose) &&
!circuit_purpose_is_hidden_service(new_purpose)) {
- hs_circ_cleanup(circ);
+ hs_circ_cleanup_on_repurpose(circ);
}
}
diff --git a/src/feature/hs/hs_circuit.c b/src/feature/hs/hs_circuit.c
index 8640129f3..a43e7f0e2 100644
--- a/src/feature/hs/hs_circuit.c
+++ b/src/feature/hs/hs_circuit.c
@@ -621,16 +621,16 @@ setup_introduce1_data(const hs_desc_intro_point_t *ip,
}
/** Helper: cleanup function for client circuit. This is for every HS version.
- * It is called from hs_circ_cleanup() entry point. */
+ * It is called from hs_circ_cleanup_on_free() entry point. */
static void
-cleanup_client_circ(circuit_t *circ)
+cleanup_on_free_client_circ(circuit_t *circ)
{
tor_assert(circ);
if (circuit_is_hs_v2(circ)) {
- rend_client_circuit_cleanup(circ);
+ rend_client_circuit_cleanup_on_free(circ);
} else if (circuit_is_hs_v3(circ)) {
- hs_client_circuit_cleanup(circ);
+ hs_client_circuit_cleanup_on_free(circ);
}
/* It is possible the circuit has an HS purpose but no identifier (rend_data
* or hs_ident). Thus possible that this passess through. */
@@ -1215,29 +1215,83 @@ hs_circ_send_establish_rendezvous(origin_circuit_t *circ)
return -1;
}
-/** We are about to close or free this <b>circ</b>. Clean it up from any
- * related HS data structures. This function can be called multiple times
- * safely for the same circuit. */
+/** Circuit cleanup strategy:
+ *
+ * What follows is a series of functions that notifies the HS subsystem of 3
+ * different circuit cleanup phase: close, free and repurpose.
+ *
+ * Tor can call any of those in any orders so they have to be safe between
+ * each other. In other words, the free should never depend on close to be
+ * called before.
+ *
+ * The "on_close()" is called from circuit_mark_for_close() which is
+ * considered the tor fast path and thus as little work as possible should
+ * done in that function. Currently, we only remove the circuit from the HS
+ * circuit map and move on.
+ *
+ * The "on_free()" is called from circuit circuit_free_() and it is very
+ * important that at the end of the function, no state or objects related to
+ * this circuit remains alive.
+ *
+ * The "on_repurpose()" is called from circuit_change_purpose() for which we
+ * simply remove it from the HS circuit map. We do not have other cleanup
+ * requirements after that.
+ *
+ * NOTE: The onion service code, specifically the service code, cleans up
+ * lingering objects or state if any of its circuit disappear which is why
+ * our cleanup strategy doesn't involve any service specific actions. As long
+ * as the circuit is removed from the HS circuit map, it won't be used.
+ */
+
+/** We are about to close this <b>circ</b>. Clean it up from any related HS
+ * data structures. This function can be called multiple times safely for the
+ * same circuit. */
void
-hs_circ_cleanup(circuit_t *circ)
+hs_circ_cleanup_on_close(circuit_t *circ)
{
tor_assert(circ);
+ /* On close, we simply remove it from the circuit map. It can not be used
+ * anymore. We keep this code path fast and lean. */
+
+ if (circ->hs_token) {
+ hs_circuitmap_remove_circuit(circ);
+ }
+}
+
+/** We are about to free this <b>circ</b>. Clean it up from any related HS
+ * data structures. This function can be called multiple times safely for the
+ * same circuit. */
+void
+hs_circ_cleanup_on_free(circuit_t *circ)
+{
+ tor_assert(circ);
+
+ /* NOTE: Bulk of the work of cleaning up a circuit is done here. */
+
if (circuit_purpose_is_hs_client(circ->purpose)) {
- cleanup_client_circ(circ);
+ cleanup_on_free_client_circ(circ);
+ }
+
+ /* We have no assurance that the given HS circuit has been closed before and
+ * thus removed from the HS map. This actually happens in unit tests. */
+ if (circ->hs_token) {
+ hs_circuitmap_remove_circuit(circ);
}
+}
+
+/** We are about to repurpose this <b>circ</b>. Clean it up from any related
+ * HS data structures. This function can be called multiple times safely for
+ * the same circuit. */
+void
+hs_circ_cleanup_on_repurpose(circuit_t *circ)
+{
+ tor_assert(circ);
+
+ /* On repurpose, we simply remove it from the circuit map but we do not do
+ * the on_free actions since we don't treat a repurpose as something we need
+ * to report in the client cache failure. */
- /* Actions that MUST happen for every circuits regardless of what was done
- * on it before. */
-
- /* Clear HS circuitmap token for this circ (if any). Very important to be
- * done after the HS subsystem has been notified of the close else the
- * circuit will not be found.
- *
- * We do this at the close if possible because from that point on, the
- * circuit is good as dead. We can't rely on removing it in the circuit
- * free() function because we open a race window between the close and free
- * where we can't register a new circuit for the same intro point. */
if (circ->hs_token) {
hs_circuitmap_remove_circuit(circ);
}
diff --git a/src/feature/hs/hs_circuit.h b/src/feature/hs/hs_circuit.h
index c817f3e37..42e5ca134 100644
--- a/src/feature/hs/hs_circuit.h
+++ b/src/feature/hs/hs_circuit.h
@@ -14,8 +14,10 @@
#include "feature/hs/hs_service.h"
-/* Cleanup function when the circuit is closed or/and freed. */
-void hs_circ_cleanup(circuit_t *circ);
+/* Cleanup function when the circuit is closed or freed. */
+void hs_circ_cleanup_on_close(circuit_t *circ);
+void hs_circ_cleanup_on_free(circuit_t *circ);
+void hs_circ_cleanup_on_repurpose(circuit_t *circ);
/* Circuit API. */
int hs_circ_service_intro_has_opened(hs_service_t *service,
diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c
index 716b6a26a..a2a47c1da 100644
--- a/src/feature/hs/hs_client.c
+++ b/src/feature/hs/hs_client.c
@@ -1525,7 +1525,7 @@ get_hs_client_auths_map(void)
/** Called when a circuit was just cleaned up. This is done right before the
* circuit is freed. */
void
-hs_client_circuit_cleanup(const circuit_t *circ)
+hs_client_circuit_cleanup_on_free(const circuit_t *circ)
{
bool has_timed_out;
rend_intro_point_failure_t failure = INTRO_POINT_FAILURE_GENERIC;
diff --git a/src/feature/hs/hs_client.h b/src/feature/hs/hs_client.h
index a0f1c7758..23effc06b 100644
--- a/src/feature/hs/hs_client.h
+++ b/src/feature/hs/hs_client.h
@@ -10,6 +10,8 @@
#define TOR_HS_CLIENT_H
#include "lib/crypt_ops/crypto_ed25519.h"
+
+#include "feature/hs/hs_circuit.h"
#include "feature/hs/hs_descriptor.h"
#include "feature/hs/hs_ident.h"
@@ -112,7 +114,7 @@ int hs_client_send_introduce1(origin_circuit_t *intro_circ,
origin_circuit_t *rend_circ);
void hs_client_circuit_has_opened(origin_circuit_t *circ);
-void hs_client_circuit_cleanup(const circuit_t *circ);
+void hs_client_circuit_cleanup_on_free(const circuit_t *circ);
int hs_client_receive_rendezvous_acked(origin_circuit_t *circ,
const uint8_t *payload,
diff --git a/src/feature/rend/rendclient.c b/src/feature/rend/rendclient.c
index 632b00c85..14484f1ce 100644
--- a/src/feature/rend/rendclient.c
+++ b/src/feature/rend/rendclient.c
@@ -1254,7 +1254,7 @@ rend_parse_service_authorization(const or_options_t *options,
/** The given circuit is being freed. Take appropriate action if it is of
* interest to the client subsystem. */
void
-rend_client_circuit_cleanup(const circuit_t *circ)
+rend_client_circuit_cleanup_on_free(const circuit_t *circ)
{
int reason, orig_reason;
bool has_timed_out, ip_is_redundant;
diff --git a/src/feature/rend/rendclient.h b/src/feature/rend/rendclient.h
index da6f4646d..63191737c 100644
--- a/src/feature/rend/rendclient.h
+++ b/src/feature/rend/rendclient.h
@@ -12,6 +12,7 @@
#ifndef TOR_RENDCLIENT_H
#define TOR_RENDCLIENT_H
+#include "feature/hs/hs_circuit.h"
#include "feature/rend/rendcache.h"
void rend_client_purge_state(void);
@@ -47,7 +48,7 @@ rend_service_authorization_t *rend_client_lookup_service_authorization(
const char *onion_address);
void rend_service_authorization_free_all(void);
-void rend_client_circuit_cleanup(const circuit_t *circ);
+void rend_client_circuit_cleanup_on_free(const circuit_t *circ);
#endif /* !defined(TOR_RENDCLIENT_H) */
More information about the tor-commits
mailing list