5 files changed, 68 insertions, 21 deletions

diff --git a/src/or/bridges.c b/src/or/bridges.c
index 461f86260f..0d4549dd16 100644
--- a/src/or/bridges.c
+++ b/src/or/bridges.c
@@ -455,8 +455,8 @@ bridge_add_from_config(bridge_line_t *bridge_line)
   b->fetch_status.schedule = DL_SCHED_BRIDGE;
   b->fetch_status.backoff = DL_SCHED_RANDOM_EXPONENTIAL;
   b->fetch_status.increment_on = DL_SCHED_INCREMENT_ATTEMPT;
-  /* This will fail if UseBridges is not set */
-  download_status_reset(&b->fetch_status);
+  /* This will fail if UseBridges is not set -- and it does. */
+  // download_status_reset(&b->fetch_status);
   b->socks_args = bridge_line->socks_args;
   if (!bridge_list)
     bridge_list = smartlist_new();
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 <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 >
-        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 <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
-     * 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 c03dac9856..6fe6b82f5e 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 <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
- * time is <b>now</b>. */
+/* Get the number of the _previous_ HS time period, given that the current 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_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)
 {
diff --git a/src/or/router.c b/src/or/router.c
index 2bc7a875f1..d2d3d12f5b 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -3607,7 +3607,7 @@ routerstatus_describe(const routerstatus_t *rs)
   return routerstatus_get_description(buf, rs);
 }
 
-/** Return a human-readable description of the extend_info_t <b>ri</b>.
+/** Return a human-readable description of the extend_info_t <b>ei</b>.
  *
  * This function is not thread-safe.  Each call to this function invalidates
  * previous values returned by this function.