Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tor Trac #33375]: Stop advertising an IPv6 exit policy when DNS is broken for IPv6 #1771

Closed
wants to merge 11 commits into from
@@ -0,0 +1,3 @@
o Major bugfixes (ipv6):
- Stop advertising an IPv6 exit policy when DNS is broken for IPv6.
Fixes bug 33375; bugfix on 0.2.4.7-alpha. Patch by Neel Chauhan.
@@ -83,6 +83,12 @@
* that the resolver is wedged? */
#define RESOLVE_MAX_TIMEOUT 300

/** If IPv6 DNS fails, how long should we not advertise an IPv6 exit policy? */
#define IPV6_FAILED_EXIT_POLICY_TIMEOUT 86400 /* 24 hours */

/** How many timeouts until we stop advertising an IPv6 exit policy? */
#define IPV6_DNS_MAX_TIMEOUTS 1000

/** Our evdns_base; this structure handles all our name lookups. */
static struct evdns_base *the_evdns_base = NULL;

@@ -109,6 +115,8 @@ static int answer_is_wildcarded(const char *ip);
static int evdns_err_is_transient(int err);
static void inform_pending_connections(cached_resolve_t *resolve);
static void make_pending_resolve_cached(cached_resolve_t *cached);
static void dns_schedule_ipv6_broken_reset(void);
static void dns_reset_ipv6_broken(evutil_socket_t fd, short event, void *args);

#ifdef DEBUG_DNS_CACHE
static void assert_cache_ok_(void);
@@ -1553,13 +1561,16 @@ evdns_callback(int result, char type, int count, int ttl, void *addresses,
++n_ipv6_timeouts;
}

if (n_ipv6_timeouts > 10 &&
n_ipv6_timeouts > n_ipv6_requests_made / 2) {
if ((n_ipv6_timeouts > IPV6_DNS_MAX_TIMEOUTS &&
n_ipv6_timeouts > n_ipv6_requests_made / 2) ||
(get_options()->TestingTorNetwork && n_ipv6_timeouts > 1)) {
if (! dns_is_broken_for_ipv6) {
log_notice(LD_EXIT, "More than half of our IPv6 requests seem to "

This comment has been minimized.

@teor2345

teor2345 Mar 20, 2020
Contributor

Please increase the minimum number of DNS queries in these lines:

    if (n_ipv6_timeouts > 10 &&
        n_ipv6_timeouts > n_ipv6_requests_made / 2) {

But set it to 1 when TestingTorNetwork is 1.

Also, please replace the numbers by named constants. One of the reasons you changed the wrong code, is that there are multiple "10"s in this file.

"have timed out. I'm going to assume I can't get AAAA "
"responses.");
mark_my_descriptor_dirty("IPv6 DNS is broken");
dns_is_broken_for_ipv6 = 1;
dns_schedule_ipv6_broken_reset();
}
}
}
@@ -2026,6 +2037,23 @@ dns_launch_correctness_checks(void)
}
}

/** If appropriate, if IPv6 DNS failed, reset the countner every 24 hours */
static void
dns_schedule_ipv6_broken_reset(void)
{
static struct event *launch_event = NULL;
struct timeval timeout;

if (!launch_event)
launch_event = tor_evtimer_new(tor_libevent_get_base(),
dns_reset_ipv6_broken, NULL);
timeout.tv_sec = IPV6_FAILED_EXIT_POLICY_TIMEOUT;
timeout.tv_usec = 0;
if (evtimer_add(launch_event, &timeout) < 0) {
log_warn(LD_BUG, "Couldn't add timer for reset ipv6 dns broken count");
}
}

/** Return true iff our DNS servers lie to us too much to be trusted. */
int
dns_seems_to_be_broken(void)
@@ -2040,6 +2068,26 @@ dns_seems_to_be_broken_for_ipv6(void)
return dns_is_broken_for_ipv6;
}

/** Reset the counter that states that IPv6 DNS is broken. */
static void
dns_reset_ipv6_broken(evutil_socket_t fd, short event, void *args)
{
(void)fd;
(void)event;
(void)args;

dns_is_broken_for_ipv6 = 0;
}

#ifdef TOR_UNIT_TESTS
/** Set dns_is_broken_for_ipv6 to the value in val for unit test. */
void
dns_set_is_broken_for_ipv6(int val)
{
dns_is_broken_for_ipv6 = val;
}
#endif /* defined(TOR_UNIT_TESTS) */

/** Forget what we've previously learned about our DNS servers' correctness. */
void
dns_reset_correctness_checks(void)
@@ -32,12 +32,17 @@ size_t dns_cache_handle_oom(time_t now, size_t min_remove_bytes);
void dns_free_all(void);
void dns_launch_correctness_checks(void);

#ifdef TOR_UNIT_TESTS
void dns_set_is_broken_for_ipv6(int val);
#endif /* defined(TOR_UNIT_TESTS) */

#else /* !defined(HAVE_MODULE_RELAY) */

#define dns_init() (0)
#define dns_seems_to_be_broken() (0)
#define has_dns_init_failed() (0)
#define dns_cache_total_allocation() (0)
#define dns_seems_to_be_broken_for_ipv6() (0)

#define dns_reset_correctness_checks() STMT_NIL

@@ -2053,7 +2053,7 @@ router_build_fresh_unsigned_routerinfo,(routerinfo_t **ri_out))
policy_is_reject_star(ri->exit_policy, AF_INET, 1) &&
policy_is_reject_star(ri->exit_policy, AF_INET6, 1);

if (options->IPv6Exit) {
if (options->IPv6Exit && !dns_seems_to_be_broken_for_ipv6()) {
char *p_tmp = policy_summarize(ri->exit_policy, AF_INET6);
if (p_tmp)
ri->ipv6_exit_policy = parse_short_policy(p_tmp);
@@ -2,6 +2,7 @@
/* See LICENSE for licensing information */

#define CONFIG_PRIVATE
#define ROUTER_PRIVATE
#define POLICIES_PRIVATE

#include "core/or/or.h"
@@ -11,6 +12,8 @@
#include "feature/dirparse/policy_parse.h"
#include "feature/hs/hs_common.h"
#include "feature/hs/hs_descriptor.h"
#include "feature/nodelist/routerlist.h"
#include "feature/relay/dns.h"
#include "feature/relay/router.h"
#include "lib/encoding/confline.h"
#include "test/test.h"
@@ -2711,6 +2714,91 @@ test_policies_fascist_firewall_choose_address(void *arg)
UNMOCK(get_options);
}

static crypto_pk_t *mocked_onionkey = NULL;

/* Returns mocked_onionkey with no checks. */
static crypto_pk_t *
mock_get_onion_key(void)
{
return mocked_onionkey;
}

static curve25519_keypair_t mocked_curve25519_onion_key;

/* Returns mocked_curve25519_onion_key with no checks. */
static const curve25519_keypair_t *
mock_get_current_curve25519_keypair(void)
{
return &mocked_curve25519_onion_key;
}

static crypto_pk_t *mocked_server_identitykey = NULL;

/* Returns mocked_server_identitykey with no checks. */
static crypto_pk_t *
mock_get_server_identity_key(void)
{
return mocked_server_identitykey;
}

static tor_cert_t *
mock_tor_cert_dup_null(const tor_cert_t *cert)
{
(void)cert;
return NULL;
}

/** Run unit tests on an exit's failed IPv6 DNS resolver if
* we have the relay module. */
static void
test_policies_reject_failed_ipv6_dns(void *arg)
{
routerinfo_t *ri = NULL;
config_line_t line;
(void)arg;

memset(&mock_options, 0, sizeof(or_options_t));
MOCK(get_options, mock_get_options);
MOCK(get_onion_key, mock_get_onion_key);
MOCK(get_current_curve25519_keypair, mock_get_current_curve25519_keypair);
MOCK(get_server_identity_key, mock_get_server_identity_key);
MOCK(tor_cert_dup, mock_tor_cert_dup_null);

line.key = (char *) "ExitPolicy";
line.value = (char *) "accept *:*";
line.next = NULL;
mock_options.ExitPolicy = &line;

mock_options.ORPort_set = 1;
mock_options.Nickname = tor_strdup("Marina");
mock_options.ExitRelay = 1;
mock_options.IPv6Exit = 1;
mock_options.Address = tor_strdup("1.1.1.1");

mocked_server_identitykey = pk_generate(0);
mocked_onionkey = pk_generate(1);
curve25519_keypair_generate(&mocked_curve25519_onion_key, 0);

dns_set_is_broken_for_ipv6(0);
router_build_fresh_unsigned_routerinfo(&ri);
tt_assert(ri->ipv6_exit_policy != NULL);
routerinfo_free(ri);

dns_set_is_broken_for_ipv6(1);
router_build_fresh_unsigned_routerinfo(&ri);
tt_assert(ri->ipv6_exit_policy == NULL);
done:
dns_set_is_broken_for_ipv6(0);
crypto_pk_free(mocked_onionkey);
crypto_pk_free(mocked_server_identitykey);
routerinfo_free(ri);
UNMOCK(get_options);
UNMOCK(get_onion_key);
UNMOCK(get_current_curve25519_keypair);
UNMOCK(get_server_identity_key);
UNMOCK(tor_cert_dup);
}

#undef TEST_IPV4_ADDR_STR
#undef TEST_IPV6_ADDR_STR
#undef TEST_IPV4_OR_PORT
@@ -2736,5 +2824,7 @@ struct testcase_t policy_tests[] = {
test_policies_fascist_firewall_allows_address, 0, NULL, NULL },
{ "fascist_firewall_choose_address",
test_policies_fascist_firewall_choose_address, 0, NULL, NULL },
{ "reject_failed_ipv6_dns", test_policies_reject_failed_ipv6_dns,
0, NULL, NULL },
END_OF_TESTCASES
};