Merge branch 'bug23466_squashed'

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 <b>desc_str</b> 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 >
+    if (BUG(cache_entry->desc->plaintext_data.revision_counter >
-        client_desc->desc->plaintext_data.revision_counter) {
+            client_desc->desc->plaintext_data.revision_counter)) {
-      log_info(LD_REND, "We already have fresher descriptor. Ignoring.");
       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 <b>cached_desc</b has
+ * expired. */
+static int
+cached_client_descriptor_has_expired(time_t now,
+                               const hs_cache_client_descriptor_t *cached_desc)
+{
+  /* We use the current consensus time to see if we should expire this
+   * descriptor since we use consensus time for all other parts of the protocol
+   * as well (e.g. to build the blinded key and compute time periods). */
+  const networkstatus_t *ns = networkstatus_get_live_consensus(now);
+  /* If we don't have a recent consensus, consider this entry expired since we
+   * will want to fetch a new HS desc when we get a live consensus. */
+  if (!ns) {
+    return 1;
+  }
+
+  if (cached_desc->expiration_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
+    /* If the entry has not expired, continue to the next cached entry */
-     * continue to the next entry in our v3 cache. */
+    if (!cached_client_descriptor_has_expired(now, entry)) {
-    if (entry->created_ts > cutoff) {
       continue;
     }
     /* Here, our entry has expired, remove and free. */

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. */
+  /* When will this entry expire? We expire cached client descriptors in the
-  time_t created_ts;
+   * 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. */

src/or/hs_common.c

@@ -235,7 +235,7 @@ get_time_period_length(void)
 }
 
 /** Get the HS time period number at time <b>now</b>. If <b>now</b> 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 <b>now</b>. */
+ *  time is <b>now</b>. If <b>now</b> 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
+/* Get the number of the _previous_ HS time period, given that the current time
- * time is <b>now</b>. */
+ * is <b>now</b>. If <b>now</b> 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 <b>now</b>. */
+/* Return the start time of the upcoming time period based on <b>now</b>. If
+   <b>now</b> 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)
 {

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. */

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;
+    parse_rfc1123_time("Sat, 27 Oct 1985 02:00:00 UTC",
-    cached_desc = lookup_v3_desc_as_client(signing_kp.pubkey.pubkey);
+                       &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: