diff --git a/src/or/hs_cache.c b/src/or/hs_cache.c index 7f2a9cbbb7..3ebe13fb4d 100644 --- a/src/or/hs_cache.c +++ b/src/or/hs_cache.c @@ -20,6 +20,9 @@ #include "hs_cache.h" +static int cached_client_descriptor_has_expired(time_t now, + const hs_cache_client_descriptor_t *cached_desc); + /********************** Directory HS cache ******************/ /* Directory descriptor cache. Map indexed by blinded key. */ @@ -356,12 +359,27 @@ store_v3_desc_as_client(hs_cache_client_descriptor_t *desc) rend_cache_increment_allocation(cache_get_client_entry_size(desc)); } -/* Query our cache and return the entry or NULL if not found. */ +/* Query our cache and return the entry or NULL if not found or if expired. */ STATIC hs_cache_client_descriptor_t * lookup_v3_desc_as_client(const uint8_t *key) { + time_t now = approx_time(); + hs_cache_client_descriptor_t *cached_desc; + tor_assert(key); - return digest256map_get(hs_cache_v3_client, key); + + /* Do the lookup */ + cached_desc = digest256map_get(hs_cache_v3_client, key); + if (!cached_desc) { + return NULL; + } + + /* Don't return expired entries */ + if (cached_client_descriptor_has_expired(now, cached_desc)) { + return NULL; + } + + return cached_desc; } /* Parse the encoded descriptor in desc_str using @@ -388,7 +406,10 @@ cache_client_desc_new(const char *desc_str, /* All is good: make a cache object for this descriptor */ client_desc = tor_malloc_zero(sizeof(hs_cache_client_descriptor_t)); ed25519_pubkey_copy(&client_desc->key, service_identity_pk); - client_desc->created_ts = approx_time(); + /* Set expiration time for this cached descriptor to be the start of the next + * time period since that's when clients need to start using the next blinded + * pk of the service (and hence will need its next descriptor). */ + client_desc->expiration_ts = hs_get_start_time_of_next_time_period(0); client_desc->desc = desc; client_desc->encoded_desc = tor_strdup(desc_str); @@ -603,9 +624,8 @@ cache_store_as_client(hs_cache_client_descriptor_t *client_desc) if (cache_entry != NULL) { /* 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 > - client_desc->desc->plaintext_data.revision_counter) { - log_info(LD_REND, "We already have fresher descriptor. Ignoring."); + if (BUG(cache_entry->desc->plaintext_data.revision_counter > + client_desc->desc->plaintext_data.revision_counter)) { cache_client_desc_free(client_desc); goto done; } @@ -621,7 +641,30 @@ cache_store_as_client(hs_cache_client_descriptor_t *client_desc) return 0; } -/* Clean the client cache using now as the current time. Return the total size +/* Return true iff the cached client descriptor at cached_descexpiration_ts <= ns->valid_after) { + return 1; + } + + return 0; +} + +/* clean the client cache using now as the current time. Return the total size * of removed bytes from the cache. */ static size_t cache_clean_v3_as_client(time_t now) @@ -635,11 +678,9 @@ cache_clean_v3_as_client(time_t now) DIGEST256MAP_FOREACH_MODIFY(hs_cache_v3_client, key, hs_cache_client_descriptor_t *, entry) { size_t entry_size; - time_t cutoff = now - rend_cache_max_entry_lifetime(); - /* If the entry has been created _after_ the cutoff, not expired so - * continue to the next entry in our v3 cache. */ - if (entry->created_ts > cutoff) { + /* If the entry has not expired, continue to the next cached entry */ + if (!cached_client_descriptor_has_expired(now, entry)) { continue; } /* Here, our entry has expired, remove and free. */ diff --git a/src/or/hs_cache.h b/src/or/hs_cache.h index 8dbc842b95..a6beaebc10 100644 --- a/src/or/hs_cache.h +++ b/src/or/hs_cache.h @@ -103,8 +103,10 @@ typedef struct hs_cache_client_descriptor_t { /* This object is indexed using the service identity public key */ ed25519_public_key_t key; - /* When was this entry created. Used to expire entries. */ - time_t created_ts; + /* When will this entry expire? We expire cached client descriptors in the + * start of the next time period, since that's when clients need to start + * using the next blinded key of the service. */ + time_t expiration_ts; /* The cached descriptor, this object is the owner. It can't be NULL. A * cache object without a valid descriptor is not possible. */ diff --git a/src/or/hs_common.c b/src/or/hs_common.c index 291d8ae8da..f10fc4d5a4 100644 --- a/src/or/hs_common.c +++ b/src/or/hs_common.c @@ -235,7 +235,7 @@ get_time_period_length(void) } /** Get the HS time period number at time now. If now is not set, - * we try to get the time ourselves. */ + * we try to get the time ourselves from a live consensus. */ uint64_t hs_get_time_period_num(time_t now) { @@ -269,22 +269,26 @@ hs_get_time_period_num(time_t now) } /** Get the number of the _upcoming_ HS time period, given that the current - * time is now. */ + * time is now. If now is not set, we try to get the time from a + * live consensus. */ uint64_t hs_get_next_time_period_num(time_t now) { return hs_get_time_period_num(now) + 1; } -/* Get the number of the _previous_ HS time period, given that the current - * time is now. */ +/* Get the number of the _previous_ HS time period, given that the current time + * is now. If now is not set, we try to get the time from a live + * consensus. */ uint64_t hs_get_previous_time_period_num(time_t now) { return hs_get_time_period_num(now) - 1; } -/* Return the start time of the upcoming time period based on now. */ +/* Return the start time of the upcoming time period based on now. If + now is not set, we try to get the time ourselves from a live + consensus. */ time_t hs_get_start_time_of_next_time_period(time_t now) { diff --git a/src/test/hs_test_helpers.c b/src/test/hs_test_helpers.c index 2753d29078..cab73196b9 100644 --- a/src/test/hs_test_helpers.c +++ b/src/test/hs_test_helpers.c @@ -104,7 +104,7 @@ hs_helper_build_hs_desc_impl(unsigned int no_ip, memcpy(&desc->plaintext_data.signing_pubkey, &signing_kp->pubkey, sizeof(ed25519_public_key_t)); - uint64_t current_time_period = hs_get_time_period_num(approx_time()); + uint64_t current_time_period = hs_get_time_period_num(0); hs_build_blinded_keypair(signing_kp, NULL, 0, current_time_period, &blinded_kp); /* Copy only the public key into the descriptor. */ diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c index 950c0483d1..91b13be862 100644 --- a/src/test/test_hs_cache.c +++ b/src/test/test_hs_cache.c @@ -14,6 +14,7 @@ #include "hs_cache.h" #include "rendcache.h" #include "directory.h" +#include "networkstatus.h" #include "connection.h" #include "proto_http.h" @@ -433,6 +434,15 @@ test_hsdir_revision_counter_check(void *arg) tor_free(published_desc_str); } +static networkstatus_t mock_ns; + +static networkstatus_t * +mock_networkstatus_get_live_consensus(time_t now) +{ + (void) now; + return &mock_ns; +} + /** Test that we can store HS descriptors in the client HS cache. */ static void test_client_cache(void *arg) @@ -441,7 +451,7 @@ test_client_cache(void *arg) ed25519_keypair_t signing_kp; hs_descriptor_t *published_desc = NULL; char *published_desc_str = NULL; - + uint8_t wanted_subcredential[DIGEST256_LEN]; response_handler_args_t *args = NULL; dir_connection_t *conn = NULL; @@ -450,6 +460,17 @@ test_client_cache(void *arg) /* Initialize HSDir cache subsystem */ init_test(); + MOCK(networkstatus_get_live_consensus, + mock_networkstatus_get_live_consensus); + + /* Set consensus 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 a valid descriptor with normal values. */ { retval = ed25519_keypair_generate(&signing_kp, 0); @@ -459,6 +480,8 @@ test_client_cache(void *arg) retval = hs_desc_encode_descriptor(published_desc, &signing_kp, &published_desc_str); tt_int_op(retval, OP_EQ, 0); + memcpy(wanted_subcredential, published_desc->subcredential, DIGEST256_LEN); + tt_assert(!tor_mem_is_zero((char*)wanted_subcredential, DIGEST256_LEN)); } /* Test handle_response_fetch_hsdesc_v3() */ @@ -478,12 +501,36 @@ test_client_cache(void *arg) retval = handle_response_fetch_hsdesc_v3(conn, args); tt_int_op(retval, == , 0); - /* fetch the descriptor and make sure it's there */ + /* Progress time a bit and attempt to clean cache: our desc should not be + * cleaned since we still in the same TP. */ { - hs_cache_client_descriptor_t *cached_desc = NULL; - cached_desc = lookup_v3_desc_as_client(signing_kp.pubkey.pubkey); + parse_rfc1123_time("Sat, 27 Oct 1985 02:00:00 UTC", + &mock_ns.valid_after); + parse_rfc1123_time("Sat, 27 Oct 1985 03:00:00 UTC", + &mock_ns.fresh_until); + parse_rfc1123_time("Sat, 27 Oct 1985 05:00:00 UTC", + &mock_ns.valid_until); + + /* fetch the descriptor and make sure it's there */ + const hs_descriptor_t *cached_desc = NULL; + cached_desc = hs_cache_lookup_as_client(&signing_kp.pubkey); tt_assert(cached_desc); - tt_str_op(cached_desc->encoded_desc, OP_EQ, published_desc_str); + tt_mem_op(cached_desc->subcredential, OP_EQ, wanted_subcredential, + DIGEST256_LEN); + } + + /* Progress time to next TP and check that desc was cleaned */ + { + parse_rfc1123_time("Sat, 27 Oct 1985 12:00:00 UTC", + &mock_ns.valid_after); + parse_rfc1123_time("Sat, 27 Oct 1985 13:00:00 UTC", + &mock_ns.fresh_until); + parse_rfc1123_time("Sat, 27 Oct 1985 15:00:00 UTC", + &mock_ns.valid_until); + + const hs_descriptor_t *cached_desc = NULL; + cached_desc = hs_cache_lookup_as_client(&signing_kp.pubkey); + tt_assert(!cached_desc); } done: