commit 692828bea558295b2480eab473f9131e85d1eb9e Author: teor (Tim Wilson-Brown) teor2345@gmail.com Date: Wed Apr 20 17:30:55 2016 +1000
Decide to advertise begindir support like we decide to advertise DirPort
Decide to advertise begindir support in a similar way to how we decide to advertise DirPort.
Fix up the associated descriptor-building unit tests.
Resolves #18616, bugfix on 0c8e042c30946faa in #12538 in 0.2.8.1-alpha. --- src/or/config.c | 5 ++--- src/or/dirserv.c | 7 +++++-- src/or/router.c | 48 ++++++++++++++++++++++++++++++++++++++++++------ src/test/test_dir.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 11 deletions(-)
diff --git a/src/or/config.c b/src/or/config.c index 5d938d1..f07ca64 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -7005,9 +7005,8 @@ get_first_listener_addrport_string(int listener_type) int get_first_advertised_port_by_type_af(int listener_type, int address_family) { - if (!configured_ports) - return 0; - SMARTLIST_FOREACH_BEGIN(configured_ports, const port_cfg_t *, cfg) { + const smartlist_t *conf_ports = get_configured_ports(); + SMARTLIST_FOREACH_BEGIN(conf_ports, const port_cfg_t *, cfg) { if (cfg->type == listener_type && !cfg->server_cfg.no_advertise && (tor_addr_family(&cfg->addr) == address_family || diff --git a/src/or/dirserv.c b/src/or/dirserv.c index a045f3a..5f848bd 100644 --- a/src/or/dirserv.c +++ b/src/or/dirserv.c @@ -1131,8 +1131,11 @@ directory_caches_unknown_auth_certs(const or_options_t *options) return dir_server_mode(options) || options->BridgeRelay; }
-/** Return 1 if we want to keep descriptors, networkstatuses, etc around - * and we're willing to serve them to others. Else return 0. +/** Return 1 if we want to keep descriptors, networkstatuses, etc around. + * Else return 0. + * Check get_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. */ int directory_caches_dir_info(const or_options_t *options) diff --git a/src/or/router.c b/src/or/router.c index 68bcf13..545753c 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -1148,10 +1148,11 @@ router_should_be_directory_server(const or_options_t *options, int dir_port) "seconds long. Raising to 1."); interval_length = 1; } - log_info(LD_GENERAL, "Calculating whether to disable dirport: effective " + log_info(LD_GENERAL, "Calculating whether to advertise %s: effective " "bwrate: %u, AccountingMax: "U64_FORMAT", " - "accounting interval length %d", effective_bw, - U64_PRINTF_ARG(options->AccountingMax), + "accounting interval length %d", + dir_port ? "dirport" : "begindir", + effective_bw, U64_PRINTF_ARG(options->AccountingMax), interval_length);
acc_bytes = options->AccountingMax; @@ -1218,6 +1219,7 @@ decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port) return dir_port; if (net_is_disabled()) return 0; + /* redundant - if DirPort is unreachable, we don't publish a descriptor */ if (!check_whether_dirport_reachable()) return 0; if (!router_get_advertised_dir_port(options, dir_port)) @@ -1229,6 +1231,39 @@ decide_to_advertise_dirport(const or_options_t *options, uint16_t dir_port) return router_should_be_directory_server(options, dir_port) ? dir_port : 0; }
+/** Look at a variety of factors, and 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, + int supports_tunnelled_dir_requests) +{ + /* Part one: reasons to publish or not publish that aren't + * worth mentioning to the user, either because they're obvious + * or because they're normal behavior. */ + + /* short circuit the rest of the function */ + if (!supports_tunnelled_dir_requests) + return 0; + if (authdir_mode(options)) /* always publish */ + return 1; + if (net_is_disabled()) + return 0; + /* redundant - if ORPort is unreachable, we don't publish a descriptor */ + if (!check_whether_orport_reachable()) + return 0; + if (!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. */ + return router_should_be_directory_server(options, 0); +} + /** Allocate and return a new extend_info_t that can be used to build * a circuit to or through the router <b>r</b>. Use the primary * address of the router unless <b>for_direct_connect</b> is true, in @@ -1924,8 +1959,8 @@ router_build_fresh_descriptor(routerinfo_t **r, extrainfo_t **e) ri->addr = addr; ri->or_port = router_get_advertised_or_port(options); ri->dir_port = router_get_advertised_dir_port(options, 0); - ri->supports_tunnelled_dir_requests = dir_server_mode(options) && - router_should_be_directory_server(options, ri->dir_port); + ri->supports_tunnelled_dir_requests = + directory_permits_begindir_requests(options); ri->cache_info.published_on = time(NULL); ri->onion_pkey = crypto_pk_dup_key(get_onion_key()); /* must invoke from * main thread */ @@ -2706,7 +2741,8 @@ router_dump_router_to_string(routerinfo_t *router, tor_free(p6); }
- if (router->supports_tunnelled_dir_requests) { + if (decide_to_advertise_begindir(options, + router->supports_tunnelled_dir_requests)) { smartlist_add(chunks, tor_strdup("tunnelled-dir-server\n")); }
diff --git a/src/test/test_dir.c b/src/test/test_dir.c index ea179fb..401c7b2 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -85,6 +85,15 @@ test_dir_nicknames(void *arg) ; }
+static smartlist_t *mocked_configured_ports = NULL; + +/** Returns mocked_configured_ports */ +static const smartlist_t * +mock_get_configured_ports(void) +{ + return mocked_configured_ports; +} + /** Run unit tests for router descriptor generation logic. */ static void test_dir_formats(void *arg) @@ -104,6 +113,7 @@ test_dir_formats(void *arg) or_options_t *options = get_options_mutable(); const addr_policy_t *p; time_t now = time(NULL); + port_cfg_t orport, dirport;
(void)arg; pk1 = pk_generate(0); @@ -185,9 +195,31 @@ test_dir_formats(void *arg) /* XXXX025 router_dump_to_string should really take this from ri.*/ options->ContactInfo = tor_strdup("Magri White " "magri@elsewhere.example.com"); + /* Skip reachability checks for DirPort and tunnelled-dir-server */ + options->AssumeReachable = 1; + + /* Fake just enough of an ORPort and DirPort to get by */ + MOCK(get_configured_ports, mock_get_configured_ports); + mocked_configured_ports = smartlist_new(); + + memset(&orport, 0, sizeof(orport)); + orport.type = CONN_TYPE_OR_LISTENER; + orport.addr.family = AF_INET; + orport.port = 9000; + smartlist_add(mocked_configured_ports, &orport); + + memset(&dirport, 0, sizeof(dirport)); + dirport.type = CONN_TYPE_DIR_LISTENER; + dirport.addr.family = AF_INET; + dirport.port = 9003; + smartlist_add(mocked_configured_ports, &dirport);
buf = router_dump_router_to_string(r1, pk2, NULL, NULL, NULL);
+ UNMOCK(get_configured_ports); + smartlist_free(mocked_configured_ports); + mocked_configured_ports = NULL; + tor_free(options->ContactInfo); tt_assert(buf);
@@ -308,6 +340,16 @@ test_dir_formats(void *arg) strlcat(buf2, "tunnelled-dir-server\n", sizeof(buf2)); strlcat(buf2, "router-sig-ed25519 ", sizeof(buf2));
+ /* Fake just enough of an ORPort to get by */ + MOCK(get_configured_ports, mock_get_configured_ports); + mocked_configured_ports = smartlist_new(); + + memset(&orport, 0, sizeof(orport)); + orport.type = CONN_TYPE_OR_LISTENER; + orport.addr.family = AF_INET; + orport.port = 9005; + smartlist_add(mocked_configured_ports, &orport); + buf = router_dump_router_to_string(r2, pk1, pk2, &r2_onion_keypair, &kp2); tt_assert(buf); buf[strlen(buf2)] = '\0'; /* Don't compare the sig; it's never the same @@ -318,6 +360,10 @@ test_dir_formats(void *arg)
buf = router_dump_router_to_string(r2, pk1, NULL, NULL, NULL);
+ UNMOCK(get_configured_ports); + smartlist_free(mocked_configured_ports); + mocked_configured_ports = NULL; + /* Reset for later */ cp = buf; rp2 = router_parse_entry_from_string((const char*)cp,NULL,1,0,NULL,NULL);