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

@@ -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.

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

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 <b>src</b> to a tor_addr_t <b>addr</b>.
- */
+ *
+ *  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)
 {

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();

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),

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 */

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*/