From d60e7df2da5bfae62750bc4b41581d47e504e4bf Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 15 Sep 2017 13:50:29 +1000 Subject: [PATCH 1/3] Explain where bridge download statuses are initialised And why we can't initialise them on config Comment-only change, follow-up to 23347. --- src/or/bridges.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/or/bridges.c b/src/or/bridges.c index 0d4549dd16..812b0ddce8 100644 --- a/src/or/bridges.c +++ b/src/or/bridges.c @@ -455,8 +455,8 @@ bridge_add_from_config(bridge_line_t *bridge_line) b->fetch_status.schedule = DL_SCHED_BRIDGE; b->fetch_status.backoff = DL_SCHED_RANDOM_EXPONENTIAL; b->fetch_status.increment_on = DL_SCHED_INCREMENT_ATTEMPT; - /* This will fail if UseBridges is not set -- and it does. */ - // download_status_reset(&b->fetch_status); + /* We can't reset the bridge's download status here, because UseBridges + * might be 0 now, and it might be changed to 1 much later. */ b->socks_args = bridge_line->socks_args; if (!bridge_list) bridge_list = smartlist_new(); @@ -625,6 +625,7 @@ fetch_bridge_descriptors(const or_options_t *options, time_t now) SMARTLIST_FOREACH_BEGIN(bridge_list, bridge_info_t *, bridge) { + /* This resets the download status on first use */ if (!download_status_is_ready(&bridge->fetch_status, now, IMPOSSIBLE_TO_DOWNLOAD)) continue; /* don't bother, no need to retry yet */ From 033691212a49d3d32f8a684a493c8c11b8887eee Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 15 Sep 2017 14:02:22 +1000 Subject: [PATCH 2/3] Make an assert into a BUG warning in the bridge code If future code asks if there are any running bridges, without checking if bridges are enabled, log a BUG warning rather than crashing. Fixes 23524 on 0.3.0.1-alpha --- changes/bug23524 | 4 ++++ src/or/bridges.c | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 changes/bug23524 diff --git a/changes/bug23524 b/changes/bug23524 new file mode 100644 index 0000000000..c8ece52930 --- /dev/null +++ b/changes/bug23524 @@ -0,0 +1,4 @@ + o Minor bugfixes (DoS-resistance): + - If future code asks if there are any running bridges, without checking + if bridges are enabled, log a BUG warning rather than crashing. + Fixes 23524 on 0.3.0.1-alpha. diff --git a/src/or/bridges.c b/src/or/bridges.c index 812b0ddce8..1eec4e39ec 100644 --- a/src/or/bridges.c +++ b/src/or/bridges.c @@ -836,7 +836,9 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache) MOCK_IMPL(int, any_bridge_descriptors_known, (void)) { - tor_assert(get_options()->UseBridges); + if (BUG(!get_options()->UseBridges)) { + return 0; + } if (!bridge_list) return 0; From 6e87c0b23e622e95d8348650f7bdf2af75ab824e Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 15 Sep 2017 14:17:41 +1000 Subject: [PATCH 3/3] Avoid an instance of the bug warning in any_bridge_descriptors_known() Part of 23524. --- src/or/directory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/or/directory.c b/src/or/directory.c index 01d9fc617c..630524db67 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -5379,7 +5379,7 @@ find_dl_schedule(const download_status_t *dls, const or_options_t *options) } case DL_SCHED_BRIDGE: /* A bridge client downloading bridge descriptors */ - if (any_bridge_descriptors_known()) { + if (options->UseBridges && any_bridge_descriptors_known()) { /* A bridge client with one or more running bridges */ return options->TestingBridgeDownloadSchedule; } else {