From fc2831558c498d6fbc0071d1ebef5c42006a4a27 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 10 Jan 2015 19:59:24 -0500 Subject: [PATCH 01/15] Split the rewrite part of rewrite-and-attach I'd also like to split out the part that sends early socks responses. --- src/or/connection_edge.c | 109 +++++++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 40 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index d8f397bd90..ed85c72d30 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -908,41 +908,32 @@ connection_ap_rewrite_and_attach_if_allowed(entry_connection_t *conn, return connection_ap_handshake_rewrite_and_attach(conn, circ, cpath); } -/** Connection conn just finished its socks handshake, or the - * controller asked us to take care of it. If circ is defined, - * then that's where we'll want to attach it. Otherwise we have to - * figure it out ourselves. - * - * First, parse whether it's a .exit address, remap it, and so on. Then - * if it's for a general circuit, try to attach it to a circuit (or launch - * one as needed), else if it's for a rendezvous circuit, fetch a - * rendezvous descriptor first (or attach/launch a circuit if the - * rendezvous descriptor is already here and fresh enough). - * - * The stream will exit from the hop - * indicated by cpath, or from the last hop in circ's cpath if - * cpath is NULL. - */ -int -connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, - origin_circuit_t *circ, - crypt_path_t *cpath) -{ - socks_request_t *socks = conn->socks_request; - hostname_type_t addresstype; - const or_options_t *options = get_options(); - tor_addr_t addr_tmp; +typedef struct { + char orig_address[MAX_SOCKS_ADDR_LEN]; /* We set this to true if this is an address we should automatically * remap to a local address in VirtualAddrNetwork */ - int automap = 0; - char orig_address[MAX_SOCKS_ADDR_LEN]; - time_t map_expires = TIME_MAX; - time_t now = time(NULL); - connection_t *base_conn = ENTRY_TO_CONN(conn); - addressmap_entry_source_t exit_source = ADDRMAPSRC_NONE; + int automap; + addressmap_entry_source_t exit_source; + time_t map_expires; +} rewrite_result_t; + +/* DOCDOC 0 if closed successfully. -1 if closed on error. 1 if not + * closed. +*/ +static int +connection_ap_handshake_rewrite(entry_connection_t *conn, + rewrite_result_t *out) +{ + socks_request_t *socks = conn->socks_request; + const or_options_t *options = get_options(); + tor_addr_t addr_tmp; + + out->automap = 0; + out->exit_source = ADDRMAPSRC_NONE; + out->map_expires = TIME_MAX; tor_strlower(socks->address); /* normalize it */ - strlcpy(orig_address, socks->address, sizeof(orig_address)); + strlcpy(out->orig_address, socks->address, sizeof(out->orig_address)); log_debug(LD_APP,"Client asked for %s:%d", safe_str_client(socks->address), socks->port); @@ -963,8 +954,8 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, if (socks->command == SOCKS_COMMAND_RESOLVE && tor_addr_parse(&addr_tmp, socks->address)<0 && options->AutomapHostsOnResolve) { - automap = addressmap_address_should_automap(socks->address, options); - if (automap) { + out->automap = addressmap_address_should_automap(socks->address, options); + if (out->automap) { const char *new_addr; int addr_type = RESOLVED_TYPE_IPV4; if (conn->socks_request->socks_version != 4) { @@ -996,15 +987,15 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, rewrite_flags |= AMR_FLAG_USE_IPV6_DNS; if (addressmap_rewrite_reverse(socks->address, sizeof(socks->address), - rewrite_flags, &map_expires)) { + rewrite_flags, &out->map_expires)) { char *result = tor_strdup(socks->address); /* remember _what_ is supposed to have been resolved. */ tor_snprintf(socks->address, sizeof(socks->address), "REVERSE[%s]", - orig_address); + out->orig_address); connection_ap_handshake_socks_resolved(conn, RESOLVED_TYPE_HOSTNAME, strlen(result), (uint8_t*)result, -1, - map_expires); + out->map_expires); connection_mark_unattached_ap(conn, END_STREAM_REASON_DONE | END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED); @@ -1025,7 +1016,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, return -1; } } - } else if (!automap) { + } else if (!out->automap) { /* For address map controls, remap the address. */ unsigned rewrite_flags = 0; if (conn->use_cached_ipv4_answers) @@ -1033,13 +1024,13 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, if (conn->use_cached_ipv6_answers) rewrite_flags |= AMR_FLAG_USE_IPV6_DNS; if (addressmap_rewrite(socks->address, sizeof(socks->address), - rewrite_flags, &map_expires, &exit_source)) { + rewrite_flags, &out->map_expires, &out->exit_source)) { control_event_stream_status(conn, STREAM_EVENT_REMAP, REMAP_STREAM_SOURCE_CACHE); } } - if (!automap && address_is_in_virtual_range(socks->address)) { + if (!out->automap && address_is_in_virtual_range(socks->address)) { /* This address was probably handed out by client_dns_get_unmapped_address, * but the mapping was discarded for some reason. We *don't* want to send * the address through Tor; that's likely to fail, and may leak @@ -1051,6 +1042,44 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, return -1; } + return 1; +} + +/** Connection conn just finished its socks handshake, or the + * controller asked us to take care of it. If circ is defined, + * then that's where we'll want to attach it. Otherwise we have to + * figure it out ourselves. + * + * First, parse whether it's a .exit address, remap it, and so on. Then + * if it's for a general circuit, try to attach it to a circuit (or launch + * one as needed), else if it's for a rendezvous circuit, fetch a + * rendezvous descriptor first (or attach/launch a circuit if the + * rendezvous descriptor is already here and fresh enough). + * + * The stream will exit from the hop + * indicated by cpath, or from the last hop in circ's cpath if + * cpath is NULL. + */ +int +connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, + origin_circuit_t *circ, + crypt_path_t *cpath) +{ + int r; + socks_request_t *socks = conn->socks_request; + hostname_type_t addresstype; + const or_options_t *options = get_options(); + connection_t *base_conn = ENTRY_TO_CONN(conn); + time_t now = time(NULL); + rewrite_result_t rr; + + memset(&rr, 0, sizeof(rr)); + if ((r = connection_ap_handshake_rewrite(conn,&rr)) != 1) + return r; + time_t map_expires = rr.map_expires; + int automap = rr.automap; + addressmap_entry_source_t exit_source = rr.exit_source; + /* Parse the address provided by SOCKS. Modify it in-place if it * specifies a hidden-service (.onion) or particular exit node (.exit). */ @@ -1175,7 +1204,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, /* Reply to resolves immediately if we can. */ if (tor_addr_parse(&answer, socks->address) >= 0) {/* is it an IP? */ /* remember _what_ is supposed to have been resolved. */ - strlcpy(socks->address, orig_address, sizeof(socks->address)); + strlcpy(socks->address, rr.orig_address, sizeof(socks->address)); connection_ap_handshake_socks_resolved_addr(conn, &answer, -1, map_expires); connection_mark_unattached_ap(conn, From cd6a57e3d560c023de849ba3cd70b4625a85a3f2 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 10 Jan 2015 20:07:35 -0500 Subject: [PATCH 02/15] Move stream-closing out of rewrite code --- src/or/connection_edge.c | 56 ++++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index ed85c72d30..c1215f7101 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -915,12 +915,14 @@ typedef struct { int automap; addressmap_entry_source_t exit_source; time_t map_expires; + + int end_reason; + int should_close; } rewrite_result_t; -/* DOCDOC 0 if closed successfully. -1 if closed on error. 1 if not - * closed. +/* DOCDOC */ -static int +static void connection_ap_handshake_rewrite(entry_connection_t *conn, rewrite_result_t *out) { @@ -931,6 +933,8 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, out->automap = 0; out->exit_source = ADDRMAPSRC_NONE; out->map_expires = TIME_MAX; + out->end_reason = 0; + out->should_close = 0; tor_strlower(socks->address); /* normalize it */ strlcpy(out->orig_address, socks->address, sizeof(out->orig_address)); @@ -944,8 +948,9 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, "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; + out->end_reason = END_STREAM_REASON_TORPROTOCOL; + out->should_close = 1; + return; } if (! conn->original_dest_address) @@ -969,8 +974,9 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, if (! new_addr) { log_warn(LD_APP, "Unable to automap address %s", escaped_safe_str(socks->address)); - connection_mark_unattached_ap(conn, END_STREAM_REASON_INTERNAL); - return -1; + out->end_reason = END_STREAM_REASON_INTERNAL; + out->should_close = 1; + return; } log_info(LD_APP, "Automapping %s to %s", escaped_safe_str_client(socks->address), @@ -996,10 +1002,10 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, strlen(result), (uint8_t*)result, -1, out->map_expires); - connection_mark_unattached_ap(conn, - END_STREAM_REASON_DONE | - END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED); - return 0; + out->end_reason = END_STREAM_REASON_DONE | + END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED; + out->should_close = 1; + return; } if (options->ClientDNSRejectInternalAddresses) { /* Don't let people try to do a reverse lookup on 10.0.0.1. */ @@ -1010,10 +1016,10 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, if (ok == 1 && tor_addr_is_internal(&addr, 0)) { connection_ap_handshake_socks_resolved(conn, RESOLVED_TYPE_ERROR, 0, NULL, -1, TIME_MAX); - connection_mark_unattached_ap(conn, - END_STREAM_REASON_SOCKSPROTOCOL | - END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED); - return -1; + out->end_reason = END_STREAM_REASON_SOCKSPROTOCOL | + END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED; + out->should_close = 1; + return; } } } else if (!out->automap) { @@ -1038,11 +1044,10 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, */ log_warn(LD_APP,"Missing mapping for virtual address '%s'. Refusing.", safe_str_client(socks->address)); - connection_mark_unattached_ap(conn, END_STREAM_REASON_INTERNAL); - return -1; + out->end_reason = END_STREAM_REASON_INTERNAL; + out->should_close = 1; + return; } - - return 1; } /** Connection conn just finished its socks handshake, or the @@ -1065,7 +1070,6 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, origin_circuit_t *circ, crypt_path_t *cpath) { - int r; socks_request_t *socks = conn->socks_request; hostname_type_t addresstype; const or_options_t *options = get_options(); @@ -1074,8 +1078,16 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, rewrite_result_t rr; memset(&rr, 0, sizeof(rr)); - if ((r = connection_ap_handshake_rewrite(conn,&rr)) != 1) - return r; + connection_ap_handshake_rewrite(conn,&rr); + + if (rr.should_close) { + connection_mark_unattached_ap(conn, rr.end_reason); + if (0 != (rr.end_reason & END_STREAM_REASON_DONE)) + return 0; + else + return -1; + } + time_t map_expires = rr.map_expires; int automap = rr.automap; addressmap_entry_source_t exit_source = rr.exit_source; From 715fdfcb7b44ae6fdad886e688708ec0d483a1f8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sat, 10 Jan 2015 20:14:55 -0500 Subject: [PATCH 03/15] document rewrite_result_t and export for testing --- src/or/connection_edge.c | 19 ++++--------------- src/or/connection_edge.h | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index c1215f7101..56e496f670 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -908,21 +908,10 @@ connection_ap_rewrite_and_attach_if_allowed(entry_connection_t *conn, return connection_ap_handshake_rewrite_and_attach(conn, circ, cpath); } -typedef struct { - char orig_address[MAX_SOCKS_ADDR_LEN]; - /* We set this to true if this is an address we should automatically - * remap to a local address in VirtualAddrNetwork */ - int automap; - addressmap_entry_source_t exit_source; - time_t map_expires; - - int end_reason; - int should_close; -} rewrite_result_t; - -/* DOCDOC -*/ -static void +/* Try to perform any map-based rewriting of the target address in conn, + * filling in the fields of out as we go. + */ +STATIC void connection_ap_handshake_rewrite(entry_connection_t *conn, rewrite_result_t *out) { diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index e6adad91d8..af564cec0c 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -143,6 +143,30 @@ STATIC int begin_cell_parse(const cell_t *cell, begin_cell_t *bcell, STATIC int connected_cell_format_payload(uint8_t *payload_out, const tor_addr_t *addr, uint32_t ttl); + + +typedef struct { + /** Original address, after we lowercased it but before we started + * mapping it. + */ + char orig_address[MAX_SOCKS_ADDR_LEN]; + /** True iff the address has been automatically remapped to a local + * address in VirtualAddrNetwork */ + int automap; + /** If this connection has a .exit address, who put it there? */ + addressmap_entry_source_t exit_source; + /** If we've rewritten the address, when does this map expire? */ + time_t map_expires; + /** If we should close the connection, this is the end_reason to pass + * to connection_mark_unattached_ap */ + int end_reason; + /** True iff we should close the connection, either because of error or + * because of successful early RESOLVED reply. */ + int should_close; +} rewrite_result_t; + +STATIC void connection_ap_handshake_rewrite(entry_connection_t *conn, + rewrite_result_t *out); #endif #endif From bba995e6660dbb49876886278f71d7d5a85f5f88 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 12 Jan 2015 21:08:43 -0500 Subject: [PATCH 04/15] Begin writing unit tests for rewrite code --- src/or/connection_edge.c | 1 + src/or/connection_edge.h | 3 +- src/test/include.am | 1 + src/test/test.c | 2 + src/test/test_entryconn.c | 265 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 src/test/test_entryconn.c diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 56e496f670..ca99a86ae0 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -924,6 +924,7 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, out->map_expires = TIME_MAX; out->end_reason = 0; out->should_close = 0; + out->orig_address[0] = 0; tor_strlower(socks->address); /* normalize it */ strlcpy(out->orig_address, socks->address, sizeof(out->orig_address)); diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index af564cec0c..d4a20e7f96 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -151,7 +151,8 @@ typedef struct { */ char orig_address[MAX_SOCKS_ADDR_LEN]; /** True iff the address has been automatically remapped to a local - * address in VirtualAddrNetwork */ + * address in VirtualAddrNetwork. (Only set true the first time we + * do the mapping.) */ int automap; /** If this connection has a .exit address, who put it there? */ addressmap_entry_source_t exit_source; diff --git a/src/test/include.am b/src/test/include.am index 9db1587da7..46db801d27 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -31,6 +31,7 @@ src_test_test_SOURCES = \ src/test/test_data.c \ src/test/test_dir.c \ src/test/test_checkdir.c \ + src/test/test_entryconn.c \ src/test/test_entrynodes.c \ src/test/test_extorport.c \ src/test/test_introduce.c \ diff --git a/src/test/test.c b/src/test/test.c index de6efaf873..65d15f12bd 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -1313,6 +1313,7 @@ extern struct testcase_t channel_tests[]; extern struct testcase_t channeltls_tests[]; extern struct testcase_t relay_tests[]; extern struct testcase_t scheduler_tests[]; +extern struct testcase_t entryconn_tests[]; static struct testgroup_t testgroups[] = { { "", test_array }, @@ -1337,6 +1338,7 @@ static struct testgroup_t testgroups[] = { { "circuitmux/", circuitmux_tests }, { "options/", options_tests }, { "entrynodes/", entrynodes_tests }, + { "entryconn/", entryconn_tests }, { "extorport/", extorport_tests }, { "control/", controller_event_tests }, { "hs/", hs_tests }, diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c new file mode 100644 index 0000000000..1c77394df0 --- /dev/null +++ b/src/test/test_entryconn.c @@ -0,0 +1,265 @@ +/* Copyright (c) 2014-2015, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#include "orconfig.h" + +#define CONNECTION_PRIVATE +#define CONNECTION_EDGE_PRIVATE + +#include "or.h" +#include "test.h" + +#include "addressmap.h" +#include "config.h" +#include "connection.h" +#include "connection_edge.h" + +static void * +entryconn_rewrite_setup(const struct testcase_t *tc) +{ + (void)tc; + entry_connection_t *ec = entry_connection_new(CONN_TYPE_AP, AF_INET); + addressmap_init(); + return ec; +} + +static int +entryconn_rewrite_teardown(const struct testcase_t *tc, void *arg) +{ + (void)tc; + entry_connection_t *ec = arg; + if (ec) + connection_free_(ENTRY_TO_CONN(ec)); + addressmap_free_all(); + return 1; +} + +static struct testcase_setup_t test_rewrite_setup = { + entryconn_rewrite_setup, entryconn_rewrite_teardown +}; + +/* Simple rewrite: no changes needed */ +static void +test_entryconn_rewrite_basic(void *arg) +{ + entry_connection_t *ec = arg; + rewrite_result_t rr; + + tt_assert(ec->socks_request); + strlcpy(ec->socks_request->address, "www.TORproject.org", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_int_op(rr.automap, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "www.torproject.org"); + tt_str_op(ec->socks_request->address, OP_EQ, "www.torproject.org"); + tt_str_op(ec->original_dest_address, OP_EQ, "www.torproject.org"); + + done: + ; +} + +/* Rewrite but reject because of disallowed .exit */ +static void +test_entryconn_rewrite_bad_dotexit(void *arg) +{ + entry_connection_t *ec = arg; + rewrite_result_t rr; + + get_options_mutable()->AllowDotExit = 0; + tt_assert(ec->socks_request); + strlcpy(ec->socks_request->address, "www.TORproject.org.foo.exit", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.should_close, OP_EQ, 1); + tt_int_op(rr.end_reason, OP_EQ, END_STREAM_REASON_TORPROTOCOL); + + done: + ; +} + +/* Automap on resolve, connect to automapped address, resolve again and get + * same answer. (IPv4) */ +static void +test_entryconn_rewrite_automap_ipv4(void *arg) +{ + entry_connection_t *ec = arg; + entry_connection_t *ec2=NULL, *ec3=NULL; + rewrite_result_t rr; + char *msg = NULL; + + ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET); + ec3 = entry_connection_new(CONN_TYPE_AP, AF_INET); + + get_options_mutable()->AutomapHostsOnResolve = 1; + get_options_mutable()->AutomapHostsSuffixes = smartlist_new(); + smartlist_add(get_options_mutable()->AutomapHostsSuffixes, tor_strdup(".")); + parse_virtual_addr_network("127.202.0.0/16", AF_INET, 0, &msg); + + /* Automap this on resolve. */ + strlcpy(ec->socks_request->address, "WWW.MIT.EDU", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_RESOLVE; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "www.mit.edu"); + tt_str_op(ec->original_dest_address, OP_EQ, "www.mit.edu"); + + tt_assert(!strcmpstart(ec->socks_request->address,"127.202.")); + + /* Connect to it and make sure we get the original address back. */ + strlcpy(ec2->socks_request->address, ec->socks_request->address, + sizeof(ec2->socks_request->address)); + + ec2->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec2, &rr); + + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, ec->socks_request->address); + tt_str_op(ec2->original_dest_address, OP_EQ, ec->socks_request->address); + tt_str_op(ec2->socks_request->address, OP_EQ, "www.mit.edu"); + + /* Resolve it again, make sure the answer is the same. */ + strlcpy(ec3->socks_request->address, "www.MIT.EDU", + sizeof(ec3->socks_request->address)); + ec3->socks_request->command = SOCKS_COMMAND_RESOLVE; + connection_ap_handshake_rewrite(ec3, &rr); + + tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "www.mit.edu"); + tt_str_op(ec3->original_dest_address, OP_EQ, "www.mit.edu"); + + tt_str_op(ec3->socks_request->address, OP_EQ, + ec->socks_request->address); + + done: + connection_free_(ENTRY_TO_CONN(ec2)); + connection_free_(ENTRY_TO_CONN(ec3)); +} + +/* Automap on resolve, connect to automapped address, resolve again and get + * same answer. (IPv6) */ +static void +test_entryconn_rewrite_automap_ipv6(void *arg) +{ + entry_connection_t *ec = arg; + entry_connection_t *ec2=NULL, *ec3=NULL; + rewrite_result_t rr; + char *msg = NULL; + connection_free_(ENTRY_TO_CONN(ec)); + ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET6); + ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET6); + ec3 = entry_connection_new(CONN_TYPE_AP, AF_INET6); + + get_options_mutable()->AutomapHostsOnResolve = 1; + get_options_mutable()->AutomapHostsSuffixes = smartlist_new(); + smartlist_add(get_options_mutable()->AutomapHostsSuffixes, tor_strdup(".")); + parse_virtual_addr_network("FE80::/32", AF_INET6, 0, &msg); + + /* Automap this on resolve. */ + strlcpy(ec->socks_request->address, "WWW.MIT.EDU", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_RESOLVE; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "www.mit.edu"); + tt_str_op(ec->original_dest_address, OP_EQ, "www.mit.edu"); + + printf("<%s>\n", ec->socks_request->address); + /* XXXX Should this [ be here? */ + tt_assert(!strcmpstart(ec->socks_request->address,"[fe80:")); + + /* Connect to it and make sure we get the original address back. */ + strlcpy(ec2->socks_request->address, ec->socks_request->address, + sizeof(ec2->socks_request->address)); + + ec2->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec2, &rr); + + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, ec->socks_request->address); + tt_str_op(ec2->original_dest_address, OP_EQ, ec->socks_request->address); + tt_str_op(ec2->socks_request->address, OP_EQ, "www.mit.edu"); + + /* Resolve it again, make sure the answer is the same. */ + strlcpy(ec3->socks_request->address, "www.MIT.EDU", + sizeof(ec3->socks_request->address)); + ec3->socks_request->command = SOCKS_COMMAND_RESOLVE; + connection_ap_handshake_rewrite(ec3, &rr); + + tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "www.mit.edu"); + tt_str_op(ec3->original_dest_address, OP_EQ, "www.mit.edu"); + + tt_str_op(ec3->socks_request->address, OP_EQ, + ec->socks_request->address); + + done: + connection_free_(ENTRY_TO_CONN(ec2)); + connection_free_(ENTRY_TO_CONN(ec3)); +} + +/* automap on resolve, reverse lookup. */ + +/* Rewrite because of cached DNS entry. */ + +/* Rewrite because of AddrmapRewrite option */ + +/* Rewrite because of control port */ + +/* Rewrite into .exit because of virtual address mapping */ + +/* Rewrite into .exit because of mapaddress */ + +/* Fail to connect to unmapped address in virtual range. */ + +/* Rewrite plus automap */ + +/* Map foo.onion to longthing.onion */ + + + +#define REWRITE(name) \ + { #name, test_entryconn_##name, TT_FORK, &test_rewrite_setup, NULL } + +struct testcase_t entryconn_tests[] = { + REWRITE(rewrite_basic), + REWRITE(rewrite_bad_dotexit), + REWRITE(rewrite_automap_ipv4), + REWRITE(rewrite_automap_ipv6), + END_OF_TESTCASES +}; + From 2e1ed0815d3f426e7e30b09f3c781425c76e72d5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 13 Jan 2015 09:42:23 -0500 Subject: [PATCH 05/15] Actually set *expires_out in addressmap_rewrite. Fixes 14193; bugfix on 35d08e30d, which went into 0.2.3.17-beta. --- changes/bug14193 | 4 ++++ src/or/addressmap.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changes/bug14193 diff --git a/changes/bug14193 b/changes/bug14193 new file mode 100644 index 0000000000..a7006685f5 --- /dev/null +++ b/changes/bug14193 @@ -0,0 +1,4 @@ + o Minor bugfixes (client DNS): + - Report the correct cached DNS expiration times. Previously, we + would report everything as "never expires." Fixes bug 14193; + bugfix on 0.2.3.17-beta. diff --git a/src/or/addressmap.c b/src/or/addressmap.c index 8ad24323b5..908d94faa1 100644 --- a/src/or/addressmap.c +++ b/src/or/addressmap.c @@ -431,7 +431,7 @@ addressmap_rewrite(char *address, size_t maxlen, if (exit_source_out) *exit_source_out = exit_source; if (expires_out) - *expires_out = TIME_MAX; + *expires_out = expires; return (rewrites > 0); } From 05a80bb46c4e71f46f71b83e7ec076fadd20c55a Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 13 Jan 2015 11:08:33 -0500 Subject: [PATCH 06/15] More unit tests for rewriting entry connection addresses --- src/or/addressmap.c | 4 + src/test/test_entryconn.c | 428 +++++++++++++++++++++++++++++++++++++- 2 files changed, 421 insertions(+), 11 deletions(-) diff --git a/src/or/addressmap.c b/src/or/addressmap.c index 908d94faa1..0f417e6369 100644 --- a/src/or/addressmap.c +++ b/src/or/addressmap.c @@ -455,6 +455,8 @@ addressmap_rewrite_reverse(char *address, size_t maxlen, unsigned flags, return 0; else if (f == AF_INET6 && !(flags & AMR_FLAG_USE_IPV6_DNS)) return 0; + /* FFFF we should reverse-map virtual addresses even if we haven't + * enabled DNS cacheing. */ } tor_asprintf(&s, "REVERSE[%s]", address); @@ -975,6 +977,8 @@ addressmap_register_virtual_address(int type, char *new_address) strmap_set(virtaddress_reversemap, new_address, vent); addressmap_register(*addrp, new_address, 2, ADDRMAPSRC_AUTOMAP, 0, 0); + /* FFFF register corresponding reverse mapping. */ + #if 0 { /* Try to catch possible bugs */ diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c index 1c77394df0..a37f1941d0 100644 --- a/src/test/test_entryconn.c +++ b/src/test/test_entryconn.c @@ -11,6 +11,7 @@ #include "addressmap.h" #include "config.h" +#include "confparse.h" #include "connection.h" #include "connection_edge.h" @@ -162,12 +163,13 @@ test_entryconn_rewrite_automap_ipv4(void *arg) static void test_entryconn_rewrite_automap_ipv6(void *arg) { - entry_connection_t *ec = arg; + (void)arg; + entry_connection_t *ec =NULL; entry_connection_t *ec2=NULL, *ec3=NULL; rewrite_result_t rr; char *msg = NULL; - connection_free_(ENTRY_TO_CONN(ec)); - ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET6); + + ec = entry_connection_new(CONN_TYPE_AP, AF_INET6); ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET6); ec3 = entry_connection_new(CONN_TYPE_AP, AF_INET6); @@ -190,8 +192,7 @@ test_entryconn_rewrite_automap_ipv6(void *arg) tt_str_op(rr.orig_address, OP_EQ, "www.mit.edu"); tt_str_op(ec->original_dest_address, OP_EQ, "www.mit.edu"); - printf("<%s>\n", ec->socks_request->address); - /* XXXX Should this [ be here? */ + /* Yes, this [ should be here. */ tt_assert(!strcmpstart(ec->socks_request->address,"[fe80:")); /* Connect to it and make sure we get the original address back. */ @@ -228,29 +229,425 @@ test_entryconn_rewrite_automap_ipv6(void *arg) ec->socks_request->address); done: + connection_free_(ENTRY_TO_CONN(ec)); connection_free_(ENTRY_TO_CONN(ec2)); connection_free_(ENTRY_TO_CONN(ec3)); } +#if 0 +/* FFFF not actually supported. */ /* automap on resolve, reverse lookup. */ +static void +test_entryconn_rewrite_automap_reverse(void *arg) +{ + entry_connection_t *ec = arg; + entry_connection_t *ec2=NULL; + rewrite_result_t rr; + char *msg = NULL; + + ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET); + + get_options_mutable()->AutomapHostsOnResolve = 1; + get_options_mutable()->AutomapHostsSuffixes = smartlist_new(); + get_options_mutable()->SafeLogging_ = SAFELOG_SCRUB_NONE; + smartlist_add(get_options_mutable()->AutomapHostsSuffixes, + tor_strdup(".bloom")); + parse_virtual_addr_network("127.80.0.0/16", AF_INET, 0, &msg); + + /* Automap this on resolve. */ + strlcpy(ec->socks_request->address, "www.poldy.BLOOM", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_RESOLVE; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "www.poldy.bloom"); + tt_str_op(ec->original_dest_address, OP_EQ, "www.poldy.bloom"); + + tt_assert(!strcmpstart(ec->socks_request->address,"127.80.")); + + strlcpy(ec2->socks_request->address, ec->socks_request->address, + sizeof(ec2->socks_request->address)); + ec2->use_cached_ipv4_answers = 1; // XXXX REMOVE. This is only there to hide a bug. + ec2->socks_request->command = SOCKS_COMMAND_RESOLVE_PTR; + connection_ap_handshake_rewrite(ec2, &rr); + + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 1); + tt_int_op(rr.end_reason, OP_EQ, + END_STREAM_REASON_DONE|END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + + done: + connection_free_(ENTRY_TO_CONN(ec2)); +} +#endif /* Rewrite because of cached DNS entry. */ +static void +test_entryconn_rewrite_cached_dns_ipv4(void *arg) +{ + entry_connection_t *ec = arg; + rewrite_result_t rr; + time_t expires = time(NULL) + 3600; + entry_connection_t *ec2=NULL; -/* Rewrite because of AddrmapRewrite option */ + ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET); -/* Rewrite because of control port */ + addressmap_register("www.friendly.example.com", + tor_strdup("240.240.241.241"), + expires, + ADDRMAPSRC_DNS, + 0, 0); -/* Rewrite into .exit because of virtual address mapping */ + strlcpy(ec->socks_request->address, "www.friendly.example.com", + sizeof(ec->socks_request->address)); + strlcpy(ec2->socks_request->address, "www.friendly.example.com", + sizeof(ec2->socks_request->address)); -/* Rewrite into .exit because of mapaddress */ + ec->socks_request->command = SOCKS_COMMAND_CONNECT; + ec2->socks_request->command = SOCKS_COMMAND_CONNECT; + + ec2->use_cached_ipv4_answers = 1; /* only ec2 gets this flag */ + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "www.friendly.example.com"); + tt_str_op(ec->socks_request->address, OP_EQ, "www.friendly.example.com"); + + connection_ap_handshake_rewrite(ec2, &rr); + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, expires); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "www.friendly.example.com"); + tt_str_op(ec2->socks_request->address, OP_EQ, "240.240.241.241"); + + done: + connection_free_(ENTRY_TO_CONN(ec2)); +} + +/* Rewrite because of cached DNS entry. */ +static void +test_entryconn_rewrite_cached_dns_ipv6(void *arg) +{ + entry_connection_t *ec = NULL; + rewrite_result_t rr; + time_t expires = time(NULL) + 3600; + entry_connection_t *ec2=NULL; + + (void)arg; + + ec = entry_connection_new(CONN_TYPE_AP, AF_INET6); + ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET6); + + addressmap_register("www.friendly.example.com", + tor_strdup("[::f00f]"), + expires, + ADDRMAPSRC_DNS, + 0, 0); + + strlcpy(ec->socks_request->address, "www.friendly.example.com", + sizeof(ec->socks_request->address)); + strlcpy(ec2->socks_request->address, "www.friendly.example.com", + sizeof(ec2->socks_request->address)); + + ec->socks_request->command = SOCKS_COMMAND_CONNECT; + ec2->socks_request->command = SOCKS_COMMAND_CONNECT; + + ec2->use_cached_ipv6_answers = 1; /* only ec2 gets this flag */ + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "www.friendly.example.com"); + tt_str_op(ec->socks_request->address, OP_EQ, "www.friendly.example.com"); + + connection_ap_handshake_rewrite(ec2, &rr); + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, expires); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "www.friendly.example.com"); + tt_str_op(ec2->socks_request->address, OP_EQ, "[::f00f]"); + + done: + connection_free_(ENTRY_TO_CONN(ec)); + connection_free_(ENTRY_TO_CONN(ec2)); +} /* Fail to connect to unmapped address in virtual range. */ +static void +test_entryconn_rewrite_unmapped_virtual(void *arg) +{ + entry_connection_t *ec = arg; + rewrite_result_t rr; + entry_connection_t *ec2 = NULL; + char *msg = NULL; -/* Rewrite plus automap */ + ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET6); -/* Map foo.onion to longthing.onion */ + parse_virtual_addr_network("18.202.0.0/16", AF_INET, 0, &msg); + parse_virtual_addr_network("[ABCD::]/16", AF_INET6, 0, &msg); + strlcpy(ec->socks_request->address, "18.202.5.5", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec, &rr); + tt_int_op(rr.should_close, OP_EQ, 1); + tt_int_op(rr.end_reason, OP_EQ, END_STREAM_REASON_INTERNAL); + tt_int_op(rr.automap, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + + strlcpy(ec2->socks_request->address, "[ABCD:9::5314:9543]", + sizeof(ec2->socks_request->address)); + ec2->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec2, &rr); + + tt_int_op(rr.should_close, OP_EQ, 1); + tt_int_op(rr.end_reason, OP_EQ, END_STREAM_REASON_INTERNAL); + tt_int_op(rr.automap, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + + done: + connection_free_(ENTRY_TO_CONN(ec2)); +} + +/* Rewrite because of mapaddress option */ +static void +test_entryconn_rewrite_mapaddress(void *arg) +{ + entry_connection_t *ec = arg; + rewrite_result_t rr; + + config_line_append(&get_options_mutable()->AddressMap, + "MapAddress", "meta metaobjects.example"); + config_register_addressmaps(get_options()); + + strlcpy(ec->socks_request->address, "meta", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_int_op(rr.automap, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(ec->socks_request->address, OP_EQ, "metaobjects.example"); + + done: + ; +} + +/* Reject reverse lookups of internal address. */ +static void +test_entryconn_rewrite_reject_internal_reverse(void *arg) +{ + entry_connection_t *ec = arg; + rewrite_result_t rr; + + strlcpy(ec->socks_request->address, "10.0.0.1", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_RESOLVE_PTR; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.should_close, OP_EQ, 1); + tt_int_op(rr.end_reason, OP_EQ, END_STREAM_REASON_SOCKSPROTOCOL | + END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED); + tt_int_op(rr.automap, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + + done: + ; +} + +/* Rewrite into .exit because of virtual address mapping */ +static void +test_entryconn_rewrite_automap_exit(void *arg) +{ + entry_connection_t *ec = arg; + entry_connection_t *ec2=NULL; + rewrite_result_t rr; + char *msg = NULL; + + ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET); + + get_options_mutable()->AutomapHostsOnResolve = 1; + get_options_mutable()->AutomapHostsSuffixes = smartlist_new(); + get_options_mutable()->AllowDotExit = 1; + smartlist_add(get_options_mutable()->AutomapHostsSuffixes, + tor_strdup(".EXIT")); + parse_virtual_addr_network("127.1.0.0/16", AF_INET, 0, &msg); + + /* Automap this on resolve. */ + strlcpy(ec->socks_request->address, "website.example.exit", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_RESOLVE; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "website.example.exit"); + tt_str_op(ec->original_dest_address, OP_EQ, "website.example.exit"); + + tt_assert(!strcmpstart(ec->socks_request->address,"127.1.")); + + /* Connect to it and make sure we get the original address back. */ + strlcpy(ec2->socks_request->address, ec->socks_request->address, + sizeof(ec2->socks_request->address)); + + ec2->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec2, &rr); + + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_AUTOMAP); + tt_str_op(rr.orig_address, OP_EQ, ec->socks_request->address); + tt_str_op(ec2->original_dest_address, OP_EQ, ec->socks_request->address); + tt_str_op(ec2->socks_request->address, OP_EQ, "website.example.exit"); + + done: + connection_free_(ENTRY_TO_CONN(ec2)); +} + +/* Rewrite into .exit because of mapaddress */ +static void +test_entryconn_rewrite_mapaddress_exit(void *arg) +{ + entry_connection_t *ec = arg; + rewrite_result_t rr; + + config_line_append(&get_options_mutable()->AddressMap, + "MapAddress", "*.example.com *.example.com.abc.exit"); + config_register_addressmaps(get_options()); + + /* Automap this on resolve. */ + strlcpy(ec->socks_request->address, "abc.example.com", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_TORRC); + tt_str_op(rr.orig_address, OP_EQ, "abc.example.com"); + tt_str_op(ec->socks_request->address, OP_EQ, "abc.example.com.abc.exit"); + done: + ; +} + +/* Map foo.onion to longthing.onion, and also automap. */ +static void +test_entryconn_rewrite_mapaddress_automap_onion(void *arg) +{ + entry_connection_t *ec = arg; + entry_connection_t *ec2 = NULL; + entry_connection_t *ec3 = NULL; + entry_connection_t *ec4 = NULL; + rewrite_result_t rr; + char *msg = NULL; + + ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET); + ec3 = entry_connection_new(CONN_TYPE_AP, AF_INET); + ec4 = entry_connection_new(CONN_TYPE_AP, AF_INET); + + get_options_mutable()->AutomapHostsOnResolve = 1; + get_options_mutable()->AutomapHostsSuffixes = smartlist_new(); + get_options_mutable()->AllowDotExit = 1; + smartlist_add(get_options_mutable()->AutomapHostsSuffixes, + tor_strdup(".onion")); + parse_virtual_addr_network("192.168.0.0/16", AF_INET, 0, &msg); + config_line_append(&get_options_mutable()->AddressMap, + "MapAddress", "foo.onion abcdefghijklmnop.onion"); + config_register_addressmaps(get_options()); + + /* Connect to foo.onion. */ + strlcpy(ec->socks_request->address, "foo.onion", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "foo.onion"); + tt_str_op(ec->socks_request->address, OP_EQ, "abcdefghijklmnop.onion"); + + /* Okay, resolve foo.onion */ + strlcpy(ec2->socks_request->address, "foo.onion", + sizeof(ec2->socks_request->address)); + ec2->socks_request->command = SOCKS_COMMAND_RESOLVE; + connection_ap_handshake_rewrite(ec2, &rr); + + tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "foo.onion"); + tt_assert(!strcmpstart(ec2->socks_request->address, "192.168.")); + + /* Now connect */ + strlcpy(ec3->socks_request->address, ec2->socks_request->address, + sizeof(ec3->socks_request->address)); + ec3->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec3, &rr); + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_assert(!strcmpstart(ec3->socks_request->address, "abcdefghijklmnop.onion")); + + /* Now resolve abcefghijklmnop.onion. */ + strlcpy(ec4->socks_request->address, "abcdefghijklmnop.onion", + sizeof(ec4->socks_request->address)); + ec4->socks_request->command = SOCKS_COMMAND_RESOLVE; + connection_ap_handshake_rewrite(ec4, &rr); + + tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "abcdefghijklmnop.onion"); + tt_assert(!strcmpstart(ec4->socks_request->address, "192.168.")); + /* XXXX doesn't work + tt_str_op(ec4->socks_request->address, OP_EQ, ec2->socks_request->address); + */ + + done: + connection_free_(ENTRY_TO_CONN(ec2)); + connection_free_(ENTRY_TO_CONN(ec3)); + connection_free_(ENTRY_TO_CONN(ec4)); + +} #define REWRITE(name) \ { #name, test_entryconn_##name, TT_FORK, &test_rewrite_setup, NULL } @@ -260,6 +657,15 @@ struct testcase_t entryconn_tests[] = { REWRITE(rewrite_bad_dotexit), REWRITE(rewrite_automap_ipv4), REWRITE(rewrite_automap_ipv6), + // REWRITE(rewrite_automap_reverse), + REWRITE(rewrite_cached_dns_ipv4), + REWRITE(rewrite_cached_dns_ipv6), + REWRITE(rewrite_unmapped_virtual), + REWRITE(rewrite_mapaddress), + REWRITE(rewrite_reject_internal_reverse), + REWRITE(rewrite_automap_exit), + REWRITE(rewrite_mapaddress_exit), + REWRITE(rewrite_mapaddress_automap_onion), END_OF_TESTCASES }; From 6cbe016cb10fe737d7c3403c0a2f7d0a361d834b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 13 Jan 2015 12:13:46 -0500 Subject: [PATCH 07/15] Fix memory leaks in entryconn tests. --- src/test/test_entryconn.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c index a37f1941d0..8efd916310 100644 --- a/src/test/test_entryconn.c +++ b/src/test/test_entryconn.c @@ -100,7 +100,6 @@ test_entryconn_rewrite_automap_ipv4(void *arg) ec3 = entry_connection_new(CONN_TYPE_AP, AF_INET); get_options_mutable()->AutomapHostsOnResolve = 1; - get_options_mutable()->AutomapHostsSuffixes = smartlist_new(); smartlist_add(get_options_mutable()->AutomapHostsSuffixes, tor_strdup(".")); parse_virtual_addr_network("127.202.0.0/16", AF_INET, 0, &msg); @@ -174,7 +173,6 @@ test_entryconn_rewrite_automap_ipv6(void *arg) ec3 = entry_connection_new(CONN_TYPE_AP, AF_INET6); get_options_mutable()->AutomapHostsOnResolve = 1; - get_options_mutable()->AutomapHostsSuffixes = smartlist_new(); smartlist_add(get_options_mutable()->AutomapHostsSuffixes, tor_strdup(".")); parse_virtual_addr_network("FE80::/32", AF_INET6, 0, &msg); @@ -248,7 +246,6 @@ test_entryconn_rewrite_automap_reverse(void *arg) ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET); get_options_mutable()->AutomapHostsOnResolve = 1; - get_options_mutable()->AutomapHostsSuffixes = smartlist_new(); get_options_mutable()->SafeLogging_ = SAFELOG_SCRUB_NONE; smartlist_add(get_options_mutable()->AutomapHostsSuffixes, tor_strdup(".bloom")); @@ -492,7 +489,6 @@ test_entryconn_rewrite_automap_exit(void *arg) ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET); get_options_mutable()->AutomapHostsOnResolve = 1; - get_options_mutable()->AutomapHostsSuffixes = smartlist_new(); get_options_mutable()->AllowDotExit = 1; smartlist_add(get_options_mutable()->AutomapHostsSuffixes, tor_strdup(".EXIT")); @@ -578,7 +574,6 @@ test_entryconn_rewrite_mapaddress_automap_onion(void *arg) ec4 = entry_connection_new(CONN_TYPE_AP, AF_INET); get_options_mutable()->AutomapHostsOnResolve = 1; - get_options_mutable()->AutomapHostsSuffixes = smartlist_new(); get_options_mutable()->AllowDotExit = 1; smartlist_add(get_options_mutable()->AutomapHostsSuffixes, tor_strdup(".onion")); From ab6bd78eca0f3edc7ed910d8567c409fe3dfdd92 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 13 Jan 2015 12:26:04 -0500 Subject: [PATCH 08/15] (Disabled, failing) test for the bug in 7555. --- src/test/test_entryconn.c | 72 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c index 8efd916310..fefd2eaf00 100644 --- a/src/test/test_entryconn.c +++ b/src/test/test_entryconn.c @@ -641,9 +641,75 @@ test_entryconn_rewrite_mapaddress_automap_onion(void *arg) connection_free_(ENTRY_TO_CONN(ec2)); connection_free_(ENTRY_TO_CONN(ec3)); connection_free_(ENTRY_TO_CONN(ec4)); - } +#if 0 +/* This fails because of #7555 */ +/* This time is the same, but we start with a mapping from a non-onion + * address. */ +static void +test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) +{ + entry_connection_t *ec = arg; + entry_connection_t *ec2 = NULL; + entry_connection_t *ec3 = NULL; + rewrite_result_t rr; + + ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET); + ec3 = entry_connection_new(CONN_TYPE_AP, AF_INET); + + get_options_mutable()->AutomapHostsOnResolve = 1; + get_options_mutable()->AllowDotExit = 1; + smartlist_add(get_options_mutable()->AutomapHostsSuffixes, + tor_strdup(".onion")); + config_line_append(&get_options_mutable()->AddressMap, + "MapAddress", "irc.example.com abcdefghijklmnop.onion"); + config_register_addressmaps(get_options()); + + /* Connect to irc.example.com */ + strlcpy(ec->socks_request->address, "irc.example.com", + sizeof(ec->socks_request->address)); + ec->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec, &rr); + + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "irc.example.com"); + tt_str_op(ec->socks_request->address, OP_EQ, "abcdefghijklmnop.onion"); + + /* Okay, resolve irc.example.com */ + strlcpy(ec2->socks_request->address, "irc.example.com", + sizeof(ec2->socks_request->address)); + ec2->socks_request->command = SOCKS_COMMAND_RESOLVE; + connection_ap_handshake_rewrite(ec2, &rr); + + // tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); + tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); + tt_str_op(rr.orig_address, OP_EQ, "irc.example.com"); + tt_assert(!strcmpstart(ec2->socks_request->address, "192.168.")); + + /* Now connect */ + strlcpy(ec3->socks_request->address, ec2->socks_request->address, + sizeof(ec3->socks_request->address)); + ec3->socks_request->command = SOCKS_COMMAND_CONNECT; + connection_ap_handshake_rewrite(ec3, &rr); + tt_int_op(rr.automap, OP_EQ, 0); + tt_int_op(rr.should_close, OP_EQ, 0); + tt_int_op(rr.end_reason, OP_EQ, 0); + tt_assert(!strcmpstart(ec3->socks_request->address, "abcdefghijklmnop.onion")); + + done: + connection_free_(ENTRY_TO_CONN(ec2)); + connection_free_(ENTRY_TO_CONN(ec3)); +} +#endif + #define REWRITE(name) \ { #name, test_entryconn_##name, TT_FORK, &test_rewrite_setup, NULL } @@ -661,6 +727,10 @@ struct testcase_t entryconn_tests[] = { REWRITE(rewrite_automap_exit), REWRITE(rewrite_mapaddress_exit), REWRITE(rewrite_mapaddress_automap_onion), + /* + This fails because of #7555 + REWRITE(rewrite_mapaddress_automap_onion2), + */ END_OF_TESTCASES }; From 9d0fab9872807ef212fadb3feb299cf6309a185f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 8 Apr 2014 18:02:03 -0400 Subject: [PATCH 09/15] Allow MapAddress and Automap to work together The trick here is to apply mapaddress first, and only then apply automapping. Otherwise, the automap checks don't get done. Fix for bug 7555; bugfix on all versions of Tor supporting both MapAddress and AutoMap. --- changes/bug7555 | 5 +++++ src/or/addressmap.c | 36 +++++++++++++++++++++++++++++------- src/or/addressmap.h | 7 +++++-- src/or/connection_edge.c | 17 +++++++++++++++-- src/test/test_config.c | 3 +-- src/test/test_entryconn.c | 8 +++----- 6 files changed, 58 insertions(+), 18 deletions(-) create mode 100644 changes/bug7555 diff --git a/changes/bug7555 b/changes/bug7555 new file mode 100644 index 0000000000..a43ff739cb --- /dev/null +++ b/changes/bug7555 @@ -0,0 +1,5 @@ + o Major bugfixes (client): + - Allow MapAddress and AutomapHostsOnResolve to work together when an + address is mapped into another address type that must be + automapped at resolve time. Fixes bug 7555; bugfix on + 0.2.0.1-alpha. diff --git a/src/or/addressmap.c b/src/or/addressmap.c index 0f417e6369..ea01894634 100644 --- a/src/or/addressmap.c +++ b/src/or/addressmap.c @@ -390,13 +390,35 @@ addressmap_rewrite(char *address, size_t maxlen, goto done; } - if (ent && ent->source == ADDRMAPSRC_DNS) { - sa_family_t f; - tor_addr_t tmp; - f = tor_addr_parse(&tmp, ent->new_address); - if (f == AF_INET && !(flags & AMR_FLAG_USE_IPV4_DNS)) - goto done; - else if (f == AF_INET6 && !(flags & AMR_FLAG_USE_IPV6_DNS)) + switch (ent->source) { + case ADDRMAPSRC_DNS: + { + sa_family_t f; + tor_addr_t tmp; + f = tor_addr_parse(&tmp, ent->new_address); + if (f == AF_INET && !(flags & AMR_FLAG_USE_IPV4_DNS)) + goto done; + else if (f == AF_INET6 && !(flags & AMR_FLAG_USE_IPV6_DNS)) + goto done; + } + break; + case ADDRMAPSRC_CONTROLLER: + case ADDRMAPSRC_TORRC: + if (!(flags & AMR_FLAG_USE_MAPADDRESS)) + goto done; + break; + case ADDRMAPSRC_AUTOMAP: + if (!(flags & AMR_FLAG_USE_AUTOMAP)) + goto done; + break; + case ADDRMAPSRC_TRACKEXIT: + if (!(flags & AMR_FLAG_USE_TRACKEXIT)) + goto done; + break; + case ADDRMAPSRC_NONE: + default: + log_warn(LD_BUG, "Unknown addrmap source value %d. Ignoring it.", + (int) ent->source); goto done; } diff --git a/src/or/addressmap.h b/src/or/addressmap.h index bb737e47f4..ff108df024 100644 --- a/src/or/addressmap.h +++ b/src/or/addressmap.h @@ -16,8 +16,11 @@ void addressmap_clean(time_t now); void addressmap_clear_configured(void); void addressmap_clear_transient(void); void addressmap_free_all(void); -#define AMR_FLAG_USE_IPV4_DNS (1u<<0) -#define AMR_FLAG_USE_IPV6_DNS (1u<<1) +#define AMR_FLAG_USE_IPV4_DNS (1u<<0) +#define AMR_FLAG_USE_IPV6_DNS (1u<<1) +#define AMR_FLAG_USE_MAPADDRESS (1u<<2) +#define AMR_FLAG_USE_AUTOMAP (1u<<3) +#define AMR_FLAG_USE_TRACKEXIT (1u<<4) int addressmap_rewrite(char *address, size_t maxlen, unsigned flags, time_t *expires_out, addressmap_entry_source_t *exit_source_out); diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index ca99a86ae0..7a77f5e20a 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -946,6 +946,15 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, if (! conn->original_dest_address) conn->original_dest_address = tor_strdup(conn->socks_request->address); + if (socks->command != SOCKS_COMMAND_RESOLVE_PTR) { + const unsigned rewrite_flags = AMR_FLAG_USE_MAPADDRESS; + if (addressmap_rewrite(socks->address, sizeof(socks->address), + rewrite_flags, &out->map_expires, &out->exit_source)) { + control_event_stream_status(conn, STREAM_EVENT_REMAP, + REMAP_STREAM_SOURCE_CACHE); + } + } + if (socks->command == SOCKS_COMMAND_RESOLVE && tor_addr_parse(&addr_tmp, socks->address)<0 && options->AutomapHostsOnResolve) { @@ -1014,16 +1023,20 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, } } else if (!out->automap) { /* For address map controls, remap the address. */ - unsigned rewrite_flags = 0; + unsigned rewrite_flags = AMR_FLAG_USE_AUTOMAP | AMR_FLAG_USE_TRACKEXIT; + addressmap_entry_source_t exit_source2; if (conn->use_cached_ipv4_answers) rewrite_flags |= AMR_FLAG_USE_IPV4_DNS; if (conn->use_cached_ipv6_answers) rewrite_flags |= AMR_FLAG_USE_IPV6_DNS; if (addressmap_rewrite(socks->address, sizeof(socks->address), - rewrite_flags, &out->map_expires, &out->exit_source)) { + rewrite_flags, &out->map_expires, &exit_source2)) { control_event_stream_status(conn, STREAM_EVENT_REMAP, REMAP_STREAM_SOURCE_CACHE); } + if (out->exit_source == ADDRMAPSRC_NONE) { + out->exit_source = exit_source2; + } } if (!out->automap && address_is_in_virtual_range(socks->address)) { diff --git a/src/test/test_config.c b/src/test/test_config.c index fb8e4020dc..b1f5017b78 100644 --- a/src/test/test_config.c +++ b/src/test/test_config.c @@ -51,8 +51,7 @@ test_config_addressmap(void *arg) /* Use old interface for now, so we don't need to rewrite the unit tests */ #define addressmap_rewrite(a,s,eo,ao) \ - addressmap_rewrite((a),(s),AMR_FLAG_USE_IPV4_DNS|AMR_FLAG_USE_IPV6_DNS, \ - (eo),(ao)) + addressmap_rewrite((a),(s), ~0, (eo),(ao)) /* MapAddress .invalidwildcard.com .torserver.exit - no match */ strlcpy(address, "www.invalidwildcard.com", sizeof(address)); diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c index fefd2eaf00..0d0f2b0b5e 100644 --- a/src/test/test_entryconn.c +++ b/src/test/test_entryconn.c @@ -643,7 +643,6 @@ test_entryconn_rewrite_mapaddress_automap_onion(void *arg) connection_free_(ENTRY_TO_CONN(ec4)); } -#if 0 /* This fails because of #7555 */ /* This time is the same, but we start with a mapping from a non-onion * address. */ @@ -654,6 +653,7 @@ test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) entry_connection_t *ec2 = NULL; entry_connection_t *ec3 = NULL; rewrite_result_t rr; + char *msg = NULL; ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET); ec3 = entry_connection_new(CONN_TYPE_AP, AF_INET); @@ -662,6 +662,7 @@ test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) get_options_mutable()->AllowDotExit = 1; smartlist_add(get_options_mutable()->AutomapHostsSuffixes, tor_strdup(".onion")); + parse_virtual_addr_network("192.168.0.0/16", AF_INET, 0, &msg); config_line_append(&get_options_mutable()->AddressMap, "MapAddress", "irc.example.com abcdefghijklmnop.onion"); config_register_addressmaps(get_options()); @@ -708,7 +709,6 @@ test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) connection_free_(ENTRY_TO_CONN(ec2)); connection_free_(ENTRY_TO_CONN(ec3)); } -#endif #define REWRITE(name) \ { #name, test_entryconn_##name, TT_FORK, &test_rewrite_setup, NULL } @@ -727,10 +727,8 @@ struct testcase_t entryconn_tests[] = { REWRITE(rewrite_automap_exit), REWRITE(rewrite_mapaddress_exit), REWRITE(rewrite_mapaddress_automap_onion), - /* - This fails because of #7555 REWRITE(rewrite_mapaddress_automap_onion2), - */ + END_OF_TESTCASES }; From 73ccf0b33f13ecd9e9e3e481576c5132b9b79a6e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 13 Jan 2015 13:42:20 -0500 Subject: [PATCH 10/15] Uncomment a test --- src/test/test_entryconn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c index 0d0f2b0b5e..0ff353d0e8 100644 --- a/src/test/test_entryconn.c +++ b/src/test/test_entryconn.c @@ -687,7 +687,7 @@ test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) ec2->socks_request->command = SOCKS_COMMAND_RESOLVE; connection_ap_handshake_rewrite(ec2, &rr); - // tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.automap, OP_EQ, 1); tt_int_op(rr.should_close, OP_EQ, 0); tt_int_op(rr.end_reason, OP_EQ, 0); tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); From 73d1d153dc0946439f65e201734d1d9eb20b576c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 13 Jan 2015 13:56:59 -0500 Subject: [PATCH 11/15] remove needless AllowDotExit in test_entryconn_rewrite_mapaddress_automap_onion2 --- src/test/test_entryconn.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c index 0ff353d0e8..0d0105e935 100644 --- a/src/test/test_entryconn.c +++ b/src/test/test_entryconn.c @@ -659,7 +659,6 @@ test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) ec3 = entry_connection_new(CONN_TYPE_AP, AF_INET); get_options_mutable()->AutomapHostsOnResolve = 1; - get_options_mutable()->AllowDotExit = 1; smartlist_add(get_options_mutable()->AutomapHostsSuffixes, tor_strdup(".onion")); parse_virtual_addr_network("192.168.0.0/16", AF_INET, 0, &msg); From 03f783c045380cdb162f243e52f20cebbfb00921 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 13 Jan 2015 13:59:39 -0500 Subject: [PATCH 12/15] remove a bogus comment --- src/test/test_entryconn.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c index 0d0105e935..3a73700cf8 100644 --- a/src/test/test_entryconn.c +++ b/src/test/test_entryconn.c @@ -643,7 +643,6 @@ test_entryconn_rewrite_mapaddress_automap_onion(void *arg) connection_free_(ENTRY_TO_CONN(ec4)); } -/* This fails because of #7555 */ /* This time is the same, but we start with a mapping from a non-onion * address. */ static void From b72acd725aa687bae7012abcc0d67aef9d32bfaa Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 14 Jan 2015 09:10:35 -0500 Subject: [PATCH 13/15] More tests as suggested by rl1987 --- src/test/test_entryconn.c | 72 ++++++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 17 deletions(-) diff --git a/src/test/test_entryconn.c b/src/test/test_entryconn.c index 3a73700cf8..4c14edefd7 100644 --- a/src/test/test_entryconn.c +++ b/src/test/test_entryconn.c @@ -643,28 +643,18 @@ test_entryconn_rewrite_mapaddress_automap_onion(void *arg) connection_free_(ENTRY_TO_CONN(ec4)); } -/* This time is the same, but we start with a mapping from a non-onion - * address. */ static void -test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) +test_entryconn_rewrite_mapaddress_automap_onion_common(entry_connection_t *ec, + int map_to_onion, + int map_to_address) { - entry_connection_t *ec = arg; entry_connection_t *ec2 = NULL; entry_connection_t *ec3 = NULL; rewrite_result_t rr; - char *msg = NULL; ec2 = entry_connection_new(CONN_TYPE_AP, AF_INET); ec3 = entry_connection_new(CONN_TYPE_AP, AF_INET); - get_options_mutable()->AutomapHostsOnResolve = 1; - smartlist_add(get_options_mutable()->AutomapHostsSuffixes, - tor_strdup(".onion")); - parse_virtual_addr_network("192.168.0.0/16", AF_INET, 0, &msg); - config_line_append(&get_options_mutable()->AddressMap, - "MapAddress", "irc.example.com abcdefghijklmnop.onion"); - config_register_addressmaps(get_options()); - /* Connect to irc.example.com */ strlcpy(ec->socks_request->address, "irc.example.com", sizeof(ec->socks_request->address)); @@ -677,7 +667,8 @@ test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); tt_str_op(rr.orig_address, OP_EQ, "irc.example.com"); - tt_str_op(ec->socks_request->address, OP_EQ, "abcdefghijklmnop.onion"); + tt_str_op(ec->socks_request->address, OP_EQ, + map_to_onion ? "abcdefghijklmnop.onion" : "irc.example.com"); /* Okay, resolve irc.example.com */ strlcpy(ec2->socks_request->address, "irc.example.com", @@ -685,13 +676,14 @@ test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) ec2->socks_request->command = SOCKS_COMMAND_RESOLVE; connection_ap_handshake_rewrite(ec2, &rr); - tt_int_op(rr.automap, OP_EQ, 1); + tt_int_op(rr.automap, OP_EQ, map_to_onion && map_to_address); tt_int_op(rr.should_close, OP_EQ, 0); tt_int_op(rr.end_reason, OP_EQ, 0); tt_i64_op(rr.map_expires, OP_EQ, TIME_MAX); tt_int_op(rr.exit_source, OP_EQ, ADDRMAPSRC_NONE); tt_str_op(rr.orig_address, OP_EQ, "irc.example.com"); - tt_assert(!strcmpstart(ec2->socks_request->address, "192.168.")); + if (map_to_onion && map_to_address) + tt_assert(!strcmpstart(ec2->socks_request->address, "192.168.")); /* Now connect */ strlcpy(ec3->socks_request->address, ec2->socks_request->address, @@ -701,13 +693,57 @@ test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) tt_int_op(rr.automap, OP_EQ, 0); tt_int_op(rr.should_close, OP_EQ, 0); tt_int_op(rr.end_reason, OP_EQ, 0); - tt_assert(!strcmpstart(ec3->socks_request->address, "abcdefghijklmnop.onion")); + if (map_to_onion) + tt_assert(!strcmpstart(ec3->socks_request->address, + "abcdefghijklmnop.onion")); done: connection_free_(ENTRY_TO_CONN(ec2)); connection_free_(ENTRY_TO_CONN(ec3)); } +/* This time is the same, but we start with a mapping from a non-onion + * address. */ +static void +test_entryconn_rewrite_mapaddress_automap_onion2(void *arg) +{ + char *msg = NULL; + get_options_mutable()->AutomapHostsOnResolve = 1; + smartlist_add(get_options_mutable()->AutomapHostsSuffixes, + tor_strdup(".onion")); + parse_virtual_addr_network("192.168.0.0/16", AF_INET, 0, &msg); + config_line_append(&get_options_mutable()->AddressMap, + "MapAddress", "irc.example.com abcdefghijklmnop.onion"); + config_register_addressmaps(get_options()); + + test_entryconn_rewrite_mapaddress_automap_onion_common(arg, 1, 1); +} + +/* Same as above, with automapped turned off */ +static void +test_entryconn_rewrite_mapaddress_automap_onion3(void *arg) +{ + config_line_append(&get_options_mutable()->AddressMap, + "MapAddress", "irc.example.com abcdefghijklmnop.onion"); + config_register_addressmaps(get_options()); + + test_entryconn_rewrite_mapaddress_automap_onion_common(arg, 1, 0); +} + +/* As above, with no mapping. */ +static void +test_entryconn_rewrite_mapaddress_automap_onion4(void *arg) +{ + char *msg = NULL; + get_options_mutable()->AutomapHostsOnResolve = 1; + smartlist_add(get_options_mutable()->AutomapHostsSuffixes, + tor_strdup(".onion")); + parse_virtual_addr_network("192.168.0.0/16", AF_INET, 0, &msg); + + test_entryconn_rewrite_mapaddress_automap_onion_common(arg, 0, 1); +} + + #define REWRITE(name) \ { #name, test_entryconn_##name, TT_FORK, &test_rewrite_setup, NULL } @@ -726,6 +762,8 @@ struct testcase_t entryconn_tests[] = { REWRITE(rewrite_mapaddress_exit), REWRITE(rewrite_mapaddress_automap_onion), REWRITE(rewrite_mapaddress_automap_onion2), + REWRITE(rewrite_mapaddress_automap_onion3), + REWRITE(rewrite_mapaddress_automap_onion4), END_OF_TESTCASES }; From 54e4aaf52c882626fe2cce0ba704d9661269ab99 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 18 Jan 2015 14:19:26 -0500 Subject: [PATCH 14/15] Fix memory leak in connection_ap_handshake_rewrite_and_attach() Spotted by asn. #14259. Bugfix on 368eb6a97 in 0.2.0.1-alpha. --- changes/bug14259 | 6 ++++++ src/or/connection_edge.c | 1 + 2 files changed, 7 insertions(+) create mode 100644 changes/bug14259 diff --git a/changes/bug14259 b/changes/bug14259 new file mode 100644 index 0000000000..1b5b9b80b3 --- /dev/null +++ b/changes/bug14259 @@ -0,0 +1,6 @@ + o Minor bugfixes (client): + - Avoid a small memory leak when we find a cached answer for a reverse + DNS lookup in a client-side DNS cache. (Remember, client-side DNS + caching is off by default, and is not recommended.) Fixes bug 14259; + bugfix on 0.2.0.1-alpha. + diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 7a77f5e20a..7dc8f9c878 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1001,6 +1001,7 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, strlen(result), (uint8_t*)result, -1, out->map_expires); + tor_free(result); out->end_reason = END_STREAM_REASON_DONE | END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED; out->should_close = 1; From 758d77130c654bcf71ba28a3ba75bd8569efc985 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 18 Jan 2015 15:10:35 -0500 Subject: [PATCH 15/15] Add a bunch of new comments to explain connection_ap_rewrite{,_and_attach} Also, do a little light refactoring to move some variable declarations around and make a few things const Also fix an obnoxious bug on checking for the DONE stream end reason. It's not a flag; it's a possible value or a variable that needs to be masked. --- src/or/connection_edge.c | 218 +++++++++++++++++++++++++++++++-------- src/or/connection_edge.h | 4 +- 2 files changed, 177 insertions(+), 45 deletions(-) diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 7dc8f9c878..06d35462ba 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -908,8 +908,9 @@ connection_ap_rewrite_and_attach_if_allowed(entry_connection_t *conn, return connection_ap_handshake_rewrite_and_attach(conn, circ, cpath); } -/* Try to perform any map-based rewriting of the target address in conn, - * filling in the fields of out as we go. +/* Try to perform any map-based rewriting of the target address in + * conn, filling in the fields of out as we go, and modifying + * conn->socks_request.address as appropriate. */ STATIC void connection_ap_handshake_rewrite(entry_connection_t *conn, @@ -919,6 +920,7 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, const or_options_t *options = get_options(); tor_addr_t addr_tmp; + /* Initialize all the fields of 'out' to reasonable defaults */ out->automap = 0; out->exit_source = ADDRMAPSRC_NONE; out->map_expires = TIME_MAX; @@ -926,12 +928,17 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, out->should_close = 0; out->orig_address[0] = 0; - tor_strlower(socks->address); /* normalize it */ + /* We convert all incoming addresses to lowercase. */ + tor_strlower(socks->address); + /* Remember the original address. */ strlcpy(out->orig_address, socks->address, sizeof(out->orig_address)); log_debug(LD_APP,"Client asked for %s:%d", safe_str_client(socks->address), socks->port); + /* Check for whether this is a .exit address. By default, those are + * disallowed when they're coming straight from the client, but you're + * allowed to have them in MapAddress commands and so forth. */ 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 " @@ -943,9 +950,18 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, return; } - if (! conn->original_dest_address) + /* Remember the original address so we can tell the user about what + * they actually said, not just what it turned into. */ + if (! conn->original_dest_address) { + /* Is the 'if' necessary here? XXXX */ conn->original_dest_address = tor_strdup(conn->socks_request->address); + } + /* First, apply MapAddress and MAPADDRESS mappings. We need to do + * these only for non-reverse lookups, since they don't exist for those. + * We need to do this before we consider automapping, since we might + * e.g. resolve irc.oftc.net into irconionaddress.onion, at which point + * we'd need to automap it. */ if (socks->command != SOCKS_COMMAND_RESOLVE_PTR) { const unsigned rewrite_flags = AMR_FLAG_USE_MAPADDRESS; if (addressmap_rewrite(socks->address, sizeof(socks->address), @@ -955,12 +971,20 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, } } + /* Now, handle automapping. Automapping happens when we're asked to + * resolve a hostname, and AutomapHostsOnResolve is set, and + * the hostname has a suffix listed in AutomapHostsSuffixes. + */ if (socks->command == SOCKS_COMMAND_RESOLVE && tor_addr_parse(&addr_tmp, socks->address)<0 && options->AutomapHostsOnResolve) { + /* Check the suffix... */ out->automap = addressmap_address_should_automap(socks->address, options); if (out->automap) { + /* If we get here, then we should apply an automapping for this. */ const char *new_addr; + /* We return an IPv4 address by default, or an IPv6 address if we + * are allowed to do so. */ int addr_type = RESOLVED_TYPE_IPV4; if (conn->socks_request->socks_version != 4) { if (!conn->ipv4_traffic_ok || @@ -968,6 +992,10 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, conn->prefer_ipv6_virtaddr) addr_type = RESOLVED_TYPE_IPV6; } + /* Okay, register the target address as automapped, and find the new + * address we're supposed to give as a resolve answer. (Return a cached + * value if we've looked up this address before. + */ new_addr = addressmap_register_virtual_address( addr_type, tor_strdup(socks->address)); if (! new_addr) { @@ -984,6 +1012,8 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, } } + /* Now handle reverse lookups, if they're in the cache. This doesn't + * happen too often, since client-side DNS caching is off by default. */ if (socks->command == SOCKS_COMMAND_RESOLVE_PTR) { unsigned rewrite_flags = 0; if (conn->use_cached_ipv4_answers) @@ -1007,6 +1037,10 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, out->should_close = 1; return; } + + /* Hang on, did we find an answer saying that this is a reverse lookup for + * an internal address? If so, we should reject it if we're condigured to + * do so. */ if (options->ClientDNSRejectInternalAddresses) { /* Don't let people try to do a reverse lookup on 10.0.0.1. */ tor_addr_t addr; @@ -1022,8 +1056,16 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, return; } } - } else if (!out->automap) { - /* For address map controls, remap the address. */ + } + + /* If we didn't automap it before, then this is still the address + * that came straight from the user, mapped according to any + * MapAddress/MAPADDRESS commands. Now other mappings, including + * previously registered Automap entries, TrackHostExits entries, + * and client-side DNS cache entries (not recommended). + */ + if (!socks->command != SOCKS_COMMAND_RESOLVE_PTR && + !out->automap) { unsigned rewrite_flags = AMR_FLAG_USE_AUTOMAP | AMR_FLAG_USE_TRACKEXIT; addressmap_entry_source_t exit_source2; if (conn->use_cached_ipv4_answers) @@ -1036,15 +1078,20 @@ connection_ap_handshake_rewrite(entry_connection_t *conn, REMAP_STREAM_SOURCE_CACHE); } if (out->exit_source == ADDRMAPSRC_NONE) { + /* If it wasn't a .exit before, maybe it turned into a .exit. Remember + * the original source of a .exit. */ out->exit_source = exit_source2; } } + /* Check to see whether we're about to use an address in the virtual + * range without actually having gotten it from an Automap. */ if (!out->automap && address_is_in_virtual_range(socks->address)) { - /* This address was probably handed out by client_dns_get_unmapped_address, - * but the mapping was discarded for some reason. We *don't* want to send - * the address through Tor; that's likely to fail, and may leak - * information. + /* This address was probably handed out by + * client_dns_get_unmapped_address, but the mapping was discarded for some + * reason. Or the user typed in a virtual address range manually. We + * *don't* want to send the address through Tor; that's likely to fail, + * and may leak information. */ log_warn(LD_APP,"Missing mapping for virtual address '%s'. Refusing.", safe_str_client(socks->address)); @@ -1075,7 +1122,6 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, crypt_path_t *cpath) { socks_request_t *socks = conn->socks_request; - hostname_type_t addresstype; const or_options_t *options = get_options(); connection_t *base_conn = ENTRY_TO_CONN(conn); time_t now = time(NULL); @@ -1085,22 +1131,27 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, connection_ap_handshake_rewrite(conn,&rr); if (rr.should_close) { + /* connection_ap_handshake_rewrite told us to close the connection, + * either because it sent back an answer, or because it sent back an + * error */ connection_mark_unattached_ap(conn, rr.end_reason); - if (0 != (rr.end_reason & END_STREAM_REASON_DONE)) + if (END_STREAM_REASON_DONE == (rr.end_reason & END_STREAM_REASON_MASK)) return 0; else return -1; } - time_t map_expires = rr.map_expires; - int automap = rr.automap; - addressmap_entry_source_t exit_source = rr.exit_source; + const time_t map_expires = rr.map_expires; + const int automap = rr.automap; + const addressmap_entry_source_t exit_source = rr.exit_source; /* 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); + const hostname_type_t addresstype = parse_extended_hostname(socks->address); + /* Now see whether the hostname is bogus. This could happen because of an + * onion hostname whose format we don't recognize. */ if (addresstype == BAD_HOSTNAME) { control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", escaped(socks->address)); @@ -1108,16 +1159,21 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, return -1; } + /* If this is a .exit hostname, strip off the .name.exit part, and + * see whether we're going to connect there, and otherwise handle it. + * (The ".exit" part got stripped off by "parse_extended_hostname"). + * + * We'll set chosen_exit_name and/or close the connection as appropriate. + */ if (addresstype == EXIT_HOSTNAME) { - /* foo.exit -- modify conn->chosen_exit_node to specify the exit - * node, and conn->address to hold only the address portion. */ - char *s = strrchr(socks->address,'.'); - - /* If StrictNodes is not set, then .exit overrides ExcludeNodes. */ + /* If StrictNodes is not set, then .exit overrides ExcludeNodes but + * not ExcludeExitNodes. */ routerset_t *excludeset = options->StrictNodes ? options->ExcludeExitNodesUnion_ : options->ExcludeExitNodes; - const node_t *node; + const node_t *node = NULL; + /* If this .exit was added by an AUTOMAP, then it came straight from + * a user. Make sure that options->AllowDotExit permits that. */ if (exit_source == ADDRMAPSRC_AUTOMAP && !options->AllowDotExit) { /* Whoops; this one is stale. It must have gotten added earlier, * when AllowDotExit was on. */ @@ -1130,6 +1186,8 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, return -1; } + /* Double-check to make sure there are no .exits coming from + * impossible/weird sources. */ 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 @@ -1144,9 +1202,12 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, } tor_assert(!automap); + /* Now, find the character before the .(name) part. */ + char *s = strrchr(socks->address,'.'); if (s) { /* The address was of the form "(stuff).(name).exit */ if (s[1] != '\0') { + /* Looks like a real .exit one. */ conn->chosen_exit_name = tor_strdup(s+1); node = node_get_by_nickname(conn->chosen_exit_name, 1); @@ -1165,7 +1226,8 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, return -1; } } else { - /* It looks like they just asked for "foo.exit". */ + /* It looks like they just asked for "foo.exit". That's a special + * form that means (foo's address).foo.exit. */ conn->chosen_exit_name = tor_strdup(socks->address); node = node_get_by_nickname(conn->chosen_exit_name, 1); @@ -1174,6 +1236,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, node_get_address_string(node, socks->address, sizeof(socks->address)); } } + /* Now make sure that the chosen exit exists... */ if (!node) { log_warn(LD_APP, @@ -1195,8 +1258,12 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, implies no. */ } + /* Now, handle everything that isn't a .onion address. */ if (addresstype != ONION_HOSTNAME) { - /* not a hidden-service request (i.e. normal or .exit) */ + /* Not a hidden-service request. It's either a hostname or an IP, + * possibly with a .exit that we stripped off. */ + + /* Check for funny characters in the address. */ if (address_is_invalid_destination(socks->address, 1)) { control_event_client_status(LOG_WARN, "SOCKS_BAD_HOSTNAME HOSTNAME=%s", escaped(socks->address)); @@ -1207,6 +1274,8 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, return -1; } + /* If we're running in Tor2webMode, we don't allow anything BUT .onion + * addresses. */ if (options->Tor2webMode) { log_warn(LD_APP, "Refusing to connect to non-hidden-service hostname %s " "because tor2web mode is enabled.", @@ -1215,6 +1284,9 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, return -1; } + /* See if this is a hostname lookup that we can answer immediately. + * (For example, an attempt to look up the IP address for an IP address.) + */ if (socks->command == SOCKS_COMMAND_RESOLVE) { tor_addr_t answer; /* Reply to resolves immediately if we can. */ @@ -1231,14 +1303,22 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, tor_assert(!automap); rep_hist_note_used_resolve(now); /* help predict this next time */ } else if (socks->command == SOCKS_COMMAND_CONNECT) { + /* Special handling for attempts to connect */ tor_assert(!automap); + /* Don't allow connections to port 0. */ if (socks->port == 0) { log_notice(LD_APP,"Application asked to connect to port 0. Refusing."); connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); return -1; } + /* You can't make connections to internal addresses, by default. + * Exceptions are begindir requests (where the address is meaningless, + * or cases where you've hand-configured a particular exit, thereby + * making the local address meaningful. */ if (options->ClientRejectInternalAddresses && !conn->use_begindir && !conn->chosen_exit_name && !circ) { + /* If we reach this point then we don't want to allow internal + * addresses. Check if we got one. */ tor_addr_t addr; if (tor_addr_hostname_is_local(socks->address) || (tor_addr_parse(&addr, socks->address) >= 0 && @@ -1273,31 +1353,46 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, connection_mark_unattached_ap(conn, END_STREAM_REASON_PRIVATE_ADDR); return -1; } - } + } /* end "if we should check for internal addresses" */ + /* Okay. We're still doing a CONNECT, and it wasn't a private + * address. Do special handling for literal IP addresses */ { tor_addr_t addr; /* XXX Duplicate call to tor_addr_parse. */ if (tor_addr_parse(&addr, socks->address) >= 0) { + /* If we reach this point, it's an IPv4 or an IPv6 address. */ sa_family_t family = tor_addr_family(&addr); + if ((family == AF_INET && ! conn->ipv4_traffic_ok) || (family == AF_INET6 && ! conn->ipv4_traffic_ok)) { + /* You can't do an IPv4 address on a v6-only socks listener, + * or vice versa. */ log_warn(LD_NET, "Rejecting SOCKS request for an IP address " "family that this listener does not support."); connection_mark_unattached_ap(conn, END_STREAM_REASON_ENTRYPOLICY); return -1; } else if (family == AF_INET6 && socks->socks_version == 4) { + /* You can't make a socks4 request to an IPv6 address. Socks4 + * doesn't support that. */ log_warn(LD_NET, "Rejecting SOCKS4 request for an IPv6 address."); connection_mark_unattached_ap(conn, END_STREAM_REASON_ENTRYPOLICY); return -1; } else if (socks->socks_version == 4 && !conn->ipv4_traffic_ok) { + /* You can't do any kind of Socks4 request when IPv4 is forbidden. + * + * XXX raise this check outside the enclosing block? */ log_warn(LD_NET, "Rejecting SOCKS4 request on a listener with " "no IPv4 traffic supported."); connection_mark_unattached_ap(conn, END_STREAM_REASON_ENTRYPOLICY); return -1; } else if (family == AF_INET6) { + /* Tell the exit: we won't accept any ipv4 connection to an IPv6 + * address. */ conn->ipv4_traffic_ok = 0; } else if (family == AF_INET) { + /* Tell the exit: we won't accept any ipv6 connection to an IPv4 + * address. */ conn->ipv6_traffic_ok = 0; } } @@ -1306,6 +1401,9 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, if (socks->socks_version == 4) conn->ipv6_traffic_ok = 0; + /* Still handling CONNECT. Now, check for exit enclaves. (Which we + * don't do on BEGINDIR, or there is a chosen exit.) + */ if (!conn->use_begindir && !conn->chosen_exit_name && !circ) { /* see if we can find a suitable enclave exit */ const node_t *r = @@ -1322,11 +1420,13 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, } } - /* warn or reject if it's using a dangerous port */ + /* Still handling CONNECT: warn or reject if it's using a dangerous + * port. */ if (!conn->use_begindir && !conn->chosen_exit_name && !circ) if (consider_plaintext_ports(conn, socks->port) < 0) return -1; + /* Remember the port so that we do predicted requests there. */ if (!conn->use_begindir) { /* help predict this next time */ rep_hist_note_used_port(now, socks->port); @@ -1335,25 +1435,41 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, rep_hist_note_used_resolve(now); /* help predict this next time */ /* no extra processing needed */ } else { + /* We should only be doing CONNECT or RESOLVE! */ tor_fragile_assert(); } + + /* Okay. At this point we've set chosen_exit_name if needed, rewritten the + * address, and decided not to reject it for any number of reasons. Now + * mark the connection as waiting for a circuit, and try to attach it! + */ base_conn->state = AP_CONN_STATE_CIRCUIT_WAIT; - if ((circ && connection_ap_handshake_attach_chosen_circuit( - conn, circ, cpath) < 0) || - (!circ && - connection_ap_handshake_attach_circuit(conn) < 0)) { + + /* If we were given a circuit to attach to, try to attach. Otherwise, + * try to find a good one and attach to that. */ + int rv; + if (circ) + rv = connection_ap_handshake_attach_chosen_circuit(conn, circ, cpath); + else + rv = connection_ap_handshake_attach_circuit(conn); + + /* If the above function returned 0 then we're waiting for a circuit. + * if it returned 1, we're attached. Both are okay. But if it returned + * -1, there was an error, so make sure the connection is marked, and + * return -1. */ + if (rv < 0) { if (!base_conn->marked_for_close) connection_mark_unattached_ap(conn, END_STREAM_REASON_CANT_ATTACH); return -1; } + return 0; } else { - /* it's a hidden-service request */ - rend_cache_entry_t *entry; - int r; - rend_service_authorization_t *client_auth; - rend_data_t *rend_data; + /* If we get here, it's a request for a .onion address! */ tor_assert(!automap); + + /* Check whether it's RESOLVE or RESOLVE_PTR. We don't handle those + * for hidden service addresses. */ if (SOCKS_COMMAND_IS_RESOLVE(socks->command)) { /* if it's a resolve request, fail it right now, rather than * building all the circuits and then realizing it won't work. */ @@ -1367,6 +1483,8 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, return -1; } + /* If we were passed a circuit, then we need to fail. .onion addresses + * only work when we launch our own circuits for now. */ if (circ) { log_warn(LD_CONTROL, "Attachstream to a circuit is not " "supported for .onion addresses currently. Failing."); @@ -1374,15 +1492,22 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, return -1; } - ENTRY_TO_EDGE_CONN(conn)->rend_data = rend_data = + /* Fill in the rend_data field so we can start doing a connection to + * a hidden service. */ + rend_data_t *rend_data = ENTRY_TO_EDGE_CONN(conn)->rend_data = tor_malloc_zero(sizeof(rend_data_t)); strlcpy(rend_data->onion_address, socks->address, sizeof(rend_data->onion_address)); log_info(LD_REND,"Got a hidden service request for ID '%s'", safe_str_client(rend_data->onion_address)); - /* see if we already have it cached */ - r = rend_cache_lookup_entry(rend_data->onion_address, -1, &entry); - if (r<0) { + + /* see if we already have a hidden service descriptor cached for this + * address. */ + rend_cache_entry_t *entry = NULL; + const int rend_cache_lookup_result = + rend_cache_lookup_entry(rend_data->onion_address, -1, &entry); + if (rend_cache_lookup_result < 0) { + /* We should already have rejected this address! */ log_warn(LD_BUG,"Invalid service name '%s'", safe_str_client(rend_data->onion_address)); connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); @@ -1393,8 +1518,10 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, * a stable circuit yet, but we know we'll need *something*. */ rep_hist_note_used_internal(now, 0, 1); - /* Look up if we have client authorization for it. */ - client_auth = rend_client_lookup_service_authorization( + /* Look up if we have client authorization configured for this hidden + * service. If we do, associate it with the rend_data. */ + rend_service_authorization_t *client_auth = + rend_client_lookup_service_authorization( rend_data->onion_address); if (client_auth) { log_info(LD_REND, "Using previously configured client authorization " @@ -1403,12 +1530,16 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, client_auth->descriptor_cookie, REND_DESC_COOKIE_LEN); rend_data->auth_type = client_auth->auth_type; } - if (r==0) { + + /* Now, we either launch an attempt to connect to the hidden service, + * or we launch an attempt to look up its descriptor, depending on + * whether we had the descriptor. */ + if (rend_cache_lookup_result == 0) { base_conn->state = AP_CONN_STATE_RENDDESC_WAIT; log_info(LD_REND, "Unknown descriptor %s. Fetching.", safe_str_client(rend_data->onion_address)); rend_client_refetch_v2_renddesc(rend_data); - } else { /* r > 0 */ + } else { /* rend_cache_lookup_result > 0 */ base_conn->state = AP_CONN_STATE_CIRCUIT_WAIT; log_info(LD_REND, "Descriptor is here. Great."); if (connection_ap_handshake_attach_circuit(conn) < 0) { @@ -1419,6 +1550,7 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn, } return 0; } + return 0; /* unreached but keeps the compiler happy */ } diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index d4a20e7f96..0cc391ed82 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -151,8 +151,8 @@ typedef struct { */ char orig_address[MAX_SOCKS_ADDR_LEN]; /** True iff the address has been automatically remapped to a local - * address in VirtualAddrNetwork. (Only set true the first time we - * do the mapping.) */ + * address in VirtualAddrNetwork. (Only set true when we do a resolve + * and get a virtual address; not when we connect to the address.) */ int automap; /** If this connection has a .exit address, who put it there? */ addressmap_entry_source_t exit_source;