[tor-commits] [tor/master] Fix revision counter bugs caused by bad SRV start time computation.

[nickm at torproject.org]

commit 5febea0d54ea986ab66dc62905a686cc23abbcb6
Author: George Kadianakis <desnacked at riseup.net>
Date:   Fri Aug 17 15:10:20 2018 +0300

    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.
---
 src/feature/hs_common/shared_random_client.c | 21 +++++++++++++----
 src/test/test_shared_random.c                | 34 ++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/src/feature/hs_common/shared_random_client.c b/src/feature/hs_common/shared_random_client.c
index ff98a719d..a13404a32 100644
--- a/src/feature/hs_common/shared_random_client.c
+++ b/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;
diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c
index d2defdf68..72e4522da 100644
--- a/src/test/test_shared_random.c
+++ b/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);