commit 9278a24729c92b9f5c670b3e1608e2cdbd8bd9a1 Author: David Goulet <dgoulet@torproject.org> Date: Tue Feb 4 09:25:55 2020 -0500 hs-v3: Remove descriptor when we remove client authorization When the ONION_CLIENT_AUTH_REMOVE command is given to tor, now also remove the descriptor associated with the client authorization credentials. Fixes #33148 Signed-off-by: David Goulet <dgoulet@torproject.org> --- changes/ticket33148 | 5 +++++ src/feature/hs/hs_cache.c | 36 ++++++++++++++++++++++++++++++ src/feature/hs/hs_cache.h | 1 + src/feature/hs/hs_client.c | 3 +++ src/test/test_hs_cache.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 100 insertions(+) diff --git a/changes/ticket33148 b/changes/ticket33148 new file mode 100644 index 000000000..1f44b4259 --- /dev/null +++ b/changes/ticket33148 @@ -0,0 +1,5 @@ + o Minor bugfixes (onion service v3, client authorization): + - When removing client authorization credentials using the control port, + also remove the associated descriptor so they don't linger and are still + usable hence making the onion service behind client authorization + reachable. Fixes bug 33148; bugfix on 0.4.3.1-alpha. diff --git a/src/feature/hs/hs_cache.c b/src/feature/hs/hs_cache.c index a7b41b835..9cf408ca3 100644 --- a/src/feature/hs/hs_cache.c +++ b/src/feature/hs/hs_cache.c @@ -847,6 +847,42 @@ hs_cache_store_as_client(const char *desc_str, return ret; } +/** Remove and free a client cache descriptor entry for the given onion + * service ed25519 public key. If the descriptor is decoded, the intro + * circuits are closed if any. + * + * This does nothing if no descriptor exists for the given key. */ +void +hs_cache_remove_as_client(const ed25519_public_key_t *key) +{ + hs_cache_client_descriptor_t *cached_desc = NULL; + + tor_assert(key); + + cached_desc = lookup_v3_desc_as_client(key->pubkey); + if (!cached_desc) { + return; + } + /* 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) { + hs_client_close_intro_circuits_from_desc(cached_desc->desc); + } + /* Remove and free. */ + remove_v3_desc_as_client(cached_desc); + cache_client_desc_free(cached_desc); + + /* Logging. */ + { + char key_b64[BASE64_DIGEST256_LEN + 1]; + digest256_to_base64(key_b64, (const char *) key); + log_info(LD_REND, "Onion service v3 descriptor '%s' removed " + "from client cache", + safe_str_client(key_b64)); + } +} + /** Clean all client caches using the current time now. */ void hs_cache_clean_as_client(time_t now) diff --git a/src/feature/hs/hs_cache.h b/src/feature/hs/hs_cache.h index ebe1621e8..bb3c77f22 100644 --- a/src/feature/hs/hs_cache.h +++ b/src/feature/hs/hs_cache.h @@ -85,6 +85,7 @@ const char * hs_cache_lookup_encoded_as_client(const struct ed25519_public_key_t *key); hs_desc_decode_status_t hs_cache_store_as_client(const char *desc_str, const struct ed25519_public_key_t *identity_pk); +void hs_cache_remove_as_client(const struct ed25519_public_key_t *key); void hs_cache_clean_as_client(time_t now); void hs_cache_purge_as_client(void); diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index 611cc5430..4599bde5b 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -1735,6 +1735,9 @@ hs_client_remove_auth_credentials(const char *hsaddress) find_and_remove_client_auth_creds_file(cred); } + /* Remove associated descriptor if any. */ + hs_cache_remove_as_client(&service_identity_pk); + client_service_authorization_free(cred); return REMOVAL_SUCCESS; } diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c index 9e0094d25..8ea550b65 100644 --- a/src/test/test_hs_cache.c +++ b/src/test/test_hs_cache.c @@ -645,6 +645,59 @@ test_client_cache_decrypt(void *arg) UNMOCK(networkstatus_get_live_consensus); } +static void +test_client_cache_remove(void *arg) +{ + int ret; + ed25519_keypair_t service_kp; + hs_descriptor_t *desc1 = NULL; + + (void) arg; + + hs_init(); + + MOCK(networkstatus_get_live_consensus, + mock_networkstatus_get_live_consensus); + + /* Set consensus time. Lookup will not return the entry if it has expired + * and it is checked against the consensus valid_after time. */ + parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC", + &mock_ns.valid_after); + parse_rfc1123_time("Sat, 26 Oct 1985 14:00:00 UTC", + &mock_ns.fresh_until); + parse_rfc1123_time("Sat, 26 Oct 1985 16:00:00 UTC", + &mock_ns.valid_until); + + /* Generate service keypair */ + tt_int_op(0, OP_EQ, ed25519_keypair_generate(&service_kp, 0)); + + /* Build a descriptor and cache it. */ + { + char *encoded; + desc1 = hs_helper_build_hs_desc_with_ip(&service_kp); + tt_assert(desc1); + ret = hs_desc_encode_descriptor(desc1, &service_kp, NULL, &encoded); + tt_int_op(ret, OP_EQ, 0); + tt_assert(encoded); + + /* Store it */ + ret = hs_cache_store_as_client(encoded, &service_kp.pubkey); + tt_int_op(ret, OP_EQ, HS_DESC_DECODE_OK); + tor_free(encoded); + tt_assert(hs_cache_lookup_as_client(&service_kp.pubkey)); + } + + /* Remove the cached entry. */ + hs_cache_remove_as_client(&service_kp.pubkey); + tt_assert(!hs_cache_lookup_as_client(&service_kp.pubkey)); + + done: + hs_descriptor_free(desc1); + hs_free_all(); + + UNMOCK(networkstatus_get_live_consensus); +} + struct testcase_t hs_cache[] = { /* Encoding tests. */ { "directory", test_directory, TT_FORK, @@ -659,6 +712,8 @@ struct testcase_t hs_cache[] = { NULL, NULL }, { "client_cache_decrypt", test_client_cache_decrypt, TT_FORK, NULL, NULL }, + { "client_cache_remove", test_client_cache_remove, TT_FORK, + NULL, NULL }, END_OF_TESTCASES };