Browse Source

Fix revision counter bugs caused by bad SRV start time computation.

Bug description: For each descriptor, its revision counter is the OPE
ciphertext of the number of seconds since the start time of its SRV value.
This bug caused us to confuse the SRV start time in the middle of the lifetime
of a descriptor in some edge-cases, which caused descriptor rejects.

Bug cause: The bug occurs when we fetch a 23:00 consensus after
midnight (e.g. at 00:08 when not all dirauths have fetched the latest 00:00
consensus). In that case, the voting schedule (which was used for SRV start
time calculation) would return a valid-after past-midnight, whereas our
consensus would be pre-midnight, and that would confuse the SRV start time
computation which is used by HS revision counters (because we would reset the
start time of SRV, without rotating descriptors).

Bug fix: We now use our local consensus time to calculate the SRV start time,
instead of the voting schedule. The voting schedule does not work as originally
envisioned in this case, because it was created for voting by dirauths and not
for scheduling stuff on clients.
George Kadianakis 5 years ago
parent
commit
5febea0d54
2 changed files with 51 additions and 4 deletions
  1. 17 4
      src/feature/hs_common/shared_random_client.c
  2. 34 0
      src/test/test_shared_random.c

+ 17 - 4
src/feature/hs_common/shared_random_client.c

@@ -51,9 +51,13 @@ get_voting_interval(void)
   return interval;
 }
 
-/* Given the time <b>now</b>, return the start time of the current round of
+/* Given the current consensus, return the start time of the current round of
  * the SR protocol. For example, if it's 23:47:08, the current round thus
- * started at 23:47:00 for a voting interval of 10 seconds. */
+ * started at 23:47:00 for a voting interval of 10 seconds.
+ *
+ * This function uses the consensus voting schedule to derive its results,
+ * instead of the actual consensus we are currently using, so it should be used
+ * for voting purposes. */
 time_t
 get_start_time_of_current_round(void)
 {
@@ -231,8 +235,17 @@ sr_state_get_start_time_of_current_protocol_run(void)
 {
   int total_rounds = SHARED_RANDOM_N_ROUNDS * SHARED_RANDOM_N_PHASES;
   int voting_interval = get_voting_interval();
-  /* Find the time the current round started. */
-  time_t beginning_of_curr_round = get_start_time_of_current_round();
+  time_t beginning_of_curr_round;
+
+  /* This function is not used for voting purposes, so if we have a live
+     consensus, use its valid-after as the beginning of the current round,
+     otherwise resort to the voting schedule which should always exist. */
+  networkstatus_t *ns = networkstatus_get_live_consensus(approx_time());
+  if (ns) {
+    beginning_of_curr_round = ns->valid_after;
+  } else {
+    beginning_of_curr_round = get_start_time_of_current_round();
+  }
 
   /* Get current SR protocol round */
   int curr_round_slot;

+ 34 - 0
src/test/test_shared_random.c

@@ -290,6 +290,40 @@ test_get_start_time_of_current_run(void *arg)
     tt_str_op("2015-04-20 00:00:00", OP_EQ, tbuf);
   }
 
+  {
+    /* We want the local time to be past midnight, but the current consensus to
+     * have valid-after 23:00 (e.g. this can happen if we fetch a new consensus
+     * at 00:08 before dircaches have a chance to get the midnight consensus).
+     *
+     * Basically, we want to cause a desynch between ns->valid_after (23:00)
+     * and the voting_schedule.interval_starts (01:00), to make sure that
+     * sr_state_get_start_time_of_current_protocol_run() handles it gracefully:
+     * It should actually follow the local consensus time and not the voting
+     * schedule (which is designed for authority voting purposes). */
+    retval = parse_rfc1123_time("Mon, 20 Apr 2015 00:00:00 UTC",
+                                &mock_consensus.fresh_until);
+    tt_int_op(retval, OP_EQ, 0);
+
+    retval = parse_rfc1123_time("Mon, 19 Apr 2015 23:00:00 UTC",
+                                &mock_consensus.valid_after);
+
+    retval = parse_rfc1123_time("Mon, 20 Apr 2015 00:08:00 UTC",
+                                &current_time);
+    tt_int_op(retval, OP_EQ, 0);
+    update_approx_time(current_time);
+    voting_schedule_recalculate_timing(get_options(), current_time);
+
+    run_start_time = sr_state_get_start_time_of_current_protocol_run();
+
+    /* Compare it with the correct result */
+    format_iso_time(tbuf, run_start_time);
+    tt_str_op("2015-04-19 00:00:00", OP_EQ, tbuf);
+    /* Check that voting_schedule.interval_starts is at 01:00 (see above) */
+    time_t interval_starts = voting_schedule_get_next_valid_after_time();
+    format_iso_time(tbuf, interval_starts);
+    tt_str_op("2015-04-20 01:00:00", OP_EQ, tbuf);
+  }
+
   /* Next test is testing it without a consensus to use the testing voting
    * interval . */
   UNMOCK(networkstatus_get_live_consensus);