From 7d70f67deaeea1a3dc80a763f13bcec5d7a2425d Mon Sep 17 00:00:00 2001 From: juga0 Date: Sun, 3 Jun 2018 09:31:19 +0000 Subject: Check bandwidth changes only if small uptime to upload a new descriptor. --- src/or/router.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/or/router.c b/src/or/router.c index 31f2ff00d2..ab916fb6e6 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2425,13 +2425,24 @@ mark_my_descriptor_dirty(const char *reason) * if our previous bandwidth estimate was exactly 0. */ #define MAX_BANDWIDTH_CHANGE_FREQ (3*60*60) +/** Maximum uptime to republish our descriptor because of large shifts in + * estimated bandwidth. */ +#define MAX_UPTIME_BANDWIDTH_CHANGE (24*60*60) + /** Check whether bandwidth has changed a lot since the last time we announced - * bandwidth. If so, mark our descriptor dirty. */ + * bandwidth while the uptime is smaller than MAX_UPTIME_BANDWIDTH_CHANGE. + * If so, mark our descriptor dirty. */ void check_descriptor_bandwidth_changed(time_t now) { static time_t last_changed = 0; uint64_t prev, cur; + + /* If the relay uptime is bigger than MAX_UPTIME_BANDWIDTH_CHANGE, + * the next regularly scheduled descriptor update (18h) will be enough */ + if (get_uptime() > MAX_UPTIME_BANDWIDTH_CHANGE) + return; + if (!router_get_my_routerinfo()) return; -- cgit v1.2.3-54-g00ecf From 6210d568ecc5c2fd1833126d68505f42e4399820 Mon Sep 17 00:00:00 2001 From: juga0 Date: Sun, 3 Jun 2018 09:40:57 +0000 Subject: Make bandwidth change factor a constant used to determine large changes in bandwidth. --- src/or/router.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index ab916fb6e6..4afba65491 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2429,6 +2429,9 @@ mark_my_descriptor_dirty(const char *reason) * estimated bandwidth. */ #define MAX_UPTIME_BANDWIDTH_CHANGE (24*60*60) +/** By which factor bandwidth shifts have to change to be considered large. */ +#define BANDWIDTH_CHANGE_FACTOR 2 + /** Check whether bandwidth has changed a lot since the last time we announced * bandwidth while the uptime is smaller than MAX_UPTIME_BANDWIDTH_CHANGE. * If so, mark our descriptor dirty. */ @@ -2449,8 +2452,8 @@ check_descriptor_bandwidth_changed(time_t now) prev = router_get_my_routerinfo()->bandwidthcapacity; cur = we_are_hibernating() ? 0 : rep_hist_bandwidth_assess(); if ((prev != cur && (!prev || !cur)) || - cur > prev*2 || - cur < prev/2) { + cur > (prev * BANDWIDTH_CHANGE_FACTOR) || + cur < (prev / BANDWIDTH_CHANGE_FACTOR) ) { if (last_changed+MAX_BANDWIDTH_CHANGE_FREQ < now || !prev) { log_info(LD_GENERAL, "Measured bandwidth has changed; rebuilding descriptor."); -- cgit v1.2.3-54-g00ecf From bbb82a52491de244fbdf5d7b6f22ae26f8b4ea19 Mon Sep 17 00:00:00 2001 From: juga0 Date: Mon, 4 Jun 2018 20:40:21 +0000 Subject: Add changes file --- changes/bug24104 | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changes/bug24104 diff --git a/changes/bug24104 b/changes/bug24104 new file mode 100644 index 0000000000..ca2a3537fa --- /dev/null +++ b/changes/bug24104 @@ -0,0 +1,4 @@ + o Minor bugfix (relay statistics): + - Update relay descriptor on bandwidth changes only when the uptime is + smaller than 24h in order to reduce the efficiency of guard discovery + attacks. Fixes bug 24104; bugfix on 0.1.1.6-alpha. -- cgit v1.2.3-54-g00ecf From 842b18ab26ac12da7e63c38413d193905b8fbed5 Mon Sep 17 00:00:00 2001 From: juga0 Date: Sat, 9 Jun 2018 10:34:17 +0000 Subject: Add test log helpers for msgs not containing str --- src/test/log_test_helpers.c | 22 +++++++++++++++++++++- src/test/log_test_helpers.h | 7 ++++++- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/test/log_test_helpers.c b/src/test/log_test_helpers.c index c788a33c17..c5368b6cbc 100644 --- a/src/test/log_test_helpers.c +++ b/src/test/log_test_helpers.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2015-2016, The Tor Project, Inc. */ +/* Copyright (c) 2015-2018, The Tor Project, Inc. */ /* See LICENSE for licensing information */ #define LOG_PRIVATE #include "torlog.h" @@ -158,6 +158,26 @@ mock_saved_log_has_message_containing(const char *msg) return 0; } +/** + * Return true iff there is not a message recorded by log capture + * that contains msg as a substring. + */ +int +mock_saved_log_has_message_not_containing(const char *msg) +{ + if (saved_logs) { + SMARTLIST_FOREACH( + saved_logs, mock_saved_log_entry_t *, m, + { + if (msg && m->generated_msg && strstr(m->generated_msg, msg)) + return 0; + } + ); + } + + return 1; +} + /** Return true iff the saved logs have any messages with severity */ int mock_saved_log_has_severity(int severity) diff --git a/src/test/log_test_helpers.h b/src/test/log_test_helpers.h index 922c68b42f..a087b913f4 100644 --- a/src/test/log_test_helpers.h +++ b/src/test/log_test_helpers.h @@ -1,4 +1,4 @@ -/* Copyright (c) 2014-2016, The Tor Project, Inc. */ +/* Copyright (c) 2014-2018, The Tor Project, Inc. */ /* See LICENSE for licensing information */ #include "or.h" @@ -24,6 +24,7 @@ void teardown_capture_of_logs(void); int mock_saved_log_has_message(const char *msg); int mock_saved_log_has_message_containing(const char *msg); +int mock_saved_log_has_message_not_containing(const char *msg); int mock_saved_log_has_severity(int severity); int mock_saved_log_has_entry(void); int mock_saved_log_n_entries(void); @@ -46,6 +47,10 @@ void mock_dump_saved_logs(void); assert_log_predicate(mock_saved_log_has_message_containing(str), \ "expected log to contain " # str); +#define expect_log_msg_not_containing(str) \ + assert_log_predicate(mock_saved_log_has_message_not_containing(str), \ + "expected log to not contain " # str); + #define expect_log_msg_containing_either(str1, str2) \ assert_log_predicate(mock_saved_log_has_message_containing(str1) || \ mock_saved_log_has_message_containing(str2), \ -- cgit v1.2.3-54-g00ecf From e13ddee1665bc83442be04820b51b425fa98e848 Mon Sep 17 00:00:00 2001 From: juga0 Date: Sat, 9 Jun 2018 12:54:08 +0000 Subject: Allow mocking rep_hist_bandwidth_assess --- src/or/rephist.c | 6 +++--- src/or/rephist.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/or/rephist.c b/src/or/rephist.c index f0bac57898..2844c4d74e 100644 --- a/src/or/rephist.c +++ b/src/or/rephist.c @@ -1,5 +1,5 @@ /* Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2016, The Tor Project, Inc. */ + * Copyright (c) 2007-2018, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** @@ -1427,8 +1427,8 @@ find_largest_max(bw_array_t *b) * * Return the smaller of these sums, divided by NUM_SECS_ROLLING_MEASURE. */ -int -rep_hist_bandwidth_assess(void) +MOCK_IMPL(int, +rep_hist_bandwidth_assess,(void)) { uint64_t w,r; r = find_largest_max(read_array); diff --git a/src/or/rephist.h b/src/or/rephist.h index ff4810a56d..6d35ac67f6 100644 --- a/src/or/rephist.h +++ b/src/or/rephist.h @@ -1,7 +1,7 @@ /* Copyright (c) 2001 Matej Pfajfar. * Copyright (c) 2001-2004, Roger Dingledine. * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2016, The Tor Project, Inc. */ + * Copyright (c) 2007-2018, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** @@ -29,7 +29,7 @@ void rep_hist_make_router_pessimal(const char *id, time_t when); void rep_hist_note_dir_bytes_read(size_t num_bytes, time_t when); void rep_hist_note_dir_bytes_written(size_t num_bytes, time_t when); -int rep_hist_bandwidth_assess(void); +MOCK_DECL(int, rep_hist_bandwidth_assess, (void)); char *rep_hist_get_bandwidth_lines(void); void rep_hist_update_state(or_state_t *state); int rep_hist_load_state(or_state_t *state, char **err); -- cgit v1.2.3-54-g00ecf From 1066fdd8d1e084ef38281c9817032e68e2a490ff Mon Sep 17 00:00:00 2001 From: juga0 Date: Sat, 9 Jun 2018 10:34:41 +0000 Subject: Add test for check_descriptor_bandwidth_changed --- src/test/include.am | 1 + src/test/test_router.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 src/test/test_router.c diff --git a/src/test/include.am b/src/test/include.am index 0ee3d1169f..7864d7d9fd 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -117,6 +117,7 @@ src_test_test_SOURCES = \ src/test/test_relaycell.c \ src/test/test_rendcache.c \ src/test/test_replay.c \ + src/test/test_router.c \ src/test/test_routerkeys.c \ src/test/test_routerlist.c \ src/test/test_routerset.c \ diff --git a/src/test/test_router.c b/src/test/test_router.c new file mode 100644 index 0000000000..a4921da9b6 --- /dev/null +++ b/src/test/test_router.c @@ -0,0 +1,120 @@ +/* Copyright (c) 2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file test_router.c + * \brief Unittests for code in src/or/router.c + **/ + +#include "or.h" +#include "hibernate.h" +#include "log_test_helpers.h" +#include "main.h" +#include "rephist.h" +#include "router.h" +#include "test.h" + +static routerinfo_t *mock_router_get_my_routerinfo_result = NULL; + +static const routerinfo_t * +mock_router_get_my_routerinfo(void) +{ + return mock_router_get_my_routerinfo_result; +} + +static long +mock_get_uptime_3h(void) +{ + return 3*60*60; +} + +static long +mock_get_uptime_1d(void) +{ + return 24*60*60; +} + +static int +mock_rep_hist_bandwidth_assess(void) +{ + return 20001; +} + +static int +mock_we_are_hibernating(void) +{ + return 0; +} + +static void +test_router_check_descriptor_bandwidth_changed(void *arg) +{ + (void)arg; + routerinfo_t routerinfo; + memset(&routerinfo, 0, sizeof(routerinfo)); + mock_router_get_my_routerinfo_result = NULL; + + MOCK(we_are_hibernating, mock_we_are_hibernating); + MOCK(router_get_my_routerinfo, mock_router_get_my_routerinfo); + mock_router_get_my_routerinfo_result = &routerinfo; + + /* When uptime is less than 24h, no previous bandwidth, no last_changed + * Uptime: 10800, last_changed: 0, Previous bw: 0, Current bw: 0 */ + routerinfo.bandwidthcapacity = 0; + MOCK(get_uptime, mock_get_uptime_3h); + setup_full_capture_of_logs(LOG_INFO); + check_descriptor_bandwidth_changed(time(NULL)); + expect_log_msg_not_containing( + "Measured bandwidth has changed; rebuilding descriptor."); + teardown_capture_of_logs(); + + /* When uptime is less than 24h, previous bandwidth, + * last_changed more than 3h ago + * Uptime: 10800, last_changed: 0, Previous bw: 10000, Current bw: 0 */ + routerinfo.bandwidthcapacity = 10000; + setup_full_capture_of_logs(LOG_INFO); + check_descriptor_bandwidth_changed(time(NULL)); + expect_log_msg_containing( + "Measured bandwidth has changed; rebuilding descriptor."); + teardown_capture_of_logs(); + + /* When uptime is less than 24h, last_changed is not more than 3h ago + * Uptime: 10800, last_changed: x, Previous bw: 10000, Current bw: 0 */ + setup_full_capture_of_logs(LOG_INFO); + check_descriptor_bandwidth_changed(time(NULL)); + expect_log_msg_not_containing( + "Measured bandwidth has changed; rebuilding descriptor."); + teardown_capture_of_logs(); + + /* When uptime is less than 24h and bandwidthcapacity does not change + * Uptime: 10800, last_changed: x, Previous bw: 10000, Current bw: 20001 */ + MOCK(rep_hist_bandwidth_assess, mock_rep_hist_bandwidth_assess); + setup_full_capture_of_logs(LOG_INFO); + check_descriptor_bandwidth_changed(time(NULL) + 6*60*60 + 1); + expect_log_msg_containing( + "Measured bandwidth has changed; rebuilding descriptor."); + UNMOCK(get_uptime); + UNMOCK(rep_hist_bandwidth_assess); + teardown_capture_of_logs(); + + /* When uptime is more than 24h */ + MOCK(get_uptime, mock_get_uptime_1d); + setup_full_capture_of_logs(LOG_INFO); + check_descriptor_bandwidth_changed(time(NULL)); + expect_log_msg_not_containing( + "Measured bandwidth has changed; rebuilding descriptor."); + UNMOCK(get_uptime); + teardown_capture_of_logs(); + + done: + UNMOCK(router_get_my_routerinfo); +} + +#define ROUTER_TEST(name, flags) \ + { #name, test_router_ ## name, flags, NULL, NULL } + +struct testcase_t router_tests[] = { + ROUTER_TEST(check_descriptor_bandwidth_changed, TT_FORK), + END_OF_TESTCASES +}; + -- cgit v1.2.3-54-g00ecf From e033d98f79a950a342a7db6909709b5dc987b2cb Mon Sep 17 00:00:00 2001 From: juga0 Date: Mon, 2 Jul 2018 08:21:43 +0000 Subject: Check descriptor bandwidth changed if not hibernating There should be a separate check to update descriptor when start or end hibernating. --- src/or/router.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/or/router.c b/src/or/router.c index 4afba65491..f1a99364e6 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2440,17 +2440,18 @@ check_descriptor_bandwidth_changed(time_t now) { static time_t last_changed = 0; uint64_t prev, cur; + int hibernating = we_are_hibernating(); /* If the relay uptime is bigger than MAX_UPTIME_BANDWIDTH_CHANGE, * the next regularly scheduled descriptor update (18h) will be enough */ - if (get_uptime() > MAX_UPTIME_BANDWIDTH_CHANGE) + if (get_uptime() > MAX_UPTIME_BANDWIDTH_CHANGE && !hibernating) return; if (!router_get_my_routerinfo()) return; prev = router_get_my_routerinfo()->bandwidthcapacity; - cur = we_are_hibernating() ? 0 : rep_hist_bandwidth_assess(); + cur = hibernating ? 0 : rep_hist_bandwidth_assess(); if ((prev != cur && (!prev || !cur)) || cur > (prev * BANDWIDTH_CHANGE_FACTOR) || cur < (prev / BANDWIDTH_CHANGE_FACTOR) ) { -- cgit v1.2.3-54-g00ecf From d4e51a2eeb173fab25d3945d045499b80cb4565c Mon Sep 17 00:00:00 2001 From: juga0 Date: Mon, 2 Jul 2018 08:48:36 +0000 Subject: Add missing router_tests to test.h --- src/test/test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/test.c b/src/test/test.c index 0fef697909..4632ea4973 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1240,6 +1240,7 @@ struct testgroup_t testgroups[] = { { "relaycell/", relaycell_tests }, { "rend_cache/", rend_cache_tests }, { "replaycache/", replaycache_tests }, + { "router/", router_tests }, { "routerkeys/", routerkeys_tests }, { "routerlist/", routerlist_tests }, { "routerset/" , routerset_tests }, -- cgit v1.2.3-54-g00ecf From 81f4223329a709e5138532b037a58c118b30dd7f Mon Sep 17 00:00:00 2001 From: juga0 Date: Mon, 2 Jul 2018 09:02:32 +0000 Subject: Test for descriptor does not change when hibernating --- src/test/test_router.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/test/test_router.c b/src/test/test_router.c index a4921da9b6..51055a3367 100644 --- a/src/test/test_router.c +++ b/src/test/test_router.c @@ -40,6 +40,12 @@ mock_rep_hist_bandwidth_assess(void) return 20001; } +static int +mock_we_are_not_hibernating(void) +{ + return 0; +} + static int mock_we_are_hibernating(void) { @@ -54,7 +60,7 @@ test_router_check_descriptor_bandwidth_changed(void *arg) memset(&routerinfo, 0, sizeof(routerinfo)); mock_router_get_my_routerinfo_result = NULL; - MOCK(we_are_hibernating, mock_we_are_hibernating); + MOCK(we_are_hibernating, mock_we_are_not_hibernating); MOCK(router_get_my_routerinfo, mock_router_get_my_routerinfo); mock_router_get_my_routerinfo_result = &routerinfo; @@ -78,6 +84,21 @@ test_router_check_descriptor_bandwidth_changed(void *arg) "Measured bandwidth has changed; rebuilding descriptor."); teardown_capture_of_logs(); + /* When uptime is less than 24h, previous bandwidth, + * last_changed more than 3h ago, and hibernating + * Uptime: 10800, last_changed: 0, Previous bw: 10000, Current bw: 0 */ + + UNMOCK(we_are_hibernating); + MOCK(we_are_hibernating, mock_we_are_hibernating); + routerinfo.bandwidthcapacity = 10000; + setup_full_capture_of_logs(LOG_INFO); + check_descriptor_bandwidth_changed(time(NULL)); + expect_log_msg_not_containing( + "Measured bandwidth has changed; rebuilding descriptor."); + teardown_capture_of_logs(); + UNMOCK(we_are_hibernating); + MOCK(we_are_hibernating, mock_we_are_not_hibernating); + /* When uptime is less than 24h, last_changed is not more than 3h ago * Uptime: 10800, last_changed: x, Previous bw: 10000, Current bw: 0 */ setup_full_capture_of_logs(LOG_INFO); @@ -103,11 +124,12 @@ test_router_check_descriptor_bandwidth_changed(void *arg) check_descriptor_bandwidth_changed(time(NULL)); expect_log_msg_not_containing( "Measured bandwidth has changed; rebuilding descriptor."); - UNMOCK(get_uptime); teardown_capture_of_logs(); done: + UNMOCK(get_uptime); UNMOCK(router_get_my_routerinfo); + UNMOCK(we_are_hibernating); } #define ROUTER_TEST(name, flags) \ -- cgit v1.2.3-54-g00ecf