February 2016 - tor-commits - lists.torproject.org

[tor/master] make check-spaces
by nickm＠torproject.org 11 Feb '16

11 Feb '16

commit 838d4dee121b311647ae4edd7e220436571c449c Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 11 12:50:55 2016 -0500 make check-spaces --- src/or/entrynodes.c | 5 +++-- src/or/or.h | 4 ++-- src/or/policies.c | 15 ++++++++------- src/test/test_policy.c | 1 - 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index a4b9350..95d9fec 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -2124,8 +2124,9 @@ launch_direct_bridge_descriptor_fetch(bridge_info_t *bridge) * it. If we */ if (!fascist_firewall_allows_address_addr(&bridge->addr, bridge->port, FIREWALL_OR_CONNECTION, 0)) { - log_notice(LD_CONFIG, "Tried to fetch a descriptor directly from a bridge, " - "but that bridge is not reachable through our firewall."); + log_notice(LD_CONFIG, "Tried to fetch a descriptor directly from a " + "bridge, but that bridge is not reachable through our " + "firewall."); return; } diff --git a/src/or/or.h b/src/or/or.h index 04bd42a..f438212 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4092,8 +4092,8 @@ typedef struct { int ClientUseIPv6; /** If true, prefer an IPv6 OR port over an IPv4 one for entry node * connections. If auto, bridge clients prefer IPv6, and other clients - * prefer IPv4. Use fascist_firewall_prefer_ipv6_orport() instead of accessing - * this value directly. */ + * prefer IPv4. Use fascist_firewall_prefer_ipv6_orport() instead of + * accessing this value directly. */ int ClientPreferIPv6ORPort; /** If true, prefer an IPv6 directory port over an IPv4 one for direct * directory connections. If auto, bridge clients prefer IPv6, and other diff --git a/src/or/policies.c b/src/or/policies.c index faa39ad..984ab6a 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -421,7 +421,8 @@ fascist_firewall_allows_address(const tor_addr_t *addr, /** Is this client configured to use IPv6? */ -int fascist_firewall_use_ipv6(const or_options_t *options) +int +fascist_firewall_use_ipv6(const or_options_t *options) { /* Clients use IPv6 if it's set, or they use bridges, or they don't use * IPv4 */ @@ -513,14 +514,14 @@ fascist_firewall_allows_address_addr(const tor_addr_t *addr, uint16_t port, if (fw_connection == FIREWALL_OR_CONNECTION) { return fascist_firewall_allows_address(addr, port, - reachable_or_addr_policy, - pref_only, - fascist_firewall_prefer_ipv6_orport(options)); + reachable_or_addr_policy, + pref_only, + fascist_firewall_prefer_ipv6_orport(options)); } else if (fw_connection == FIREWALL_DIR_CONNECTION) { return fascist_firewall_allows_address(addr, port, - reachable_dir_addr_policy, - pref_only, - fascist_firewall_prefer_ipv6_dirport(options)); + reachable_dir_addr_policy, + pref_only, + fascist_firewall_prefer_ipv6_dirport(options)); } else { log_warn(LD_BUG, "Bad firewall_connection_t value %d.", fw_connection); diff --git a/src/test/test_policy.c b/src/test/test_policy.c index c044d9f..3688909 100644 --- a/src/test/test_policy.c +++ b/src/test/test_policy.c @@ -1546,7 +1546,6 @@ test_policies_fascist_firewall_choose_address(void *arg) FIREWALL_DIR_CONNECTION, 1) == &ipv6_dir_ap); - /* In the default configuration (Auto / IPv6 off), bridge clients should * still use IPv6, and only prefer it for bridges configured with an IPv6 * address, regardless of ClientUseIPv6. */

1 0

[tor/master] Add a helper to search for strings in the log, and change option tests to use this helper instead of looking at specific indices in the log list
by nickm＠torproject.org 11 Feb '16

11 Feb '16

commit 8627a40fbab223386c9d13bb63e4e5d52a795286 Author: Ola Bini <ola(a)olabini.se> Date: Fri Jan 29 11:38:54 2016 -0500 Add a helper to search for strings in the log, and change option tests to use this helper instead of looking at specific indices in the log list --- src/test/log_test_helpers.c | 17 +++++ src/test/log_test_helpers.h | 1 + src/test/test_options.c | 159 ++++++++++++++++++++++---------------------- 3 files changed, 97 insertions(+), 80 deletions(-) diff --git a/src/test/log_test_helpers.c b/src/test/log_test_helpers.c index 51b5f9b..88d28e1 100644 --- a/src/test/log_test_helpers.c +++ b/src/test/log_test_helpers.c @@ -83,6 +83,23 @@ mock_saved_logs(void) return saved_logs; } +int +mock_saved_log_has_message(const char *msg) +{ + int has_msg = 0; + if (saved_logs) { + SMARTLIST_FOREACH(saved_logs, mock_saved_log_entry_t *, m, + { + if (msg && m->generated_msg && + !strcmp(msg, m->generated_msg)) { + has_msg = 1; + } + }); + } + + return has_msg; +} + void mock_saving_logv(int severity, log_domain_mask_t domain, const char *funcname, const char *suffix, diff --git a/src/test/log_test_helpers.h b/src/test/log_test_helpers.h index af8e8a6..3a565c6 100644 --- a/src/test/log_test_helpers.h +++ b/src/test/log_test_helpers.h @@ -26,6 +26,7 @@ void teardown_capture_of_logs(int prev); const char *mock_saved_log_at(int ix); int mock_saved_severity_at(int ix); int mock_saved_log_number(void); +int mock_saved_log_has_message(const char *msg); #endif diff --git a/src/test/test_options.c b/src/test/test_options.c index e00c802..275fee2 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -367,6 +367,14 @@ free_options_test_data(options_test_data_t *td) tor_free(td); } +#define expect_log_msg(str) \ + tt_assert_msg(mock_saved_log_has_message(str), \ + "expected log to contain " # str); + +#define expect_no_log_msg(str) \ + tt_assert_msg(!mock_saved_log_has_message(str), \ + "expected log to not contain " # str); + static void test_options_validate__uname_for_server(void *ignored) { @@ -379,7 +387,7 @@ test_options_validate__uname_for_server(void *ignored) MOCK(get_uname, fixed_get_uname); fixed_get_uname_result = "Windows 95"; options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(0), OP_EQ, "Tor is running as a server, but you" + expect_log_msg("Tor is running as a server, but you" " are running Windows 95; this probably won't work. See https://www" ".torproject.org/docs/faq.html#BestOSForRelay for details.\n"); tor_free(msg); @@ -387,7 +395,7 @@ test_options_validate__uname_for_server(void *ignored) fixed_get_uname_result = "Windows 98"; mock_clean_saved_logs(); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(0), OP_EQ, "Tor is running as a server, but you" + expect_log_msg("Tor is running as a server, but you" " are running Windows 98; this probably won't work. See https://www" ".torproject.org/docs/faq.html#BestOSForRelay for details.\n"); tor_free(msg); @@ -395,7 +403,7 @@ test_options_validate__uname_for_server(void *ignored) fixed_get_uname_result = "Windows Me"; mock_clean_saved_logs(); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(0), OP_EQ, "Tor is running as a server, but you" + expect_log_msg("Tor is running as a server, but you" " are running Windows Me; this probably won't work. See https://www" ".torproject.org/docs/faq.html#BestOSForRelay for details.\n"); tor_free(msg); @@ -512,7 +520,7 @@ test_options_validate__contactinfo(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(mock_saved_log_at(0), OP_EQ, + expect_log_msg( "Your ContactInfo config option is not" " set. Please consider setting it, so we can contact you if your" " server is misconfigured or something else goes wrong.\n"); @@ -524,7 +532,7 @@ test_options_validate__contactinfo(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(mock_saved_log_at(0), OP_NE, + expect_no_log_msg( "Your ContactInfo config option is not" " set. Please consider setting it, so we can contact you if your" " server is misconfigured or something else goes wrong.\n"); @@ -632,7 +640,7 @@ test_options_validate__authdir(void *ignored) tt_int_op(ret, OP_EQ, -1); tt_str_op(msg, OP_EQ, "Failed to resolve/guess local address. See logs for" " details."); - tt_str_op(mock_saved_log_at(0), OP_EQ, "Could not resolve local Address " + expect_log_msg("Could not resolve local Address " "'this.should.not_exist.example.org'. Failing.\n"); tor_free(msg); @@ -763,7 +771,7 @@ test_options_validate__authdir(void *ignored) "SchedulerLowWaterMark__ 10\n"); mock_clean_saved_logs(); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(0), OP_EQ, "Authoritative directory servers " + expect_log_msg("Authoritative directory servers " "can't set UseEntryGuards. Disabling.\n"); tt_int_op(tdata->opt->UseEntryGuards, OP_EQ, 0); tor_free(msg); @@ -777,7 +785,7 @@ test_options_validate__authdir(void *ignored) "SchedulerLowWaterMark__ 10\n"); mock_clean_saved_logs(); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(0), OP_EQ, "Authoritative directories always try" + expect_log_msg("Authoritative directories always try" " to download extra-info documents. Setting DownloadExtraInfo.\n"); tt_int_op(tdata->opt->DownloadExtraInfo, OP_EQ, 1); tor_free(msg); @@ -792,7 +800,7 @@ test_options_validate__authdir(void *ignored) "SchedulerLowWaterMark__ 10\n"); mock_clean_saved_logs(); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(0), OP_NE, "Authoritative directories always try" + expect_no_log_msg("Authoritative directories always try" " to download extra-info documents. Setting DownloadExtraInfo.\n"); tt_int_op(tdata->opt->DownloadExtraInfo, OP_EQ, 1); tor_free(msg); @@ -935,7 +943,7 @@ test_options_validate__relay_with_hidden_services(void *ignored) ); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(1), OP_EQ, + expect_log_msg( "Tor is currently configured as a relay and a hidden service. " "That's not very secure: you should probably run your hidden servi" "ce in a separate Tor process, at least -- see " @@ -1145,7 +1153,7 @@ test_options_validate__exclude_nodes(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(mock_saved_log_at(0), OP_EQ, + expect_log_msg( "You have asked to exclude certain relays from all positions " "in your circuits. Expect hidden services and other Tor " "features to be broken in unpredictable ways.\n"); @@ -1158,7 +1166,7 @@ test_options_validate__exclude_nodes(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(mock_saved_log_at(0), OP_NE, + expect_no_log_msg( "You have asked to exclude certain relays from all positions " "in your circuits. Expect hidden services and other Tor " "features to be broken in unpredictable ways.\n"); @@ -1183,8 +1191,7 @@ test_options_validate__scheduler(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(mock_saved_log_at(0), OP_EQ, - "Bad SchedulerLowWaterMark__ option\n"); + expect_log_msg("Bad SchedulerLowWaterMark__ option\n"); tor_free(msg); // TODO: this test cannot run on platforms where UINT32_MAX == UINT64_MAX. @@ -1207,8 +1214,7 @@ test_options_validate__scheduler(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(mock_saved_log_at(0), OP_EQ, - "Bad SchedulerHighWaterMark option\n"); + expect_log_msg("Bad SchedulerHighWaterMark option\n"); tor_free(msg); done: @@ -1283,8 +1289,7 @@ test_options_validate__tlsec(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(mock_saved_log_at(0), OP_EQ, - "Unrecognized TLSECGroup: Falling back to the default.\n"); + expect_log_msg("Unrecognized TLSECGroup: Falling back to the default.\n"); tt_assert(!tdata->opt->TLSECGroup); tor_free(msg); @@ -1295,7 +1300,7 @@ test_options_validate__tlsec(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(mock_saved_log_at(0), OP_NE, + expect_no_log_msg( "Unrecognized TLSECGroup: Falling back to the default.\n"); tt_assert(tdata->opt->TLSECGroup); tor_free(msg); @@ -1307,7 +1312,7 @@ test_options_validate__tlsec(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(mock_saved_log_at(0), OP_NE, + expect_no_log_msg( "Unrecognized TLSECGroup: Falling back to the default.\n"); tt_assert(tdata->opt->TLSECGroup); tor_free(msg); @@ -1360,8 +1365,7 @@ test_options_validate__recommended_packages(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_int_op(mock_saved_log_number(), OP_EQ, 1); - tt_str_op(mock_saved_log_at(0), OP_NE, "Invalid RecommendedPackage line " + expect_no_log_msg("Invalid RecommendedPackage line " "invalid-package-line will be ignored\n"); done: @@ -1455,9 +1459,7 @@ test_options_validate__paths_needed(void *ignored) tt_int_op(ret, OP_EQ, -1); tt_assert(tdata->opt->PathsNeededToBuildCircuits > 0.24 && tdata->opt->PathsNeededToBuildCircuits < 0.26); - tt_int_op(mock_saved_log_number(), OP_EQ, 1); - tt_str_op(mock_saved_log_at(0), OP_EQ, - "PathsNeededToBuildCircuits is too low. Increasing to 0.25\n"); + expect_log_msg("PathsNeededToBuildCircuits is too low. Increasing to 0.25\n"); tor_free(msg); free_options_test_data(tdata); @@ -1471,8 +1473,7 @@ test_options_validate__paths_needed(void *ignored) tt_int_op(ret, OP_EQ, -1); tt_assert(tdata->opt->PathsNeededToBuildCircuits > 0.94 && tdata->opt->PathsNeededToBuildCircuits < 0.96); - tt_int_op(mock_saved_log_number(), OP_EQ, 1); - tt_str_op(mock_saved_log_at(0), OP_EQ, "PathsNeededToBuildCircuits is " + expect_log_msg("PathsNeededToBuildCircuits is " "too high. Decreasing to 0.95\n"); tor_free(msg); @@ -1638,11 +1639,10 @@ test_options_validate__reachable_addresses(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_int_op(mock_saved_log_number(), OP_EQ, 6); - tt_str_op(mock_saved_log_at(1), OP_EQ, "Converting FascistFirewall config " + expect_log_msg("Converting FascistFirewall config " "option to new format: \"ReachableDirAddresses *:80\"\n"); tt_str_op(tdata->opt->ReachableDirAddresses->value, OP_EQ, "*:80"); - tt_str_op(mock_saved_log_at(2), OP_EQ, "Converting FascistFirewall config " + expect_log_msg("Converting FascistFirewall config " "option to new format: \"ReachableORAddresses *:443\"\n"); tt_str_op(tdata->opt->ReachableORAddresses->value, OP_EQ, "*:443"); tor_free(msg); @@ -1675,8 +1675,7 @@ test_options_validate__reachable_addresses(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_int_op(mock_saved_log_number(), OP_EQ, 5); - tt_str_op(mock_saved_log_at(1), OP_EQ, "Converting FascistFirewall and " + expect_log_msg("Converting FascistFirewall and " "FirewallPorts config options to new format: " "\"ReachableAddresses *:123\"\n"); tt_str_op(tdata->opt->ReachableAddresses->value, OP_EQ, "*:123"); @@ -2056,7 +2055,7 @@ test_options_validate__publish_server_descriptor(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(mock_saved_log_at(1), OP_EQ, "Can't set a DirPort on a bridge " + expect_log_msg("Can't set a DirPort on a bridge " "relay; disabling DirPort\n"); tt_assert(!tdata->opt->DirPort_lines); tt_assert(!tdata->opt->DirPort_set); @@ -2158,7 +2157,7 @@ test_options_validate__hidserv(void *ignored) tdata->opt->MinUptimeHidServDirectoryV2 = -1; ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(1), OP_EQ, "MinUptimeHidServDirectoryV2 " + expect_log_msg("MinUptimeHidServDirectoryV2 " "option must be at least 0 seconds. Changing to 0.\n"); tt_int_op(tdata->opt->MinUptimeHidServDirectoryV2, OP_EQ, 0); tor_free(msg); @@ -2169,7 +2168,7 @@ test_options_validate__hidserv(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(1), OP_EQ, "RendPostPeriod option is too short;" + expect_log_msg("RendPostPeriod option is too short;" " raising to 600 seconds.\n"); tt_int_op(tdata->opt->RendPostPeriod, OP_EQ, 600); tor_free(msg); @@ -2180,7 +2179,7 @@ test_options_validate__hidserv(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(1), OP_EQ, "RendPostPeriod is too large; " + expect_log_msg("RendPostPeriod is too large; " "clipping to 302400s.\n"); tt_int_op(tdata->opt->RendPostPeriod, OP_EQ, 302400); tor_free(msg); @@ -2206,7 +2205,7 @@ test_options_validate__predicted_ports(void *ignored) TEST_OPTIONS_DEFAULT_VALUES); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(1), OP_EQ, "PredictedPortsRelevanceTime is too " + expect_log_msg("PredictedPortsRelevanceTime is too " "large; clipping to 3600s.\n"); tt_int_op(tdata->opt->PredictedPortsRelevanceTime, OP_EQ, 3600); @@ -2433,7 +2432,7 @@ test_options_validate__circuits(void *ignored) tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES "MaxCircuitDirtiness 2592001\n"); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(1), OP_EQ, "MaxCircuitDirtiness option is too " + expect_log_msg("MaxCircuitDirtiness option is too " "high; setting to 30 days.\n"); tt_int_op(tdata->opt->MaxCircuitDirtiness, OP_EQ, 2592000); tor_free(msg); @@ -2443,7 +2442,7 @@ test_options_validate__circuits(void *ignored) tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES "CircuitStreamTimeout 1\n"); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(2), OP_EQ, "CircuitStreamTimeout option is too" + expect_log_msg("CircuitStreamTimeout option is too" " short; raising to 10 seconds.\n"); tt_int_op(tdata->opt->CircuitStreamTimeout, OP_EQ, 10); tor_free(msg); @@ -2453,7 +2452,7 @@ test_options_validate__circuits(void *ignored) tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES "CircuitStreamTimeout 111\n"); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(2), OP_NE, "CircuitStreamTimeout option is too" + expect_no_log_msg("CircuitStreamTimeout option is too" " short; raising to 10 seconds.\n"); tt_int_op(tdata->opt->CircuitStreamTimeout, OP_EQ, 111); tor_free(msg); @@ -2463,7 +2462,7 @@ test_options_validate__circuits(void *ignored) tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES "HeartbeatPeriod 1\n"); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(2), OP_EQ, "HeartbeatPeriod option is too short;" + expect_log_msg("HeartbeatPeriod option is too short;" " raising to 1800 seconds.\n"); tt_int_op(tdata->opt->HeartbeatPeriod, OP_EQ, 1800); tor_free(msg); @@ -2473,7 +2472,7 @@ test_options_validate__circuits(void *ignored) tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES "HeartbeatPeriod 1982\n"); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(2), OP_NE, "HeartbeatPeriod option is too short;" + expect_no_log_msg("HeartbeatPeriod option is too short;" " raising to 1800 seconds.\n"); tt_int_op(tdata->opt->HeartbeatPeriod, OP_EQ, 1982); tor_free(msg); @@ -2484,7 +2483,7 @@ test_options_validate__circuits(void *ignored) "CircuitBuildTimeout 1\n" ); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(1), OP_EQ, "CircuitBuildTimeout is shorter (1" + expect_log_msg("CircuitBuildTimeout is shorter (1" " seconds) than the recommended minimum (10 seconds), and " "LearnCircuitBuildTimeout is disabled. If tor isn't working, " "raise this value or enable LearnCircuitBuildTimeout.\n"); @@ -2496,7 +2495,7 @@ test_options_validate__circuits(void *ignored) "CircuitBuildTimeout 11\n" ); options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); - tt_str_op(mock_saved_log_at(1), OP_NE, "CircuitBuildTimeout is shorter (1 " + expect_no_log_msg("CircuitBuildTimeout is shorter (1 " "seconds) than the recommended minimum (10 seconds), and " "LearnCircuitBuildTimeout is disabled. If tor isn't working, " "raise this value or enable LearnCircuitBuildTimeout.\n"); @@ -2590,7 +2589,7 @@ test_options_validate__rend(void *ignored) ); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(1), OP_EQ, "UseEntryGuards is disabled, but you" + expect_log_msg("UseEntryGuards is disabled, but you" " have configured one or more hidden services on this Tor " "instance. Your hidden services will be very easy to locate using" " a well-known attack -- see http://freehaven.net/anonbib/#hs-" @@ -2607,7 +2606,7 @@ test_options_validate__rend(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(1), OP_NE, "UseEntryGuards is disabled, but you" + expect_no_log_msg("UseEntryGuards is disabled, but you" " have configured one or more hidden services on this Tor " "instance. Your hidden services will be very easy to locate using" " a well-known attack -- see http://freehaven.net/anonbib/#hs-" @@ -2710,7 +2709,7 @@ test_options_validate__accounting(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(4), OP_EQ, "Using accounting with a hidden " + expect_log_msg("Using accounting with a hidden " "service and an ORPort is risky: your hidden service(s) and " "your public address will all turn off at the same time, " "which may alert observers that they are being run by the " @@ -2727,7 +2726,7 @@ test_options_validate__accounting(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(4), OP_NE, "Using accounting with a hidden " + expect_no_log_msg("Using accounting with a hidden " "service and an ORPort is risky: your hidden service(s) and " "your public address will all turn off at the same time, " "which may alert observers that they are being run by the " @@ -2746,7 +2745,7 @@ test_options_validate__accounting(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(3), OP_EQ, "Using accounting with multiple " + expect_log_msg("Using accounting with multiple " "hidden services is risky: they will all turn off at the same" " time, which may alert observers that they are being run by " "the same party.\n"); @@ -2957,7 +2956,7 @@ test_options_validate__proxy(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_EQ, "HTTPProxy configured, but no SOCKS " + expect_log_msg("HTTPProxy configured, but no SOCKS " "proxy or HTTPS proxy configured. Watch out: this configuration " "will proxy unencrypted directory connections only.\n"); tor_free(msg); @@ -2970,7 +2969,7 @@ test_options_validate__proxy(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, "HTTPProxy configured, but no SOCKS " + expect_no_log_msg("HTTPProxy configured, but no SOCKS " "proxy or HTTPS proxy configured. Watch out: this configuration " "will proxy unencrypted directory connections only.\n"); tor_free(msg); @@ -2983,7 +2982,7 @@ test_options_validate__proxy(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, "HTTPProxy configured, but no SOCKS " + expect_no_log_msg("HTTPProxy configured, but no SOCKS " "proxy or HTTPS proxy configured. Watch out: this configuration " "will proxy unencrypted directory connections only.\n"); tor_free(msg); @@ -2996,7 +2995,7 @@ test_options_validate__proxy(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, + expect_no_log_msg( "HTTPProxy configured, but no SOCKS proxy or HTTPS proxy " "configured. Watch out: this configuration will proxy " "unencrypted directory connections only.\n"); @@ -3166,7 +3165,7 @@ test_options_validate__control(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_EQ, + expect_log_msg( "ControlPort is open, but no authentication method has been " "configured. This means that any program on your computer can " "reconfigure your Tor. That's bad! You should upgrade your Tor" @@ -3182,7 +3181,7 @@ test_options_validate__control(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, + expect_no_log_msg( "ControlPort is open, but no authentication method has been " "configured. This means that any program on your computer can " "reconfigure your Tor. That's bad! You should upgrade your Tor " @@ -3199,7 +3198,7 @@ test_options_validate__control(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, + expect_no_log_msg( "ControlPort is open, but no authentication method has been " "configured. This means that any program on your computer can " "reconfigure your Tor. That's bad! You should upgrade your Tor " @@ -3214,7 +3213,7 @@ test_options_validate__control(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, + expect_no_log_msg( "ControlPort is open, but no authentication method has been " "configured. This means that any program on your computer can " "reconfigure your Tor. That's bad! You should upgrade your Tor " @@ -3229,7 +3228,7 @@ test_options_validate__control(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_EQ, + expect_log_msg( "ControlSocket is world writable, but no authentication method has" " been configured. This means that any program on your computer " "can reconfigure your Tor. That's bad! You should upgrade your " @@ -3245,7 +3244,7 @@ test_options_validate__control(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, + expect_no_log_msg( "ControlSocket is world writable, but no authentication method has" " been configured. This means that any program on your computer " "can reconfigure your Tor. That's bad! You should upgrade your " @@ -3262,7 +3261,7 @@ test_options_validate__control(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, + expect_no_log_msg( "ControlSocket is world writable, but no authentication method has" " been configured. This means that any program on your computer " "can reconfigure your Tor. That's bad! You should upgrade your " @@ -3277,7 +3276,7 @@ test_options_validate__control(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, + expect_no_log_msg( "ControlSocket is world writable, but no authentication method has" " been configured. This means that any program on your computer " "can reconfigure your Tor. That's bad! You should upgrade your " @@ -3292,7 +3291,7 @@ test_options_validate__control(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_EQ, + expect_log_msg( "CookieAuthFileGroupReadable is set, but will have no effect: you " "must specify an explicit CookieAuthFile to have it " "group-readable.\n"); @@ -3306,7 +3305,7 @@ test_options_validate__control(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, + expect_no_log_msg( "CookieAuthFileGroupReadable is set, but will have no effect: you " "must specify an explicit CookieAuthFile to have it " "group-readable.\n"); @@ -3343,7 +3342,7 @@ test_options_validate__families(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_EQ, + expect_log_msg( "Listing a family for a bridge relay is not supported: it can " "reveal bridge fingerprints to censors. You should also make sure " "you aren't listing this bridge's fingerprint in any other " @@ -3357,7 +3356,7 @@ test_options_validate__families(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, + expect_no_log_msg( "Listing a family for a bridge relay is not supported: it can " "reveal bridge fingerprints to censors. You should also make sure " "you aren't listing this bridge's fingerprint in any other " @@ -3438,7 +3437,7 @@ test_options_validate__dir_auth(void *ignored) tt_str_op(msg, OP_EQ, "Directory authority/fallback line did not parse. See logs for " "details."); - tt_str_op(mock_saved_log_at(2), OP_EQ, + expect_log_msg( "You cannot set both DirAuthority and Alternate*Authority.\n"); tor_free(msg); @@ -3524,7 +3523,7 @@ test_options_validate__transport(void *ignored) tt_int_op(ret, OP_EQ, -1); tt_str_op(msg, OP_EQ, "Invalid client transport line. See logs for details."); - tt_str_op(mock_saved_log_at(3), OP_EQ, + expect_log_msg( "Too few arguments on ClientTransportPlugin line.\n"); tor_free(msg); @@ -3546,7 +3545,7 @@ test_options_validate__transport(void *ignored) tt_int_op(ret, OP_EQ, -1); tt_str_op(msg, OP_EQ, "Invalid server transport line. See logs for details."); - tt_str_op(mock_saved_log_at(3), OP_EQ, + expect_log_msg( "Too few arguments on ServerTransportPlugin line.\n"); tor_free(msg); @@ -3557,7 +3556,7 @@ test_options_validate__transport(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(3), OP_EQ, + expect_log_msg( "Tor is not configured as a relay but you specified a " "ServerTransportPlugin line (\"foo exec bar\"). The " "ServerTransportPlugin line will be ignored.\n"); @@ -3575,7 +3574,7 @@ test_options_validate__transport(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(3), OP_NE, + expect_no_log_msg( "Tor is not configured as a relay but you specified a " "ServerTransportPlugin line (\"foo exec bar\"). The " "ServerTransportPlugin line will be ignored.\n"); @@ -3599,7 +3598,7 @@ test_options_validate__transport(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(3), OP_EQ, + expect_log_msg( "You need at least a single managed-proxy to specify a transport " "listen address. The ServerTransportListenAddr line will be " "ignored.\n"); @@ -3618,7 +3617,7 @@ test_options_validate__transport(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(3), OP_NE, + expect_no_log_msg( "You need at least a single managed-proxy to specify a transport " "listen address. The ServerTransportListenAddr line will be " "ignored.\n"); @@ -3687,7 +3686,7 @@ test_options_validate__constrained_sockets(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_EQ, "You have requested constrained " + expect_log_msg("You have requested constrained " "socket buffers while also serving directory entries via DirPort." " It is strongly suggested that you disable serving directory" " requests when system TCP buffer resources are scarce.\n"); @@ -3701,7 +3700,7 @@ test_options_validate__constrained_sockets(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, + expect_no_log_msg( "You have requested constrained socket buffers while also serving" " directory entries via DirPort. It is strongly suggested that " "you disable serving directory requests when system TCP buffer " @@ -3824,7 +3823,7 @@ test_options_validate__v3_auth(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_EQ, "V3AuthVotingInterval does not divide" + expect_log_msg("V3AuthVotingInterval does not divide" " evenly into 24 hours.\n"); tor_free(msg); @@ -3837,7 +3836,7 @@ test_options_validate__v3_auth(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, "V3AuthVotingInterval does not divide" + expect_no_log_msg("V3AuthVotingInterval does not divide" " evenly into 24 hours.\n"); tor_free(msg); @@ -3852,7 +3851,7 @@ test_options_validate__v3_auth(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(3), OP_EQ, "V3AuthVotingInterval is very low. " + expect_log_msg("V3AuthVotingInterval is very low. " "This may lead to failure to synchronise for a consensus.\n"); tor_free(msg); @@ -3985,7 +3984,7 @@ test_options_validate__exits(void *ignored) ); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_EQ, "You have set AllowSingleHopExits; " + expect_log_msg("You have set AllowSingleHopExits; " "now your relay will allow others to make one-hop exits. However," " since by default most clients avoid relays that set this option," " most clients will ignore you.\n"); @@ -3999,7 +3998,7 @@ test_options_validate__exits(void *ignored) mock_clean_saved_logs(); ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, 0); - tt_str_op(mock_saved_log_at(2), OP_NE, "You have set AllowSingleHopExits; " + expect_no_log_msg("You have set AllowSingleHopExits; " "now your relay will allow others to make one-hop exits. However," " since by default most clients avoid relays that set this option," " most clients will ignore you.\n"); @@ -4044,7 +4043,7 @@ test_options_validate__testing_options(void *ignored) mock_clean_saved_logs(); \ ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg);\ tt_int_op(ret, OP_EQ, 0); \ - tt_str_op(mock_saved_log_at(3), OP_EQ, #name " is insanely high.\n"); \ + expect_log_msg( #name " is insanely high.\n"); \ tor_free(msg); \ STMT_END

1 0

[tor/master] Fix existing options_validate unit tests for ClientUseIPv4
by nickm＠torproject.org 11 Feb '16

11 Feb '16

commit 13db39b8563b42cdf47b1feb546d33217c30c824 Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Fri Jan 29 08:13:11 2016 +1100 Fix existing options_validate unit tests for ClientUseIPv4 --- src/test/test_options.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index 275fee2..35f9f67 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -327,6 +327,7 @@ fixed_get_uname(void) "V3AuthVoteDelay 20\n" \ "V3AuthDistDelay 20\n" \ "V3AuthNIntervalsValid 3\n" \ + "ClientUseIPv4 1\n" \ "VirtualAddrNetworkIPv4 127.192.0.0/10\n" \ "VirtualAddrNetworkIPv6 [FE80::]/10\n" \ "SchedulerHighWaterMark__ 42\n" \ @@ -1698,6 +1699,10 @@ test_options_validate__reachable_addresses(void *ignored) tt_str_op(tdata->opt->ReachableAddresses->value, OP_EQ, "*:82"); tor_free(msg); +#define SERVERS_REACHABLE_MSG "Servers must be able to freely connect to" \ + " the rest of the Internet, so they must not set Reachable*Addresses or" \ + " FascistFirewall or FirewallPorts or ClientUseIPv4 0." + free_options_test_data(tdata); tdata = get_options_test_data("ReachableAddresses *:82\n" "ORListenAddress 127.0.0.1:5555\n" @@ -1709,9 +1714,7 @@ test_options_validate__reachable_addresses(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "Servers must be able to freely connect to the rest of" - " the Internet, so they must not set Reachable*Addresses or" - " FascistFirewall."); + tt_str_op(msg, OP_EQ, SERVERS_REACHABLE_MSG); tor_free(msg); free_options_test_data(tdata); @@ -1725,9 +1728,7 @@ test_options_validate__reachable_addresses(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "Servers must be able to freely connect to the rest of" - " the Internet, so they must not set Reachable*Addresses or" - " FascistFirewall."); + tt_str_op(msg, OP_EQ, SERVERS_REACHABLE_MSG); tor_free(msg); free_options_test_data(tdata); @@ -1741,9 +1742,7 @@ test_options_validate__reachable_addresses(void *ignored) ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); tt_int_op(ret, OP_EQ, -1); - tt_str_op(msg, OP_EQ, "Servers must be able to freely connect to the rest of" - " the Internet, so they must not set Reachable*Addresses or" - " FascistFirewall."); + tt_str_op(msg, OP_EQ, SERVERS_REACHABLE_MSG); tor_free(msg); done: @@ -1760,6 +1759,7 @@ test_options_validate__use_bridges(void *ignored) char *msg; options_test_data_t *tdata = get_options_test_data( "UseBridges 1\n" + "ClientUseIPv4 1\n" "ORListenAddress 127.0.0.1:5555\n" "ORPort 955\n" "MaxClientCircuitsPending 1\n"

1 0

[tor/master] Report malformed options in options_validate unit tests
by nickm＠torproject.org 11 Feb '16

11 Feb '16

commit 26f68a771c2d3df12d0dce20f37ee6549e16c920 Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Fri Jan 29 08:15:14 2016 +1100 Report malformed options in options_validate unit tests --- src/test/test_options.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/src/test/test_options.c b/src/test/test_options.c index 35f9f67..eb2d332 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -339,22 +339,47 @@ typedef struct { or_options_t *def_opt; } options_test_data_t; +static void free_options_test_data(options_test_data_t *td); + static options_test_data_t * get_options_test_data(const char *conf) { + int rv = -1; + char *msg = NULL; config_line_t *cl=NULL; options_test_data_t *result = tor_malloc(sizeof(options_test_data_t)); result->opt = options_new(); result->old_opt = options_new(); result->def_opt = options_new(); - config_get_lines(conf, &cl, 1); - config_assign(&options_format, result->opt, cl, 0, 0, NULL); + rv = config_get_lines(conf, &cl, 1); + tt_assert(rv == 0); + rv = config_assign(&options_format, result->opt, cl, 0, 0, &msg); + if (msg) { + /* Display the parse error message by comparing it with an empty string */ + tt_str_op(msg, OP_EQ, ""); + } + tt_assert(rv == 0); config_free_lines(cl); result->opt->LogTimeGranularity = 1; result->opt->TokenBucketRefillInterval = 1; - config_get_lines(TEST_OPTIONS_OLD_VALUES, &cl, 1); - config_assign(&options_format, result->def_opt, cl, 0, 0, NULL); + rv = config_get_lines(TEST_OPTIONS_OLD_VALUES, &cl, 1); + tt_assert(rv == 0); + rv = config_assign(&options_format, result->def_opt, cl, 0, 0, &msg); + if (msg) { + /* Display the parse error message by comparing it with an empty string */ + tt_str_op(msg, OP_EQ, ""); + } + tt_assert(rv == 0); + +done: config_free_lines(cl); + if (rv != 0) { + free_options_test_data(result); + result = NULL; + /* Callers expect a non-NULL result, so just die if we can't provide one. + */ + tor_assert(0); + } return result; }

1 0

[tor/master] Add unit tests for ClientUseIPv[4, 6] and ClientPreferIPv6[OR, Dir]Port
by nickm＠torproject.org 11 Feb '16

11 Feb '16

commit 1dae4dac12de391a7aea7b375628a7898168cc12 Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Fri Jan 29 09:12:07 2016 +1100 Add unit tests for ClientUseIPv[4,6] and ClientPreferIPv6[OR,Dir]Port --- src/test/test_options.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) diff --git a/src/test/test_options.c b/src/test/test_options.c index eb2d332..e15c881 100644 --- a/src/test/test_options.c +++ b/src/test/test_options.c @@ -1770,6 +1770,104 @@ test_options_validate__reachable_addresses(void *ignored) tt_str_op(msg, OP_EQ, SERVERS_REACHABLE_MSG); tor_free(msg); + free_options_test_data(tdata); + tdata = get_options_test_data("ClientUseIPv4 0\n" + "ORListenAddress 127.0.0.1:5555\n" + "ORPort 955\n" + "MaxClientCircuitsPending 1\n" + "ConnLimit 1\n" + "SchedulerHighWaterMark__ 42\n" + "SchedulerLowWaterMark__ 10\n"); + + ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); + tt_int_op(ret, OP_EQ, -1); + tt_str_op(msg, OP_EQ, SERVERS_REACHABLE_MSG); + tor_free(msg); + + /* Test IPv4-only clients setting IPv6 preferences */ + +#define WARN_PLEASE_USE_IPV6_OR_LOG_MSG \ + "ClientPreferIPv6ORPort 1 is ignored unless tor is using IPv6. " \ + "Please set ClientUseIPv6 1, ClientUseIPv4 0, or configure bridges.\n" + +#define WARN_PLEASE_USE_IPV6_DIR_LOG_MSG \ + "ClientPreferIPv6DirPort 1 is ignored unless tor is using IPv6. " \ + "Please set ClientUseIPv6 1, ClientUseIPv4 0, or configure bridges.\n" + + free_options_test_data(tdata); + tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES + "ClientUseIPv4 1\n" + "ClientUseIPv6 0\n" + "UseBridges 0\n" + "ClientPreferIPv6ORPort 1\n"); + + ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); + tt_int_op(ret, OP_EQ, 0); + expect_log_msg(WARN_PLEASE_USE_IPV6_OR_LOG_MSG); + tor_free(msg); + + free_options_test_data(tdata); + tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES + "ClientUseIPv4 1\n" + "ClientUseIPv6 0\n" + "UseBridges 0\n" + "ClientPreferIPv6DirPort 1\n"); + + ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); + tt_int_op(ret, OP_EQ, 0); + expect_log_msg(WARN_PLEASE_USE_IPV6_DIR_LOG_MSG); + tor_free(msg); + + /* Now test an IPv4/IPv6 client setting IPv6 preferences */ + + free_options_test_data(tdata); + tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES + "ClientUseIPv4 1\n" + "ClientUseIPv6 1\n" + "ClientPreferIPv6ORPort 1\n" + "ClientPreferIPv6DirPort 1\n"); + + ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); + tt_int_op(ret, OP_EQ, 0); + tt_ptr_op(msg, OP_EQ, NULL); + + /* Now test an IPv6 client setting IPv6 preferences */ + + free_options_test_data(tdata); + tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES + "ClientUseIPv6 1\n" + "ClientPreferIPv6ORPort 1\n" + "ClientPreferIPv6DirPort 1\n"); + + ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); + tt_int_op(ret, OP_EQ, 0); + tt_ptr_op(msg, OP_EQ, NULL); + + /* And an implicit (IPv4 disabled) IPv6 client setting IPv6 preferences */ + + free_options_test_data(tdata); + tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES + "ClientUseIPv4 0\n" + "ClientPreferIPv6ORPort 1\n" + "ClientPreferIPv6DirPort 1\n"); + + ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); + tt_int_op(ret, OP_EQ, 0); + tt_ptr_op(msg, OP_EQ, NULL); + + /* And an implicit (bridge) client setting IPv6 preferences */ + + free_options_test_data(tdata); + tdata = get_options_test_data(TEST_OPTIONS_DEFAULT_VALUES + "UseBridges 1\n" + "Bridge 127.0.0.1:12345\n" + "ClientPreferIPv6ORPort 1\n" + "ClientPreferIPv6DirPort 1\n"); + + ret = options_validate(tdata->old_opt, tdata->opt, tdata->def_opt, 0, &msg); + tt_int_op(ret, OP_EQ, 0); + tt_ptr_op(msg, OP_EQ, NULL); + done: teardown_capture_of_logs(previous_log); free_options_test_data(tdata);

1 0

[tor/master] Make bridge clients prefer the configured bridge address
by nickm＠torproject.org 11 Feb '16

11 Feb '16

commit b316c87bc90969b2bf724bc2dd695e3f362955b8 Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Wed Feb 3 23:52:39 2016 +1100 Make bridge clients prefer the configured bridge address When ClientPreferIPv6ORPort is auto, bridges prefer the configured bridge ORPort address. Otherwise, they use the value of the option. Other clients prefer IPv4 ORPorts if ClientPreferIPv6ORPort is auto. When ClientPreferIPv6DirPort is auto, all clients prefer IPv4 DirPorts. --- doc/tor.1.txt | 12 ++++++------ src/or/entrynodes.c | 22 ++++++++++++++++------ src/or/nodelist.c | 24 +++++++++++------------- src/or/policies.c | 18 +++++++----------- src/test/test_entrynodes.c | 10 ++++++---- src/test/test_policy.c | 25 +++++++++++++++---------- 6 files changed, 61 insertions(+), 50 deletions(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 87d976b..4c9c53d 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -1504,17 +1504,17 @@ The following options are useful only for clients (that is, if If this option is set to 1, Tor prefers a directory port with an IPv6 address over one with IPv4, for direct connections, if a given directory server has both. (Tor also prefers an IPv6 DirPort if IPv4Client is set to - 0.) If this option is set to auto, Tor bridge clients prefer IPv6, and - other clients prefer IPv4. Other things may influence the choice. This - option breaks a tie to the favor of IPv6. (Default: auto) + 0.) If this option is set to auto, clients prefer IPv4. Other things may + influence the choice. This option breaks a tie to the favor of IPv6. + (Default: auto) [[ClientPreferIPv6ORPort]] **ClientPreferIPv6ORPort** **0**|**1**|**auto**:: If this option is set to 1, Tor prefers an OR port with an IPv6 address over one with IPv4 if a given entry node has both. (Tor also prefers an IPv6 ORPort if IPv4Client is set to 0.) If this option is set - to auto, Tor bridge clients prefer IPv6, and other clients prefer IPv4. - Other things may influence the choice. This option breaks a tie to the - favor of IPv6. (Default: auto) + to auto, Tor bridge clients prefer the configured bridge address, and + other clients prefer IPv4. Other things may influence the choice. This + option breaks a tie to the favor of IPv6. (Default: auto) [[PathsNeededToBuildCircuits]] **PathsNeededToBuildCircuits** __NUM__:: Tor clients don't build circuits for user traffic until they know diff --git a/src/or/entrynodes.c b/src/or/entrynodes.c index d6bef65..a4b9350 100644 --- a/src/or/entrynodes.c +++ b/src/or/entrynodes.c @@ -2240,6 +2240,7 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node) * does so through an address from any source other than node_get_addr(). */ tor_addr_t addr; + const or_options_t *options = get_options(); if (node->ri) { routerinfo_t *ri = node->ri; @@ -2272,9 +2273,15 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node) } } - /* Mark which address to use based on which bridge_t we got. */ - node->ipv6_preferred = (tor_addr_family(&bridge->addr) == AF_INET6 && - !tor_addr_is_null(&node->ri->ipv6_addr)); + if (options->ClientPreferIPv6ORPort == -1) { + /* Mark which address to use based on which bridge_t we got. */ + node->ipv6_preferred = (tor_addr_family(&bridge->addr) == AF_INET6 && + !tor_addr_is_null(&node->ri->ipv6_addr)); + } else { + /* Mark which address to use based on user preference */ + node->ipv6_preferred = (fascist_firewall_prefer_ipv6_orport(options) && + !tor_addr_is_null(&node->ri->ipv6_addr)); + } /* XXXipv6 we lack support for falling back to another address for the same relay, warn the user */ @@ -2283,10 +2290,13 @@ rewrite_node_address_for_bridge(const bridge_info_t *bridge, node_t *node) node_get_pref_orport(node, &ap); log_notice(LD_CONFIG, "Bridge '%s' has both an IPv4 and an IPv6 address. " - "Will prefer using its %s address (%s).", + "Will prefer using its %s address (%s) based on %s.", ri->nickname, - tor_addr_family(&ap.addr) == AF_INET6 ? "IPv6" : "IPv4", - fmt_addrport(&ap.addr, ap.port)); + node->ipv6_preferred ? "IPv6" : "IPv4", + fmt_addrport(&ap.addr, ap.port), + options->ClientPreferIPv6ORPort == -1 ? + "the configured Bridge address" : + "ClientPreferIPv6ORPort"); } } if (node->rs) { diff --git a/src/or/nodelist.c b/src/or/nodelist.c index d7cada9..23e9b0e 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -981,10 +981,6 @@ node_has_ipv6_dirport(const node_t *node) * i) the node_t says that it prefers IPv6 * or * ii) the router has no IPv4 OR address. - * or - * iii) our preference is for IPv6 addresses. - * (This extra step is needed in case our preferences have changed since - * node->ipv6_preferred was set at the time the consensus was loaded.) */ int node_ipv6_or_preferred(const node_t *node) @@ -993,10 +989,12 @@ node_ipv6_or_preferred(const node_t *node) tor_addr_port_t ipv4_addr; node_assert_ok(node); + /* XX/teor - node->ipv6_preferred is set from + * fascist_firewall_prefer_ipv6_orport() each time the consensus is loaded. + */ if (!fascist_firewall_use_ipv6(options)) { return 0; - } else if (node->ipv6_preferred || node_get_prim_orport(node, &ipv4_addr) - || fascist_firewall_prefer_ipv6_orport(get_options())) { + } else if (node->ipv6_preferred || node_get_prim_orport(node, &ipv4_addr)) { return node_has_ipv6_orport(node); } return 0; @@ -1077,13 +1075,9 @@ node_get_pref_ipv6_orport(const node_t *node, tor_addr_port_t *ap_out) * * We prefer the IPv6 address if the router has an IPv6 address, * and we can use IPv6 addresses, and: - * i) the node_t says that it prefers IPv6 + * i) the router has no IPv4 Dir address. * or - * ii) the router has no IPv4 Dir address. - * or - * iii) our preference is for IPv6 addresses. - * (This extra step is needed in case our preferences have changed since - * node->ipv6_preferred was set at the time the consensus was loaded.) + * ii) our preference is for IPv6 Dir addresses. */ int node_ipv6_dir_preferred(const node_t *node) @@ -1092,9 +1086,13 @@ node_ipv6_dir_preferred(const node_t *node) tor_addr_port_t ipv4_addr; node_assert_ok(node); + /* node->ipv6_preferred is set from fascist_firewall_prefer_ipv6_orport(), + * so we can't use it to determine DirPort IPv6 preference. + * This means that bridge clients will use IPv4 DirPorts by default. + */ if (!fascist_firewall_use_ipv6(options)) { return 0; - } else if (node->ipv6_preferred || node_get_prim_dirport(node, &ipv4_addr) + } else if (node_get_prim_dirport(node, &ipv4_addr) || fascist_firewall_prefer_ipv6_dirport(get_options())) { return node_has_ipv6_dirport(node); } diff --git a/src/or/policies.c b/src/or/policies.c index 734558d..e2cece5 100644 --- a/src/or/policies.c +++ b/src/or/policies.c @@ -458,6 +458,13 @@ fascist_firewall_prefer_ipv6_impl(const or_options_t *options) int fascist_firewall_prefer_ipv6_orport(const or_options_t *options) { + /* node->ipv6_preferred is set from fascist_firewall_prefer_ipv6_orport() + * each time the consensus is loaded. + * If our preferences change, we will only reset ipv6_preferred on the node + * when the next consensus is loaded. But the consensus is realoded when the + * configuration changes after a HUP. So as long as the result of this + * function only depends on Tor's options, everything should work ok. + */ int pref_ipv6 = fascist_firewall_prefer_ipv6_impl(options); if (pref_ipv6 >= 0) { @@ -469,11 +476,6 @@ fascist_firewall_prefer_ipv6_orport(const or_options_t *options) return 1; } - /* For bridge clients, ClientPreferIPv6ORPort auto means "prefer IPv6". */ - if (options->UseBridges && options->ClientPreferIPv6ORPort != 0) { - return 1; - } - return 0; } @@ -493,12 +495,6 @@ fascist_firewall_prefer_ipv6_dirport(const or_options_t *options) return 1; } - /* For bridge clients, ClientPreferIPv6ORPort auto means "prefer IPv6". - * XX/teor - do bridge clients ever use a DirPort? */ - if (options->UseBridges && options->ClientPreferIPv6DirPort != 0) { - return 1; - } - return 0; } diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index 14baa8c..fd19db0 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -9,14 +9,16 @@ #include "or.h" #include "test.h" + +#include "config.h" #include "entrynodes.h" -#include "routerparse.h" #include "nodelist.h" -#include "util.h" +#include "policies.h" #include "routerlist.h" +#include "routerparse.h" #include "routerset.h" #include "statefile.h" -#include "config.h" +#include "util.h" #include "test_helpers.h" @@ -826,7 +828,7 @@ test_node_preferred_orport(void *arg) * ClientUseIPv4 is 0 */ mocked_options.ClientUseIPv4 = 0; mocked_options.ClientUseIPv6 = 1; - node.ipv6_preferred = 0; + node.ipv6_preferred = fascist_firewall_prefer_ipv6_orport(&mocked_options); node_get_pref_orport(&node, &ap); tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr)); tt_assert(ap.port == ipv6_port); diff --git a/src/test/test_policy.c b/src/test/test_policy.c index b4cbfb2..c044d9f 100644 --- a/src/test/test_policy.c +++ b/src/test/test_policy.c @@ -1510,21 +1510,25 @@ test_policies_fascist_firewall_choose_address(void *arg) FIREWALL_DIR_CONNECTION, 1) == &ipv4_dir_ap); - /* Auto (Preferring IPv6 for bridge clients) */ + /* Auto: + * - bridge clients prefer the configured bridge OR address, + * - other clients prefer IPv4 OR by default, + * - all clients prefer IPv4 Dir by default. + */ mock_options.ClientPreferIPv6ORPort = -1; mock_options.ClientPreferIPv6DirPort = -1; tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0, FIREWALL_OR_CONNECTION, 0) - == &ipv6_or_ap); + == &ipv4_or_ap); tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0, FIREWALL_OR_CONNECTION, 1) - == &ipv6_or_ap); + == &ipv4_or_ap); tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0, FIREWALL_DIR_CONNECTION, 0) - == &ipv6_dir_ap); + == &ipv4_dir_ap); tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0, FIREWALL_DIR_CONNECTION, 1) - == &ipv6_dir_ap); + == &ipv4_dir_ap); /* Preferring IPv6 */ mock_options.ClientPreferIPv6ORPort = 1; @@ -1544,22 +1548,23 @@ test_policies_fascist_firewall_choose_address(void *arg) /* In the default configuration (Auto / IPv6 off), bridge clients should - * still use and prefer IPv6 regardless of ClientUseIPv6. */ + * still use IPv6, and only prefer it for bridges configured with an IPv6 + * address, regardless of ClientUseIPv6. */ mock_options.ClientUseIPv6 = 0; mock_options.ClientPreferIPv6ORPort = -1; mock_options.ClientPreferIPv6DirPort = -1; tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0, FIREWALL_OR_CONNECTION, 0) - == &ipv6_or_ap); + == &ipv4_or_ap); tt_assert(fascist_firewall_choose_address(&ipv4_or_ap, &ipv6_or_ap, 0, FIREWALL_OR_CONNECTION, 1) - == &ipv6_or_ap); + == &ipv4_or_ap); tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0, FIREWALL_DIR_CONNECTION, 0) - == &ipv6_dir_ap); + == &ipv4_dir_ap); tt_assert(fascist_firewall_choose_address(&ipv4_dir_ap, &ipv6_dir_ap, 0, FIREWALL_DIR_CONNECTION, 1) - == &ipv6_dir_ap); + == &ipv4_dir_ap); /* Choose an address with IPv4 on */ memset(&mock_options, 0, sizeof(or_options_t));

1 0

[tor/master] Merge branch 'feature17840-v11-tests_truncated'
by nickm＠torproject.org 11 Feb '16

11 Feb '16

commit 2b5ff5259408bf7dc17939c6f15eb9e1e68215fa Merge: ba2be81 b316c87 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Feb 11 12:45:51 2016 -0500 Merge branch 'feature17840-v11-tests_truncated' src/test/log_test_helpers.h | 1 + src/test/test_options.c | 124 ++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 116 insertions(+), 9 deletions(-) diff --cc src/test/log_test_helpers.h index 298237d,3a565c6..02f31a5 --- a/src/test/log_test_helpers.h +++ b/src/test/log_test_helpers.h @@@ -23,33 -23,10 +23,34 @@@ void mock_clean_saved_logs(void) const smartlist_t *mock_saved_logs(void); int setup_capture_of_logs(int new_level); void teardown_capture_of_logs(int prev); -const char *mock_saved_log_at(int ix); -int mock_saved_severity_at(int ix); -int mock_saved_log_number(void); ++ int mock_saved_log_has_message(const char *msg); +int mock_saved_log_has_severity(int severity); +int mock_saved_log_has_entry(void); + +#define expect_log_msg(str) \ + tt_assert_msg(mock_saved_log_has_message(str), \ + "expected log to contain " # str); + +#define expect_no_log_msg(str) \ + tt_assert_msg(!mock_saved_log_has_message(str), \ + "expected log to not contain " # str); + +#define expect_log_severity(severity) \ + tt_assert_msg(mock_saved_log_has_severity(severity), \ + "expected log to contain severity " # severity); + +#define expect_no_log_severity(severity) \ + tt_assert_msg(!mock_saved_log_has_severity(severity), \ + "expected log to not contain severity " # severity); + +#define expect_log_entry() \ + tt_assert_msg(mock_saved_log_has_entry(), \ + "expected log to contain entries"); + +#define expect_no_log_entry() \ + tt_assert_msg(!mock_saved_log_has_entry(), \ + "expected log to not contain entries"); #endif

1 0

[tor/master] Fix *_get_all_orports to use ipv6_orport
by nickm＠torproject.org 11 Feb '16

11 Feb '16

commit 4460feaf2850ef0fb027a2d01786a5bbaee056dc Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Tue Dec 22 10:42:09 2015 +1100 Fix *_get_all_orports to use ipv6_orport node_get_all_orports and router_get_all_orports incorrectly used or_port with IPv6 addresses. They now use ipv6_orport. Also refactor and remove duplicated code. --- src/common/address.c | 53 ++++++++++++++++++++++++++++++++++++ src/common/address.h | 21 +++++++++++++++ src/or/nodelist.c | 76 ++++++++++++++++++++++++++++++++++++++++------------ src/or/router.c | 26 +++++------------- 4 files changed, 140 insertions(+), 36 deletions(-) diff --git a/src/common/address.c b/src/common/address.c index 69a8098..19e9fdd 100644 --- a/src/common/address.c +++ b/src/common/address.c @@ -908,6 +908,59 @@ tor_addr_is_loopback(const tor_addr_t *addr) } } +/* Is addr valid? + * Checks that addr is non-NULL and not tor_addr_is_null(). + * If for_listening is true, IPv4 addr 0.0.0.0 is allowed. + * It means "bind to all addresses on the local machine". */ +int +tor_addr_is_valid(const tor_addr_t *addr, int for_listening) +{ + /* NULL addresses are invalid regardless of for_listening */ + if (addr == NULL) { + return 0; + } + + /* Only allow IPv4 0.0.0.0 for_listening. */ + if (for_listening && addr->family == AF_INET + && tor_addr_to_ipv4h(addr) == 0) { + return 1; + } + + /* Otherwise, the address is valid if it's not tor_addr_is_null() */ + return !tor_addr_is_null(addr); +} + +/* Is the network-order IPv4 address v4n_addr valid? + * Checks that addr is not zero. + * Except if for_listening is true, where IPv4 addr 0.0.0.0 is allowed. */ +int +tor_addr_is_valid_ipv4n(uint32_t v4n_addr, int for_listening) +{ + /* Any IPv4 address is valid with for_listening. */ + if (for_listening) { + return 1; + } + + /* Otherwise, zero addresses are invalid. */ + return v4n_addr != 0; +} + +/* Is port valid? + * Checks that port is not 0. + * Except if for_listening is true, where port 0 is allowed. + * It means "OS chooses a port". */ +int +tor_port_is_valid(uint16_t port, int for_listening) +{ + /* Any port value is valid with for_listening. */ + if (for_listening) { + return 1; + } + + /* Otherwise, zero ports are invalid. */ + return port != 0; +} + /** Set dest to equal the IPv4 address in v4addr (given in * network order). */ void diff --git a/src/common/address.h b/src/common/address.h index 684ba65..918b024 100644 --- a/src/common/address.h +++ b/src/common/address.h @@ -267,6 +267,27 @@ void tor_addr_from_in6(tor_addr_t *dest, const struct in6_addr *in6); int tor_addr_is_null(const tor_addr_t *addr); int tor_addr_is_loopback(const tor_addr_t *addr); +int tor_addr_is_valid(const tor_addr_t *addr, int for_listening); +int tor_addr_is_valid_ipv4n(uint32_t v4n_addr, int for_listening); +#define tor_addr_is_valid_ipv4h(v4h_addr, for_listening) \ + tor_addr_is_valid_ipv4n(htonl(v4h_addr), (for_listening)) +int tor_port_is_valid(uint16_t port, int for_listening); +/* Are addr and port both valid? */ +#define tor_addr_port_is_valid(addr, port, for_listening) \ + (tor_addr_is_valid((addr), (for_listening)) && \ + tor_port_is_valid((port), (for_listening))) +/* Are ap->addr and ap->port both valid? */ +#define tor_addr_port_is_valid_ap(ap, for_listening) \ + tor_addr_port_is_valid(&(ap)->addr, (ap)->port, (for_listening)) +/* Are the network-order v4addr and port both valid? */ +#define tor_addr_port_is_valid_ipv4n(v4n_addr, port, for_listening) \ + (tor_addr_is_valid_ipv4n((v4n_addr), (for_listening)) && \ + tor_port_is_valid((port), (for_listening))) +/* Are the host-order v4addr and port both valid? */ +#define tor_addr_port_is_valid_ipv4h(v4h_addr, port, for_listening) \ + (tor_addr_is_valid_ipv4h((v4h_addr), (for_listening)) && \ + tor_port_is_valid((port), (for_listening))) + int tor_addr_port_split(int severity, const char *addrport, char **address_out, uint16_t *port_out); diff --git a/src/or/nodelist.c b/src/or/nodelist.c index fc27207..a1d99e9 100644 --- a/src/or/nodelist.c +++ b/src/or/nodelist.c @@ -754,6 +754,40 @@ node_exit_policy_is_exact(const node_t *node, sa_family_t family) return 1; } +/* Check if the "addr" and port_field fields from r are a valid non-listening + * address/port. If so, set valid to true and add a newly allocated + * tor_addr_port_t containing "addr" and port_field to sl. + * "addr" is an IPv4 host-order address and port_field is a uint16_t. + * r is typically a routerinfo_t or routerstatus_t. + */ +#define SL_ADD_NEW_IPV4_AP(r, port_field, sl, valid) \ + STMT_BEGIN \ + if (tor_addr_port_is_valid_ipv4h((r)->addr, (r)->port_field, 0)) { \ + valid = 1; \ + tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); \ + tor_addr_from_ipv4h(&ap->addr, (r)->addr); \ + ap->port = (r)->port_field; \ + smartlist_add((sl), ap); \ + } \ + STMT_END + +/* Check if the "addr" and port_field fields from r are a valid non-listening + * address/port. If so, set valid to true and add a newly allocated + * tor_addr_port_t containing "addr" and port_field to sl. + * "addr" is a tor_addr_t and port_field is a uint16_t. + * r is typically a routerinfo_t or routerstatus_t. + */ +#define SL_ADD_NEW_IPV6_AP(r, port_field, sl, valid) \ + STMT_BEGIN \ + if (tor_addr_port_is_valid(&(r)->ipv6_addr, (r)->port_field, 0)) { \ + valid = 1; \ + tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); \ + tor_addr_copy(&ap->addr, &(r)->ipv6_addr); \ + ap->port = (r)->port_field; \ + smartlist_add((sl), ap); \ + } \ + STMT_END + /** Return list of tor_addr_port_t with all OR ports (in the sense IP * addr + TCP port) for node. Caller must free all elements * using tor_free() and free the list using smartlist_free(). @@ -766,30 +800,38 @@ smartlist_t * node_get_all_orports(const node_t *node) { smartlist_t *sl = smartlist_new(); + int valid = 0; + /* Find a valid IPv4 address and port */ if (node->ri != NULL) { - if (node->ri->addr != 0) { - tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); - tor_addr_from_ipv4h(&ap->addr, node->ri->addr); - ap->port = node->ri->or_port; - smartlist_add(sl, ap); - } - if (!tor_addr_is_null(&node->ri->ipv6_addr)) { - tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); - tor_addr_copy(&ap->addr, &node->ri->ipv6_addr); - ap->port = node->ri->or_port; - smartlist_add(sl, ap); - } - } else if (node->rs != NULL) { - tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); - tor_addr_from_ipv4h(&ap->addr, node->rs->addr); - ap->port = node->rs->or_port; - smartlist_add(sl, ap); + SL_ADD_NEW_IPV4_AP(node->ri, or_port, sl, valid); + } + + /* If we didn't find a valid address/port in the ri, try the rs */ + if (!valid && node->rs != NULL) { + SL_ADD_NEW_IPV4_AP(node->rs, or_port, sl, valid); + } + + /* Find a valid IPv6 address and port */ + valid = 0; + if (node->ri != NULL) { + SL_ADD_NEW_IPV6_AP(node->ri, ipv6_orport, sl, valid); + } + + if (!valid && node->rs != NULL) { + SL_ADD_NEW_IPV6_AP(node->rs, ipv6_orport, sl, valid); + } + + if (!valid && node->md != NULL) { + SL_ADD_NEW_IPV6_AP(node->md, ipv6_orport, sl, valid); } return sl; } +#undef SL_ADD_NEW_IPV4_AP +#undef SL_ADD_NEW_IPV6_AP + /** Wrapper around node_get_prim_orport for backward compatibility. */ void diff --git a/src/or/router.c b/src/or/router.c index c35f629..ba32d77 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -3350,28 +3350,16 @@ router_free_all(void) /** Return a smartlist of tor_addr_port_t's with all the OR ports of ri. Note that freeing of the items in the list as well as - the smartlist itself is the callers responsibility. - - XXX duplicating code from node_get_all_orports(). */ + the smartlist itself is the callers responsibility. */ smartlist_t * router_get_all_orports(const routerinfo_t *ri) { - smartlist_t *sl = smartlist_new(); tor_assert(ri); - - if (ri->addr != 0) { - tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); - tor_addr_from_ipv4h(&ap->addr, ri->addr); - ap->port = ri->or_port; - smartlist_add(sl, ap); - } - if (!tor_addr_is_null(&ri->ipv6_addr)) { - tor_addr_port_t *ap = tor_malloc(sizeof(tor_addr_port_t)); - tor_addr_copy(&ap->addr, &ri->ipv6_addr); - ap->port = ri->or_port; - smartlist_add(sl, ap); - } - - return sl; + node_t fake_node; + memset(&fake_node, 0, sizeof(fake_node)); + /* we don't modify ri, fake_node is passed as a const node_t * + */ + fake_node.ri = (routerinfo_t *)ri; + return node_get_all_orports(&fake_node); }

1 0

[tor/master] Choose directory servers by IPv4/IPv6 preferences
by nickm＠torproject.org 11 Feb '16

11 Feb '16

commit e72cbf7a4e346f0d379961520db8bea7e9249f88 Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Fri Dec 18 11:29:47 2015 +1100 Choose directory servers by IPv4/IPv6 preferences Add unit tests, refactor pick_directory functions. --- src/or/connection.c | 6 +- src/or/connection.h | 6 +- src/or/directory.c | 180 +++++++++++++++++++++++++++--------- src/or/routerlist.c | 224 +++++++++++++++++++++++++++++++-------------- src/or/routerlist.h | 2 + src/test/test_routerlist.c | 73 +++++++++++++++ 6 files changed, 375 insertions(+), 116 deletions(-) diff --git a/src/or/connection.c b/src/or/connection.c index 9765a8e..d59e07b 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -4252,10 +4252,10 @@ connection_write_to_buf_impl_,(const char *string, size_t len, /** Return a connection with given type, address, port, and purpose; * or NULL if no such connection exists (or if all such connections are marked * for close). */ -connection_t * -connection_get_by_type_addr_port_purpose(int type, +MOCK_IMPL(connection_t *, +connection_get_by_type_addr_port_purpose,(int type, const tor_addr_t *addr, uint16_t port, - int purpose) + int purpose)) { CONN_GET_TEMPLATE(conn, (conn->type == type && diff --git a/src/or/connection.h b/src/or/connection.h index 59ea6d8..ec5c394 100644 --- a/src/or/connection.h +++ b/src/or/connection.h @@ -186,9 +186,9 @@ connection_get_outbuf_len(connection_t *conn) connection_t *connection_get_by_global_id(uint64_t id); connection_t *connection_get_by_type(int type); -connection_t *connection_get_by_type_addr_port_purpose(int type, - const tor_addr_t *addr, - uint16_t port, int purpose); +MOCK_DECL(connection_t *,connection_get_by_type_addr_port_purpose,(int type, + const tor_addr_t *addr, + uint16_t port, int purpose)); connection_t *connection_get_by_type_state(int type, int state); connection_t *connection_get_by_type_state_rendquery(int type, int state, const char *rendquery); diff --git a/src/or/directory.c b/src/or/directory.c index d5531d8..438b5d8 100644 --- a/src/or/directory.c +++ b/src/or/directory.c @@ -82,18 +82,18 @@ static void dir_microdesc_download_failed(smartlist_t *failed, static void note_client_request(int purpose, int compressed, size_t bytes); static int client_likes_consensus(networkstatus_t *v, const char *want_url); -static void directory_initiate_command_rend(const tor_addr_t *addr, - uint16_t or_port, - uint16_t dir_port, - const char *digest, - uint8_t dir_purpose, - uint8_t router_purpose, - dir_indirection_t indirection, - const char *resource, - const char *payload, - size_t payload_len, - time_t if_modified_since, - const rend_data_t *rend_query); +static void directory_initiate_command_rend( + const tor_addr_port_t *or_addr_port, + const tor_addr_port_t *dir_addr_port, + const char *digest, + uint8_t dir_purpose, + uint8_t router_purpose, + dir_indirection_t indirection, + const char *resource, + const char *payload, + size_t payload_len, + time_t if_modified_since, + const rend_data_t *rend_query); /********* START VARIABLES **********/ @@ -624,8 +624,10 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status, { const or_options_t *options = get_options(); const node_t *node; - tor_addr_t addr; + tor_addr_port_t use_or_ap, use_dir_ap; const int anonymized_connection = dirind_is_anon(indirection); + int have_or = 0, have_dir = 0; + node = node_get_by_id(status->identity_digest); if (!node && anonymized_connection) { @@ -634,7 +636,6 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status, routerstatus_describe(status)); return; } - tor_addr_from_ipv4h(&addr, status->addr); if (options->ExcludeNodes && options->StrictNodes && routerset_contains_routerstatus(options->ExcludeNodes, status, -1)) { @@ -646,13 +647,68 @@ directory_initiate_command_routerstatus_rend(const routerstatus_t *status, return; } - directory_initiate_command_rend(&addr, - status->or_port, status->dir_port, - status->identity_digest, - dir_purpose, router_purpose, - indirection, resource, - payload, payload_len, if_modified_since, - rend_query); + /* At this point, if we are a clients making a direct connection to a + * directory server, we have selected a server that has at least one address + * allowed by ClientUseIPv4/6 and Reachable{"",OR,Dir}Addresses. This + * selection uses the preference in ClientPreferIPv6{OR,Dir}Port, if + * possible. (If UseBridges is set, clients ignore all these settings.) + * + * Now we use a similar process to select an address for the relay, + * but simply use the other address if the one we want isn't allowed by + * the firewall. + * + * (When Tor uploads and downloads a hidden service descriptor, it uses + * DIRIND_ANONYMOUS, except for Tor2Web, which uses DIRIND_ONEHOP. + * So this code will only modify the address for Tor2Web's HS descriptor + * fetches. Even Single Onion Servers (NYI) use DIRIND_ANONYMOUS, to avoid + * HSDirs denying service by rejecting descriptors.) + */ + + /* Initialise the OR / Dir addresses */ + tor_addr_make_null(&use_or_ap.addr, AF_UNSPEC); + use_or_ap.port = 0; + tor_addr_make_null(&use_dir_ap.addr, AF_UNSPEC); + use_dir_ap.port = 0; + + if (anonymized_connection) { + /* Use the primary (IPv4) OR address if we're making an indirect + * connection. */ + tor_addr_from_ipv4h(&use_or_ap.addr, status->addr); + use_or_ap.port = status->or_port; + have_or = 1; + } else { + /* We use an IPv6 address if we have one and we prefer it. + * Use the preferred address and port if they are reachable, otherwise, + * use the alternate address and port (if any). + */ + have_or = fascist_firewall_choose_address_rs(status, + FIREWALL_OR_CONNECTION, 0, + &use_or_ap); + } + + have_dir = fascist_firewall_choose_address_rs(status, + FIREWALL_DIR_CONNECTION, 0, + &use_dir_ap); + + /* We rejected both addresses. This isn't great. */ + if (!have_or && !have_dir) { + log_info(LD_DIR, "Rejected both the OR and Dir address when launching a " + "directory connection to: IPv4 %s OR %d Dir %d IPv6 %s OR %d " + "Dir %d", fmt_addr32(status->addr), status->or_port, + status->dir_port, fmt_addr(&status->ipv6_addr), + status->ipv6_orport, status->dir_port); + return; + } + + /* XX/teor - we don't retry the alternate OR/Dir address if this one fails. + * (See #6772.) Instead, we'll retry another directory on failure. */ + + directory_initiate_command_rend(&use_or_ap, &use_dir_ap, + status->identity_digest, + dir_purpose, router_purpose, + indirection, resource, + payload, payload_len, if_modified_since, + rend_query); } /** Launch a new connection to the directory server status to @@ -874,17 +930,21 @@ directory_command_should_use_begindir(const or_options_t *options, if (indirection == DIRIND_DIRECT_CONN || indirection == DIRIND_ANON_DIRPORT) return 0; if (indirection == DIRIND_ONEHOP) - if (!fascist_firewall_allows_address_or(addr, or_port) || + if (!fascist_firewall_allows_address_addr(addr, or_port, + FIREWALL_OR_CONNECTION, 0) || directory_fetches_from_authorities(options)) return 0; /* We're firewalled or are acting like a relay -- also no. */ return 1; } -/** Helper for directory_initiate_command_routerstatus: send the - * command to a server whose address is address, whose IP is - * addr, whose directory port is dir_port, whose tor version - * supports_begindir, and whose identity key digest is - * digest. */ +/** Helper for directory_initiate_command_rend: send the + * command to a server whose address is _addr, whose OR port is + * or_port, whose directory port is dir_port, whose identity key + * digest is digest, with purposes dir_purpose and + * router_purpose, making an (in)direct connection as specified in + * indirection, with command resource, payload of + * payload_len, and asking for a result only if_modified_since. + */ void directory_initiate_command(const tor_addr_t *_addr, uint16_t or_port, uint16_t dir_port, @@ -894,7 +954,27 @@ directory_initiate_command(const tor_addr_t *_addr, const char *payload, size_t payload_len, time_t if_modified_since) { - directory_initiate_command_rend(_addr, or_port, dir_port, + /* Assume _addr applies to both the ORPort and the DirPort, but use the + * null tor_addr if ORPort or DirPort are 0. */ + tor_addr_port_t or_ap, dir_ap; + + if (or_port) { + tor_addr_copy(&or_ap.addr, _addr); + } else { + /* the family doesn't matter here, so make it the same as _addr */ + tor_addr_make_null(&or_ap.addr, tor_addr_family(_addr)); + } + or_ap.port = or_port; + + if (dir_port) { + tor_addr_copy(&dir_ap.addr, _addr); + } else { + /* the family doesn't matter here, so make it the same as _addr */ + tor_addr_make_null(&dir_ap.addr, tor_addr_family(_addr)); + } + dir_ap.port = dir_port; + + directory_initiate_command_rend(&or_ap, &dir_ap, digest, dir_purpose, router_purpose, indirection, resource, payload, payload_len, @@ -914,10 +994,11 @@ is_sensitive_dir_purpose(uint8_t dir_purpose) } /** Same as directory_initiate_command(), but accepts rendezvous data to - * fetch a hidden service descriptor. */ + * fetch a hidden service descriptor, and takes its address & port arguments + * as tor_addr_port_t. */ static void -directory_initiate_command_rend(const tor_addr_t *_addr, - uint16_t or_port, uint16_t dir_port, +directory_initiate_command_rend(const tor_addr_port_t *or_addr_port, + const tor_addr_port_t *dir_addr_port, const char *digest, uint8_t dir_purpose, uint8_t router_purpose, dir_indirection_t indirection, @@ -926,19 +1007,24 @@ directory_initiate_command_rend(const tor_addr_t *_addr, time_t if_modified_since, const rend_data_t *rend_query) { + tor_assert(or_addr_port); + tor_assert(dir_addr_port); + tor_assert(or_addr_port->port || dir_addr_port->port); + tor_assert(digest); + dir_connection_t *conn; const or_options_t *options = get_options(); int socket_error = 0; - int use_begindir = directory_command_should_use_begindir(options, _addr, - or_port, router_purpose, indirection); + const int use_begindir = directory_command_should_use_begindir(options, + &or_addr_port->addr, or_addr_port->port, + router_purpose, indirection); const int anonymized_connection = dirind_is_anon(indirection); - tor_addr_t addr; - - tor_assert(_addr); - tor_assert(or_port || dir_port); - tor_assert(digest); + const int or_connection = use_begindir || anonymized_connection; - tor_addr_copy(&addr, _addr); + tor_addr_t addr; + tor_addr_copy(&addr, &(or_connection ? or_addr_port : dir_addr_port)->addr); + uint16_t port = (or_connection ? or_addr_port : dir_addr_port)->port; + uint16_t dir_port = dir_addr_port->port; log_debug(LD_DIR, "anonymized %d, use_begindir %d.", anonymized_connection, use_begindir); @@ -954,13 +1040,23 @@ directory_initiate_command_rend(const tor_addr_t *_addr, /* ensure that we don't make direct connections when a SOCKS server is * configured. */ - if (!anonymized_connection && !use_begindir && !options->HTTPProxy && + if (!or_connection && !options->HTTPProxy && (options->Socks4Proxy || options->Socks5Proxy)) { log_warn(LD_DIR, "Cannot connect to a directory server through a " "SOCKS proxy!"); return; } + if (or_connection && (!or_addr_port->port + || tor_addr_is_null(&or_addr_port->addr))) { + log_warn(LD_DIR, "Cannot make an OR connection without an OR port."); + return; + } else if (!or_connection && (!dir_addr_port->port + || tor_addr_is_null(&dir_addr_port->addr))) { + log_warn(LD_DIR, "Cannot make a Dir connection without a Dir port."); + return; + } + /* ensure we don't make excess connections when we're already downloading * a consensus during bootstrap */ if (connection_dir_avoid_extra_connection_for_purpose(dir_purpose)) { @@ -971,7 +1067,7 @@ directory_initiate_command_rend(const tor_addr_t *_addr, /* set up conn so it's got all the data we need to remember */ tor_addr_copy(&conn->base_.addr, &addr); - conn->base_.port = use_begindir ? or_port : dir_port; + conn->base_.port = port; conn->base_.address = tor_dup_addr(&addr); memcpy(conn->identity_digest, digest, DIGEST_LEN); @@ -989,7 +1085,7 @@ directory_initiate_command_rend(const tor_addr_t *_addr, if (rend_query) conn->rend_data = rend_data_dup(rend_query); - if (!anonymized_connection && !use_begindir) { + if (!or_connection) { /* then we want to connect to dirport directly */ if (options->HTTPProxy) { diff --git a/src/or/routerlist.c b/src/or/routerlist.c index 804ff29..4a3c1cd 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1462,9 +1462,122 @@ router_pick_dirserver_generic(smartlist_t *sourcelist, return router_pick_trusteddirserver_impl(sourcelist, type, flags, NULL); } +/* Check if we already have a directory fetch from ap, for serverdesc + * (including extrainfo) or microdesc documents. + * If so, return 1, if not, return 0. + * Also returns 0 if addr is NULL, tor_addr_is_null(addr), or dir_port is 0. + */ +STATIC int +router_is_already_dir_fetching(const tor_addr_port_t *ap, int serverdesc, + int microdesc) +{ + if (!ap || tor_addr_is_null(&ap->addr) || !ap->port) { + return 0; + } + + /* XX/teor - we're not checking tunnel connections here, see #17848 + */ + if (serverdesc && ( + connection_get_by_type_addr_port_purpose( + CONN_TYPE_DIR, &ap->addr, ap->port, DIR_PURPOSE_FETCH_SERVERDESC) + || connection_get_by_type_addr_port_purpose( + CONN_TYPE_DIR, &ap->addr, ap->port, DIR_PURPOSE_FETCH_EXTRAINFO))) { + return 1; + } + + if (microdesc && ( + connection_get_by_type_addr_port_purpose( + CONN_TYPE_DIR, &ap->addr, ap->port, DIR_PURPOSE_FETCH_MICRODESC))) { + return 1; + } + + return 0; +} + +/* Check if we already have a directory fetch from ds, for serverdesc + * (including extrainfo) or microdesc documents. + * If so, return 1, if not, return 0. + */ +static int +router_is_already_dir_fetching_ds(const dir_server_t *ds, + int serverdesc, + int microdesc) +{ + tor_addr_port_t ipv4_dir_ap, ipv6_dir_ap; + + /* Assume IPv6 DirPort is the same as IPv4 DirPort */ + tor_addr_from_ipv4h(&ipv4_dir_ap.addr, ds->addr); + ipv4_dir_ap.port = ds->dir_port; + tor_addr_copy(&ipv6_dir_ap.addr, &ds->ipv6_addr); + ipv6_dir_ap.port = ds->dir_port; + + return (router_is_already_dir_fetching(&ipv4_dir_ap, serverdesc, microdesc) + || router_is_already_dir_fetching(&ipv6_dir_ap, serverdesc, microdesc)); +} + +/* Check if we already have a directory fetch from rs, for serverdesc + * (including extrainfo) or microdesc documents. + * If so, return 1, if not, return 0. + */ +static int +router_is_already_dir_fetching_rs(const routerstatus_t *rs, + int serverdesc, + int microdesc) +{ + tor_addr_port_t ipv4_dir_ap, ipv6_dir_ap; + + /* Assume IPv6 DirPort is the same as IPv4 DirPort */ + tor_addr_from_ipv4h(&ipv4_dir_ap.addr, rs->addr); + ipv4_dir_ap.port = rs->dir_port; + tor_addr_copy(&ipv6_dir_ap.addr, &rs->ipv6_addr); + ipv6_dir_ap.port = rs->dir_port; + + return (router_is_already_dir_fetching(&ipv4_dir_ap, serverdesc, microdesc) + || router_is_already_dir_fetching(&ipv6_dir_ap, serverdesc, microdesc)); +} + /** How long do we avoid using a directory server after it's given us a 503? */ #define DIR_503_TIMEOUT (60*60) +/* Common retry code for router_pick_directory_server_impl and + * router_pick_trusteddirserver_impl. Retry with the non-preferred IP version. + * Must be called before RETRY_WITHOUT_EXCLUDE(). + * + * If we got no result, and we are applying IP preferences, and we are a + * client that could use an alternate IP version, try again with the + * opposite preferences. */ +#define RETRY_ALTERNATE_IP_VERSION(retry_label) \ + STMT_BEGIN \ + if (result == NULL && try_ip_pref && options->ClientUseIPv4 \ + && options->ClientUseIPv6 && !server_mode(options) && n_not_preferred \ + && !n_busy) { \ + n_excluded = 0; \ + n_busy = 0; \ + try_ip_pref = 0; \ + n_not_preferred = 0; \ + goto retry_label; \ + } \ + STMT_END \ + +/* Common retry code for router_pick_directory_server_impl and + * router_pick_trusteddirserver_impl. Retry without excluding nodes, but with + * the preferred IP version. Must be called after RETRY_ALTERNATE_IP_VERSION(). + * + * If we got no result, and we are excluding nodes, and StrictNodes is + * not set, try again without excluding nodes. */ +#define RETRY_WITHOUT_EXCLUDE(retry_label) \ + STMT_BEGIN \ + if (result == NULL && try_excluding && !options->StrictNodes \ + && n_excluded && !n_busy) { \ + try_excluding = 0; \ + n_excluded = 0; \ + n_busy = 0; \ + try_ip_pref = 1; \ + n_not_preferred = 0; \ + goto retry_label; \ + } \ + STMT_END + /** Pick a random running valid directory server/mirror from our * routerlist. Arguments are as for router_pick_directory_server(), except: * @@ -1489,11 +1602,12 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags, const int no_microdesc_fetching = (flags & PDS_NO_EXISTING_MICRODESC_FETCH); const int for_guard = (flags & PDS_FOR_GUARD); int try_excluding = 1, n_excluded = 0, n_busy = 0; + int try_ip_pref = 1, n_not_preferred = 0; if (!consensus) return NULL; - retry_without_exclude: + retry_search: direct = smartlist_new(); tunnel = smartlist_new(); @@ -1506,7 +1620,6 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags, SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) { int is_trusted, is_trusted_extrainfo; int is_overloaded; - tor_addr_t addr; const routerstatus_t *status = node->rs; const country_t country = node->country; if (!status) @@ -1537,36 +1650,32 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags, continue; } - /* XXXX IP6 proposal 118 */ - tor_addr_from_ipv4h(&addr, status->addr); - - if (no_serverdesc_fetching && ( - connection_get_by_type_addr_port_purpose( - CONN_TYPE_DIR, &addr, status->dir_port, DIR_PURPOSE_FETCH_SERVERDESC) - || connection_get_by_type_addr_port_purpose( - CONN_TYPE_DIR, &addr, status->dir_port, DIR_PURPOSE_FETCH_EXTRAINFO) - )) { - ++n_busy; - continue; - } - - if (no_microdesc_fetching && connection_get_by_type_addr_port_purpose( - CONN_TYPE_DIR, &addr, status->dir_port, DIR_PURPOSE_FETCH_MICRODESC) - ) { + if (router_is_already_dir_fetching_rs(status, + no_serverdesc_fetching, + no_microdesc_fetching)) { ++n_busy; continue; } is_overloaded = status->last_dir_503_at + DIR_503_TIMEOUT > now; - if ((!fascistfirewall || - fascist_firewall_allows_address_or(&addr, status->or_port))) + /* We use an IPv6 address if we have one and we prefer it. + * Add the router if its preferred address and port are reachable. + * If we don't get any routers, we'll try again with the non-preferred + * address for each router (if any). (To ensure correct load-balancing + * we try routers that only have one address both times.) + */ + if (!fascistfirewall || + fascist_firewall_allows_rs(status, FIREWALL_OR_CONNECTION, + try_ip_pref)) smartlist_add(is_trusted ? trusted_tunnel : is_overloaded ? overloaded_tunnel : tunnel, (void*)node); - else if (!fascistfirewall || - fascist_firewall_allows_address_dir(&addr, status->dir_port)) + else if (fascist_firewall_allows_rs(status, FIREWALL_DIR_CONNECTION, + try_ip_pref)) smartlist_add(is_trusted ? trusted_direct : is_overloaded ? overloaded_direct : direct, (void*)node); + else if (!tor_addr_is_null(&status->ipv6_addr)) + ++n_not_preferred; } SMARTLIST_FOREACH_END(node); if (smartlist_len(tunnel)) { @@ -1595,15 +1704,9 @@ router_pick_directory_server_impl(dirinfo_type_t type, int flags, smartlist_free(overloaded_direct); smartlist_free(overloaded_tunnel); - if (result == NULL && try_excluding && !options->StrictNodes && n_excluded - && !n_busy) { - /* If we got no result, and we are excluding nodes, and StrictNodes is - * not set, try again without excluding nodes. */ - try_excluding = 0; - n_excluded = 0; - n_busy = 0; - goto retry_without_exclude; - } + RETRY_ALTERNATE_IP_VERSION(retry_search); + + RETRY_WITHOUT_EXCLUDE(retry_search); if (n_busy_out) *n_busy_out = n_busy; @@ -1658,11 +1761,12 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist, smartlist_t *pick_from; int n_busy = 0; int try_excluding = 1, n_excluded = 0; + int try_ip_pref = 1, n_not_preferred = 0; if (!sourcelist) return NULL; - retry_without_exclude: + retry_search: direct = smartlist_new(); tunnel = smartlist_new(); @@ -1673,7 +1777,6 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist, { int is_overloaded = d->fake_status.last_dir_503_at + DIR_503_TIMEOUT > now; - tor_addr_t addr; if (!d->is_running) continue; if ((type & d->type) == 0) continue; @@ -1689,35 +1792,27 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist, continue; } - /* XXXX IP6 proposal 118 */ - tor_addr_from_ipv4h(&addr, d->addr); - - if (no_serverdesc_fetching) { - if (connection_get_by_type_addr_port_purpose( - CONN_TYPE_DIR, &addr, d->dir_port, DIR_PURPOSE_FETCH_SERVERDESC) - || connection_get_by_type_addr_port_purpose( - CONN_TYPE_DIR, &addr, d->dir_port, DIR_PURPOSE_FETCH_EXTRAINFO)) { - //log_debug(LD_DIR, "We have an existing connection to fetch " - // "descriptor from %s; delaying",d->description); - ++n_busy; - continue; - } - } - if (no_microdesc_fetching) { - if (connection_get_by_type_addr_port_purpose( - CONN_TYPE_DIR, &addr, d->dir_port, DIR_PURPOSE_FETCH_MICRODESC)) { - ++n_busy; - continue; - } + if (router_is_already_dir_fetching_ds(d, no_serverdesc_fetching, + no_microdesc_fetching)) { + ++n_busy; + continue; } - if (d->or_port && - (!fascistfirewall || - fascist_firewall_allows_address_or(&addr, d->or_port))) + /* We use an IPv6 address if we have one and we prefer it. + * Add the router if its preferred address and port are reachable. + * If we don't get any routers, we'll try again with the non-preferred + * address for each router (if any). (To ensure correct load-balancing + * we try routers that only have one address both times.) + */ + if (!fascistfirewall || + fascist_firewall_allows_dir_server(d, FIREWALL_OR_CONNECTION, + try_ip_pref)) smartlist_add(is_overloaded ? overloaded_tunnel : tunnel, (void*)d); - else if (!fascistfirewall || - fascist_firewall_allows_address_dir(&addr, d->dir_port)) + else if (fascist_firewall_allows_dir_server(d, FIREWALL_DIR_CONNECTION, + try_ip_pref)) smartlist_add(is_overloaded ? overloaded_direct : direct, (void*)d); + else if (!tor_addr_is_null(&d->ipv6_addr)) + ++n_not_preferred; } SMARTLIST_FOREACH_END(d); @@ -1744,19 +1839,12 @@ router_pick_trusteddirserver_impl(const smartlist_t *sourcelist, smartlist_free(overloaded_direct); smartlist_free(overloaded_tunnel); - if (result == NULL && try_excluding && !options->StrictNodes && n_excluded - && !n_busy) { - /* If we got no result, and we are excluding nodes, and StrictNodes is - * not set, try again without excluding nodes. */ - try_excluding = 0; - n_excluded = 0; - n_busy = 0; - goto retry_without_exclude; - } + RETRY_ALTERNATE_IP_VERSION(retry_search); + + RETRY_WITHOUT_EXCLUDE(retry_search); if (n_busy_out) *n_busy_out = n_busy; - return result; } diff --git a/src/or/routerlist.h b/src/or/routerlist.h index 4c828d6..ac286d9 100644 --- a/src/or/routerlist.h +++ b/src/or/routerlist.h @@ -243,6 +243,8 @@ MOCK_DECL(STATIC was_router_added_t, extrainfo_insert, MOCK_DECL(STATIC void, initiate_descriptor_downloads, (const routerstatus_t *source, int purpose, smartlist_t *digests, int lo, int hi, int pds_flags)); +STATIC int router_is_already_dir_fetching(const tor_addr_port_t *ap, + int serverdesc, int microdesc); #endif diff --git a/src/test/test_routerlist.c b/src/test/test_routerlist.c index 1bc5e4b..7ec6525 100644 --- a/src/test/test_routerlist.c +++ b/src/test/test_routerlist.c @@ -5,6 +5,7 @@ #include "or.h" #include "routerlist.h" #include "directory.h" +#include "connection.h" #include "test.h" /* 4 digests + 3 sep + pre + post + NULL */ @@ -94,12 +95,84 @@ test_routerlist_launch_descriptor_downloads(void *arg) smartlist_free(downloadable); } +connection_t *mocked_connection = NULL; + +/* Mock connection_get_by_type_addr_port_purpose by returning + * mocked_connection. */ +static connection_t * +mock_connection_get_by_type_addr_port_purpose(int type, + const tor_addr_t *addr, + uint16_t port, int purpose) +{ + (void)type; + (void)addr; + (void)port; + (void)purpose; + + return mocked_connection; +} + +#define TEST_ADDR_STR "127.0.0.1" +#define TEST_DIR_PORT 12345 + +static void +test_routerlist_router_is_already_dir_fetching(void *arg) +{ + (void)arg; + tor_addr_port_t test_ap, null_addr_ap, zero_port_ap; + + /* Setup */ + tor_addr_parse(&test_ap.addr, TEST_ADDR_STR); + test_ap.port = TEST_DIR_PORT; + tor_addr_make_null(&null_addr_ap.addr, AF_INET6); + null_addr_ap.port = TEST_DIR_PORT; + tor_addr_parse(&zero_port_ap.addr, TEST_ADDR_STR); + zero_port_ap.port = 0; + MOCK(connection_get_by_type_addr_port_purpose, + mock_connection_get_by_type_addr_port_purpose); + + /* Test that we never get 1 from a NULL connection */ + mocked_connection = NULL; + tt_assert(router_is_already_dir_fetching(&test_ap, 1, 1) == 0); + tt_assert(router_is_already_dir_fetching(&test_ap, 1, 0) == 0); + tt_assert(router_is_already_dir_fetching(&test_ap, 0, 1) == 0); + /* We always expect 0 in these cases */ + tt_assert(router_is_already_dir_fetching(&test_ap, 0, 0) == 0); + tt_assert(router_is_already_dir_fetching(NULL, 1, 1) == 0); + tt_assert(router_is_already_dir_fetching(&null_addr_ap, 1, 1) == 0); + tt_assert(router_is_already_dir_fetching(&zero_port_ap, 1, 1) == 0); + + /* Test that we get 1 with a connection in the appropriate circumstances */ + mocked_connection = connection_new(CONN_TYPE_DIR, AF_INET); + tt_assert(router_is_already_dir_fetching(&test_ap, 1, 1) == 1); + tt_assert(router_is_already_dir_fetching(&test_ap, 1, 0) == 1); + tt_assert(router_is_already_dir_fetching(&test_ap, 0, 1) == 1); + + /* Test that we get 0 even with a connection in the appropriate + * circumstances */ + tt_assert(router_is_already_dir_fetching(&test_ap, 0, 0) == 0); + tt_assert(router_is_already_dir_fetching(NULL, 1, 1) == 0); + tt_assert(router_is_already_dir_fetching(&null_addr_ap, 1, 1) == 0); + tt_assert(router_is_already_dir_fetching(&zero_port_ap, 1, 1) == 0); + + done: + /* If a connection is never set up, connection_free chokes on it. */ + buf_free(mocked_connection->inbuf); + buf_free(mocked_connection->outbuf); + tor_free(mocked_connection); + UNMOCK(connection_get_by_type_addr_port_purpose); +} + +#undef TEST_ADDR_STR +#undef TEST_DIR_PORT + #define NODE(name, flags) \ { #name, test_routerlist_##name, (flags), NULL, NULL } struct testcase_t routerlist_tests[] = { NODE(initiate_descriptor_downloads, 0), NODE(launch_descriptor_downloads, 0), + NODE(router_is_already_dir_fetching, TT_FORK), END_OF_TESTCASES };

1 0

[tor/master] Choose OR Entry Guards using IPv4/IPv6 preferences
by nickm＠torproject.org 11 Feb '16

11 Feb '16

commit 268608c0a0605e596d1a884ee35d432c88bac38b Author: teor (Tim Wilson-Brown) <teor2345(a)gmail.com> Date: Fri Dec 18 11:28:54 2015 +1100 Choose OR Entry Guards using IPv4/IPv6 preferences Update unit tests. --- src/or/circuitbuild.c | 15 +++----- src/or/or.h | 4 +- src/or/routerlist.c | 18 +++++++-- src/or/routerlist.h | 3 +- src/test/test_entrynodes.c | 93 +++++++++++++++++++++++++++++++++++++++++----- 5 files changed, 108 insertions(+), 25 deletions(-) diff --git a/src/or/circuitbuild.c b/src/or/circuitbuild.c index d44fcd7..dcb9de3 100644 --- a/src/or/circuitbuild.c +++ b/src/or/circuitbuild.c @@ -1778,7 +1778,7 @@ pick_tor2web_rendezvous_node(router_crn_flags_t flags, router_add_running_nodes_to_smartlist(all_live_nodes, allow_invalid, 0, 0, 0, - need_desc); + need_desc, 0); /* Filter all_live_nodes to only add live *and* whitelisted RPs to * the list whitelisted_live_rps. */ @@ -2144,7 +2144,9 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state) const node_t *choice; smartlist_t *excluded; const or_options_t *options = get_options(); - router_crn_flags_t flags = CRN_NEED_GUARD|CRN_NEED_DESC; + /* If possible, choose an entry server with a preferred address, + * otherwise, choose one with an allowed address */ + router_crn_flags_t flags = CRN_NEED_GUARD|CRN_NEED_DESC|CRN_PREF_ADDR; const node_t *node; if (state && options->UseEntryGuards && @@ -2161,12 +2163,6 @@ choose_good_entry_server(uint8_t purpose, cpath_build_state_t *state) * family. */ nodelist_add_node_and_family(excluded, node); } - /* Exclude all ORs that we can't reach through our firewall */ - smartlist_t *nodes = nodelist_get_list(); - SMARTLIST_FOREACH(nodes, const node_t *, node, { - if (!fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION, 0)) - smartlist_add(excluded, (void*)node); - }); /* and exclude current entry guards and their families, * unless we're in a test network, and excluding guards * would exclude all nodes (i.e. we're in an incredibly small tor network, @@ -2332,8 +2328,9 @@ extend_info_from_node(const node_t *node, int for_direct_connect) if (node->ri == NULL && (node->rs == NULL || node->md == NULL)) return NULL; + /* Choose a preferred address first, but fall back to an allowed address*/ if (for_direct_connect) - node_get_pref_orport(node, &ap); + fascist_firewall_choose_address_node(node, FIREWALL_OR_CONNECTION, 0, &ap); else node_get_prim_orport(node, &ap); diff --git a/src/or/or.h b/src/or/or.h index b1765d1..412789c 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -5221,7 +5221,9 @@ typedef enum { CRN_ALLOW_INVALID = 1<<3, /* XXXX not used, apparently. */ CRN_WEIGHT_AS_EXIT = 1<<5, - CRN_NEED_DESC = 1<<6 + CRN_NEED_DESC = 1<<6, + /* On clients, only provide nodes that satisfy ClientPreferIPv6OR */ + CRN_PREF_ADDR = 1<<7 } router_crn_flags_t; /** Return value for router_add_to_routerlist() and dirserv_add_descriptor() */ diff --git a/src/or/routerlist.c b/src/or/routerlist.c index c45854c..804ff29 100644 --- a/src/or/routerlist.c +++ b/src/or/routerlist.c @@ -1826,7 +1826,8 @@ routerlist_add_node_and_family(smartlist_t *sl, const routerinfo_t *router) void router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid, int need_uptime, int need_capacity, - int need_guard, int need_desc) + int need_guard, int need_desc, + int pref_addr) { /* XXXX MOVE */ SMARTLIST_FOREACH_BEGIN(nodelist_get_list(), const node_t *, node) { if (!node->is_running || @@ -1838,6 +1839,9 @@ router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid, continue; if (node_is_unreliable(node, need_uptime, need_capacity, need_guard)) continue; + /* Choose a node with a preferred OR address */ + if (!fascist_firewall_allows_node(node, FIREWALL_OR_CONNECTION, pref_addr)) + continue; smartlist_add(sl, (void *)node); } SMARTLIST_FOREACH_END(node); @@ -2299,6 +2303,10 @@ node_sl_choose_by_bandwidth(const smartlist_t *sl, * If CRN_NEED_DESC is set in flags, we only consider nodes that * have a routerinfo or microdescriptor -- that is, enough info to be * used to build a circuit. + * If CRN_PREF_ADDR is set in flags, we only consider nodes that + * have an address that is preferred by the ClientPreferIPv6ORPort setting + * (regardless of this flag, we exclude nodes that aren't allowed by the + * firewall, including ClientUseIPv4 0 and ClientUseIPv6 0). */ const node_t * router_choose_random_node(smartlist_t *excludedsmartlist, @@ -2311,6 +2319,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist, const int allow_invalid = (flags & CRN_ALLOW_INVALID) != 0; const int weight_for_exit = (flags & CRN_WEIGHT_AS_EXIT) != 0; const int need_desc = (flags & CRN_NEED_DESC) != 0; + const int pref_addr = (flags & CRN_PREF_ADDR) != 0; smartlist_t *sl=smartlist_new(), *excludednodes=smartlist_new(); @@ -2336,7 +2345,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist, router_add_running_nodes_to_smartlist(sl, allow_invalid, need_uptime, need_capacity, - need_guard, need_desc); + need_guard, need_desc, pref_addr); log_debug(LD_CIRC, "We found %d running nodes.", smartlist_len(sl)); @@ -2365,7 +2374,7 @@ router_choose_random_node(smartlist_t *excludedsmartlist, choice = node_sl_choose_by_bandwidth(sl, rule); smartlist_free(sl); - if (!choice && (need_uptime || need_capacity || need_guard)) { + if (!choice && (need_uptime || need_capacity || need_guard || pref_addr)) { /* try once more -- recurse but with fewer restrictions. */ log_info(LD_CIRC, "We couldn't find any live%s%s%s routers; falling back " @@ -2373,7 +2382,8 @@ router_choose_random_node(smartlist_t *excludedsmartlist, need_capacity?", fast":"", need_uptime?", stable":"", need_guard?", guard":""); - flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD); + flags &= ~ (CRN_NEED_UPTIME|CRN_NEED_CAPACITY|CRN_NEED_GUARD| + CRN_PREF_ADDR); choice = router_choose_random_node( excludedsmartlist, excludedset, flags); } diff --git a/src/or/routerlist.h b/src/or/routerlist.h index 339e34a..4c828d6 100644 --- a/src/or/routerlist.h +++ b/src/or/routerlist.h @@ -61,7 +61,8 @@ void router_reset_status_download_failures(void); int routers_have_same_or_addrs(const routerinfo_t *r1, const routerinfo_t *r2); void router_add_running_nodes_to_smartlist(smartlist_t *sl, int allow_invalid, int need_uptime, int need_capacity, - int need_guard, int need_desc); + int need_guard, int need_desc, + int pref_addr); const routerinfo_t *routerlist_find_my_routerinfo(void); uint32_t router_get_advertised_bandwidth(const routerinfo_t *router); diff --git a/src/test/test_entrynodes.c b/src/test/test_entrynodes.c index 87276db..e4947e0 100644 --- a/src/test/test_entrynodes.c +++ b/src/test/test_entrynodes.c @@ -70,6 +70,14 @@ fake_network_setup(const struct testcase_t *testcase) return dummy_state; } +static or_options_t mocked_options; + +static const or_options_t * +mock_get_options(void) +{ + return &mocked_options; +} + /** Test choose_random_entry() with none of our routers being guard nodes. */ static void test_choose_random_entry_no_guards(void *arg) @@ -78,6 +86,14 @@ test_choose_random_entry_no_guards(void *arg) (void) arg; + MOCK(get_options, mock_get_options); + + /* Check that we get a guard if it passes preferred + * address settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 1; + mocked_options.ClientPreferIPv6ORPort = 0; + /* Try to pick an entry even though none of our routers are guards. */ chosen_entry = choose_random_entry(NULL); @@ -86,8 +102,35 @@ test_choose_random_entry_no_guards(void *arg) can't find a proper entry guard. */ tt_assert(chosen_entry); + /* And with the other IP version active */ + mocked_options.ClientUseIPv6 = 1; + chosen_entry = choose_random_entry(NULL); + tt_assert(chosen_entry); + + /* Check that we don't get a guard if it doesn't pass mandatory address + * settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 0; + mocked_options.ClientPreferIPv6ORPort = 0; + + chosen_entry = choose_random_entry(NULL); + + /* If we don't allow IPv4 at all, we don't get a guard*/ + tt_assert(!chosen_entry); + + /* Check that we get a guard if it passes allowed but not preferred address + * settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 1; + mocked_options.ClientUseIPv6 = 1; + mocked_options.ClientPreferIPv6ORPort = 1; + + chosen_entry = choose_random_entry(NULL); + tt_assert(chosen_entry); + done: - ; + memset(&mocked_options, 0, sizeof(mocked_options)); + UNMOCK(get_options); } /** Test choose_random_entry() with only one of our routers being a @@ -101,17 +144,55 @@ test_choose_random_entry_one_possible_guard(void *arg) (void) arg; + MOCK(get_options, mock_get_options); + /* Set one of the nodes to be a guard. */ our_nodelist = nodelist_get_list(); the_guard = smartlist_get(our_nodelist, 4); /* chosen by fair dice roll */ the_guard->is_possible_guard = 1; + /* Check that we get the guard if it passes preferred + * address settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 1; + mocked_options.ClientPreferIPv6ORPort = 0; + /* Pick an entry. Make sure we pick the node we marked as guard. */ chosen_entry = choose_random_entry(NULL); tt_ptr_op(chosen_entry, OP_EQ, the_guard); + /* And with the other IP version active */ + mocked_options.ClientUseIPv6 = 1; + chosen_entry = choose_random_entry(NULL); + tt_ptr_op(chosen_entry, OP_EQ, the_guard); + + /* Check that we don't get a guard if it doesn't pass mandatory address + * settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 0; + mocked_options.ClientPreferIPv6ORPort = 0; + + chosen_entry = choose_random_entry(NULL); + + /* If we don't allow IPv4 at all, we don't get a guard*/ + tt_assert(!chosen_entry); + + /* Check that we get a node if it passes allowed but not preferred + * address settings */ + memset(&mocked_options, 0, sizeof(mocked_options)); + mocked_options.ClientUseIPv4 = 1; + mocked_options.ClientUseIPv6 = 1; + mocked_options.ClientPreferIPv6ORPort = 1; + + chosen_entry = choose_random_entry(NULL); + + /* We disable the guard check and the preferred address check at the same + * time, so we can't be sure we get the guard */ + tt_assert(chosen_entry); + done: - ; + memset(&mocked_options, 0, sizeof(mocked_options)); + UNMOCK(get_options); } /** Helper to conduct tests for populate_live_entry_guards(). @@ -624,14 +705,6 @@ test_entry_is_live(void *arg) ; /* XXX */ } -static or_options_t mocked_options; - -static const or_options_t * -mock_get_options(void) -{ - return &mocked_options; -} - #define TEST_IPV4_ADDR "123.45.67.89" #define TEST_IPV6_ADDR "[1234:5678:90ab:cdef::]"

1 0