October 2017 - tor-commits - lists.torproject.org

[tor/maint-0.3.2] Tweak the documentation for BridgeDistribution
by nickm＠torproject.org 24 Oct '17

24 Oct '17

commit 3581f93d273dfd9fbc56de907b7e3b77575614e1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 23 10:38:19 2017 -0400 Tweak the documentation for BridgeDistribution Note that it will have no effect yet; note that the default is "any". --- changes/feature18329 | 5 ++++- doc/tor.1.txt | 7 ++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/changes/feature18329 b/changes/feature18329 index 7fdb659e8..f534c7614 100644 --- a/changes/feature18329 +++ b/changes/feature18329 @@ -2,5 +2,8 @@ - Bridge relays can now set the BridgeDistribution config option to add a "bridge-distribution-request" line to their bridge descriptor, which tells BridgeDB how they'd like their bridge address to be - given out. Implements tickets 18329. + given out. (Note that as of Oct 2017, BridgeDB does not yet implement + this feature.) As a side benefit, this feature provides a way + to distinguish bridge descriptors from non-bridge descriptors. + Implements tickets 18329. diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 7da5ba3fe..52e81860b 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1651,7 +1651,12 @@ is non-zero): If set along with BridgeRelay, Tor will include a new line in its bridge descriptor which indicates to the BridgeDB service how it would like its bridge address to be given out. Set it to "none" if - you want BridgeDB to avoid distributing your bridge address. + you want BridgeDB to avoid distributing your bridge address, or "any" to + let BridgeDB decide. (Default: any) + + + Note: as of Oct 2017, the BridgeDB part of this option is not yet + implemented. Until BridgeDB is updated to obey this option, your + bridge will make this request, but it will not (yet) be obeyed. [[ContactInfo]] **ContactInfo** __email_address__:: Administrative contact information for this relay or bridge. This line

1 0

[tor/maint-0.3.2] Add new BridgeDistribution config option
by nickm＠torproject.org 24 Oct '17

24 Oct '17

commit ebab5215252f9fa810ae091c335c5ae6e619faaf Author: Roger Dingledine <arma(a)torproject.org> Date: Sun Nov 13 02:39:16 2016 -0500 Add new BridgeDistribution config option Bridge relays can use it to add a "bridge-distribution-request" line to their bridge descriptor, which tells BridgeDB how they'd like their bridge address to be given out. Implements tickets 18329. --- changes/feature18329 | 6 ++++++ doc/tor.1.txt | 6 ++++++ src/or/config.c | 7 +++++++ src/or/or.h | 4 ++++ src/or/router.c | 8 ++++++++ 5 files changed, 31 insertions(+) diff --git a/changes/feature18329 b/changes/feature18329 new file mode 100644 index 000000000..7fdb659e8 --- /dev/null +++ b/changes/feature18329 @@ -0,0 +1,6 @@ + o Minor features: + - Bridge relays can now set the BridgeDistribution config option to + add a "bridge-distribution-request" line to their bridge descriptor, + which tells BridgeDB how they'd like their bridge address to be + given out. Implements tickets 18329. + diff --git a/doc/tor.1.txt b/doc/tor.1.txt index f1068cedc..7da5ba3fe 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1647,6 +1647,12 @@ is non-zero): server descriptor to the bridge database, rather than to the public directory authorities. +[[BridgeDistribution]] **BridgeDistribution** __string__:: + If set along with BridgeRelay, Tor will include a new line in its + bridge descriptor which indicates to the BridgeDB service how it + would like its bridge address to be given out. Set it to "none" if + you want BridgeDB to avoid distributing your bridge address. + [[ContactInfo]] **ContactInfo** __email_address__:: Administrative contact information for this relay or bridge. This line can be used to contact you if your relay or bridge is misconfigured or diff --git a/src/or/config.c b/src/or/config.c index 08cc5cdf5..1bdfdf484 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -182,6 +182,7 @@ static config_var_t option_vars_[] = { V(BridgePassword, STRING, NULL), V(BridgeRecordUsageByCountry, BOOL, "1"), V(BridgeRelay, BOOL, "0"), + V(BridgeDistribution, STRING, NULL), V(CellStatistics, BOOL, "0"), V(LearnCircuitBuildTimeout, BOOL, "1"), V(CircuitBuildTimeout, INTERVAL, "0"), @@ -3346,6 +3347,10 @@ options_validate(or_options_t *old_options, or_options_t *options, options->DirPort_set = 0; } + if (options->BridgeDistribution && !options->BridgeRelay) { + REJECT("You have set BridgeDistribution, yet you didn't set BridgeRelay!"); + } + if (options->MinUptimeHidServDirectoryV2 < 0) { log_warn(LD_CONFIG, "MinUptimeHidServDirectoryV2 option must be at " "least 0 seconds. Changing to 0."); @@ -4497,6 +4502,8 @@ options_transition_affects_descriptor(const or_options_t *old_options, get_effective_bwburst(old_options) != get_effective_bwburst(new_options) || !opt_streq(old_options->ContactInfo, new_options->ContactInfo) || + !opt_streq(old_options->BridgeDistribution, + new_options->BridgeDistribution) || !opt_streq(old_options->MyFamily, new_options->MyFamily) || !opt_streq(old_options->AccountingStart, new_options->AccountingStart) || old_options->AccountingMax != new_options->AccountingMax || diff --git a/src/or/or.h b/src/or/or.h index 33fe8b96c..3d61cfa05 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3638,6 +3638,10 @@ typedef struct { int BridgeAuthoritativeDir; /**< Boolean: is this an authoritative directory * that aggregates bridge descriptors? */ + /** If set on a bridge relay, it will include this value on a new + * "bridge-distribution-request" line in its bridge descriptor. */ + char *BridgeDistribution; + /** If set on a bridge authority, it will answer requests on its dirport * for bridge statuses -- but only if the requests use this password. */ char *BridgePassword; diff --git a/src/or/router.c b/src/or/router.c index 6d3a32a60..780d0444f 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2867,6 +2867,14 @@ router_dump_router_to_string(routerinfo_t *router, smartlist_add_asprintf(chunks, "contact %s\n", ci); } + if (options->BridgeRelay && options->BridgeDistribution && + strlen(options->BridgeDistribution)) { + const char *bd = options->BridgeDistribution; + if (strchr(bd, '\n') || strchr(bd, '\r')) + bd = escaped(bd); + smartlist_add_asprintf(chunks, "bridge-distribution-request %s\n", bd); + } + if (router->onion_curve25519_pkey) { char kbuf[128]; base64_encode(kbuf, sizeof(kbuf),

1 0

[tor/maint-0.3.2] test: Add unittest for descriptors with BridgeDistribution option.
by nickm＠torproject.org 24 Oct '17

24 Oct '17

commit 02cde0d9398c43de3a67133b7982d9df6962fe32 Author: Isis Lovecruft <isis(a)torproject.org> Date: Mon Oct 23 19:44:06 2017 +0000 test: Add unittest for descriptors with BridgeDistribution option. --- src/or/config.c | 3 +- src/or/config.h | 1 + src/test/include.am | 1 + src/test/test.c | 1 + src/test/test_config.c | 63 ++++++++++++++++++++++++++++ src/test/test_router.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 176 insertions(+), 2 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 4d08a0795..0b1e6bed1 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -654,7 +654,6 @@ static int parse_ports(or_options_t *options, int validate_only, static int check_server_ports(const smartlist_t *ports, const or_options_t *options, int *num_low_ports_out); -static int check_bridge_distribution_setting(const char *bd); static int validate_data_directory(or_options_t *options); static int write_configuration_file(const char *fname, const or_options_t *options); @@ -6370,7 +6369,7 @@ warn_client_dns_cache(const char *option, int disabling) * returned. If the option string contains an invalid character, -1 is * returned. **/ -static int +STATIC int check_bridge_distribution_setting(const char *bd) { if (bd == NULL) diff --git a/src/or/config.h b/src/or/config.h index 664553251..096937cb6 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -199,6 +199,7 @@ STATIC int parse_port_config(smartlist_t *out, const char *defaultaddr, int defaultport, const unsigned flags); +STATIC int check_bridge_distribution_setting(const char *bd); #endif #endif diff --git a/src/test/include.am b/src/test/include.am index 8ecfaf10c..6cefc6c5a 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -115,6 +115,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.c b/src/test/test.c index 9a41b976b..733732609 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1220,6 +1220,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 }, diff --git a/src/test/test_config.c b/src/test/test_config.c index 89f9b3e2a..396f06adf 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -4890,6 +4890,65 @@ test_config_parse_port_config__ports__server_options(void *data) config_free_lines(config_port_valid); config_port_valid = NULL; } +/* If we're not configured to be a bridge, but we set + * BridgeDistribution, then options_validate () should return -1. */ +static void +test_config_check_bridge_distribution_setting_not_a_bridge(void *arg) { + or_options_t* options = get_options_mutable(); + or_options_t* old_options = options; + or_options_t* default_options = options; + char* message = (char*)(""); + int ret; + + (void)arg; + + options->BridgeRelay = 0; + options->BridgeDistribution = (char*)("https"); + + ret = options_validate(old_options, options, default_options, 0, &message); + + tt_int_op(ret, OP_EQ, -1); + done: + return; +} + +/* If the BridgeDistribution setting was valid, 0 should be returned. */ +static void +test_config_check_bridge_distribution_setting_valid(void *arg) { + int ret = check_bridge_distribution_setting("https"); + + (void)arg; + + tt_int_op(ret, OP_EQ, 0); + done: + return; +} + +/* If the BridgeDistribution setting was invalid, -1 should be returned. */ +static void +test_config_check_bridge_distribution_setting_invalid(void *arg) { + int ret = check_bridge_distribution_setting("hyphens-are-not-allowed"); + + (void)arg; + + tt_int_op(ret, OP_EQ, -1); + done: + return; +} + +/* If the BridgeDistribution setting was unrecognised, a warning should be + * logged and 0 should be returned. */ +static void +test_config_check_bridge_distribution_setting_unrecognised(void *arg) { + int ret = check_bridge_distribution_setting("unicorn"); + + (void)arg; + + tt_int_op(ret, OP_EQ, 0); + done: + return; +} + #define CONFIG_TEST(name, flags) \ { #name, test_config_ ## name, flags, NULL, NULL } @@ -4916,6 +4975,10 @@ struct testcase_t config_tests[] = { CONFIG_TEST(parse_port_config__ports__no_ports_given, 0), CONFIG_TEST(parse_port_config__ports__server_options, 0), CONFIG_TEST(parse_port_config__ports__ports_given, 0), + CONFIG_TEST(check_bridge_distribution_setting_not_a_bridge, TT_FORK), + CONFIG_TEST(check_bridge_distribution_setting_valid, 0), + CONFIG_TEST(check_bridge_distribution_setting_invalid, 0), + CONFIG_TEST(check_bridge_distribution_setting_unrecognised, 0), END_OF_TESTCASES }; diff --git a/src/test/test_router.c b/src/test/test_router.c new file mode 100644 index 000000000..64434035e --- /dev/null +++ b/src/test/test_router.c @@ -0,0 +1,109 @@ +/* Copyright (c) 2017, The Tor Project, Inc. */ +/* Copyright (c) 2017, isis agora lovecruft */ +/* See LICENSE for licensing information */ + +/** + * \file test_router.c + * \brief Unittests for code in src/or/router.c + **/ + +#include "or.h" +#include "config.h" +#include "crypto_curve25519.h" +#include "crypto_ed25519.h" +#include "router.h" +#include "routerlist.h" + +/* Test suite stuff */ +#include "test.h" + +NS_DECL(const routerinfo_t *, router_get_my_routerinfo, (void)); + +static routerinfo_t* mock_routerinfo; + +static const routerinfo_t* +NS(router_get_my_routerinfo)(void) { + crypto_pk_t* ident_key; + crypto_pk_t* tap_key; + time_t now; + + if (!mock_routerinfo) { + /* Mock the published timestamp, otherwise router_dump_router_to_string() + * will poop its pants. */ + time(&now); + + /* We'll need keys, or router_dump_router_to_string() would return NULL. */ + ident_key = pk_generate(0); + tap_key = pk_generate(0); + + tor_assert(ident_key != NULL); + tor_assert(tap_key != NULL); + + mock_routerinfo = tor_malloc_zero(sizeof(routerinfo_t)); + mock_routerinfo->nickname = tor_strdup("ConlonNancarrow"); + mock_routerinfo->addr = 123456789; + mock_routerinfo->or_port = 443; + mock_routerinfo->platform = tor_strdup("unittest"); + mock_routerinfo->cache_info.published_on = now; + mock_routerinfo->identity_pkey = crypto_pk_dup_key(ident_key); + mock_routerinfo->onion_pkey = crypto_pk_dup_key(tap_key); + mock_routerinfo->bandwidthrate = 9001; + mock_routerinfo->bandwidthburst = 9002; + } + + return mock_routerinfo; +} + +/* If no distribution option was set, then check_bridge_distribution_setting() + * should have set it to "any". */ +static void +test_router_dump_router_to_string_no_bridge_distribution_method(void *arg) { + const char* needle = "bridge-distribution-request any"; + or_options_t* options = get_options_mutable(); + routerinfo_t* router = NULL; + curve25519_keypair_t ntor_keypair; + ed25519_keypair_t signing_keypair; + char* desc = NULL; + char* found = NULL; + (void)arg; + + NS_MOCK(router_get_my_routerinfo); + + options->ORPort_set = 1; + options->BridgeRelay = 1; + + /* Generate keys which router_dump_router_to_string() expects to exist. */ + tt_int_op(0, ==, curve25519_keypair_generate(&ntor_keypair, 0)); + tt_int_op(0, ==, ed25519_keypair_generate(&signing_keypair, 0)); + + /* Set up part of our routerinfo_t so that we don't trigger any other + * assertions in router_dump_router_to_string(). */ + router = (routerinfo_t*)router_get_my_routerinfo(); + tt_ptr_op(router, !=, NULL); + + router->onion_curve25519_pkey = &ntor_keypair.pubkey; + + /* Generate our server descriptor and ensure that the substring + * "bridge-distribution-request any" occurs somewhere within it. */ + desc = router_dump_router_to_string(router, + router->identity_pkey, + router->onion_pkey, + &ntor_keypair, + &signing_keypair); + tt_ptr_op(desc, !=, NULL); + found = strstr(desc, needle); + tt_ptr_op(found, !=, NULL); + + done: + NS_UNMOCK(router_get_my_routerinfo); + + tor_free(desc); +} + +#define ROUTER_TEST(name, flags) \ + { #name, test_router_ ## name, flags, NULL, NULL } + +struct testcase_t router_tests[] = { + ROUTER_TEST(dump_router_to_string_no_bridge_distribution_method, TT_FORK), + END_OF_TESTCASES +};

1 0

[tor/maint-0.3.2] Adjust test to allow hyphens too.
by nickm＠torproject.org 24 Oct '17

24 Oct '17

commit 44e8aaa9aa57bf6b9fce1712a8f77e5f9979392c Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 24 19:33:38 2017 -0400 Adjust test to allow hyphens too. --- src/test/test_config.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/test_config.c b/src/test/test_config.c index 396f06adf..593d3ca8f 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -4927,10 +4927,14 @@ test_config_check_bridge_distribution_setting_valid(void *arg) { /* If the BridgeDistribution setting was invalid, -1 should be returned. */ static void test_config_check_bridge_distribution_setting_invalid(void *arg) { - int ret = check_bridge_distribution_setting("hyphens-are-not-allowed"); + int ret = check_bridge_distribution_setting("hyphens-are-allowed"); (void)arg; + tt_int_op(ret, OP_EQ, 0); + + ret = check_bridge_distribution_setting("asterisks*are*forbidden"); + tt_int_op(ret, OP_EQ, -1); done: return;

1 0

[tor/maint-0.3.2] Always set bridge-distribution-request on bridges' descriptors.
by nickm＠torproject.org 24 Oct '17

24 Oct '17

commit 613b18f0afd865b0fc5daf8d17da241396100dcd Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Oct 23 10:52:42 2017 -0400 Always set bridge-distribution-request on bridges' descriptors. Also, warn the user if the BridgeDistribution option is unrecognized, and reject the value if it is invalid. --- src/or/config.c | 43 ++++++++++++++++++++++++++++++++++++++++--- src/or/router.c | 10 +++++++--- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 1bdfdf484..7f45bb2cb 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -654,7 +654,7 @@ static int parse_ports(or_options_t *options, int validate_only, static int check_server_ports(const smartlist_t *ports, const or_options_t *options, int *num_low_ports_out); - +static int check_bridge_distribution_setting(const char *bd); static int validate_data_directory(or_options_t *options); static int write_configuration_file(const char *fname, const or_options_t *options); @@ -3347,10 +3347,16 @@ options_validate(or_options_t *old_options, or_options_t *options, options->DirPort_set = 0; } - if (options->BridgeDistribution && !options->BridgeRelay) { - REJECT("You have set BridgeDistribution, yet you didn't set BridgeRelay!"); + if (options->BridgeDistribution) { + if (!options->BridgeRelay) { + REJECT("You set BridgeDistribution, but you didn't set BridgeRelay!"); + } + if (check_bridge_distribution_setting(options->BridgeDistribution) < 0) { + REJECT("Invalid BridgeDistribution value."); + } } + if (options->MinUptimeHidServDirectoryV2 < 0) { log_warn(LD_CONFIG, "MinUptimeHidServDirectoryV2 option must be at " "least 0 seconds. Changing to 0."); @@ -6344,6 +6350,37 @@ warn_client_dns_cache(const char *option, int disabling) "to your destination."); } +/** Warn if bd is an unrecognized bridge distribution setting; + * return -1 if it is invalid. */ +static int +check_bridge_distribution_setting(const char *bd) +{ + if (bd == NULL) + return 0; + + const char *RECOGNIZED[] = { + "none", "any", "https", "email", "moat", "hyphae" + }; + unsigned i; + for (i = 0; i < ARRAY_LENGTH(RECOGNIZED); ++i) { + if (!strcmp(bd, RECOGNIZED[i])) + return 0; + } + + const char *cp = bd; + // Method = (KeywordChar | "_") + + while (TOR_ISALNUM(*cp) || *cp == '-' || *cp == '_') + ++cp; + + if (*cp == 0) { + log_warn(LD_CONFIG, "Unrecognized BridgeDistribution value %s. I'll " + "assume you know what you are doing...", escaped(bd)); + return 0; // we reached the end of the string; all is well + } else { + return -1; // we found a bad character in the string. + } +} + /** * Parse port configuration for a single port type. * diff --git a/src/or/router.c b/src/or/router.c index 780d0444f..553264efa 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2867,9 +2867,13 @@ router_dump_router_to_string(routerinfo_t *router, smartlist_add_asprintf(chunks, "contact %s\n", ci); } - if (options->BridgeRelay && options->BridgeDistribution && - strlen(options->BridgeDistribution)) { - const char *bd = options->BridgeDistribution; + if (options->BridgeRelay) { + const char *bd; + if (options->BridgeDistribution && strlen(options->BridgeDistribution)) { + bd = options->BridgeDistribution; + } else { + bd = "any"; + } if (strchr(bd, '\n') || strchr(bd, '\r')) bd = escaped(bd); smartlist_add_asprintf(chunks, "bridge-distribution-request %s\n", bd);

1 0

[tor/maint-0.3.2] doc: Improve documentation for check_bridge_distribution_setting().
by nickm＠torproject.org 24 Oct '17

24 Oct '17

commit b0e10f23ba2b03f275ef4acf2183a02042e6cded Author: Isis Lovecruft <isis(a)torproject.org> Date: Mon Oct 23 19:37:52 2017 +0000 doc: Improve documentation for check_bridge_distribution_setting(). --- src/or/config.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/or/config.c b/src/or/config.c index 7f45bb2cb..4d08a0795 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -6350,8 +6350,26 @@ warn_client_dns_cache(const char *option, int disabling) "to your destination."); } -/** Warn if bd is an unrecognized bridge distribution setting; - * return -1 if it is invalid. */ +/** + * Validate the configured bridge distribution method from a BridgeDistribution + * config line. + * + * The input bd, is a string taken from the BridgeDistribution config + * line (if present). If the option wasn't set, return 0 immediately. The + * BridgeDistribution option is then validated. Currently valid, recognised + * options are: + * + * - "none" + * - "any" + * - "https" + * - "email" + * - "moat" + * - "hyphae" + * + * If the option string is unrecognised, a warning will be logged and 0 is + * returned. If the option string contains an invalid character, -1 is + * returned. + **/ static int check_bridge_distribution_setting(const char *bd) {

1 0

[tor/maint-0.3.2] Merge branch 'feature18329_029_squashed' into maint-0.3.2
by nickm＠torproject.org 24 Oct '17

24 Oct '17

commit b95ef19a77e2e04c740bbbe16d3b0ed7a5239aee Merge: 594cf9249 f3af74ccf Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 24 19:35:32 2017 -0400 Merge branch 'feature18329_029_squashed' into maint-0.3.2 changes/feature18329 | 2 +- src/test/test_config.c | 18 +++++++++++++----- src/test/test_router.c | 7 +++++-- 3 files changed, 19 insertions(+), 8 deletions(-)

1 0

[tor/maint-0.3.2] Updates to fix check-spaces/check-changes warnings
by nickm＠torproject.org 24 Oct '17

24 Oct '17

commit f3af74ccf594c07f7edbf0bdfa576b9af21563d9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 24 19:35:16 2017 -0400 Updates to fix check-spaces/check-changes warnings --- changes/feature18329 | 2 +- src/or/config.c | 1 - src/test/test_config.c | 12 ++++++++---- src/test/test_router.c | 7 +++++-- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/changes/feature18329 b/changes/feature18329 index f534c7614..1dabf5024 100644 --- a/changes/feature18329 +++ b/changes/feature18329 @@ -1,4 +1,4 @@ - o Minor features: + o Minor features (bridge): - Bridge relays can now set the BridgeDistribution config option to add a "bridge-distribution-request" line to their bridge descriptor, which tells BridgeDB how they'd like their bridge address to be diff --git a/src/or/config.c b/src/or/config.c index 0b1e6bed1..330e8dcdd 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -3355,7 +3355,6 @@ options_validate(or_options_t *old_options, or_options_t *options, } } - if (options->MinUptimeHidServDirectoryV2 < 0) { log_warn(LD_CONFIG, "MinUptimeHidServDirectoryV2 option must be at " "least 0 seconds. Changing to 0."); diff --git a/src/test/test_config.c b/src/test/test_config.c index 593d3ca8f..978a24b81 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -4893,7 +4893,8 @@ test_config_parse_port_config__ports__server_options(void *data) /* If we're not configured to be a bridge, but we set * BridgeDistribution, then options_validate () should return -1. */ static void -test_config_check_bridge_distribution_setting_not_a_bridge(void *arg) { +test_config_check_bridge_distribution_setting_not_a_bridge(void *arg) +{ or_options_t* options = get_options_mutable(); or_options_t* old_options = options; or_options_t* default_options = options; @@ -4914,7 +4915,8 @@ test_config_check_bridge_distribution_setting_not_a_bridge(void *arg) { /* If the BridgeDistribution setting was valid, 0 should be returned. */ static void -test_config_check_bridge_distribution_setting_valid(void *arg) { +test_config_check_bridge_distribution_setting_valid(void *arg) +{ int ret = check_bridge_distribution_setting("https"); (void)arg; @@ -4926,7 +4928,8 @@ test_config_check_bridge_distribution_setting_valid(void *arg) { /* If the BridgeDistribution setting was invalid, -1 should be returned. */ static void -test_config_check_bridge_distribution_setting_invalid(void *arg) { +test_config_check_bridge_distribution_setting_invalid(void *arg) +{ int ret = check_bridge_distribution_setting("hyphens-are-allowed"); (void)arg; @@ -4943,7 +4946,8 @@ test_config_check_bridge_distribution_setting_invalid(void *arg) { /* If the BridgeDistribution setting was unrecognised, a warning should be * logged and 0 should be returned. */ static void -test_config_check_bridge_distribution_setting_unrecognised(void *arg) { +test_config_check_bridge_distribution_setting_unrecognised(void *arg) +{ int ret = check_bridge_distribution_setting("unicorn"); (void)arg; diff --git a/src/test/test_router.c b/src/test/test_router.c index 64434035e..4e96e2453 100644 --- a/src/test/test_router.c +++ b/src/test/test_router.c @@ -22,7 +22,8 @@ NS_DECL(const routerinfo_t *, router_get_my_routerinfo, (void)); static routerinfo_t* mock_routerinfo; static const routerinfo_t* -NS(router_get_my_routerinfo)(void) { +NS(router_get_my_routerinfo)(void) +{ crypto_pk_t* ident_key; crypto_pk_t* tap_key; time_t now; @@ -57,7 +58,8 @@ NS(router_get_my_routerinfo)(void) { /* If no distribution option was set, then check_bridge_distribution_setting() * should have set it to "any". */ static void -test_router_dump_router_to_string_no_bridge_distribution_method(void *arg) { +test_router_dump_router_to_string_no_bridge_distribution_method(void *arg) +{ const char* needle = "bridge-distribution-request any"; or_options_t* options = get_options_mutable(); routerinfo_t* router = NULL; @@ -107,3 +109,4 @@ struct testcase_t router_tests[] = { ROUTER_TEST(dump_router_to_string_no_bridge_distribution_method, TT_FORK), END_OF_TESTCASES }; +

1 0

[tor/maint-0.3.2] Merge branch 'feature18329_029_squashed' into maint-0.3.2
by nickm＠torproject.org 24 Oct '17

24 Oct '17

commit 594cf92498c8ea12dc0b19f743d6b88d4a98f1eb Merge: 122a7f884 02cde0d93 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 24 19:35:28 2017 -0400 Merge branch 'feature18329_029_squashed' into maint-0.3.2 changes/feature18329 | 9 ++++ doc/tor.1.txt | 11 +++++ src/or/config.c | 62 +++++++++++++++++++++++++++- src/or/config.h | 2 + src/or/or.h | 4 ++ src/or/router.c | 12 ++++++ src/test/include.am | 1 + src/test/test.c | 1 + src/test/test_config.c | 63 ++++++++++++++++++++++++++++ src/test/test_router.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++ 10 files changed, 273 insertions(+), 1 deletion(-) diff --cc src/or/config.c index a2353b94d,0b1e6bed1..b123d4935 --- a/src/or/config.c +++ b/src/or/config.c @@@ -253,15 -182,14 +253,16 @@@ static config_var_t option_vars_[] = V(BridgePassword, STRING, NULL), V(BridgeRecordUsageByCountry, BOOL, "1"), V(BridgeRelay, BOOL, "0"), + V(BridgeDistribution, STRING, NULL), V(CellStatistics, BOOL, "0"), + V(PaddingStatistics, BOOL, "1"), V(LearnCircuitBuildTimeout, BOOL, "1"), V(CircuitBuildTimeout, INTERVAL, "0"), - V(CircuitIdleTimeout, INTERVAL, "1 hour"), + OBSOLETE("CircuitIdleTimeout"), + V(CircuitsAvailableTimeout, INTERVAL, "0"), V(CircuitStreamTimeout, INTERVAL, "0"), V(CircuitPriorityHalflife, DOUBLE, "-100.0"), /*negative:'Use default'*/ - V(ClientDNSRejectInternalAddresses, BOOL,"1"), + V(TestingClientDNSRejectInternalAddresses, BOOL,"1"), V(ClientOnly, BOOL, "0"), V(ClientPreferIPv6ORPort, AUTOBOOL, "auto"), V(ClientPreferIPv6DirPort, AUTOBOOL, "auto"), @@@ -3504,14 -3346,16 +3504,23 @@@ options_validate(or_options_t *old_opti options->DirPort_set = 0; } + if (server_mode(options) && options->ConnectionPadding != -1) { + REJECT("Relays must use 'auto' for the ConnectionPadding setting."); + } + + if (server_mode(options) && options->ReducedConnectionPadding != 0) { + REJECT("Relays cannot set ReducedConnectionPadding. "); + } + + if (options->BridgeDistribution) { + if (!options->BridgeRelay) { + REJECT("You set BridgeDistribution, but you didn't set BridgeRelay!"); + } + if (check_bridge_distribution_setting(options->BridgeDistribution) < 0) { + REJECT("Invalid BridgeDistribution value."); + } + } + - if (options->MinUptimeHidServDirectoryV2 < 0) { log_warn(LD_CONFIG, "MinUptimeHidServDirectoryV2 option must be at " "least 0 seconds. Changing to 0."); @@@ -4686,7 -4507,9 +4695,9 @@@ options_transition_affects_descriptor(c get_effective_bwburst(old_options) != get_effective_bwburst(new_options) || !opt_streq(old_options->ContactInfo, new_options->ContactInfo) || + !opt_streq(old_options->BridgeDistribution, + new_options->BridgeDistribution) || - !opt_streq(old_options->MyFamily, new_options->MyFamily) || + !config_lines_eq(old_options->MyFamily, new_options->MyFamily) || !opt_streq(old_options->AccountingStart, new_options->AccountingStart) || old_options->AccountingMax != new_options->AccountingMax || old_options->AccountingRule != new_options->AccountingRule || @@@ -6584,11 -6350,65 +6595,60 @@@ warn_client_dns_cache(const char *optio } /** + * Validate the configured bridge distribution method from a BridgeDistribution + * config line. + * + * The input bd, is a string taken from the BridgeDistribution config + * line (if present). If the option wasn't set, return 0 immediately. The + * BridgeDistribution option is then validated. Currently valid, recognised + * options are: + * + * - "none" + * - "any" + * - "https" + * - "email" + * - "moat" + * - "hyphae" + * + * If the option string is unrecognised, a warning will be logged and 0 is + * returned. If the option string contains an invalid character, -1 is + * returned. + **/ + STATIC int + check_bridge_distribution_setting(const char *bd) + { + if (bd == NULL) + return 0; + + const char *RECOGNIZED[] = { + "none", "any", "https", "email", "moat", "hyphae" + }; + unsigned i; + for (i = 0; i < ARRAY_LENGTH(RECOGNIZED); ++i) { + if (!strcmp(bd, RECOGNIZED[i])) + return 0; + } + + const char *cp = bd; + // Method = (KeywordChar | "_") + + while (TOR_ISALNUM(*cp) || *cp == '-' || *cp == '_') + ++cp; + + if (*cp == 0) { + log_warn(LD_CONFIG, "Unrecognized BridgeDistribution value %s. I'll " + "assume you know what you are doing...", escaped(bd)); + return 0; // we reached the end of the string; all is well + } else { + return -1; // we found a bad character in the string. + } + } + + /** * Parse port configuration for a single port type. * - * Read entries of the "FooPort" type from the list ports, and - * entries of the "FooListenAddress" type from the list - * listenaddrs. Two syntaxes are supported: a legacy syntax - * where FooPort is at most a single entry containing a port number and - * where FooListenAddress has any number of address:port combinations; - * and a new syntax where there are no FooListenAddress entries and - * where FooPort can have any number of entries of the format - * "[Address:][Port] IsolationOptions". + * Read entries of the "FooPort" type from the list ports. Syntax is + * that FooPort can have any number of entries of the format + * "[Address:][Port] IsolationOptions". * * In log messages, describe the port type as portname. * diff --cc src/or/config.h index af945f16b,096937cb6..efdd8c59b --- a/src/or/config.h +++ b/src/or/config.h @@@ -203,7 -199,8 +203,9 @@@ STATIC int parse_port_config(smartlist_ const char *defaultaddr, int defaultport, const unsigned flags); ++ + STATIC int check_bridge_distribution_setting(const char *bd); -#endif +#endif /* defined(CONFIG_PRIVATE) */ -#endif +#endif /* !defined(TOR_CONFIG_H) */ diff --cc src/test/test_config.c index 58268705e,396f06adf..68eb5419c --- a/src/test/test_config.c +++ b/src/test/test_config.c @@@ -4784,668 -4890,65 +4784,727 @@@ test_config_parse_port_config__ports__s config_free_lines(config_port_valid); config_port_valid = NULL; } +static void +test_config_parse_log_severity(void *data) +{ + int ret; + const char *severity_log_lines[] = { + "debug file /tmp/debug.log", + "debug\tfile /tmp/debug.log", + "[handshake]debug [~net,~mm]info notice stdout", + "[handshake]debug\t[~net,~mm]info\tnotice\tstdout", + NULL + }; + int i; + log_severity_list_t *severity; + + (void) data; + + severity = tor_malloc(sizeof(log_severity_list_t)); + for (i = 0; severity_log_lines[i]; i++) { + memset(severity, 0, sizeof(log_severity_list_t)); + ret = parse_log_severity_config(&severity_log_lines[i], severity); + tt_int_op(ret, OP_EQ, 0); + } + + done: + tor_free(severity); +} + +static void +test_config_include_limit(void *data) +{ + (void)data; + + config_line_t *result = NULL; + char *torrc_path = NULL; + char *dir = tor_strdup(get_fname("test_include_limit")); + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&torrc_path, "%s"PATH_SEPARATOR"torrc", dir); + char torrc_contents[1000]; + tor_snprintf(torrc_contents, sizeof(torrc_contents), "%%include %s", + torrc_path); + tt_int_op(write_str_to_file(torrc_path, torrc_contents, 0), OP_EQ, 0); + + tt_int_op(config_get_lines_include(torrc_contents, &result, 0, NULL), + OP_EQ, -1); + + done: + config_free_lines(result); + tor_free(torrc_path); + tor_free(dir); +} + +static void +test_config_include_does_not_exist(void *data) +{ + (void)data; + + config_line_t *result = NULL; + char *dir = tor_strdup(get_fname("test_include_does_not_exist")); + char *missing_path = NULL; + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&missing_path, "%s"PATH_SEPARATOR"missing", dir); + char torrc_contents[1000]; + tor_snprintf(torrc_contents, sizeof(torrc_contents), "%%include %s", + missing_path); + + tt_int_op(config_get_lines_include(torrc_contents, &result, 0, NULL), + OP_EQ, -1); + + done: + config_free_lines(result); + tor_free(dir); + tor_free(missing_path); +} + +static void +test_config_include_error_in_included_file(void *data) +{ + (void)data; + config_line_t *result = NULL; + + char *dir = tor_strdup(get_fname("test_error_in_included_file")); + char *invalid_path = NULL; + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&invalid_path, "%s"PATH_SEPARATOR"invalid", dir); + tt_int_op(write_str_to_file(invalid_path, "unclosed \"", 0), OP_EQ, 0); + + char torrc_contents[1000]; + tor_snprintf(torrc_contents, sizeof(torrc_contents), "%%include %s", + invalid_path); + + tt_int_op(config_get_lines_include(torrc_contents, &result, 0, NULL), + OP_EQ, -1); + + done: + config_free_lines(result); + tor_free(dir); + tor_free(invalid_path); +} + +static void +test_config_include_empty_file_folder(void *data) +{ + (void)data; + config_line_t *result = NULL; + + char *folder_path = NULL; + char *file_path = NULL; + char *dir = tor_strdup(get_fname("test_include_empty_file_folder")); + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&folder_path, "%s"PATH_SEPARATOR"empty_dir", dir); +#ifdef _WIN32 + tt_int_op(mkdir(folder_path), OP_EQ, 0); +#else + tt_int_op(mkdir(folder_path, 0700), OP_EQ, 0); +#endif + tor_asprintf(&file_path, "%s"PATH_SEPARATOR"empty_file", dir); + tt_int_op(write_str_to_file(file_path, "", 0), OP_EQ, 0); + + char torrc_contents[1000]; + tor_snprintf(torrc_contents, sizeof(torrc_contents), + "%%include %s\n" + "%%include %s\n", + folder_path, file_path); + + int include_used; + tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used), + OP_EQ, 0); + tt_ptr_op(result, OP_EQ, NULL); + tt_int_op(include_used, OP_EQ, 1); + + done: + config_free_lines(result); + tor_free(folder_path); + tor_free(file_path); + tor_free(dir); +} + +#ifndef _WIN32 +static void +test_config_include_no_permission(void *data) +{ + (void)data; + config_line_t *result = NULL; + + char *folder_path = NULL; + char *dir = NULL; + if (geteuid() == 0) + tt_skip(); + + dir = tor_strdup(get_fname("test_include_forbidden_folder")); + tt_ptr_op(dir, OP_NE, NULL); + + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); + + tor_asprintf(&folder_path, "%s"PATH_SEPARATOR"forbidden_dir", dir); + tt_int_op(mkdir(folder_path, 0100), OP_EQ, 0); + + char torrc_contents[1000]; + tor_snprintf(torrc_contents, sizeof(torrc_contents), + "%%include %s\n", + folder_path); + + int include_used; + tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used), + OP_EQ, -1); + tt_ptr_op(result, OP_EQ, NULL); + + done: + config_free_lines(result); + tor_free(folder_path); + if (dir) + chmod(dir, 0700); + tor_free(dir); +} +#endif + +static void +test_config_include_recursion_before_after(void *data) +{ + (void)data; + + config_line_t *result = NULL; + char *torrc_path = NULL; + char *dir = tor_strdup(get_fname("test_include_recursion_before_after")); + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&torrc_path, "%s"PATH_SEPARATOR"torrc", dir); + + char file_contents[1000]; + const int limit = MAX_INCLUDE_RECURSION_LEVEL; + int i; + // Loop backwards so file_contents has the contents of the first file by the + // end of the loop + for (i = limit; i > 0; i--) { + if (i < limit) { + tor_snprintf(file_contents, sizeof(file_contents), + "Test %d\n" + "%%include %s%d\n" + "Test %d\n", + i, torrc_path, i + 1, 2 * limit - i); + } else { + tor_snprintf(file_contents, sizeof(file_contents), "Test %d\n", i); + } + + if (i > 1) { + char *file_path = NULL; + tor_asprintf(&file_path, "%s%d", torrc_path, i); + tt_int_op(write_str_to_file(file_path, file_contents, 0), OP_EQ, 0); + tor_free(file_path); + } + } + + int include_used; + tt_int_op(config_get_lines_include(file_contents, &result, 0, &include_used), + OP_EQ, 0); + tt_ptr_op(result, OP_NE, NULL); + tt_int_op(include_used, OP_EQ, 1); + + int len = 0; + config_line_t *next; + for (next = result; next != NULL; next = next->next) { + char expected[10]; + tor_snprintf(expected, sizeof(expected), "%d", len + 1); + tt_str_op(next->key, OP_EQ, "Test"); + tt_str_op(next->value, OP_EQ, expected); + len++; + } + tt_int_op(len, OP_EQ, 2 * limit - 1); + + done: + config_free_lines(result); + tor_free(dir); + tor_free(torrc_path); +} + +static void +test_config_include_recursion_after_only(void *data) +{ + (void)data; + + config_line_t *result = NULL; + char *torrc_path = NULL; + char *dir = tor_strdup(get_fname("test_include_recursion_after_only")); + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&torrc_path, "%s"PATH_SEPARATOR"torrc", dir); + + char file_contents[1000]; + const int limit = MAX_INCLUDE_RECURSION_LEVEL; + int i; + // Loop backwards so file_contents has the contents of the first file by the + // end of the loop + for (i = limit; i > 0; i--) { + int n = (i - limit - 1) * -1; + if (i < limit) { + tor_snprintf(file_contents, sizeof(file_contents), + "%%include %s%d\n" + "Test %d\n", + torrc_path, i + 1, n); + } else { + tor_snprintf(file_contents, sizeof(file_contents), "Test %d\n", n); + } + + if (i > 1) { + char *file_path = NULL; + tor_asprintf(&file_path, "%s%d", torrc_path, i); + tt_int_op(write_str_to_file(file_path, file_contents, 0), OP_EQ, 0); + tor_free(file_path); + } + } + + int include_used; + tt_int_op(config_get_lines_include(file_contents, &result, 0, &include_used), + OP_EQ, 0); + tt_ptr_op(result, OP_NE, NULL); + tt_int_op(include_used, OP_EQ, 1); + + int len = 0; + config_line_t *next; + for (next = result; next != NULL; next = next->next) { + char expected[10]; + tor_snprintf(expected, sizeof(expected), "%d", len + 1); + tt_str_op(next->key, OP_EQ, "Test"); + tt_str_op(next->value, OP_EQ, expected); + len++; + } + tt_int_op(len, OP_EQ, limit); + + done: + config_free_lines(result); + tor_free(dir); + tor_free(torrc_path); +} + +static void +test_config_include_folder_order(void *data) +{ + (void)data; + + config_line_t *result = NULL; + char *torrcd = NULL; + char *path = NULL; + char *path2 = NULL; + char *dir = tor_strdup(get_fname("test_include_folder_order")); + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&torrcd, "%s"PATH_SEPARATOR"%s", dir, "torrc.d"); + +#ifdef _WIN32 + tt_int_op(mkdir(torrcd), OP_EQ, 0); +#else + tt_int_op(mkdir(torrcd, 0700), OP_EQ, 0); +#endif + + // test that files in subfolders are ignored + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", torrcd, "subfolder"); + +#ifdef _WIN32 + tt_int_op(mkdir(path), OP_EQ, 0); +#else + tt_int_op(mkdir(path, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&path2, "%s"PATH_SEPARATOR"%s", path, "01_ignore"); + tt_int_op(write_str_to_file(path2, "ShouldNotSee 1\n", 0), OP_EQ, 0); + tor_free(path); + + // test that files starting with . are ignored + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", torrcd, ".dot"); + tt_int_op(write_str_to_file(path, "ShouldNotSee 2\n", 0), OP_EQ, 0); + tor_free(path); + + // test file order + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", torrcd, "01_1st"); + tt_int_op(write_str_to_file(path, "Test 1\n", 0), OP_EQ, 0); + tor_free(path); + + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", torrcd, "02_2nd"); + tt_int_op(write_str_to_file(path, "Test 2\n", 0), OP_EQ, 0); + tor_free(path); + + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", torrcd, "aa_3rd"); + tt_int_op(write_str_to_file(path, "Test 3\n", 0), OP_EQ, 0); + tor_free(path); + + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", torrcd, "ab_4th"); + tt_int_op(write_str_to_file(path, "Test 4\n", 0), OP_EQ, 0); + tor_free(path); + + char torrc_contents[1000]; + tor_snprintf(torrc_contents, sizeof(torrc_contents), + "%%include %s\n", + torrcd); + + int include_used; + tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used), + OP_EQ, 0); + tt_ptr_op(result, OP_NE, NULL); + tt_int_op(include_used, OP_EQ, 1); + + int len = 0; + config_line_t *next; + for (next = result; next != NULL; next = next->next) { + char expected[10]; + tor_snprintf(expected, sizeof(expected), "%d", len + 1); + tt_str_op(next->key, OP_EQ, "Test"); + tt_str_op(next->value, OP_EQ, expected); + len++; + } + tt_int_op(len, OP_EQ, 4); + + done: + config_free_lines(result); + tor_free(torrcd); + tor_free(path); + tor_free(path2); + tor_free(dir); +} + +static void +test_config_include_path_syntax(void *data) +{ + (void)data; + + config_line_t *result = NULL; + char *dir = tor_strdup(get_fname("test_include_path_syntax")); + char *esc_dir = NULL, *dir_with_pathsep = NULL, + *esc_dir_with_pathsep = NULL, *torrc_contents = NULL; + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + esc_dir = esc_for_log(dir); + tor_asprintf(&dir_with_pathsep, "%s%s", dir, PATH_SEPARATOR); + esc_dir_with_pathsep = esc_for_log(dir_with_pathsep); + + tor_asprintf(&torrc_contents, + "%%include %s\n" + "%%include %s%s \n" // space to avoid suppressing newline + "%%include %s\n", + esc_dir, + dir, PATH_SEPARATOR, + esc_dir_with_pathsep); + + int include_used; + tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used), + OP_EQ, 0); + tt_ptr_op(result, OP_EQ, NULL); + tt_int_op(include_used, OP_EQ, 1); + + done: + config_free_lines(result); + tor_free(dir); + tor_free(torrc_contents); + tor_free(esc_dir); + tor_free(dir_with_pathsep); + tor_free(esc_dir_with_pathsep); +} + +static void +test_config_include_not_processed(void *data) +{ + (void)data; + + char torrc_contents[1000] = "%include does_not_exist\n"; + config_line_t *result = NULL; + tt_int_op(config_get_lines(torrc_contents, &result, 0),OP_EQ, 0); + tt_ptr_op(result, OP_NE, NULL); + + int len = 0; + config_line_t *next; + for (next = result; next != NULL; next = next->next) { + tt_str_op(next->key, OP_EQ, "%include"); + tt_str_op(next->value, OP_EQ, "does_not_exist"); + len++; + } + tt_int_op(len, OP_EQ, 1); + + done: + config_free_lines(result); +} + +static void +test_config_include_has_include(void *data) +{ + (void)data; + + config_line_t *result = NULL; + char *dir = tor_strdup(get_fname("test_include_has_include")); + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + char torrc_contents[1000] = "Test 1\n"; + int include_used; + + tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used), + OP_EQ, 0); + tt_int_op(include_used, OP_EQ, 0); + config_free_lines(result); + + tor_snprintf(torrc_contents, sizeof(torrc_contents), "%%include %s\n", dir); + tt_int_op(config_get_lines_include(torrc_contents, &result, 0,&include_used), + OP_EQ, 0); + tt_int_op(include_used, OP_EQ, 1); + + done: + config_free_lines(result); + tor_free(dir); +} + +static void +test_config_include_flag_both_without(void *data) +{ + (void)data; + + char *errmsg = NULL; + char conf_empty[1000]; + tor_snprintf(conf_empty, sizeof(conf_empty), + "DataDirectory %s\n", + get_fname(NULL)); + // test with defaults-torrc and torrc without include + int ret = options_init_from_string(conf_empty, conf_empty, CMD_RUN_UNITTESTS, + NULL, &errmsg); + tt_int_op(ret, OP_EQ, 0); + + const or_options_t *options = get_options(); + tt_int_op(options->IncludeUsed, OP_EQ, 0); + + done: + tor_free(errmsg); +} + +static void +test_config_include_flag_torrc_only(void *data) +{ + (void)data; + + char *errmsg = NULL; + char *path = NULL; + char *dir = tor_strdup(get_fname("test_include_flag_torrc_only")); + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", dir, "dummy"); + tt_int_op(write_str_to_file(path, "\n", 0), OP_EQ, 0); + + char conf_empty[1000]; + tor_snprintf(conf_empty, sizeof(conf_empty), + "DataDirectory %s\n", + get_fname(NULL)); + char conf_include[1000]; + tor_snprintf(conf_include, sizeof(conf_include), "%%include %s", path); + + // test with defaults-torrc without include and torrc with include + int ret = options_init_from_string(conf_empty, conf_include, + CMD_RUN_UNITTESTS, NULL, &errmsg); + tt_int_op(ret, OP_EQ, 0); + + const or_options_t *options = get_options(); + tt_int_op(options->IncludeUsed, OP_EQ, 1); + + done: + tor_free(errmsg); + tor_free(path); + tor_free(dir); +} + +static void +test_config_include_flag_defaults_only(void *data) +{ + (void)data; + + char *errmsg = NULL; + char *path = NULL; + char *dir = tor_strdup(get_fname("test_include_flag_defaults_only")); + tt_ptr_op(dir, OP_NE, NULL); + +#ifdef _WIN32 + tt_int_op(mkdir(dir), OP_EQ, 0); +#else + tt_int_op(mkdir(dir, 0700), OP_EQ, 0); +#endif + + tor_asprintf(&path, "%s"PATH_SEPARATOR"%s", dir, "dummy"); + tt_int_op(write_str_to_file(path, "\n", 0), OP_EQ, 0); + + char conf_empty[1000]; + tor_snprintf(conf_empty, sizeof(conf_empty), + "DataDirectory %s\n", + get_fname(NULL)); + char conf_include[1000]; + tor_snprintf(conf_include, sizeof(conf_include), "%%include %s", path); + + // test with defaults-torrc with include and torrc without include + int ret = options_init_from_string(conf_include, conf_empty, + CMD_RUN_UNITTESTS, NULL, &errmsg); + tt_int_op(ret, OP_EQ, 0); + + const or_options_t *options = get_options(); + tt_int_op(options->IncludeUsed, OP_EQ, 0); + + done: + tor_free(errmsg); + tor_free(path); + tor_free(dir); +} + +static void +test_config_dup_and_filter(void *arg) +{ + (void)arg; + /* Test normal input. */ + config_line_t *line = NULL; + config_line_append(&line, "abc", "def"); + config_line_append(&line, "ghi", "jkl"); + config_line_append(&line, "ABCD", "mno"); + + config_line_t *line_dup = config_lines_dup_and_filter(line, "aBc"); + tt_ptr_op(line_dup, OP_NE, NULL); + tt_ptr_op(line_dup->next, OP_NE, NULL); + tt_ptr_op(line_dup->next->next, OP_EQ, NULL); + + tt_str_op(line_dup->key, OP_EQ, "abc"); + tt_str_op(line_dup->value, OP_EQ, "def"); + tt_str_op(line_dup->next->key, OP_EQ, "ABCD"); + tt_str_op(line_dup->next->value, OP_EQ, "mno"); + + /* empty output */ + config_free_lines(line_dup); + line_dup = config_lines_dup_and_filter(line, "skdjfsdkljf"); + tt_ptr_op(line_dup, OP_EQ, NULL); + + /* empty input */ + config_free_lines(line_dup); + line_dup = config_lines_dup_and_filter(NULL, "abc"); + tt_ptr_op(line_dup, OP_EQ, NULL); + + done: + config_free_lines(line); + config_free_lines(line_dup); +} + + /* If we're not configured to be a bridge, but we set + * BridgeDistribution, then options_validate () should return -1. */ + static void + test_config_check_bridge_distribution_setting_not_a_bridge(void *arg) { + or_options_t* options = get_options_mutable(); + or_options_t* old_options = options; + or_options_t* default_options = options; + char* message = (char*)(""); + int ret; + + (void)arg; + + options->BridgeRelay = 0; + options->BridgeDistribution = (char*)("https"); + + ret = options_validate(old_options, options, default_options, 0, &message); + + tt_int_op(ret, OP_EQ, -1); + done: + return; + } + + /* If the BridgeDistribution setting was valid, 0 should be returned. */ + static void + test_config_check_bridge_distribution_setting_valid(void *arg) { + int ret = check_bridge_distribution_setting("https"); + + (void)arg; + + tt_int_op(ret, OP_EQ, 0); + done: + return; + } + + /* If the BridgeDistribution setting was invalid, -1 should be returned. */ + static void + test_config_check_bridge_distribution_setting_invalid(void *arg) { + int ret = check_bridge_distribution_setting("hyphens-are-not-allowed"); + + (void)arg; + + tt_int_op(ret, OP_EQ, -1); + done: + return; + } + + /* If the BridgeDistribution setting was unrecognised, a warning should be + * logged and 0 should be returned. */ + static void + test_config_check_bridge_distribution_setting_unrecognised(void *arg) { + int ret = check_bridge_distribution_setting("unicorn"); + + (void)arg; + + tt_int_op(ret, OP_EQ, 0); + done: + return; + } + #define CONFIG_TEST(name, flags) \ { #name, test_config_ ## name, flags, NULL, NULL } @@@ -5471,24 -4974,11 +5530,28 @@@ struct testcase_t config_tests[] = CONFIG_TEST(parse_port_config__ports__no_ports_given, 0), CONFIG_TEST(parse_port_config__ports__server_options, 0), CONFIG_TEST(parse_port_config__ports__ports_given, 0), + CONFIG_TEST(parse_log_severity, 0), + CONFIG_TEST(include_limit, 0), + CONFIG_TEST(include_does_not_exist, 0), + CONFIG_TEST(include_error_in_included_file, 0), + CONFIG_TEST(include_empty_file_folder, 0), +#ifndef _WIN32 + CONFIG_TEST(include_no_permission, 0), +#endif + CONFIG_TEST(include_recursion_before_after, 0), + CONFIG_TEST(include_recursion_after_only, 0), + CONFIG_TEST(include_folder_order, 0), + CONFIG_TEST(include_path_syntax, 0), + CONFIG_TEST(include_not_processed, 0), + CONFIG_TEST(include_has_include, 0), + CONFIG_TEST(include_flag_both_without, TT_FORK), + CONFIG_TEST(include_flag_torrc_only, TT_FORK), + CONFIG_TEST(include_flag_defaults_only, TT_FORK), + CONFIG_TEST(dup_and_filter, 0), + CONFIG_TEST(check_bridge_distribution_setting_not_a_bridge, TT_FORK), + CONFIG_TEST(check_bridge_distribution_setting_valid, 0), + CONFIG_TEST(check_bridge_distribution_setting_invalid, 0), + CONFIG_TEST(check_bridge_distribution_setting_unrecognised, 0), END_OF_TESTCASES };

1 0

[nyx/master] Fix unit tests
by atagar＠torproject.org 24 Oct '17

24 Oct '17

commit c6c16ebc42908f56d064711492da6787c946c493 Author: Damian Johnson <atagar(a)torproject.org> Date: Tue Oct 24 10:17:43 2017 -0700 Fix unit tests Lots of breakages have slipped in while attempting to improve our performance. Most were timing related due to our switch to static sleeps. --- nyx/__init__.py | 1 + nyx/panel/__init__.py | 5 ++--- nyx/panel/connection.py | 10 +++++++--- nyx/panel/header.py | 4 ++-- nyx/tracker.py | 11 ++++++----- run_tests.py | 2 +- test/panel/connection.py | 4 ++-- test/panel/header.py | 2 +- test/tracker/connection_tracker.py | 8 ++++---- test/tracker/port_usage_tracker.py | 2 +- test/tracker/resource_tracker.py | 2 +- 11 files changed, 28 insertions(+), 23 deletions(-) diff --git a/nyx/__init__.py b/nyx/__init__.py index d9a9b52..5947294 100644 --- a/nyx/__init__.py +++ b/nyx/__init__.py @@ -268,6 +268,7 @@ def our_address(default = None): return CACHED_ADDRESS if CACHED_ADDRESS is not None else default + def show_message(message = None, *attr, **kwargs): """ Shows a message in our header. diff --git a/nyx/panel/__init__.py b/nyx/panel/__init__.py index ed32887..b75964c 100644 --- a/nyx/panel/__init__.py +++ b/nyx/panel/__init__.py @@ -31,10 +31,9 @@ import inspect import threading import time +import nyx import nyx.curses -from nyx import PAUSE_TIME, nyx_interface - __all__ = [ 'config', 'connection', @@ -207,7 +206,7 @@ class DaemonPanel(Panel, threading.Thread): sleep_until = last_ran + self._update_rate + 0.1 while not self._halt and time.time() < sleep_until: - time.sleep(PAUSE_TIME) + time.sleep(nyx.PAUSE_TIME) continue # done waiting, try again diff --git a/nyx/panel/connection.py b/nyx/panel/connection.py index 3fe283b..3002d67 100644 --- a/nyx/panel/connection.py +++ b/nyx/panel/connection.py @@ -11,12 +11,13 @@ import itertools import re import time +import nyx import nyx.curses import nyx.panel import nyx.popups import nyx.tracker -from nyx import PAUSE_TIME, nyx_interface, tor_controller, our_address +from nyx import nyx_interface, tor_controller from nyx.curses import WHITE, NORMAL, BOLD, HIGHLIGHT from nyx.menu import MenuItem, Submenu, RadioMenuItem, RadioGroup @@ -505,7 +506,7 @@ class ConnectionPanel(nyx.panel.DaemonPanel): elif self._halt: return else: - time.sleep(PAUSE_TIME) + time.sleep(nyx.PAUSE_TIME) controller = tor_controller() LAST_RETRIEVED_CIRCUITS = controller.get_circuits([]) @@ -607,7 +608,10 @@ def _draw_line(subwindow, x, y, line, is_selected, width, current_time): def _draw_address_column(subwindow, x, y, line, attr): - src = '%s:%s' % (our_address(line.connection.local_address), line.connection.local_port if line.line_type == LineType.CONNECTION else '') + if line.line_type == LineType.CONNECTION: + src = '%s:%s' % (nyx.our_address(line.connection.local_address), line.connection.local_port) + else: + src = nyx.our_address(line.connection.local_address) if line.line_type == LineType.CIRCUIT_HEADER and line.circuit.status != 'BUILT': dst = 'Building...' diff --git a/nyx/panel/header.py b/nyx/panel/header.py index 3751d56..0820a21 100644 --- a/nyx/panel/header.py +++ b/nyx/panel/header.py @@ -23,7 +23,7 @@ import nyx.popups import nyx.tracker from stem.util import conf, log -from nyx import nyx_interface, tor_controller, our_address +from nyx import nyx_interface, tor_controller from nyx.curses import RED, GREEN, YELLOW, CYAN, WHITE, BOLD, HIGHLIGHT @@ -268,7 +268,7 @@ class Sampling(object): 'version': str(controller.get_version('Unknown')).split()[0], 'version_status': controller.get_info('status/version/current', 'Unknown'), - 'address': or_listeners[0][0] if (or_listeners and or_listeners[0][0] != '0.0.0.0') else our_address('Unknown'), + 'address': or_listeners[0][0] if (or_listeners and or_listeners[0][0] != '0.0.0.0') else nyx.our_address('Unknown'), 'or_port': or_listeners[0][1] if or_listeners else '', 'dir_port': controller.get_conf('DirPort', '0'), 'control_port': str(control_listeners[0][1]) if control_listeners else None, diff --git a/nyx/tracker.py b/nyx/tracker.py index 6d6ab64..fb41f9e 100644 --- a/nyx/tracker.py +++ b/nyx/tracker.py @@ -60,7 +60,7 @@ import stem.control import stem.descriptor.router_status_entry import stem.util.log -from nyx import PAUSE_TIME, tor_controller, our_address +from nyx import tor_controller from stem.util import conf, connection, enum, proc, str_tools, system CONFIG = conf.config_dict('nyx', { @@ -387,10 +387,10 @@ class Daemon(threading.Thread): def run(self): while not self._halt: if self._is_paused or time.time() - self._last_ran < self._rate: - sleep_until = self._last_ran + self._rate + 0.1 + sleep_until = self._last_ran + self._rate while not self._halt and time.time() < sleep_until: - time.sleep(PAUSE_TIME) + time.sleep(nyx.PAUSE_TIME) continue # done waiting, try again @@ -903,7 +903,7 @@ class ConsensusTracker(object): controller = tor_controller() - if our_address() == address: + if nyx.our_address() == address: fingerprint = controller.get_info('fingerprint', None) ports = controller.get_ports(stem.control.Listener.OR, None) @@ -924,9 +924,10 @@ class ConsensusTracker(object): controller = tor_controller() if fingerprint == controller.get_info('fingerprint', None): + my_address = nyx.our_address() my_or_ports = controller.get_ports(stem.control.Listener.OR, []) - if our_address() and len(my_or_ports) == 1: + if my_address and len(my_or_ports) == 1: return (my_address, my_or_ports[0]) return nyx.cache().relay_address(fingerprint, default) diff --git a/run_tests.py b/run_tests.py index d923178..a9dd98a 100755 --- a/run_tests.py +++ b/run_tests.py @@ -30,7 +30,7 @@ SRC_PATHS = [os.path.join(NYX_BASE, path) for path in ( @nyx.uses_settings def main(): - nyx.TESTING = True + nyx.PAUSE_TIME = 0.000001 # make pauses negligibly low since our tests trigger things rapidly test_config = stem.util.conf.get_config('test') test_config.load(os.path.join(NYX_BASE, 'test', 'settings.cfg')) diff --git a/test/panel/connection.py b/test/panel/connection.py index f01d99a..c95f31a 100644 --- a/test/panel/connection.py +++ b/test/panel/connection.py @@ -195,8 +195,8 @@ class TestConnectionPanel(unittest.TestCase): @require_curses @patch('nyx.panel.connection.tor_controller') + @patch('nyx.our_address', Mock(return_value = '82.121.9.9')) def test_draw_line(self, tor_controller_mock): - tor_controller_mock().get_info.return_value = '82.121.9.9' tor_controller_mock().is_geoip_unavailable.return_value = False test_data = (( @@ -222,8 +222,8 @@ class TestConnectionPanel(unittest.TestCase): @require_curses @patch('nyx.panel.connection.tor_controller') + @patch('nyx.our_address', Mock(return_value = '82.121.9.9')) def test_draw_address_column(self, tor_controller_mock): - tor_controller_mock().get_info.return_value = '82.121.9.9' tor_controller_mock().is_geoip_unavailable.return_value = False test_data = (( diff --git a/test/panel/header.py b/test/panel/header.py index 805c8c7..73259da 100644 --- a/test/panel/header.py +++ b/test/panel/header.py @@ -87,6 +87,7 @@ class TestHeaderPanel(unittest.TestCase): @patch('time.time', Mock(return_value = 1234.5)) @patch('os.times', Mock(return_value = (0.08, 0.03, 0.0, 0.0, 18759021.31))) @patch('os.uname', Mock(return_value = ('Linux', 'odin', '3.5.0-54-generic', '#81~precise1-Ubuntu SMP Tue Jul 15 04:05:58 UTC 2014', 'i686'))) + @patch('nyx.our_address', Mock(return_value = '174.21.17.28')) @patch('stem.util.system.start_time', Mock(return_value = 5678)) @patch('stem.util.proc.file_descriptors_used', Mock(return_value = 89)) def test_sample(self, consensus_tracker_mock, resource_tracker_mock, tor_controller_mock): @@ -101,7 +102,6 @@ class TestHeaderPanel(unittest.TestCase): tor_controller_mock().get_info.side_effect = lambda param, default = None: { 'fingerprint': '1A94D1A794FCB2F8B6CBC179EF8FDD4008A98D3B', 'status/version/current': 'recommended', - 'address': '174.21.17.28', 'process/descriptor-limit': 678, }[param] diff --git a/test/tracker/connection_tracker.py b/test/tracker/connection_tracker.py index 39ded42..50faef1 100644 --- a/test/tracker/connection_tracker.py +++ b/test/tracker/connection_tracker.py @@ -49,17 +49,17 @@ class TestConnectionTracker(unittest.TestCase): get_value_mock.side_effect = IOError() with ConnectionTracker(0.01) as daemon: - time.sleep(0.03) + time.sleep(0.015) self.assertEqual([connection.Resolver.NETSTAT, connection.Resolver.LSOF], daemon._resolvers) self.assertEqual([], daemon.get_value()) - time.sleep(0.05) + time.sleep(0.025) self.assertEqual([connection.Resolver.LSOF], daemon._resolvers) self.assertEqual([], daemon.get_value()) - time.sleep(0.05) + time.sleep(0.035) self.assertEqual([], daemon._resolvers) self.assertEqual([], daemon.get_value()) @@ -96,7 +96,7 @@ class TestConnectionTracker(unittest.TestCase): self.assertEqual(1, len(connections)) self.assertEqual(STEM_CONNECTIONS[0].remote_address, connections[0].remote_address) - self.assertTrue(first_start_time < connections[0].start_time < time.time()) + self.assertTrue(first_start_time <= connections[0].start_time <= time.time()) self.assertTrue(connections[0].is_legacy) second_start_time = time.time() diff --git a/test/tracker/port_usage_tracker.py b/test/tracker/port_usage_tracker.py index edf1157..7e69f21 100644 --- a/test/tracker/port_usage_tracker.py +++ b/test/tracker/port_usage_tracker.py @@ -111,7 +111,7 @@ class TestPortUsageTracker(unittest.TestCase): self.assertEqual(0, daemon._failure_count) daemon.query([37277, 51849], []) - time.sleep(0.03) + time.sleep(0.015) self.assertTrue(daemon.is_alive()) time.sleep(0.1) self.assertFalse(daemon.is_alive()) diff --git a/test/tracker/resource_tracker.py b/test/tracker/resource_tracker.py index 3f9d927..25837f1 100644 --- a/test/tracker/resource_tracker.py +++ b/test/tracker/resource_tracker.py @@ -94,7 +94,7 @@ class TestResourceTracker(unittest.TestCase): tor_controller_mock().get_pid.return_value = 12345 with ResourceTracker(0.01) as daemon: - time.sleep(0.03) + time.sleep(0.015) self.assertEqual(True, daemon._use_proc) resources = daemon.get_value()

1 0