From d3224bad42957bf2e1751c7a1731c8956e003530 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 27 Aug 2007 15:33:58 +0000 Subject: [PATCH] r14227@Kushana: nickm | 2007-08-27 11:33:28 -0400 Add a new ClientDNSRejectInternalAddresses option (default: on) to refuse to believe that any address can map to or from an internal address. This blocks some kinds of potential browser-based attacks, especially on hosts using DNSPort. Also clarify behavior in some comments. Backport candiate? svn:r11287 --- ChangeLog | 6 ++++++ doc/tor.1.in | 7 +++++++ src/common/util.c | 4 +++- src/or/config.c | 5 ++++- src/or/connection_edge.c | 13 +++++++++++++ src/or/or.h | 8 +++++++- src/or/relay.c | 28 ++++++++++++++++++++++++---- 7 files changed, 64 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index 41af219d5d..e48d7ecda0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +Changes in version 0.2.0.6-alpha - 2007-??-?? + o Minor features (security): + - As a client, do not believe any server that tells us that any address + maps to an internal address space. + + Changes in version 0.2.0.6-alpha - 2007-08-26 o New directory authorities: - Set up Tonga as the default bridge directory authority. diff --git a/doc/tor.1.in b/doc/tor.1.in index e53a89e263..1b57b17161 100644 --- a/doc/tor.1.in +++ b/doc/tor.1.in @@ -676,6 +676,13 @@ Bind to this address to listen for DNS connections. (Default: 127.0.0.1). .LP .TP +\fBClientDNSRejectInternalAddresses\fP \fR\fB0\fR|\fB1\fR\fP +If true, Tor does not believe any anonymously retrieved DNS answer that tells +it that an address resolves to an internal address (like 127.0.0.1 or +192.168.0.1). This option prevents certain browser-based attacks; don't turn +it off unless you know what you're doing. (Default: 1). +.LP +.TP \fBDownloadExtraInfo\fP \fR\fB0\fR|\fB1\fR\fP If true, Tor downloads and caches "extra-info" documents. These documents contain information about servers other than the information diff --git a/src/common/util.c b/src/common/util.c index 87af9556c0..b3d3932578 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -2601,7 +2601,9 @@ tor_addr_to_str(char *dest, const tor_addr_t *addr, int len) } /** Convert the string in src to a tor_addr_t addr. - */ + * + * Return an address family on success, or -1 if an invalid address string is + * provided. */ int tor_addr_from_str(tor_addr_t *addr, const char *src) { diff --git a/src/or/config.c b/src/or/config.c index 3c7341efc9..6f747ed473 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -143,6 +143,8 @@ static config_var_t _option_vars[] = { VAR("Bridge", LINELIST, Bridges, NULL), VAR("CircuitBuildTimeout", INTERVAL, CircuitBuildTimeout, "1 minute"), VAR("CircuitIdleTimeout", INTERVAL, CircuitIdleTimeout, "1 hour"), + VAR("ClientDNSRejectInternalAddresses", BOOL, + ClientDNSRejectInternalAddresses, "1"), VAR("ClientOnly", BOOL, ClientOnly, "0"), VAR("ConnLimit", UINT, ConnLimit, "1000"), VAR("ConstrainedSockets", BOOL, ConstrainedSockets, "0"), @@ -827,7 +829,8 @@ options_act_reversible(or_options_t *old_options, char **msg) int logs_marked = 0; /* Daemonize _first_, since we only want to open most of this stuff in - * the subprocess. */ + * the subprocess. Libevent bases can't be reliably inherited across + * processes. */ if (running_tor && options->RunAsDaemon) { /* No need to roll back, since you can't change the value. */ start_daemon(); diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 77a95f6764..f4431bacf2 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1247,6 +1247,19 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn, END_STREAM_REASON_FLAG_ALREADY_SOCKS_REPLIED); return 0; } + if (options->ClientDNSRejectInternalAddresses) { + /* Don't let people try to do a reverse lookup on 10.0.0.1. */ + tor_addr_t addr; + if (tor_addr_from_str(&addr, socks->address) >= 0 && + 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; + } + } } else if (!automap) { /* For address map controls, remap the address. */ if (addressmap_rewrite(socks->address, sizeof(socks->address), diff --git a/src/or/or.h b/src/or/or.h index 3c88425414..aded806136 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -191,7 +191,8 @@ #define DEFAULT_DNS_TTL (30*60) /** How long can a TTL be before we stop believing it? */ #define MAX_DNS_TTL (3*60*60) -/** How small can a TTL be before we stop believing it? */ +/** How small can a TTL be before we stop believing it? Provides rudimentary + * pinning. */ #define MIN_DNS_TTL (60) /** How often do we rotate onion keys? */ @@ -2093,6 +2094,11 @@ typedef struct { * if we are a cache). For authorities, this is always true. */ int DownloadExtraInfo; + /** If true, do not believe anybody who tells us that a domain resolves + * to an internal address, or that an internal address has a PTR mapping. + * Helps avoid some cross-site attacks. */ + int ClientDNSRejectInternalAddresses; + /** The length of time that we think a consensus should be fresh. */ int V3AuthVotingInterval; /** The length of time we think it will take to distribute votes */ diff --git a/src/or/relay.c b/src/or/relay.c index 68b8a5f82d..d9ca3760f9 100644 --- a/src/or/relay.c +++ b/src/or/relay.c @@ -900,9 +900,14 @@ connection_edge_process_relay_cell_not_open( if (rh->length >= 4) { uint32_t addr = ntohl(get_uint32(cell->payload+RELAY_HEADER_SIZE)); int ttl; - if (!addr) { + if (!addr || (get_options()->ClientDNSRejectInternalAddresses && + is_internal_IP(addr, 0))) { + char buf[INET_NTOA_BUF_LEN]; + struct in_addr a; + a.s_addr = htonl(addr); + tor_inet_ntoa(&a, buf, sizeof(buf)); log_info(LD_APP, - "...but it claims the IP address was 0.0.0.0. Closing."); + "...but it claims the IP address was %s. Closing.", buf); connection_edge_end(conn, END_STREAM_REASON_TORPROTOCOL); connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); return 0; @@ -946,13 +951,28 @@ connection_edge_process_relay_cell_not_open( connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); return 0; } + answer_type = cell->payload[RELAY_HEADER_SIZE]; if (rh->length >= answer_len+6) ttl = (int)ntohl(get_uint32(cell->payload+RELAY_HEADER_SIZE+ 2+answer_len)); else ttl = -1; - - answer_type = cell->payload[RELAY_HEADER_SIZE]; + if (answer_type == RESOLVED_TYPE_IPV4 && answer_len >= 4) { + uint32_t addr = ntohl(get_uint32(cell->payload+RELAY_HEADER_SIZE+2)); + if (get_options()->ClientDNSRejectInternalAddresses && + is_internal_IP(addr, 0)) { + char buf[INET_NTOA_BUF_LEN]; + struct in_addr a; + a.s_addr = htonl(addr); + tor_inet_ntoa(&a, buf, sizeof(buf)); + log_info(LD_APP,"Got a resolve with answer %s. Rejecting.", buf); + connection_ap_handshake_socks_resolved(conn, + RESOLVED_TYPE_ERROR_TRANSIENT, + 0, NULL, 0, TIME_MAX); + connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); + return 0; + } + } connection_ap_handshake_socks_resolved(conn, answer_type, cell->payload[RELAY_HEADER_SIZE+1], /*answer_len*/