Don't set HSDir index if we don't have a live consensus.

We also had to alter the SRV functions to take a consensus as optional
input, since we might be setting our HSDir index using a consensus that
is currently being processed and won't be returned by the
networkstatus_get_live_consensus() function.

This change has two results:

a) It makes sure we are using a fresh consensus with the right SRV value
   when we are calculating the HSDir hash ring.

b) It ensures that we will not use the sr_get_current/previous()
   functions when we don't have a consensus which would have falsely
   triggered the disaster SRV logic.

src/or/hs_common.c

@@ -1058,12 +1058,13 @@ hs_build_hsdir_index(const ed25519_public_key_t *identity_pk,
 
 /* Return a newly allocated buffer containing the current shared random value
  * or if not present, a disaster value is computed using the given time period
- * number. This function can't fail. */
+ * number. If a consensus is provided in <b>ns</b>, use it to get the SRV
+ * value. This function can't fail. */
 uint8_t *
-hs_get_current_srv(uint64_t time_period_num)
+hs_get_current_srv(uint64_t time_period_num, const networkstatus_t *ns)
 {
   uint8_t *sr_value = tor_malloc_zero(DIGEST256_LEN);
-  const sr_srv_t *current_srv = sr_get_current();
+  const sr_srv_t *current_srv = sr_get_current(ns);
 
   if (current_srv) {
     memcpy(sr_value, current_srv->value, sizeof(current_srv->value));
@@ -1078,10 +1079,10 @@ hs_get_current_srv(uint64_t time_period_num)
  * value or if not present, a disaster value is computed using the given time
  * period number. This function can't fail. */
 uint8_t *
-hs_get_previous_srv(uint64_t time_period_num)
+hs_get_previous_srv(uint64_t time_period_num, const networkstatus_t *ns)
 {
   uint8_t *sr_value = tor_malloc_zero(DIGEST256_LEN);
-  const sr_srv_t *previous_srv = sr_get_previous();
+  const sr_srv_t *previous_srv = sr_get_previous(ns);
 
   if (previous_srv) {
     memcpy(sr_value, previous_srv->value, sizeof(previous_srv->value));

src/or/hs_common.h

@@ -200,8 +200,10 @@ link_specifier_t *hs_link_specifier_dup(const link_specifier_t *lspec);
 MOCK_DECL(int, hs_overlap_mode_is_active,
           (const networkstatus_t *consensus, time_t now));
 
-uint8_t *hs_get_current_srv(uint64_t time_period_num);
-uint8_t *hs_get_previous_srv(uint64_t time_period_num);
+uint8_t *hs_get_current_srv(uint64_t time_period_num,
+                            const networkstatus_t *ns);
+uint8_t *hs_get_previous_srv(uint64_t time_period_num,
+                             const networkstatus_t *ns);
 
 void hs_build_hsdir_index(const ed25519_public_key_t *identity_pk,
                           const uint8_t *srv, uint64_t period_num,

src/or/networkstatus.c

@@ -1393,14 +1393,21 @@ networkstatus_get_latest_consensus_by_flavor,(consensus_flavor_t f))
 MOCK_IMPL(networkstatus_t *,
 networkstatus_get_live_consensus,(time_t now))
 {
-  if (networkstatus_get_latest_consensus() &&
-      networkstatus_get_latest_consensus()->valid_after <= now &&
-      now <= networkstatus_get_latest_consensus()->valid_until)
-    return networkstatus_get_latest_consensus();
+  networkstatus_t *ns = networkstatus_get_latest_consensus();
+  if (ns && networkstatus_is_live(ns, now))
+    return ns;
   else
     return NULL;
 }
 
+/** Given a consensus in <b>ns</b>, validate that it's currently live and
+ *  unexpired. */
+int
+networkstatus_is_live(const networkstatus_t *ns, time_t now)
+{
+  return (ns->valid_after <= now && now <= ns->valid_until);
+}
+
 /** Determine if <b>consensus</b> is valid or expired recently enough that
  * we can still use it.
  *

src/or/networkstatus.h

@@ -81,6 +81,7 @@ MOCK_DECL(networkstatus_t *,networkstatus_get_latest_consensus,(void));
 MOCK_DECL(networkstatus_t *,networkstatus_get_latest_consensus_by_flavor,
           (consensus_flavor_t f));
 MOCK_DECL(networkstatus_t *, networkstatus_get_live_consensus,(time_t now));
+int networkstatus_is_live(const networkstatus_t *ns, time_t now);
 int networkstatus_consensus_reasonably_live(const networkstatus_t *consensus,
                                             time_t now);
 int networkstatus_valid_until_is_reasonably_live(time_t valid_until,

src/or/nodelist.c

@@ -179,14 +179,19 @@ node_get_or_create(const char *identity_digest)
 static void
 node_set_hsdir_index(node_t *node, const networkstatus_t *ns)
 {
-  time_t now = time(NULL);
+  time_t now = approx_time();
   const ed25519_public_key_t *node_identity_pk;
   uint8_t *next_hsdir_index_srv = NULL, *current_hsdir_index_srv = NULL;
   uint64_t next_time_period_num, current_time_period_num;
 
   tor_assert(node);
   tor_assert(ns);
 
+  if (!networkstatus_is_live(ns, now)) {
+    log_info(LD_GENERAL, "Not setting hsdir index with a non-live consensus.");
+    goto done;
+  }
+
   node_identity_pk = node_get_ed25519_id(node);
   if (node_identity_pk == NULL) {
     log_debug(LD_GENERAL, "ed25519 identity public key not found when "
@@ -205,15 +210,15 @@ node_set_hsdir_index(node_t *node, const networkstatus_t *ns)
      * period, we have to use the current SRV and use the previous SRV for the
      * current time period. If the current or previous SRV can't be found, the
      * disaster one is returned. */
-    next_hsdir_index_srv = hs_get_current_srv(next_time_period_num);
+    next_hsdir_index_srv = hs_get_current_srv(next_time_period_num, ns);
     /* The following can be confusing so again, in overlap mode, we use our
      * previous SRV for our _current_ hsdir index. */
-    current_hsdir_index_srv = hs_get_previous_srv(current_time_period_num);
+    current_hsdir_index_srv = hs_get_previous_srv(current_time_period_num, ns);
   } else {
     /* If NOT in overlap mode, we only need to compute the current hsdir index
      * for the ongoing time period and thus the current SRV. If it can't be
      * found, the disaster one is returned. */
-    current_hsdir_index_srv = hs_get_current_srv(current_time_period_num);
+    current_hsdir_index_srv = hs_get_current_srv(current_time_period_num, ns);
   }
 
   /* Build the current hsdir index. */

src/or/shared_random.c

@@ -1393,23 +1393,45 @@ sr_get_previous_for_control(void)
 /* Return current shared random value from the latest consensus. Caller can
  * NOT keep a reference to the returned pointer. Return NULL if none. */
 const sr_srv_t *
-sr_get_current(void)
+sr_get_current(const networkstatus_t *ns)
 {
-  const networkstatus_t *c = networkstatus_get_latest_consensus();
-  if (c) {
-    return c->sr_info.current_srv;
+  const networkstatus_t *consensus;
+
+  /* Use provided ns else get a live one */
+  if (ns) {
+    consensus = ns;
+  } else {
+    consensus = networkstatus_get_live_consensus(approx_time());
+  }
+  /* Ideally we would never be asked for an SRV without a live consensus. Make
+   * sure this assumption is correct. */
+  tor_assert_nonfatal(consensus);
+
+  if (consensus) {
+    return consensus->sr_info.current_srv;
   }
   return NULL;
 }
 
 /* Return previous shared random value from the latest consensus. Caller can
  * NOT keep a reference to the returned pointer. Return NULL if none. */
 const sr_srv_t *
-sr_get_previous(void)
+sr_get_previous(const networkstatus_t *ns)
 {
-  const networkstatus_t *c = networkstatus_get_latest_consensus();
-  if (c) {
-    return c->sr_info.previous_srv;
+  const networkstatus_t *consensus;
+
+  /* Use provided ns else get a live one */
+  if (ns) {
+    consensus = ns;
+  } else {
+    consensus = networkstatus_get_live_consensus(approx_time());
+  }
+  /* Ideally we would never be asked for an SRV without a live consensus. Make
+   * sure this assumption is correct. */
+  tor_assert_nonfatal(consensus);
+
+  if (consensus) {
+    return consensus->sr_info.previous_srv;
   }
   return NULL;
 }

src/or/shared_random.h

@@ -130,8 +130,8 @@ sr_commit_t *sr_generate_our_commit(time_t timestamp,
 char *sr_get_current_for_control(void);
 char *sr_get_previous_for_control(void);
 
-const sr_srv_t *sr_get_current(void);
-const sr_srv_t *sr_get_previous(void);
+const sr_srv_t *sr_get_current(const networkstatus_t *ns);
+const sr_srv_t *sr_get_previous(const networkstatus_t *ns);
 
 #ifdef SHARED_RANDOM_PRIVATE