[tor-commits] [tor/master] touchups and refactorings on bug 18616 branch
nickm at torproject.org
nickm at torproject.org
Tue May 17 15:10:35 UTC 2016
commit 06031b441eb92f575f0a1c2ac264dc0daee4fbf4
Author: Roger Dingledine <arma at torproject.org>
Date: Mon May 16 17:43:47 2016 -0400
touchups and refactorings on bug 18616 branch
no behavior changes
---
changes/bug18616 | 20 +++++++++++++-------
src/or/circuitbuild.c | 2 +-
src/or/circuituse.c | 5 +++--
src/or/control.c | 15 +++++++++------
src/or/dirserv.c | 4 ++--
src/or/main.c | 4 ++--
src/or/rephist.c | 9 ++++++---
src/or/router.c | 48 +++++++++++++++++++++---------------------------
src/or/router.h | 4 ++--
9 files changed, 59 insertions(+), 52 deletions(-)
diff --git a/changes/bug18616 b/changes/bug18616
index d760f15..ec59e84 100644
--- a/changes/bug18616
+++ b/changes/bug18616
@@ -1,8 +1,14 @@
o Major bugfixes (directory mirrors):
- - Fix broken DirPort self-checks. Decide to advertise begindir
- support the same way we decide to advertise DirPorts.
- Add additional config options that might change the content of
- a relay's descriptor.
- Avoid checking ORPort reachability when the network is disabled.
- Resolves #18616, bugfix on 0c8e042c30946faa in #12538 in
- 0.2.8.1-alpha. Patch by "teor".
+ - Decide whether to advertise begindir support the same way we decide
+ whether to advertise our DirPort. These decisions being out of sync
+ led to surprising behavior like advertising begindir support when
+ our hibernation config options made us not advertise a DirPort.
+ Resolves bug 18616; bugfix on 0.2.8.1-alpha. Patch by teor.
+
+ o Minor bugfixes:
+ - Consider more config options when relays decide whether to regenerate
+ their descriptor. Fixes more of bug 12538; bugfix on 0.2.8.1-alpha.
+ - Resolve some edge cases where we might launch an ORPort reachability
+ check even when DisableNetwork is set. Noticed while fixing bug
+ 18616; bugfix on 0.2.3.9-alpha.
+
diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c
index a5a933e..820724a 100644
--- a/src/or/circuitbuild.c
+++ b/src/or/circuitbuild.c
@@ -984,7 +984,7 @@ circuit_send_next_onion_skin(origin_circuit_t *circ)
}
control_event_client_status(LOG_NOTICE, "CIRCUIT_ESTABLISHED");
clear_broken_connection_map(1);
- if (server_mode(options) && !check_whether_orport_reachable()) {
+ if (server_mode(options) && !check_whether_orport_reachable(options)) {
inform_testing_reachability();
consider_testing_reachability(1, 1);
}
diff --git a/src/or/circuituse.c b/src/or/circuituse.c
index 31003ea..f96a3b1 100644
--- a/src/or/circuituse.c
+++ b/src/or/circuituse.c
@@ -1426,7 +1426,7 @@ static void
circuit_testing_opened(origin_circuit_t *circ)
{
if (have_performed_bandwidth_test ||
- !check_whether_orport_reachable()) {
+ !check_whether_orport_reachable(get_options())) {
/* either we've already done everything we want with testing circuits,
* or this testing circuit became open due to a fluke, e.g. we picked
* a last hop where we already had the connection open due to an
@@ -1443,7 +1443,8 @@ circuit_testing_opened(origin_circuit_t *circ)
static void
circuit_testing_failed(origin_circuit_t *circ, int at_last_hop)
{
- if (server_mode(get_options()) && check_whether_orport_reachable())
+ const or_options_t *options = get_options();
+ if (server_mode(options) && check_whether_orport_reachable(options))
return;
log_info(LD_GENERAL,
diff --git a/src/or/control.c b/src/or/control.c
index 655b4dd..a2ae2e9 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -2143,6 +2143,7 @@ getinfo_helper_events(control_connection_t *control_conn,
const char *question, char **answer,
const char **errmsg)
{
+ const or_options_t *options = get_options();
(void) control_conn;
if (!strcmp(question, "circuit-status")) {
smartlist_t *status = smartlist_new();
@@ -2279,17 +2280,19 @@ getinfo_helper_events(control_connection_t *control_conn,
*answer = tor_strdup(directories_have_accepted_server_descriptor()
? "1" : "0");
} else if (!strcmp(question, "status/reachability-succeeded/or")) {
- *answer = tor_strdup(check_whether_orport_reachable() ? "1" : "0");
+ *answer = tor_strdup(check_whether_orport_reachable(options) ?
+ "1" : "0");
} else if (!strcmp(question, "status/reachability-succeeded/dir")) {
- *answer = tor_strdup(check_whether_dirport_reachable() ? "1" : "0");
+ *answer = tor_strdup(check_whether_dirport_reachable(options) ?
+ "1" : "0");
} else if (!strcmp(question, "status/reachability-succeeded")) {
tor_asprintf(answer, "OR=%d DIR=%d",
- check_whether_orport_reachable() ? 1 : 0,
- check_whether_dirport_reachable() ? 1 : 0);
+ check_whether_orport_reachable(options) ? 1 : 0,
+ check_whether_dirport_reachable(options) ? 1 : 0);
} else if (!strcmp(question, "status/bootstrap-phase")) {
*answer = tor_strdup(last_sent_bootstrap_message);
} else if (!strcmpstart(question, "status/version/")) {
- int is_server = server_mode(get_options());
+ int is_server = server_mode(options);
networkstatus_t *c = networkstatus_get_latest_consensus();
version_status_t status;
const char *recommended;
@@ -2331,7 +2334,7 @@ getinfo_helper_events(control_connection_t *control_conn,
}
*answer = bridge_stats;
} else if (!strcmp(question, "status/fresh-relay-descs")) {
- if (!server_mode(get_options())) {
+ if (!server_mode(options)) {
*errmsg = "Only relays have descriptors";
return -1;
}
diff --git a/src/or/dirserv.c b/src/or/dirserv.c
index 5f848bd..846951c 100644
--- a/src/or/dirserv.c
+++ b/src/or/dirserv.c
@@ -1133,9 +1133,9 @@ directory_caches_unknown_auth_certs(const or_options_t *options)
/** Return 1 if we want to keep descriptors, networkstatuses, etc around.
* Else return 0.
- * Check get_options()->DirPort_set and directory_permits_begindir_requests()
+ * Check options->DirPort_set and directory_permits_begindir_requests()
* to see if we are willing to serve these directory documents to others via
- * the DirPort and begindir over ORPort, respectively.
+ * the DirPort and begindir-over-ORPort, respectively.
*/
int
directory_caches_dir_info(const or_options_t *options)
diff --git a/src/or/main.c b/src/or/main.c
index a2cf5b1..86fb2bd 100644
--- a/src/or/main.c
+++ b/src/or/main.c
@@ -2094,7 +2094,7 @@ second_elapsed_callback(periodic_timer_t *timer, void *arg)
TIMEOUT_UNTIL_UNREACHABILITY_COMPLAINT) {
/* every 20 minutes, check and complain if necessary */
const routerinfo_t *me = router_get_my_routerinfo();
- if (me && !check_whether_orport_reachable()) {
+ if (me && !check_whether_orport_reachable(options)) {
char *address = tor_dup_ip(me->addr);
log_warn(LD_CONFIG,"Your server (%s:%d) has not managed to confirm that "
"its ORPort is reachable. Relays do not publish descriptors "
@@ -2107,7 +2107,7 @@ second_elapsed_callback(periodic_timer_t *timer, void *arg)
tor_free(address);
}
- if (me && !check_whether_dirport_reachable()) {
+ if (me && !check_whether_dirport_reachable(options)) {
char *address = tor_dup_ip(me->addr);
log_warn(LD_CONFIG,
"Your server (%s:%d) has not managed to confirm that its "
diff --git a/src/or/rephist.c b/src/or/rephist.c
index fe0ca91..04ed7ae 100644
--- a/src/or/rephist.c
+++ b/src/or/rephist.c
@@ -1867,14 +1867,17 @@ any_predicted_circuits(time_t now)
int
rep_hist_circbuilding_dormant(time_t now)
{
+ const or_options_t *options = get_options();
+
if (any_predicted_circuits(now))
return 0;
/* see if we'll still need to build testing circuits */
- if (server_mode(get_options()) &&
- (!check_whether_orport_reachable() || !circuit_enough_testing_circs()))
+ if (server_mode(options) &&
+ (!check_whether_orport_reachable(options) ||
+ !circuit_enough_testing_circs()))
return 0;
- if (!check_whether_dirport_reachable())
+ if (!check_whether_dirport_reachable(options))
return 0;
return 1;
diff --git a/src/or/router.c b/src/or/router.c
index 8f369f8..57c0618 100644
--- a/src/or/router.c
+++ b/src/or/router.c
@@ -1079,7 +1079,7 @@ router_reset_reachability(void)
can_reach_or_port = can_reach_dir_port = 0;
}
-/** Return 1 if we won't do reachability checks, because:
+/** Return 1 if we won't do reachability checks, because:
* - AssumeReachable is set, or
* - the network is disabled.
* Otherwise, return 0.
@@ -1100,9 +1100,8 @@ router_reachability_checks_disabled(const or_options_t *options)
* - the network is disabled.
*/
int
-check_whether_orport_reachable(void)
+check_whether_orport_reachable(const or_options_t *options)
{
- const or_options_t *options = get_options();
int reach_checks_disabled = router_reachability_checks_disabled(options);
return reach_checks_disabled ||
can_reach_or_port;
@@ -1118,9 +1117,8 @@ check_whether_orport_reachable(void)
* - the network is disabled.
*/
int
-check_whether_dirport_reachable(void)
+check_whether_dirport_reachable(const or_options_t *options)
{
- const or_options_t *options = get_options();
int reach_checks_disabled = router_reachability_checks_disabled(options) ||
!options->DirPort_set;
return reach_checks_disabled ||
@@ -1258,18 +1256,14 @@ decide_to_advertise_dir_impl(const or_options_t *options,
!router_get_advertised_or_port(options))
return 0;
- /* Part two: reasons to publish or not publish that the user
- * might find surprising. router_should_be_directory_server()
- * considers config options that make us choose not to publish. */
+ /* Part two: consider config options that could make us choose to
+ * publish or not publish that the user might find surprising. */
return router_should_be_directory_server(options, dir_port);
}
-/** Look at a variety of factors, and return 0 if we don't want to
+/** Front-end to decide_to_advertise_dir_impl(): return 0 if we don't want to
* advertise the fact that we have a DirPort open, else return the
* DirPort we want to advertise.
- *
- * Log a helpful message if we change our mind about whether to publish
- * a DirPort.
*/
static int
decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port)
@@ -1278,11 +1272,8 @@ decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port)
return decide_to_advertise_dir_impl(options, dir_port, 0) ? dir_port : 0;
}
-/** Look at a variety of factors, and return 0 if we don't want to
+/** Front-end to decide_to_advertise_dir_impl(): return 0 if we don't want to
* advertise the fact that we support begindir requests, else return 1.
- *
- * Log a helpful message if we change our mind about whether to advertise
- * support for begindir.
*/
static int
decide_to_advertise_begindir(const or_options_t *options,
@@ -1324,9 +1315,9 @@ void
consider_testing_reachability(int test_or, int test_dir)
{
const routerinfo_t *me = router_get_my_routerinfo();
- int orport_reachable = check_whether_orport_reachable();
- tor_addr_t addr;
const or_options_t *options = get_options();
+ int orport_reachable = check_whether_orport_reachable(options);
+ tor_addr_t addr;
if (!me)
return;
@@ -1359,7 +1350,7 @@ consider_testing_reachability(int test_or, int test_dir)
/* XXX IPv6 self testing */
tor_addr_from_ipv4h(&addr, me->addr);
- if (test_dir && !check_whether_dirport_reachable() &&
+ if (test_dir && !check_whether_dirport_reachable(options) &&
!connection_get_by_type_addr_port_purpose(
CONN_TYPE_DIR, &addr, me->dir_port,
DIR_PURPOSE_FETCH_SERVERDESC)) {
@@ -1378,18 +1369,19 @@ void
router_orport_found_reachable(void)
{
const routerinfo_t *me = router_get_my_routerinfo();
+ const or_options_t *options = get_options();
if (!can_reach_or_port && me) {
char *address = tor_dup_ip(me->addr);
log_notice(LD_OR,"Self-testing indicates your ORPort is reachable from "
"the outside. Excellent.%s",
- get_options()->PublishServerDescriptor_ != NO_DIRINFO
- && check_whether_dirport_reachable() ?
+ options->PublishServerDescriptor_ != NO_DIRINFO
+ && check_whether_dirport_reachable(options) ?
" Publishing server descriptor." : "");
can_reach_or_port = 1;
mark_my_descriptor_dirty("ORPort found reachable");
/* This is a significant enough change to upload immediately,
* at least in a test network */
- if (get_options()->TestingTorNetwork == 1) {
+ if (options->TestingTorNetwork == 1) {
reschedule_descriptor_update_check();
}
control_event_server_status(LOG_NOTICE,
@@ -1404,19 +1396,20 @@ void
router_dirport_found_reachable(void)
{
const routerinfo_t *me = router_get_my_routerinfo();
+ const or_options_t *options = get_options();
if (!can_reach_dir_port && me) {
char *address = tor_dup_ip(me->addr);
log_notice(LD_DIRSERV,"Self-testing indicates your DirPort is reachable "
"from the outside. Excellent.%s",
- get_options()->PublishServerDescriptor_ != NO_DIRINFO
- && check_whether_orport_reachable() ?
+ options->PublishServerDescriptor_ != NO_DIRINFO
+ && check_whether_orport_reachable(options) ?
" Publishing server descriptor." : "");
can_reach_dir_port = 1;
- if (decide_to_advertise_dirport(get_options(), me->dir_port)) {
+ if (decide_to_advertise_dirport(options, me->dir_port)) {
mark_my_descriptor_dirty("DirPort found reachable");
/* This is a significant enough change to upload immediately,
* at least in a test network */
- if (get_options()->TestingTorNetwork == 1) {
+ if (options->TestingTorNetwork == 1) {
reschedule_descriptor_update_check();
}
}
@@ -1633,7 +1626,8 @@ decide_if_publishable_server(void)
if (!router_get_advertised_or_port(options))
return 0;
- return check_whether_orport_reachable() && check_whether_dirport_reachable();
+ return check_whether_orport_reachable(options) &&
+ check_whether_dirport_reachable(options);
}
/** Initiate server descriptor upload as reasonable (if server is publishable,
diff --git a/src/or/router.h b/src/or/router.h
index 5165462..73bfea1 100644
--- a/src/or/router.h
+++ b/src/or/router.h
@@ -39,8 +39,8 @@ int router_initialize_tls_context(void);
int init_keys(void);
int init_keys_client(void);
-int check_whether_orport_reachable(void);
-int check_whether_dirport_reachable(void);
+int check_whether_orport_reachable(const or_options_t *options);
+int check_whether_dirport_reachable(const or_options_t *options);
int dir_server_mode(const or_options_t *options);
void consider_testing_reachability(int test_or, int test_dir);
void router_orport_found_reachable(void);
More information about the tor-commits
mailing list