[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[tor-commits] [tor/master] hs-v3: Cancel active descriptor directory connections before uploading



commit 7150214baf2385d0e64fd11fe4138932675d444f
Author: David Goulet <dgoulet@xxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxx>
---
 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



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits