From f0776f6d53cf341e9992efe4a5ae2cc870dcd74f Mon Sep 17 00:00:00 2001 From: David Goulet Date: Tue, 10 Mar 2020 09:50:11 -0400 Subject: [PATCH 1/2] hs-v3: Don't close intro circuit for an encrypted descriptor To close intro circuits, we need a valid decrypted descriptor in order to find the circuit by authentication key. However, we can have encrypted descriptors in the client cache that are waiting for a client authorization token. This commit doesn't attempt to close intro circuits for that situation because we are unable to access the authentication key for each intro circuit. Signed-off-by: David Goulet --- changes/ticket33458 | 6 ++++++ src/feature/hs/hs_cache.c | 7 +++++-- 2 files changed, 11 insertions(+), 2 deletions(-) create mode 100644 changes/ticket33458 diff --git a/changes/ticket33458 b/changes/ticket33458 new file mode 100644 index 00000000000..ba18065c077 --- /dev/null +++ b/changes/ticket33458 @@ -0,0 +1,6 @@ + o Minor bugfixes (onion service client): + - Don't attempt to close introduction circuits if we have an encrypted + descriptor waiting for a valid client authorization token. It lead to a + non fatal assert due to the missing descriptor. Fixes bug 33458; bugfix on + 0.4.2.1-alpha. + diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c index 9cf408ca3ec..b81bdfe2cf4 100644 --- a/src/feature/hs/hs_cache.c +++ b/src/feature/hs/hs_cache.c @@ -743,8 +743,11 @@ cache_clean_v3_as_client(time_t now) /* 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); + * be in use. It might be an encrypted descriptor and thus the decrypted + * version doesn't exists. */ + if (entry->desc) { + 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 From 4b5992a2ce125fa631987100ee8b3c41e766590f Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 11 Mar 2020 10:14:00 -0400 Subject: [PATCH 2/2] fixup! hs-v3: Don't close intro circuit for an encrypted descriptor --- src/feature/hs/hs_cache.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c index b81bdfe2cf4..72d014db1bf 100644 --- a/src/feature/hs/hs_cache.c +++ b/src/feature/hs/hs_cache.c @@ -659,9 +659,10 @@ 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) { + /* Either the cache entry or the new entry is without a decrypted + * descriptor, we replace it regardless. We can't inspect the plaintext + * data for the revision counter. */ + if (cache_entry->desc == NULL || client_desc->desc == NULL) { remove_v3_desc_as_client(cache_entry); cache_client_desc_free(cache_entry); goto store;