commit de343b4e421c0c651eaac1d52d23c3c792bee73a Author: Taylor Yu catalyst@torproject.org Date: Fri May 4 17:16:06 2018 -0500
Improve tolerance for dirauths with skewed clocks
Previously, an authority with a clock more than 60 seconds ahead could cause a client with a correct clock to warn that the client's clock was behind. Now the clocks of a majority of directory authorities have to be ahead of the client before this warning will occur.
Relax the early-consensus check so that a client's clock must be 60 seconds behind the earliest time that a given sufficiently-signed consensus could possibly be available.
Add a new unit test that calls warn_early_consensus() directly.
Fixes bug 25756; bugfix on 0.2.2.25-alpha. --- changes/bug25756 | 7 +++++ src/or/networkstatus.c | 14 +++++++--- src/test/test_routerlist.c | 64 +++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 79 insertions(+), 6 deletions(-)
diff --git a/changes/bug25756 b/changes/bug25756 new file mode 100644 index 000000000..ff5ac0391 --- /dev/null +++ b/changes/bug25756 @@ -0,0 +1,7 @@ + o Minor bugfixes (error reporting): + - Improve tolerance for directory authorities with skewed clocks. + Previously, an authority with a clock more than 60 seconds ahead + could cause a client with a correct clock to warn that the + client's clock was behind. Now the clocks of a majority of + directory authorities have to be ahead of the client before this + warning will occur. Fixes bug 25756; bugfix on 0.2.2.25-alpha. diff --git a/src/or/networkstatus.c b/src/or/networkstatus.c index d1c217119..51b2f4af1 100644 --- a/src/or/networkstatus.c +++ b/src/or/networkstatus.c @@ -1777,10 +1777,18 @@ warn_early_consensus(const networkstatus_t *c, const char *flavor, long delta = now - c->valid_after; char *flavormsg = NULL;
-/** If a consensus appears more than this many seconds before its declared - * valid-after time, declare that our clock is skewed. */ +/** If a consensus appears more than this many seconds before it could + * possibly be a sufficiently-signed consensus, declare that our clock + * is skewed. */ #define EARLY_CONSENSUS_NOTICE_SKEW 60 - if (now >= c->valid_after - EARLY_CONSENSUS_NOTICE_SKEW) + + /* We assume that if a majority of dirauths have accurate clocks, + * the earliest that a dirauth with a skewed clock could possibly + * publish a sufficiently-signed consensus is (valid_after - + * dist_seconds). Before that time, the skewed dirauth would be + * unable to obtain enough authority signatures for the consensus to + * be valid. */ + if (now >= c->valid_after - c->dist_seconds - EARLY_CONSENSUS_NOTICE_SKEW) return;
format_iso_time(tbuf, c->valid_after); diff --git a/src/test/test_routerlist.c b/src/test/test_routerlist.c index 6dd58d313..701227c1c 100644 --- a/src/test/test_routerlist.c +++ b/src/test/test_routerlist.c @@ -692,6 +692,62 @@ test_early_consensus(void *arg) UNMOCK(clock_skew_warning); }
+/** Test warn_early_consensus(), expecting no warning */ +static void +test_warn_early_consensus_no(const networkstatus_t *c, time_t now, + long offset) +{ + mock_apparent_skew = 0; + setup_capture_of_logs(LOG_WARN); + warn_early_consensus(c, "microdesc", now + offset); + expect_no_log_msg_containing("behind the time published in the consensus"); + tt_int_op(mock_apparent_skew, OP_EQ, 0); + done: + teardown_capture_of_logs(); +} + +/** Test warn_early_consensus(), expecting a warning */ +static void +test_warn_early_consensus_yes(const networkstatus_t *c, time_t now, + long offset) +{ + mock_apparent_skew = 0; + setup_capture_of_logs(LOG_WARN); + warn_early_consensus(c, "microdesc", now + offset); + /* Can't use expect_single_log_msg() because of unrecognized authorities */ + expect_log_msg_containing("behind the time published in the consensus"); + tt_int_op(mock_apparent_skew, OP_EQ, offset); + done: + teardown_capture_of_logs(); +} + +/** + * Test warn_early_consensus() directly, checking both the non-warning + * case (consensus is not early) and the warning case (consensus is + * early). Depends on EARLY_CONSENSUS_NOTICE_SKEW=60. + */ +static void +test_warn_early_consensus(void *arg) +{ + networkstatus_t *c = NULL; + time_t now = time(NULL); + + (void)arg; + c = tor_malloc_zero(sizeof *c); + c->valid_after = now; + c->dist_seconds = 300; + mock_apparent_skew = 0; + MOCK(clock_skew_warning, mock_clock_skew_warning); + test_warn_early_consensus_no(c, now, 60); + test_warn_early_consensus_no(c, now, 0); + test_warn_early_consensus_no(c, now, -60); + test_warn_early_consensus_no(c, now, -360); + test_warn_early_consensus_yes(c, now, -361); + test_warn_early_consensus_yes(c, now, -600); + UNMOCK(clock_skew_warning); + tor_free(c); +} + #define NODE(name, flags) \ { #name, test_routerlist_##name, (flags), NULL, NULL } #define ROUTER(name,flags) \ @@ -711,11 +767,13 @@ struct testcase_t routerlist_tests[] = { ROUTER(pick_directory_server_impl, TT_FORK), { "directory_guard_fetch_with_no_dirinfo", test_directory_guard_fetch_with_no_dirinfo, TT_FORK, NULL, NULL }, - /* These depend on construct_consensus() setting valid_after=now+1000 */ + /* These depend on construct_consensus() setting + * valid_after=now+1000 and dist_seconds=250 */ TIMELY("timely_consensus1", "1010"), TIMELY("timely_consensus2", "1000"), - TIMELY("timely_consensus3", "940"), - EARLY("early_consensus1", "939"), + TIMELY("timely_consensus3", "690"), + EARLY("early_consensus1", "689"), + { "warn_early_consensus", test_warn_early_consensus, 0, NULL, NULL }, END_OF_TESTCASES };