From 5febea0d54ea986ab66dc62905a686cc23abbcb6 Mon Sep 17 00:00:00 2001 From: George Kadianakis Date: Fri, 17 Aug 2018 15:10:20 +0300 Subject: [PATCH] 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 ff98a719db..a13404a329 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 now, 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 d2defdf680..72e4522da1 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", + ¤t_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);