From ebab5215252f9fa810ae091c335c5ae6e619faaf Mon Sep 17 00:00:00 2001 From: Roger Dingledine Date: Sun, 13 Nov 2016 02:39:16 -0500 Subject: 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. --- src/or/config.c | 7 +++++++ src/or/or.h | 4 ++++ src/or/router.c | 8 ++++++++ 3 files changed, 19 insertions(+) (limited to 'src') diff --git a/src/or/config.c b/src/or/config.c index 08cc5cdf57..1bdfdf4846 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 33fe8b96c4..3d61cfa051 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 6d3a32a60c..780d0444f3 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), -- cgit v1.2.3-54-g00ecf From 613b18f0afd865b0fc5daf8d17da241396100dcd Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 23 Oct 2017 10:52:42 -0400 Subject: 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(-) (limited to 'src') diff --git a/src/or/config.c b/src/or/config.c index 1bdfdf4846..7f45bb2cba 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 780d0444f3..553264efa8 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); -- cgit v1.2.3-54-g00ecf From b0e10f23ba2b03f275ef4acf2183a02042e6cded Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 23 Oct 2017 19:37:52 +0000 Subject: doc: Improve documentation for check_bridge_distribution_setting(). --- src/or/config.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/or/config.c b/src/or/config.c index 7f45bb2cba..4d08a07957 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) { -- cgit v1.2.3-54-g00ecf From 02cde0d9398c43de3a67133b7982d9df6962fe32 Mon Sep 17 00:00:00 2001 From: Isis Lovecruft Date: Mon, 23 Oct 2017 19:44:06 +0000 Subject: 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(-) create mode 100644 src/test/test_router.c (limited to 'src') diff --git a/src/or/config.c b/src/or/config.c index 4d08a07957..0b1e6bed14 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 6645532514..096937cb6d 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 8ecfaf10c6..6cefc6c5a5 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 9a41b976b8..7337326091 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 89f9b3e2aa..396f06adff 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 0000000000..64434035ef --- /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 +}; -- cgit v1.2.3-54-g00ecf