summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--changes/bug233479
-rw-r--r--doc/tor.1.txt21
-rw-r--r--src/or/bridges.c22
-rw-r--r--src/or/bridges.h2
-rw-r--r--src/or/config.c16
-rw-r--r--src/or/directory.c13
-rw-r--r--src/or/or.h4
-rw-r--r--src/test/test_dir.c48
-rw-r--r--src/test/test_entrynodes.c2
-rw-r--r--src/test/test_options.c1
10 files changed, 112 insertions, 26 deletions
diff --git a/changes/bug23347 b/changes/bug23347
new file mode 100644
index 0000000000..e73aa48f01
--- /dev/null
+++ b/changes/bug23347
@@ -0,0 +1,9 @@
+ o Minor fixes (bridge client bootstrap):
+ - Make bridge clients with no running bridges try to download
+ bridge descriptors immediately. But when bridge clients have
+ running bridges, make them wait at least 3 hours before
+ refreshing recently received bridge descriptors.
+ Download schedules used to start with an implicit 0, but the
+ fix for 17750 changed this undocumented behaviour, and made
+ bridge clients hang for 15 minutes before bootstrapping.
+ Fixes bug 23347, not in any released version of Tor.
diff --git a/doc/tor.1.txt b/doc/tor.1.txt
index aec1767059..c375e032d3 100644
--- a/doc/tor.1.txt
+++ b/doc/tor.1.txt
@@ -1582,8 +1582,8 @@ The following options are useful only for clients (that is, if
live consensus). Only used by clients fetching from a list of fallback
directory mirrors. This schedule is advanced by (potentially concurrent)
connection attempts, unlike other schedules, which are advanced by
- connection failures. (Default: 10, 11, 3600, 10800, 25200, 54000,
- 111600, 262800)
+ connection failures. (Default: 6, 11, 3600, 10800, 25200, 54000, 111600,
+ 262800)
[[ClientBootstrapConsensusFallbackDownloadSchedule]] **ClientBootstrapConsensusFallbackDownloadSchedule** __N__,__N__,__...__::
Schedule for when clients should download consensuses from fallback
@@ -1613,7 +1613,7 @@ The following options are useful only for clients (that is, if
[[ClientBootstrapConsensusMaxInProgressTries]] **ClientBootstrapConsensusMaxInProgressTries** __NUM__::
Try this many simultaneous connections to download a consensus before
- waiting for one to complete, timeout, or error out. (Default: 4)
+ waiting for one to complete, timeout, or error out. (Default: 3)
SERVER OPTIONS
--------------
@@ -2486,7 +2486,8 @@ The following options are used for running a testing Tor network.
TestingClientDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60
TestingServerConsensusDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60
TestingClientConsensusDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60
- TestingBridgeDownloadSchedule 60, 30, 30, 60
+ TestingBridgeDownloadSchedule 10, 30, 60
+ TestingBridgeBootstrapDownloadSchedule 0, 0, 5, 10, 15, 20, 30, 60
TestingClientMaxIntervalWithoutRequest 5 seconds
TestingDirConnectionMaxStall 30 seconds
TestingConsensusMaxDownloadTries 80
@@ -2551,8 +2552,16 @@ The following options are used for running a testing Tor network.
1800, 3600, 3600, 3600, 10800, 21600, 43200)
[[TestingBridgeDownloadSchedule]] **TestingBridgeDownloadSchedule** __N__,__N__,__...__::
- Schedule for when clients should download bridge descriptors. Changing this
- requires that **TestingTorNetwork** is set. (Default: 3600, 900, 900, 3600)
+ Schedule for when clients should download each bridge descriptor when they
+ know that one or more of their configured bridges are running. Changing
+ this requires that **TestingTorNetwork** is set. (Default: 10800, 25200,
+ 54000, 111600, 262800)
+
+[[TestingBridgeBootstrapDownloadSchedule]] **TestingBridgeBootstrapDownloadSchedule** __N__,__N__,__...__::
+ Schedule for when clients should download each bridge descriptor when they
+ have just started, or when they can not contact any of their bridges.
+ Changing this requires that **TestingTorNetwork** is set. (Default: 0, 30,
+ 90, 600, 3600, 10800, 25200, 54000, 111600, 262800)
[[TestingClientMaxIntervalWithoutRequest]] **TestingClientMaxIntervalWithoutRequest** __N__ **seconds**|**minutes**::
When directory clients have only a few descriptors to request, they batch
diff --git a/src/or/bridges.c b/src/or/bridges.c
index 0818fb0812..461f86260f 100644
--- a/src/or/bridges.c
+++ b/src/or/bridges.c
@@ -454,6 +454,9 @@ bridge_add_from_config(bridge_line_t *bridge_line)
b->transport_name = bridge_line->transport_name;
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 */
+ download_status_reset(&b->fetch_status);
b->socks_args = bridge_line->socks_args;
if (!bridge_list)
bridge_list = smartlist_new();
@@ -632,8 +635,13 @@ fetch_bridge_descriptors(const or_options_t *options, time_t now)
continue;
}
- /* schedule another fetch as if this one will fail, in case it does */
- download_status_failed(&bridge->fetch_status, 0);
+ /* schedule the next attempt
+ * we can't increment after a failure, because sometimes we use the
+ * bridge authority, and sometimes we use the bridge direct */
+ download_status_increment_attempt(
+ &bridge->fetch_status,
+ safe_str_client(fmt_and_decorate_addr(&bridge->addr)),
+ now);
can_use_bridge_authority = !tor_digest_is_zero(bridge->identity) &&
num_bridge_auths;
@@ -787,8 +795,12 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache)
if (bridge) { /* if we actually want to use this one */
node_t *node;
/* it's here; schedule its re-fetch for a long time from now. */
- if (!from_cache)
+ if (!from_cache) {
+ /* This schedules the re-fetch at a constant interval, which produces
+ * a pattern of bridge traffic. But it's better than trying all
+ * configured briges several times in the first few minutes. */
download_status_reset(&bridge->fetch_status);
+ }
node = node_get_mutable_by_id(ri->cache_info.identity_digest);
tor_assert(node);
@@ -820,8 +832,8 @@ learned_bridge_descriptor(routerinfo_t *ri, int from_cache)
* We use this function to decide if we're ready to start building
* circuits through our bridges, or if we need to wait until the
* directory "server/authority" requests finish. */
-int
-any_bridge_descriptors_known(void)
+MOCK_IMPL(int,
+any_bridge_descriptors_known, (void))
{
tor_assert(get_options()->UseBridges);
diff --git a/src/or/bridges.h b/src/or/bridges.h
index 3bfc782f9a..19341818f4 100644
--- a/src/or/bridges.h
+++ b/src/or/bridges.h
@@ -45,7 +45,7 @@ void bridge_add_from_config(struct bridge_line_t *bridge_line);
void retry_bridge_descriptor_fetch_directly(const char *digest);
void fetch_bridge_descriptors(const or_options_t *options, time_t now);
void learned_bridge_descriptor(routerinfo_t *ri, int from_cache);
-int any_bridge_descriptors_known(void);
+MOCK_DECL(int, any_bridge_descriptors_known, (void));
const smartlist_t *get_socks_args_by_bridge_addrport(const tor_addr_t *addr,
uint16_t port);
diff --git a/src/or/config.c b/src/or/config.c
index 9d6cc9a26b..9a7251c41f 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -587,7 +587,16 @@ static config_var_t option_vars_[] = {
* blackholed. Clients will try 3 directories simultaneously.
* (Relays never use simultaneous connections.) */
V(ClientBootstrapConsensusMaxInProgressTries, UINT, "3"),
- V(TestingBridgeDownloadSchedule, CSV_INTERVAL, "1200, 900, 900, 3600"),
+ /* When a client has any running bridges, check each bridge occasionally,
+ * whether or not that bridge is actually up. */
+ V(TestingBridgeDownloadSchedule, CSV_INTERVAL,
+ "10800, 25200, 54000, 111600, 262800"),
+ /* When a client is just starting, or has no running bridges, check each
+ * bridge a few times quickly, and then try again later. These schedules
+ * are much longer than the other schedules, because we try each and every
+ * configured bridge with this schedule. */
+ V(TestingBridgeBootstrapDownloadSchedule, CSV_INTERVAL,
+ "0, 30, 90, 600, 3600, 10800, 25200, 54000, 111600, 262800"),
V(TestingClientMaxIntervalWithoutRequest, INTERVAL, "10 minutes"),
V(TestingDirConnectionMaxStall, INTERVAL, "5 minutes"),
V(TestingConsensusMaxDownloadTries, UINT, "8"),
@@ -649,7 +658,9 @@ static const config_var_t testing_tor_network_defaults[] = {
"15, 20, 30, 60"),
V(TestingClientConsensusDownloadSchedule, CSV_INTERVAL, "0, 0, 5, 10, "
"15, 20, 30, 60"),
- V(TestingBridgeDownloadSchedule, CSV_INTERVAL, "60, 30, 30, 60"),
+ V(TestingBridgeDownloadSchedule, CSV_INTERVAL, "10, 30, 60"),
+ V(TestingBridgeBootstrapDownloadSchedule, CSV_INTERVAL, "0, 0, 5, 10, "
+ "15, 20, 30, 60"),
V(TestingClientMaxIntervalWithoutRequest, INTERVAL, "5 seconds"),
V(TestingDirConnectionMaxStall, INTERVAL, "30 seconds"),
V(TestingConsensusMaxDownloadTries, UINT, "80"),
@@ -4098,6 +4109,7 @@ options_validate(or_options_t *old_options, or_options_t *options,
CHECK_DEFAULT(TestingServerConsensusDownloadSchedule);
CHECK_DEFAULT(TestingClientConsensusDownloadSchedule);
CHECK_DEFAULT(TestingBridgeDownloadSchedule);
+ CHECK_DEFAULT(TestingBridgeBootstrapDownloadSchedule);
CHECK_DEFAULT(TestingClientMaxIntervalWithoutRequest);
CHECK_DEFAULT(TestingDirConnectionMaxStall);
CHECK_DEFAULT(TestingConsensusMaxDownloadTries);
diff --git a/src/or/directory.c b/src/or/directory.c
index 007235d108..fed4277dec 100644
--- a/src/or/directory.c
+++ b/src/or/directory.c
@@ -5362,7 +5362,14 @@ find_dl_schedule(const download_status_t *dls, const or_options_t *options)
}
}
case DL_SCHED_BRIDGE:
- return options->TestingBridgeDownloadSchedule;
+ /* A bridge client downloading bridge descriptors */
+ if (any_bridge_descriptors_known()) {
+ /* A bridge client with one or more running bridges */
+ return options->TestingBridgeDownloadSchedule;
+ } else {
+ /* A bridge client with no running bridges */
+ return options->TestingBridgeBootstrapDownloadSchedule;
+ }
default:
tor_assert(0);
}
@@ -5683,8 +5690,8 @@ download_status_get_initial_delay_from_now(const download_status_t *dls)
* (We find the zeroth element of the download schedule, and set
* next_attempt_at to be the appropriate offset from 'now'. In most
* cases this means setting it to 'now', so the item will be immediately
- * downloadable; in the case of bridge descriptors, the zeroth element
- * is an hour from now.) */
+ * downloadable; when using authorities with fallbacks, there is a few seconds'
+ * delay.) */
void
download_status_reset(download_status_t *dls)
{
diff --git a/src/or/or.h b/src/or/or.h
index 161d80ed96..de03a99035 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -4340,6 +4340,10 @@ typedef struct {
* altered on testing networks. */
smartlist_t *TestingBridgeDownloadSchedule;
+ /** Schedule for when clients should download bridge descriptors when they
+ * have no running bridges. Only altered on testing networks. */
+ smartlist_t *TestingBridgeBootstrapDownloadSchedule;
+
/** When directory clients have only a few descriptors to request, they
* batch them until they have more, or until this amount of time has
* passed. Only altered on testing networks. */
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index b6b0936f83..dd910cd0c5 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -18,6 +18,7 @@
#define RELAY_PRIVATE
#include "or.h"
+#include "bridges.h"
#include "confparse.h"
#include "config.h"
#include "control.h"
@@ -4245,7 +4246,7 @@ test_dir_download_status_increment(void *arg)
/* Put it in the options */
mock_options = &test_options;
reset_options(mock_options, &mock_get_options_calls);
- mock_options->TestingBridgeDownloadSchedule = schedule;
+ mock_options->TestingBridgeBootstrapDownloadSchedule = schedule;
mock_options->TestingClientDownloadSchedule = schedule;
MOCK(get_options, mock_get_options);
@@ -4425,6 +4426,7 @@ test_dir_download_status_increment(void *arg)
/* Check that failure increments do happen on attempt-based schedules,
* but that the retry is set at the end of time */
+ mock_options->UseBridges = 1;
mock_get_options_calls = 0;
next_at = download_status_increment_failure(&dls_attempt, 404, "test", 0,
current_time);
@@ -4539,6 +4541,7 @@ test_dir_download_status_increment(void *arg)
tt_int_op(download_status_get_n_failures(&dls_attempt), OP_EQ, 0);
tt_int_op(download_status_get_n_attempts(&dls_attempt), OP_EQ, 0);
tt_int_op(mock_get_options_calls, OP_GE, 1);
+ mock_options->UseBridges = 0;
/* Check that attempt increments don't happen on failure-based schedules,
* and that the attempt is set at the end of time */
@@ -5859,9 +5862,17 @@ mock_networkstatus_consensus_can_use_extra_fallbacks(
return mock_networkstatus_consensus_can_use_extra_fallbacks_value;
}
-/* data is a 2 character nul-terminated string.
+static int mock_any_bridge_descriptors_known_value = 0;
+static int
+mock_any_bridge_descriptors_known(void)
+{
+ return mock_any_bridge_descriptors_known_value;
+}
+
+/* data is a 3 character nul-terminated string.
* If data[0] is 'b', set bootstrapping, anything else means not bootstrapping
* If data[1] is 'f', set extra fallbacks, anything else means no extra
+ * If data[2] is 'f', set running bridges, anything else means no extra
* fallbacks.
*/
static void
@@ -5869,7 +5880,7 @@ test_dir_find_dl_schedule(void* data)
{
const char *str = (const char *)data;
- tt_assert(strlen(data) == 2);
+ tt_assert(strlen(data) == 3);
if (str[0] == 'b') {
mock_networkstatus_consensus_is_bootstrapping_value = 1;
@@ -5883,15 +5894,23 @@ test_dir_find_dl_schedule(void* data)
mock_networkstatus_consensus_can_use_extra_fallbacks_value = 0;
}
+ if (str[2] == 'r') {
+ mock_any_bridge_descriptors_known_value = 1;
+ } else {
+ mock_any_bridge_descriptors_known_value = 0;
+ }
+
MOCK(networkstatus_consensus_is_bootstrapping,
mock_networkstatus_consensus_is_bootstrapping);
MOCK(networkstatus_consensus_can_use_extra_fallbacks,
mock_networkstatus_consensus_can_use_extra_fallbacks);
+ MOCK(any_bridge_descriptors_known,
+ mock_any_bridge_descriptors_known);
download_status_t dls;
smartlist_t server, client, server_cons, client_cons;
smartlist_t client_boot_auth_only_cons, client_boot_auth_cons;
- smartlist_t client_boot_fallback_cons, bridge;
+ smartlist_t client_boot_fallback_cons, bridge, bridge_bootstrap;
mock_options = tor_malloc(sizeof(or_options_t));
reset_options(mock_options, &mock_get_options_calls);
@@ -5908,6 +5927,7 @@ test_dir_find_dl_schedule(void* data)
mock_options->ClientBootstrapConsensusFallbackDownloadSchedule =
&client_boot_fallback_cons;
mock_options->TestingBridgeDownloadSchedule = &bridge;
+ mock_options->TestingBridgeBootstrapDownloadSchedule = &bridge_bootstrap;
dls.schedule = DL_SCHED_GENERIC;
/* client */
@@ -5996,11 +6016,17 @@ test_dir_find_dl_schedule(void* data)
dls.schedule = DL_SCHED_BRIDGE;
/* client */
mock_options->ClientOnly = 1;
- tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge);
+ mock_options->UseBridges = 1;
+ if (any_bridge_descriptors_known()) {
+ tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge);
+ } else {
+ tt_ptr_op(find_dl_schedule(&dls, mock_options), OP_EQ, &bridge_bootstrap);
+ }
done:
UNMOCK(networkstatus_consensus_is_bootstrapping);
UNMOCK(networkstatus_consensus_can_use_extra_fallbacks);
+ UNMOCK(any_bridge_descriptors_known);
UNMOCK(get_options);
tor_free(mock_options);
mock_options = NULL;
@@ -6165,10 +6191,14 @@ struct testcase_t dir_tests[] = {
DIR(dump_unparseable_descriptors, 0),
DIR(populate_dump_desc_fifo, 0),
DIR(populate_dump_desc_fifo_2, 0),
- DIR_ARG(find_dl_schedule, TT_FORK, "bf"),
- DIR_ARG(find_dl_schedule, TT_FORK, "ba"),
- DIR_ARG(find_dl_schedule, TT_FORK, "cf"),
- DIR_ARG(find_dl_schedule, TT_FORK, "ca"),
+ DIR_ARG(find_dl_schedule, TT_FORK, "bfd"),
+ DIR_ARG(find_dl_schedule, TT_FORK, "bad"),
+ DIR_ARG(find_dl_schedule, TT_FORK, "cfd"),
+ DIR_ARG(find_dl_schedule, TT_FORK, "cad"),
+ DIR_ARG(find_dl_schedule, TT_FORK, "bfr"),
+ DIR_ARG(find_dl_schedule, TT_FORK, "bar"),
+ DIR_ARG(find_dl_schedule, TT_FORK, "cfr"),
+ DIR_ARG(find_dl_schedule, TT_FORK, "car"),
DIR(assumed_flags, 0),
DIR(networkstatus_compute_bw_weights_v10, 0),
END_OF_TESTCASES
diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c
index f30bb5ffa1..d87670d82b 100644
--- a/src/test/test_entrynodes.c
+++ b/src/test/test_entrynodes.c
@@ -917,6 +917,7 @@ test_entry_guard_node_filter(void *arg)
routerset_parse(get_options_mutable()->ExcludeNodes, "144.144.0.0/16", "");
/* 4: Bridge. */
+ get_options_mutable()->UseBridges = 1;
sweep_bridge_list();
bl = tor_malloc_zero(sizeof(bridge_line_t));
tor_addr_from_ipv4h(&bl->addr, n[4]->rs->addr);
@@ -924,6 +925,7 @@ test_entry_guard_node_filter(void *arg)
memcpy(bl->digest, n[4]->identity, 20);
bridge_add_from_config(bl);
bl = NULL; // prevent free.
+ get_options_mutable()->UseBridges = 0;
/* 5: Unreachable. This stays in the filter, but isn't in usable-filtered */
g[5]->last_tried_to_connect = approx_time(); // prevent retry.
diff --git a/src/test/test_options.c b/src/test/test_options.c
index 92e6982823..65b678cf5b 100644
--- a/src/test/test_options.c
+++ b/src/test/test_options.c
@@ -2232,6 +2232,7 @@ test_options_validate__testing(void *ignored)
ENSURE_DEFAULT(TestingServerConsensusDownloadSchedule, 3000);
ENSURE_DEFAULT(TestingClientConsensusDownloadSchedule, 3000);
ENSURE_DEFAULT(TestingBridgeDownloadSchedule, 3000);
+ ENSURE_DEFAULT(TestingBridgeBootstrapDownloadSchedule, 3000);
ENSURE_DEFAULT(TestingClientMaxIntervalWithoutRequest, 3000);
ENSURE_DEFAULT(TestingDirConnectionMaxStall, 3000);
ENSURE_DEFAULT(TestingConsensusMaxDownloadTries, 3000);