commit bad091590e3d4db6995ccfe5efa03756e5e7234f Author: Georg Koppen <gk@torproject.org> Date: Sun Aug 23 19:54:50 2020 +0000 No need for the 0.4.3.6 patch file anymore Thanks cypherpunk. --- ...ve-accessor-semantic-of-client-cached-obj.patch | 212 --------------------- 1 file changed, 212 deletions(-) diff --git a/projects/tor/0001-hs-v3-Improve-accessor-semantic-of-client-cached-obj.patch b/projects/tor/0001-hs-v3-Improve-accessor-semantic-of-client-cached-obj.patch deleted file mode 100644 index 1432039..0000000 --- a/projects/tor/0001-hs-v3-Improve-accessor-semantic-of-client-cached-obj.patch +++ /dev/null @@ -1,212 +0,0 @@ -From 1810771799dd0b434ac2b5926297d64e383582e1 Mon Sep 17 00:00:00 2001 -From: David Goulet <dgoulet@torproject.org> -Date: Tue, 10 Mar 2020 10:58:51 -0400 -Subject: [PATCH] hs-v3: Improve accessor semantic of client cached object - -Add an inline helper function that indicates if the cached object contains a -decrypted descriptor or not. - -The descriptor object is NULL if tor is unable to decrypt it (lacking client -authorization) and some actions need to be done only when we have a decrypted -object. - -This improves code semantic. - -Fixes #33458 - -Signed-off-by: David Goulet <dgoulet@torproject.org> ---- - src/feature/hs/hs_cache.c | 64 +++++++++++++++++++++++++++++++-------- - src/test/test_hs_client.c | 46 ++++++++++++++++++++++++++++ - 2 files changed, 98 insertions(+), 12 deletions(-) - -diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c -index 9cf408ca3e..44cd2505fd 100644 ---- a/src/feature/hs/hs_cache.c -+++ b/src/feature/hs/hs_cache.c -@@ -27,6 +27,21 @@ - static int cached_client_descriptor_has_expired(time_t now, - const hs_cache_client_descriptor_t *cached_desc); - -+/** Helper function: Return true iff the cache entry has a decrypted -+ * descriptor. -+ * -+ * A NULL desc object in the entry means that we were not able to decrypt the -+ * descriptor because we are likely lacking client authorization. It is still -+ * a valid entry but some operations can't be done without the decrypted -+ * descriptor thus this function MUST be used to safe guard access to the -+ * decrypted desc object. */ -+static inline bool -+entry_has_decrypted_descriptor(const hs_cache_client_descriptor_t *entry) -+{ -+ tor_assert(entry); -+ return (entry->desc != NULL); -+} -+ - /********************** Directory HS cache ******************/ - - /** Directory descriptor cache. Map indexed by blinded key. */ -@@ -341,8 +356,23 @@ static digest256map_t *hs_cache_client_intro_state; - static size_t - cache_get_client_entry_size(const hs_cache_client_descriptor_t *entry) - { -- return sizeof(*entry) + -- strlen(entry->encoded_desc) + hs_desc_obj_size(entry->desc); -+ size_t size = 0; -+ -+ if (entry == NULL) { -+ goto end; -+ } -+ size += sizeof(*entry); -+ -+ if (entry->encoded_desc) { -+ size += strlen(entry->encoded_desc); -+ } -+ -+ if (entry_has_decrypted_descriptor(entry)) { -+ size += hs_desc_obj_size(entry->desc); -+ } -+ -+ end: -+ return size; - } - - /** Remove a given descriptor from our cache. */ -@@ -659,14 +689,20 @@ cache_store_as_client(hs_cache_client_descriptor_t *client_desc) - * client authorization. */ - cache_entry = lookup_v3_desc_as_client(client_desc->key.pubkey); - if (cache_entry != NULL) { -- /* Signalling an undecrypted descriptor. We'll always replace the one we -- * have with the new one just fetched. */ -- if (cache_entry->desc == NULL) { -+ /* If the current or the new cache entry don't have a decrypted descriptor -+ * (missing client authorization), we always replace the current one with -+ * the new one. Reason is that we can't inspect the revision counter -+ * within the plaintext data so we blindly replace. */ -+ if (!entry_has_decrypted_descriptor(cache_entry) || -+ !entry_has_decrypted_descriptor(client_desc)) { - remove_v3_desc_as_client(cache_entry); - cache_client_desc_free(cache_entry); - goto store; - } - -+ /* From this point on, we know that the decrypted descriptor is in the -+ * current entry and new object thus safe to access. */ -+ - /* If we have an entry in our cache that has a revision counter greater - * than the one we just fetched, discard the one we fetched. */ - if (cache_entry->desc->plaintext_data.revision_counter > -@@ -740,11 +776,15 @@ cache_clean_v3_as_client(time_t now) - MAP_DEL_CURRENT(key); - entry_size = cache_get_client_entry_size(entry); - bytes_removed += entry_size; -+ - /* We just removed an old descriptor. We need to close all intro circuits -- * so we don't have leftovers that can be selected while lacking a -- * descriptor. We leave the rendezvous circuits opened because they could -- * be in use. */ -- hs_client_close_intro_circuits_from_desc(entry->desc); -+ * if the descriptor is decrypted so we don't have leftovers that can be -+ * selected while lacking a descriptor. Circuits are selected by intro -+ * authentication key thus we need the descriptor. We leave the rendezvous -+ * circuits opened because they could be in use. */ -+ if (entry_has_decrypted_descriptor(entry)) { -+ hs_client_close_intro_circuits_from_desc(entry->desc); -+ } - /* Entry is not in the cache anymore, destroy it. */ - cache_client_desc_free(entry); - /* Update our OOM. We didn't use the remove() function because we are in -@@ -793,7 +833,7 @@ hs_cache_lookup_as_client(const ed25519_public_key_t *key) - tor_assert(key); - - cached_desc = lookup_v3_desc_as_client(key->pubkey); -- if (cached_desc && cached_desc->desc) { -+ if (cached_desc && entry_has_decrypted_descriptor(cached_desc)) { - return cached_desc->desc; - } - -@@ -866,7 +906,7 @@ hs_cache_remove_as_client(const ed25519_public_key_t *key) - /* If we have a decrypted/decoded descriptor, attempt to close its - * introduction circuit(s). We shouldn't have circuit(s) without a - * descriptor else it will lead to a failure. */ -- if (cached_desc->desc) { -+ if (entry_has_decrypted_descriptor(cached_desc)) { - hs_client_close_intro_circuits_from_desc(cached_desc->desc); - } - /* Remove and free. */ -@@ -995,7 +1035,7 @@ hs_cache_client_new_auth_parse(const ed25519_public_key_t *service_pk) - } - - cached_desc = lookup_v3_desc_as_client(service_pk->pubkey); -- if (cached_desc == NULL || cached_desc->desc != NULL) { -+ if (cached_desc == NULL || entry_has_decrypted_descriptor(cached_desc)) { - /* No entry for that service or the descriptor is already decoded. */ - goto end; - } -diff --git a/src/test/test_hs_client.c b/src/test/test_hs_client.c -index 5f7fe9c404..bff71d2645 100644 ---- a/src/test/test_hs_client.c -+++ b/src/test/test_hs_client.c -@@ -965,6 +965,7 @@ test_close_intro_circuits_new_desc(void *arg) - (void) arg; - - hs_init(); -+ rend_cache_init(); - - /* This is needed because of the client cache expiration timestamp is based - * on having a consensus. See cached_client_descriptor_has_expired(). */ -@@ -989,6 +990,51 @@ test_close_intro_circuits_new_desc(void *arg) - circ->purpose = CIRCUIT_PURPOSE_C_INTRODUCING; - ocirc = TO_ORIGIN_CIRCUIT(circ); - -+ /* Build a descriptor _without_ client authorization and thus not -+ * decryptable. Make sure the close circuit code path is not triggered. */ -+ { -+ char *desc_encoded = NULL; -+ uint8_t descriptor_cookie[HS_DESC_DESCRIPTOR_COOKIE_LEN]; -+ curve25519_keypair_t client_kp; -+ hs_descriptor_t *desc = NULL; -+ -+ tt_int_op(0, OP_EQ, curve25519_keypair_generate(&client_kp, 0)); -+ crypto_rand((char *) descriptor_cookie, sizeof(descriptor_cookie)); -+ -+ desc = hs_helper_build_hs_desc_with_client_auth(descriptor_cookie, -+ &client_kp.pubkey, -+ &service_kp); -+ tt_assert(desc); -+ ret = hs_desc_encode_descriptor(desc, &service_kp, descriptor_cookie, -+ &desc_encoded); -+ tt_int_op(ret, OP_EQ, 0); -+ /* Associate descriptor intro key with the dummy circuit. */ -+ const hs_desc_intro_point_t *ip = -+ smartlist_get(desc->encrypted_data.intro_points, 0); -+ tt_assert(ip); -+ ocirc->hs_ident = hs_ident_circuit_new(&service_kp.pubkey); -+ ed25519_pubkey_copy(ô->hs_ident->intro_auth_pk, -+ &ip->auth_key_cert->signed_key); -+ hs_descriptor_free(desc); -+ tt_assert(desc_encoded); -+ /* Put it in the cache. Should not be decrypted since the client -+ * authorization creds were not added to the global map. */ -+ ret = hs_cache_store_as_client(desc_encoded, &service_kp.pubkey); -+ tor_free(desc_encoded); -+ tt_int_op(ret, OP_EQ, HS_DESC_DECODE_NEED_CLIENT_AUTH); -+ -+ /* Clean cache with a future timestamp. It will trigger the clean up and -+ * attempt to close the circuit but only if the descriptor is decryptable. -+ * Cache object should be removed and circuit untouched. */ -+ hs_cache_clean_as_client(mock_ns.valid_after + (60 * 60 * 24)); -+ tt_assert(!hs_cache_lookup_as_client(&service_kp.pubkey)); -+ -+ /* Make sure the circuit still there. */ -+ tt_assert(circuit_get_next_intro_circ(NULL, true)); -+ /* Get rid of the ident, it will be replaced in the next tests. */ -+ hs_ident_circuit_free(ocirc->hs_ident); -+ } -+ - /* Build the first descriptor and cache it. */ - { - char *encoded; --- -2.20.1 -