[tor-commits] [tor/master] hs-v3: Cancel active descriptor directory connections before uploading
nickm at torproject.org
nickm at torproject.org
Tue Sep 12 15:11:16 UTC 2017
commit 7150214baf2385d0e64fd11fe4138932675d444f
Author: David Goulet <dgoulet at torproject.org>
Date: Mon Sep 11 13:16:23 2017 -0400
hs-v3: Cancel active descriptor directory connections before uploading
It is possible that two descriptor upload requests are launched in a very
short time frame which can lead to the second request finishing before the
first one and where that first one will make the HSDir send back a 400
malformed descriptor leading to a warning.
To avoid such, cancel all active directory connections for the specific
descriptor we are about to upload.
Note that this race is still possible on the HSDir side which triggers a log
info to be printed out but that is fine.
Fixes #23457
Signed-off-by: David Goulet <dgoulet at torproject.org>
---
src/or/hs_client.c | 3 ++-
src/or/hs_ident.c | 16 ++++++++++++++++
src/or/hs_ident.h | 8 ++++++++
src/or/hs_service.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/src/or/hs_client.c b/src/or/hs_client.c
index f85ebc847..2f8953ba7 100644
--- a/src/or/hs_client.c
+++ b/src/or/hs_client.c
@@ -156,7 +156,8 @@ directory_launch_v3_desc_fetch(const ed25519_public_key_t *onion_identity_pk,
}
/* Copy onion pk to a dir_ident so that we attach it to the dir conn */
- ed25519_pubkey_copy(&hs_conn_dir_ident.identity_pk, onion_identity_pk);
+ hs_ident_dir_conn_init(onion_identity_pk, &blinded_pubkey,
+ &hs_conn_dir_ident);
/* Setup directory request */
directory_request_t *req =
diff --git a/src/or/hs_ident.c b/src/or/hs_ident.c
index df3928515..b0e4e36a9 100644
--- a/src/or/hs_ident.c
+++ b/src/or/hs_ident.c
@@ -65,6 +65,22 @@ hs_ident_dir_conn_free(hs_ident_dir_conn_t *ident)
tor_free(ident);
}
+/* Initialized the allocated ident object with identity_pk and blinded_pk.
+ * None of them can be NULL since a valid directory connection identifier must
+ * have all fields set. */
+void
+hs_ident_dir_conn_init(const ed25519_public_key_t *identity_pk,
+ const ed25519_public_key_t *blinded_pk,
+ hs_ident_dir_conn_t *ident)
+{
+ tor_assert(identity_pk);
+ tor_assert(blinded_pk);
+ tor_assert(ident);
+
+ ed25519_pubkey_copy(&ident->identity_pk, identity_pk);
+ ed25519_pubkey_copy(&ident->blinded_pk, blinded_pk);
+}
+
/* Return a newly allocated edge connection identifier. The given public key
* identity_pk is copied into the identifier. */
hs_ident_edge_conn_t *
diff --git a/src/or/hs_ident.h b/src/or/hs_ident.h
index cfcde781d..101c1cfff 100644
--- a/src/or/hs_ident.h
+++ b/src/or/hs_ident.h
@@ -96,6 +96,11 @@ typedef struct hs_ident_dir_conn_t {
* in the onion address. */
ed25519_public_key_t identity_pk;
+ /* The blinded public key used to uniquely identify the descriptor that this
+ * directory connection identifier is for. Only used by the service-side code
+ * to fine control descriptor uploads. */
+ ed25519_public_key_t blinded_pk;
+
/* XXX: Client authorization. */
} hs_ident_dir_conn_t;
@@ -120,6 +125,9 @@ hs_ident_circuit_t *hs_ident_circuit_dup(const hs_ident_circuit_t *src);
/* Directory connection identifier API. */
hs_ident_dir_conn_t *hs_ident_dir_conn_dup(const hs_ident_dir_conn_t *src);
void hs_ident_dir_conn_free(hs_ident_dir_conn_t *ident);
+void hs_ident_dir_conn_init(const ed25519_public_key_t *identity_pk,
+ const ed25519_public_key_t *blinded_pk,
+ hs_ident_dir_conn_t *ident);
/* Edge connection identifier API. */
hs_ident_edge_conn_t *hs_ident_edge_conn_new(
diff --git a/src/or/hs_service.c b/src/or/hs_service.c
index 5759aa812..d32a120bc 100644
--- a/src/or/hs_service.c
+++ b/src/or/hs_service.c
@@ -14,6 +14,7 @@
#include "circuitlist.h"
#include "circuituse.h"
#include "config.h"
+#include "connection.h"
#include "directory.h"
#include "main.h"
#include "networkstatus.h"
@@ -641,6 +642,41 @@ count_desc_circuit_established(const hs_service_descriptor_t *desc)
return count;
}
+/* For a given service and descriptor of that service, close all active
+ * directory connections. */
+static void
+close_directory_connections(const hs_service_t *service,
+ const hs_service_descriptor_t *desc)
+{
+ unsigned int count = 0;
+ smartlist_t *dir_conns;
+
+ tor_assert(service);
+ tor_assert(desc);
+
+ /* Close pending HS desc upload connections for the blinded key of 'desc'. */
+ dir_conns = connection_list_by_type_purpose(CONN_TYPE_DIR,
+ DIR_PURPOSE_UPLOAD_HSDESC);
+ SMARTLIST_FOREACH_BEGIN(dir_conns, connection_t *, conn) {
+ dir_connection_t *dir_conn = TO_DIR_CONN(conn);
+ if (ed25519_pubkey_eq(&dir_conn->hs_ident->identity_pk,
+ &service->keys.identity_pk) &&
+ ed25519_pubkey_eq(&dir_conn->hs_ident->blinded_pk,
+ &desc->blinded_kp.pubkey)) {
+ connection_mark_for_close(conn);
+ count++;
+ continue;
+ }
+ } SMARTLIST_FOREACH_END(conn);
+
+ log_info(LD_REND, "Closed %u active service directory connections for "
+ "descriptor %s of service %s",
+ count, safe_str_client(ed25519_fmt(&desc->blinded_kp.pubkey)),
+ safe_str_client(service->onion_address));
+ /* We don't have ownership of the objects in this list. */
+ smartlist_free(dir_conns);
+}
+
/* Close all rendezvous circuits for the given service. */
static void
close_service_rp_circuits(hs_service_t *service)
@@ -2137,7 +2173,9 @@ upload_descriptor_to_hsdir(const hs_service_t *service,
}
/* Setup the connection identifier. */
- ed25519_pubkey_copy(&ident.identity_pk, &service->keys.identity_pk);
+ hs_ident_dir_conn_init(&service->keys.identity_pk, &desc->blinded_kp.pubkey,
+ &ident);
+
/* This is our resource when uploading which is used to construct the URL
* with the version number: "/tor/hs/<version>/publish". */
tor_snprintf(version_str, sizeof(version_str), "%u",
@@ -2387,6 +2425,13 @@ upload_descriptor_to_all(const hs_service_t *service,
tor_assert(service);
tor_assert(desc);
+ /* We'll first cancel any directory request that are ongoing for this
+ * descriptor. It is possible that we can trigger multiple uploads in a
+ * short time frame which can lead to a race where the second upload arrives
+ * before the first one leading to a 400 malformed descriptor response from
+ * the directory. Closing all pending requests avoids that. */
+ close_directory_connections(service, desc);
+
/* Get our list of responsible HSDir. */
responsible_dirs = smartlist_new();
/* The parameter 0 means that we aren't a client so tell the function to use
More information about the tor-commits
mailing list