commit 35d08e30d89e5882b708a2cc6cb728f5393b2528 Author: Nick Mathewson nickm@torproject.org Date: Fri May 11 17:00:41 2012 -0400
An attempt at bug3940 and making AllowDotExit 0 work with MapAddress
This time, I follow grarpamp's suggestion and move the check for .exit+AllowDotExit 0 to the top of connection_ap_rewrite_and_attach, before any rewriting occurs. This way, .exit addresses are forbidden as they arrive from a socks connection or a DNSPort request, and not otherwise.
It _is_ a little more complicated than that, though. We need to treat any .exit addresses whose source is TrackHostExits as meaning that we can retry without that exit. We also need to treat any .exit address that comes from an AutomapHostsOnResolve operation as user-provided (and thus forbidden if AllowDotExits==0), so that transitioning from AllowDotExits==1 to AllowDotExits==0 will actually turn off automapped .exit addresses. --- changes/bug3940_redux | 5 ++ src/or/connection_edge.c | 99 +++++++++++++++++++++++++++++---------------- src/or/connection_edge.h | 5 +- src/or/or.h | 3 + src/or/relay.c | 2 +- src/test/test.c | 8 ++-- src/test/test_config.c | 46 +++++++++++----------- 7 files changed, 103 insertions(+), 65 deletions(-)
diff --git a/changes/bug3940_redux b/changes/bug3940_redux new file mode 100644 index 0000000..7733740 --- /dev/null +++ b/changes/bug3940_redux @@ -0,0 +1,5 @@ + o Major bugfixes: + - Change the AllowDotExit rules so they should actually work. + We now enforce AllowDotExit only immediately after receiving + an address via SOCKS or DNSPort: other sources are free to provide + .exit addresses after the resolution occurs. diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 5ef56a6..b15e135 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1095,13 +1095,17 @@ addressmap_match_superdomains(char *address) * address changed; false otherwise. Set *<b>expires_out</b> to the * expiry time of the result, or to <b>time_max</b> if the result does * not expire. + * + * DOCDOC exit_source_out; */ int -addressmap_rewrite(char *address, size_t maxlen, time_t *expires_out) +addressmap_rewrite(char *address, size_t maxlen, time_t *expires_out, + addressmap_entry_source_t *exit_source_out) { addressmap_entry_t *ent; int rewrites; time_t expires = TIME_MAX; + addressmap_entry_source_t exit_source = ADDRMAPSRC_NONE;
for (rewrites = 0; rewrites < 16; rewrites++) { int exact_match = 0; @@ -1117,9 +1121,7 @@ addressmap_rewrite(char *address, size_t maxlen, time_t *expires_out) /* This is a rule like *.example.com example.com, and we just got * "example.com" */ tor_free(addr_orig); - if (expires_out) - *expires_out = expires; - return rewrites > 0; + goto done; }
exact_match = 1; @@ -1127,9 +1129,7 @@ addressmap_rewrite(char *address, size_t maxlen, time_t *expires_out)
if (!ent || !ent->new_address) { tor_free(addr_orig); - if (expires_out) - *expires_out = expires; - return (rewrites > 0); /* done, no rewrite needed */ + goto done; }
if (ent->dst_wildcard && !exact_match) { @@ -1139,6 +1139,12 @@ addressmap_rewrite(char *address, size_t maxlen, time_t *expires_out) strlcpy(address, ent->new_address, maxlen); }
+ if (!strcmpend(address, ".exit") && + strcmpend(addr_orig, ".exit") && + exit_source == ADDRMAPSRC_NONE) { + exit_source = ent->source; + } + log_info(LD_APP, "Addressmap: rewriting %s to %s", addr_orig, escaped_safe_str_client(address)); if (ent->expires > 1 && ent->expires < expires) @@ -1149,9 +1155,13 @@ addressmap_rewrite(char *address, size_t maxlen, time_t *expires_out) "Loop detected: we've rewritten %s 16 times! Using it as-is.", escaped_safe_str_client(address)); /* it's fine to rewrite a rewrite, but don't loop forever */ + + done: + if (exit_source_out) + *exit_source_out = exit_source; if (expires_out) *expires_out = TIME_MAX; - return 1; + return (rewrites > 0); }
/** If we have a cached reverse DNS entry for the address stored in the @@ -1782,11 +1792,9 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, int automap = 0; char orig_address[MAX_SOCKS_ADDR_LEN]; time_t map_expires = TIME_MAX; - /* This will be set to true iff the address starts out as a non-.exit - address, and we remap it to one because of an entry in the addressmap. */ - int remapped_to_exit = 0; time_t now = time(NULL); connection_t *base_conn = ENTRY_TO_CONN(conn); + addressmap_entry_source_t exit_source = ADDRMAPSRC_NONE;
tor_strlower(socks->address); /* normalize it */ strlcpy(orig_address, socks->address, sizeof(orig_address)); @@ -1794,6 +1802,16 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, safe_str_client(socks->address), socks->port);
+ if (!strcmpend(socks->address, ".exit") && !options->AllowDotExit) { + log_warn(LD_APP, "The ".exit" notation is disabled in Tor due to " + "security risks. Set AllowDotExit in your torrc to enable " + "it (at your own risk)."); + control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", + escaped(socks->address)); + connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); + return -1; + } + if (! conn->original_dest_address) conn->original_dest_address = tor_strdup(conn->socks_request->address);
@@ -1854,16 +1872,11 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, } } } else if (!automap) { - int started_without_chosen_exit = strcasecmpend(socks->address, ".exit"); /* For address map controls, remap the address. */ if (addressmap_rewrite(socks->address, sizeof(socks->address), - &map_expires)) { + &map_expires, &exit_source)) { control_event_stream_status(conn, STREAM_EVENT_REMAP, REMAP_STREAM_SOURCE_CACHE); - if (started_without_chosen_exit && - !strcasecmpend(socks->address, ".exit") && - map_expires < TIME_MAX) - remapped_to_exit = 1; } }
@@ -1882,8 +1895,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, /* Parse the address provided by SOCKS. Modify it in-place if it * specifies a hidden-service (.onion) or particular exit node (.exit). */ - addresstype = parse_extended_hostname(socks->address, - remapped_to_exit || options->AllowDotExit); + addresstype = parse_extended_hostname(socks->address);
if (addresstype == BAD_HOSTNAME) { control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", @@ -1902,14 +1914,42 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, options->_ExcludeExitNodesUnion : options->ExcludeExitNodes; const node_t *node;
+ if (exit_source == ADDRMAPSRC_AUTOMAP && !options->AllowDotExit) { + /* Whoops; this one is stale. It must have gotten added earlier, + * when AllowDotExit was on. */ + log_warn(LD_APP,"Stale automapped address for '%s.exit', with " + "AllowDotExit disabled. Refusing.", + safe_str_client(socks->address)); + control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", + escaped(socks->address)); + connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); + return -1; + } + + if (exit_source == ADDRMAPSRC_DNS || + (exit_source == ADDRMAPSRC_NONE && !options->AllowDotExit)) { + /* It shouldn't be possible to get a .exit address from any of these + * sources. */ + log_warn(LD_BUG,"Address '%s.exit', with impossible source for the .exit " + "part. Refusing.", + safe_str_client(socks->address)); + control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", + escaped(socks->address)); + connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); + return -1; + } + tor_assert(!automap); if (s) { /* The address was of the form "(stuff).(name).exit */ if (s[1] != '\0') { conn->chosen_exit_name = tor_strdup(s+1); node = node_get_by_nickname(conn->chosen_exit_name, 1); - if (remapped_to_exit) /* 5 tries before it expires the addressmap */ + + if (exit_source == ADDRMAPSRC_TRACKEXIT) { + /* We 5 tries before it expires the addressmap */ conn->chosen_exit_retries = TRACKHOSTEXITS_RETRIES; + } *s = 0; } else { /* Oops, the address was (stuff)..exit. That's not okay. */ @@ -3407,17 +3447,14 @@ connection_ap_can_use_exit(const entry_connection_t *conn, const node_t *exit) * If address is of the form "y.onion" with a badly-formed handle y: * Return BAD_HOSTNAME and log a message. * - * If address is of the form "y.exit" and <b>allowdotexit</b> is true: + * If address is of the form "y.exit": * Put a NUL after y and return EXIT_HOSTNAME. * - * If address is of the form "y.exit" and <b>allowdotexit</b> is false: - * Return BAD_HOSTNAME and log a message. - * * Otherwise: * Return NORMAL_HOSTNAME and change nothing. */ hostname_type_t -parse_extended_hostname(char *address, int allowdotexit) +parse_extended_hostname(char *address) { char *s; char query[REND_SERVICE_ID_LEN_BASE32+1]; @@ -3426,16 +3463,8 @@ parse_extended_hostname(char *address, int allowdotexit) if (!s) return NORMAL_HOSTNAME; /* no dot, thus normal */ if (!strcmp(s+1,"exit")) { - if (allowdotexit) { - *s = 0; /* NUL-terminate it */ - return EXIT_HOSTNAME; /* .exit */ - } else { - log_warn(LD_APP, "The ".exit" notation is disabled in Tor due to " - "security risks. Set AllowDotExit in your torrc to enable " - "it."); - /* FFFF send a controller event too to notify Vidalia users */ - return BAD_HOSTNAME; - } + *s = 0; /* NUL-terminate it */ + return EXIT_HOSTNAME; /* .exit */ } if (strcmp(s+1,"onion")) return NORMAL_HOSTNAME; /* neither .exit nor .onion, thus normal */ diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index 47c9c45..b69e476 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -74,7 +74,8 @@ void addressmap_clean(time_t now); void addressmap_clear_configured(void); void addressmap_clear_transient(void); void addressmap_free_all(void); -int addressmap_rewrite(char *address, size_t maxlen, time_t *expires_out); +int addressmap_rewrite(char *address, size_t maxlen, time_t *expires_out, + addressmap_entry_source_t *exit_source_out); int addressmap_have_mapping(const char *address, int update_timeout);
void addressmap_register(const char *address, char *new_address, @@ -100,7 +101,7 @@ int connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, typedef enum hostname_type_t { NORMAL_HOSTNAME, ONION_HOSTNAME, EXIT_HOSTNAME, BAD_HOSTNAME } hostname_type_t; -hostname_type_t parse_extended_hostname(char *address, int allowdotexit); +hostname_type_t parse_extended_hostname(char *address);
#if defined(HAVE_NET_IF_H) && defined(HAVE_NET_PFVAR_H) int get_pf_socket(void); diff --git a/src/or/or.h b/src/or/or.h index b8fee64..f1260d1 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -3856,6 +3856,9 @@ typedef enum { /** We're remapping this address because we got a DNS resolution from a * Tor server that told us what its value was. */ ADDRMAPSRC_DNS, + + /** DOCDOC */ + ADDRMAPSRC_NONE } addressmap_entry_source_t;
/********************************* control.c ***************************/ diff --git a/src/or/relay.c b/src/or/relay.c index 38a563f..ad98e05 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -761,7 +761,7 @@ connection_ap_process_end_not_open( /* rewrite it to an IP if we learned one. */ if (addressmap_rewrite(conn->socks_request->address, sizeof(conn->socks_request->address), - NULL)) { + NULL, NULL)) { control_event_stream_status(conn, STREAM_EVENT_REMAP, 0); } if (conn->chosen_exit_optional || diff --git a/src/test/test.c b/src/test/test.c index 7f196aa..829e806 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1287,10 +1287,10 @@ test_rend_fns(void) char address3[] = "fooaddress.exit"; char address4[] = "www.torproject.org";
- test_assert(BAD_HOSTNAME == parse_extended_hostname(address1, 1)); - test_assert(ONION_HOSTNAME == parse_extended_hostname(address2, 1)); - test_assert(EXIT_HOSTNAME == parse_extended_hostname(address3, 1)); - test_assert(NORMAL_HOSTNAME == parse_extended_hostname(address4, 1)); + test_assert(BAD_HOSTNAME == parse_extended_hostname(address1)); + test_assert(ONION_HOSTNAME == parse_extended_hostname(address2)); + test_assert(EXIT_HOSTNAME == parse_extended_hostname(address3)); + test_assert(NORMAL_HOSTNAME == parse_extended_hostname(address4));
pk1 = pk_generate(0); pk2 = pk_generate(1); diff --git a/src/test/test_config.c b/src/test/test_config.c index d8161de..4c77af5 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -42,56 +42,56 @@ test_config_addressmap(void *arg)
/* MapAddress .invalidwildcard.com .torserver.exit - no match */ strlcpy(address, "www.invalidwildcard.com", sizeof(address)); - test_assert(!addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(!addressmap_rewrite(address, sizeof(address), &expires, NULL));
/* MapAddress *invalidasterisk.com .torserver.exit - no match */ strlcpy(address, "www.invalidasterisk.com", sizeof(address)); - test_assert(!addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(!addressmap_rewrite(address, sizeof(address), &expires, NULL));
/* Where no mapping for FQDN match on top-level domain */ /* MapAddress .google.com .torserver.exit */ strlcpy(address, "reader.google.com", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "reader.torserver.exit");
/* MapAddress *.yahoo.com *.google.com.torserver.exit */ strlcpy(address, "reader.yahoo.com", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "reader.google.com.torserver.exit");
/*MapAddress *.cnn.com www.cnn.com */ strlcpy(address, "cnn.com", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "www.cnn.com");
/* MapAddress .cn.com www.cnn.com */ strlcpy(address, "www.cn.com", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "www.cnn.com");
/* MapAddress ex.com www.cnn.com - no match */ strlcpy(address, "www.ex.com", sizeof(address)); - test_assert(!addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(!addressmap_rewrite(address, sizeof(address), &expires, NULL));
/* MapAddress ey.com *.cnn.com - invalid expression */ strlcpy(address, "ey.com", sizeof(address)); - test_assert(!addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(!addressmap_rewrite(address, sizeof(address), &expires, NULL));
/* Where mapping for FQDN match on FQDN */ strlcpy(address, "www.google.com", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "3.3.3.3");
strlcpy(address, "www.torproject.org", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "1.1.1.1");
strlcpy(address, "other.torproject.org", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "this.torproject.org.otherserver.exit");
strlcpy(address, "test.torproject.org", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "2.2.2.2");
/* Test a chain of address mappings and the order in which they were added: @@ -100,17 +100,17 @@ test_config_addressmap(void *arg) "MapAddress 4.4.4.4 5.5.5.5" */ strlcpy(address, "www.example.org", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "5.5.5.5");
/* Test infinite address mapping results in no change */ strlcpy(address, "www.infiniteloop.org", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "www.infiniteloop.org");
/* Test we don't find false positives */ strlcpy(address, "www.example.com", sizeof(address)); - test_assert(!addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(!addressmap_rewrite(address, sizeof(address), &expires, NULL));
/* Test top-level-domain matching a bit harder */ addressmap_clear_configured(); @@ -122,23 +122,23 @@ test_config_addressmap(void *arg) config_register_addressmaps(get_options());
strlcpy(address, "www.abc.com", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "www.abc.torserver.exit");
strlcpy(address, "www.def.com", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "www.def.torserver.exit");
strlcpy(address, "www.torproject.org", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "1.1.1.1");
strlcpy(address, "test.torproject.org", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "1.1.1.1");
strlcpy(address, "torproject.net", sizeof(address)); - test_assert(addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(addressmap_rewrite(address, sizeof(address), &expires, NULL)); test_streq(address, "2.2.2.2");
/* We don't support '*' as a mapping directive */ @@ -148,13 +148,13 @@ test_config_addressmap(void *arg) config_register_addressmaps(get_options());
strlcpy(address, "www.abc.com", sizeof(address)); - test_assert(!addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(!addressmap_rewrite(address, sizeof(address), &expires, NULL));
strlcpy(address, "www.def.net", sizeof(address)); - test_assert(!addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(!addressmap_rewrite(address, sizeof(address), &expires, NULL));
strlcpy(address, "www.torproject.org", sizeof(address)); - test_assert(!addressmap_rewrite(address, sizeof(address), &expires)); + test_assert(!addressmap_rewrite(address, sizeof(address), &expires, NULL));
done: ;