Browse Source

More DNS fixes. Send meaningful TTLs back to the client when
possible. Cache at the server side independently from the TTL, to
prevent attackers from probing the server to see who has been asking
for what hostnames. (Hi, Dan Kaminski!)

Also, clean some whitespace.


svn:r6526

Nick Mathewson 18 years ago
parent
commit
c660a0f6a2
7 changed files with 85 additions and 44 deletions
  1. 4 4
      Makefile.am
  2. 6 1
      doc/tor-spec.txt
  3. 1 1
      src/common/compat.c
  4. 7 8
      src/or/connection_edge.c
  5. 56 25
      src/or/dns.c
  6. 1 0
      src/or/eventdns_tor.h
  7. 10 5
      src/or/or.h

+ 4 - 4
Makefile.am

@@ -55,8 +55,8 @@ doxygen:
 
 # Avoid strlcpy.c, strlcat.c, tree.h
 check-spaces:
-	./contrib/checkSpace.pl -C             \
-	        src/common/*.h                 \
-		src/common/[^s]*.c             \
-		src/or/[^t]*.[ch] src/or/t*.c
+	./contrib/checkSpace.pl -C                    \
+	        src/common/*.h                        \
+		src/common/[^as]*.c                   \
+		src/or/[^et]*.[ch] src/or/t*.c src/or/eventdns_tor.h
 

+ 6 - 1
doc/tor-spec.txt

@@ -571,7 +571,12 @@ when do we rotate which keys (tls, link, etc)?
        The IPv6 address to which the connection was made [16 octets]
        A number of seconds (TTL) for which the address may be cached [4 octets]
    [XXXX Versions of Tor before 0.1.1.6 ignore and do not generate the TTL
-   field.  No version of Tor currently generates the IPv6 format.]
+   field.  No version of Tor currently generates the IPv6 format.
+
+   Tor servers before 0.1.2.0 set the TTL field to a fixed value.  Later
+   versions set the TTL to the last value seen from a DNS server, and expire
+   their own cached entries after a fixed interval.  This prevents certain
+   attacks.]
 
    The OP waits for a RELAY_CONNECTED cell before sending any data.
    Once a connection has been established, the OP and exit node

+ 1 - 1
src/common/compat.c

@@ -131,7 +131,7 @@ tor_mmap_file(const char *filename, size_t *size)
   *size += (page_size + (page_size-(*size%page_size)));
 
   string = mmap(0, *size, PROT_READ, MAP_PRIVATE, fd, 0);
-  if(string == MAP_FAILED) {
+  if (string == MAP_FAILED) {
     log_warn(LD_FS,"Could not mmap file \"%s\": %s", filename,
              strerror(errno));
     return NULL;

+ 7 - 8
src/or/connection_edge.c

@@ -196,8 +196,7 @@ connection_edge_end(connection_t *conn, char reason, crypt_path_t *cpath_layer)
   if (reason == END_STREAM_REASON_EXITPOLICY &&
       !connection_edge_is_rendezvous_stream(conn)) {
     set_uint32(payload+1, htonl(conn->addr));
-    /* XXXX fill with a real TTL! */
-    set_uint32(payload+5, htonl(MAX_DNS_ENTRY_AGE));
+    set_uint32(payload+5, htonl(dns_clip_ttl(conn->address_ttl)));
     payload_len += 8;
   }
 
@@ -297,7 +296,7 @@ connection_edge_finished_connecting(connection_t *conn)
     char connected_payload[8];
     set_uint32(connected_payload, htonl(conn->addr));
     set_uint32(connected_payload+4,
-               htonl(MAX_DNS_ENTRY_AGE)); /* XXXX fill with a real TTL */
+               htonl(dns_clip_ttl(conn->address_ttl)));
     if (connection_edge_send_command(conn, circuit_get_by_edge_conn(conn),
         RELAY_COMMAND_CONNECTED, connected_payload, 8, conn->cpath_layer) < 0)
       return 0; /* circuit is closed, don't continue */
@@ -682,7 +681,7 @@ client_dns_incr_failures(const char *address)
   addressmap_entry_t *ent = strmap_get(addressmap, address);
   if (!ent) {
     ent = tor_malloc_zero(sizeof(addressmap_entry_t));
-    ent->expires = time(NULL)+MAX_DNS_ENTRY_AGE;
+    ent->expires = time(NULL) + MAX_DNS_ENTRY_AGE;
     strmap_set(addressmap,address,ent);
   }
   ++ent->num_resolve_failures;
@@ -726,10 +725,10 @@ client_dns_set_addressmap(const char *address, uint32_t val,
   char extendedval[INET_NTOA_BUF_LEN+MAX_HEX_NICKNAME_LEN+10];
   char valbuf[INET_NTOA_BUF_LEN];
 
-  tor_assert(address); tor_assert(val);
+  tor_assert(address);
+  tor_assert(val);
 
-  if (ttl<0 || ttl>MAX_DNS_ENTRY_AGE)
-    ttl = MAX_DNS_ENTRY_AGE;
+  ttl = dns_clip_ttl(ttl);
 
   if (tor_inet_aton(address, &in))
     return; /* If address was an IP address already, don't add a mapping. */
@@ -1817,7 +1816,7 @@ connection_exit_connect(connection_t *conn)
     char connected_payload[8];
     set_uint32(connected_payload, htonl(conn->addr));
     set_uint32(connected_payload+4,
-               htonl(MAX_DNS_ENTRY_AGE)); /* XXXX fill with a real TTL */
+               htonl(dns_clip_ttl(conn->address_ttl)));
     connection_edge_send_command(conn, circuit_get_by_edge_conn(conn),
                                  RELAY_COMMAND_CONNECTED,
                                  connected_payload, 8, conn->cpath_layer);

+ 56 - 25
src/or/dns.c

@@ -71,6 +71,7 @@ typedef struct cached_resolve_t {
 #define CACHE_STATE_VALID 1
 #define CACHE_STATE_FAILED 2
   uint32_t expire; /**< Remove items from cache after this time. */
+  uint32_t ttl; /**< What TTL did the nameserver tell us? */
   pending_connection_t *pending_connections;
   struct cached_resolve_t *next;
 } cached_resolve_t;
@@ -79,8 +80,7 @@ static void purge_expired_resolves(uint32_t now);
 static void dns_purge_resolve(cached_resolve_t *resolve);
 static void dns_found_answer(char *address, uint32_t addr, char outcome,
                              uint32_t ttl);
-static void send_resolved_cell(connection_t *conn, uint8_t answer_type,
-                               uint32_t ttl);
+static void send_resolved_cell(connection_t *conn, uint8_t answer_type);
 static int assign_to_dnsworker(connection_t *exitconn);
 #ifndef USE_EVENTDNS
 static int dnsworker_main(void *data);
@@ -130,6 +130,28 @@ dns_init(void)
 #endif
 }
 
+uint32_t
+dns_clip_ttl(uint32_t ttl)
+{
+  if (ttl < MIN_DNS_TTL)
+    return MIN_DNS_TTL;
+  else if (ttl > MAX_DNS_TTL)
+    return MAX_DNS_TTL;
+  else
+    return ttl;
+}
+
+static uint32_t
+dns_get_expiry_ttl(uint32_t ttl)
+{
+  if (ttl < MIN_DNS_TTL)
+    return MIN_DNS_TTL;
+  else if (ttl > MAX_DNS_ENTRY_AGE)
+    return MAX_DNS_ENTRY_AGE;
+  else
+    return ttl;
+}
+
 /** Helper: free storage held by an entry in the DNS cache. */
 static void
 _free_cached_resolve(cached_resolve_t *r)
@@ -212,12 +234,14 @@ purge_expired_resolves(uint32_t now)
 /** Send a response to the RESOVLE request of a connection. answer_type must
  *  be one of RESOLVED_TYPE_(IPV4|ERROR|ERROR_TRANSIENT) */
 static void
-send_resolved_cell(connection_t *conn, uint8_t answer_type, uint32_t ttl)
+send_resolved_cell(connection_t *conn, uint8_t answer_type)
 {
   char buf[RELAY_PAYLOAD_SIZE];
   size_t buflen;
+  uint32_t ttl;
 
   buf[0] = answer_type;
+  ttl = dns_clip_ttl(conn->address_ttl);
 
   switch (answer_type)
     {
@@ -289,8 +313,9 @@ dns_resolve(connection_t *exitconn)
    * know the answer. */
   if (tor_inet_aton(exitconn->address, &in) != 0) {
     exitconn->addr = ntohl(in.s_addr);
+    exitconn->address_ttl = DEFAULT_DNS_TTL;
     if (exitconn->purpose == EXIT_PURPOSE_RESOLVE)
-      send_resolved_cell(exitconn, RESOLVED_TYPE_IPV4, MAX_DNS_ENTRY_AGE);
+      send_resolved_cell(exitconn, RESOLVED_TYPE_IPV4);
     return 1;
   }
 
@@ -305,9 +330,6 @@ dns_resolve(connection_t *exitconn)
   strlcpy(search.address, exitconn->address, sizeof(search.address));
   resolve = HT_FIND(cache_map, &cache_root, &search);
   if (resolve && resolve->expire > now) { /* already there */
-    // XXXX Security problem: this leaks a time at which somebody asked for
-    // XXXX the address.  That's a problem.
-    unsigned int ttl = (unsigned int) resolve->expire - now;
     switch (resolve->state) {
       case CACHE_STATE_PENDING:
         /* add us to the pending list */
@@ -323,16 +345,17 @@ dns_resolve(connection_t *exitconn)
         return 0;
       case CACHE_STATE_VALID:
         exitconn->addr = resolve->addr;
+        exitconn->address_ttl = resolve->ttl;
         log_debug(LD_EXIT,"Connection (fd %d) found cached answer for %s",
                   exitconn->s, escaped_safe_str(exitconn->address));
         if (exitconn->purpose == EXIT_PURPOSE_RESOLVE)
-          send_resolved_cell(exitconn, RESOLVED_TYPE_IPV4, ttl);
+          send_resolved_cell(exitconn, RESOLVED_TYPE_IPV4);
         return 1;
       case CACHE_STATE_FAILED:
         log_debug(LD_EXIT,"Connection (fd %d) found cached error for %s",
                   exitconn->s, escaped_safe_str(exitconn->address));
         if (exitconn->purpose == EXIT_PURPOSE_RESOLVE)
-          send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR, ttl);
+          send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR);
         circ = circuit_get_by_edge_conn(exitconn);
         if (circ)
           circuit_detach_stream(circ, exitconn);
@@ -345,7 +368,7 @@ dns_resolve(connection_t *exitconn)
   /* not there, need to add it */
   resolve = tor_malloc_zero(sizeof(cached_resolve_t));
   resolve->state = CACHE_STATE_PENDING;
-  resolve->expire = now + MAX_DNS_ENTRY_AGE;
+  resolve->expire = now + DEFAULT_DNS_TTL; /* this will get replaced. */
   strlcpy(resolve->address, exitconn->address, sizeof(resolve->address));
 
   /* add us to the pending list */
@@ -557,7 +580,8 @@ dns_found_answer(char *address, uint32_t addr, char outcome, uint32_t ttl)
     resolve->state = (outcome == DNS_RESOLVE_SUCCEEDED) ?
       CACHE_STATE_VALID : CACHE_STATE_FAILED;
     resolve->addr = addr;
-    resolve->expire = time(NULL) + ttl;
+    resolve->expire = time(NULL) + dns_get_expiry_ttl(ttl);
+    resolve->ttl = ttl;
     insert_resolve(resolve);
     return;
   }
@@ -577,7 +601,8 @@ dns_found_answer(char *address, uint32_t addr, char outcome, uint32_t ttl)
   /* tor_assert(resolve->state == CACHE_STATE_PENDING); */
 
   resolve->addr = addr;
-  resolve->expire = time(NULL) + ttl;
+  resolve->expire = time(NULL) + dns_get_expiry_ttl(ttl);
+  resolve->ttl = ttl;
   if (outcome == DNS_RESOLVE_SUCCEEDED)
     resolve->state = CACHE_STATE_VALID;
   else
@@ -587,6 +612,7 @@ dns_found_answer(char *address, uint32_t addr, char outcome, uint32_t ttl)
     pend = resolve->pending_connections;
     assert_connection_ok(pend->conn,time(NULL));
     pend->conn->addr = resolve->addr;
+    pend->conn->address_ttl = resolve->ttl;
     pendconn = pend->conn; /* don't pass complex things to the
                               connection_mark_for_close macro */
 
@@ -599,7 +625,7 @@ dns_found_answer(char *address, uint32_t addr, char outcome, uint32_t ttl)
         /* This detach must happen after we send the end cell. */
         circuit_detach_stream(circuit_get_by_edge_conn(pendconn), pendconn);
       } else {
-        send_resolved_cell(pendconn, RESOLVED_TYPE_ERROR, ttl);
+        send_resolved_cell(pendconn, RESOLVED_TYPE_ERROR);
         /* This detach must happen after we send the resolved cell. */
         circuit_detach_stream(circuit_get_by_edge_conn(pendconn), pendconn);
       }
@@ -623,7 +649,7 @@ dns_found_answer(char *address, uint32_t addr, char outcome, uint32_t ttl)
         /* prevent double-remove.  This isn't really an accurate state,
          * but it does the right thing. */
         pendconn->state = EXIT_CONN_STATE_RESOLVEFAILED;
-        send_resolved_cell(pendconn, RESOLVED_TYPE_IPV4, ttl);
+        send_resolved_cell(pendconn, RESOLVED_TYPE_IPV4);
         circ = circuit_get_by_edge_conn(pendconn);
         tor_assert(circ);
         circuit_detach_stream(circ, pendconn);
@@ -663,7 +689,7 @@ assign_to_dnsworker(connection_t *exitconn)
   if (!dnsconn) {
     log_warn(LD_EXIT,"no idle dns workers. Failing.");
     if (exitconn->purpose == EXIT_PURPOSE_RESOLVE)
-      send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR_TRANSIENT, 0);
+      send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR_TRANSIENT);
     goto err;
   }
 
@@ -1051,28 +1077,30 @@ eventdns_callback(int result, char type, int count, int ttl, void *addresses,
                   void *arg)
 {
   char *string_address = arg;
+  int status = DNS_RESOLVE_FAILED_PERMANENT;
+  uint32_t addr = 0;
   if (result == DNS_ERR_NONE) {
     if (type == DNS_IPv4_A && count) {
       uint32_t *addrs = addresses;
-      dns_found_answer(string_address, addrs[0], DNS_RESOLVE_SUCCEEDED, ttl);
+      addr = addrs[0];
+      status = DNS_RESOLVE_SUCCEEDED;
     } else if (count) {
       log_warn(LD_EXIT, "eventdns returned only non-IPv4 answers for %s.",
                escaped_safe_str(string_address));
-      dns_found_answer(string_address, 0, DNS_RESOLVE_FAILED_PERMANENT, ttl);
     } else {
       log_warn(LD_BUG, "eventdns returned no addresses or error for %s!",
                escaped_safe_str(string_address));
-      dns_found_answer(string_address, 0, DNS_RESOLVE_FAILED_PERMANENT, ttl);
     }
   } else {
-    int err = eventdns_err_is_transient(result)
-      ? DNS_RESOLVE_FAILED_TRANSIENT : DNS_RESOLVE_FAILED_PERMANENT;
-    dns_found_answer(string_address, 0, err, ttl);
+    if (eventdns_err_is_transient(result))
+      status = DNS_RESOLVE_FAILED_TRANSIENT;
   }
+  dns_found_answer(string_address, addr, status, ttl);
   tor_free(string_address);
 }
 
-static int assign_to_dnsworker(connection_t *exitconn)
+static int
+assign_to_dnsworker(connection_t *exitconn)
 {
   char *addr = tor_strdup(exitconn->address);
   int r = eventdns_resolve(exitconn->address, DNS_QUERY_NO_SEARCH,
@@ -1082,9 +1110,11 @@ static int assign_to_dnsworker(connection_t *exitconn)
              escaped_safe_str(addr), r);
     if (exitconn->purpose == EXIT_PURPOSE_RESOLVE) {
       if (eventdns_err_is_transient(r))
-        send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR_TRANSIENT, 0);
-      else
-        send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR, MAX_DNS_ENTRY_AGE);
+        send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR_TRANSIENT);
+      else {
+        exitconn->address_ttl = DEFAULT_DNS_TTL;
+        send_resolved_cell(exitconn, RESOLVED_TYPE_ERROR);
+      }
     }
     dns_cancel_pending_resolve(addr);/* also sends end and frees */
     tor_free(addr);
@@ -1092,3 +1122,4 @@ static int assign_to_dnsworker(connection_t *exitconn)
   return r ? -1 : 0;
 }
 #endif /* USE_EVENTDNS */
+

+ 1 - 0
src/or/eventdns_tor.h

@@ -3,3 +3,4 @@
 #define DNS_USE_OPENSSL_FOR_ID
 typedef unsigned int uint;
 typedef unsigned char u_char;
+

+ 10 - 5
src/or/or.h

@@ -161,12 +161,14 @@
 #define MAX_HEADERS_SIZE 50000
 #define MAX_BODY_SIZE 500000
 
-#ifdef TOR_PERF
-/** How long do we keep DNS cache entries before purging them? */
-#define MAX_DNS_ENTRY_AGE (150*60)
-#else
+/** How long do we keep DNS cache entries before purging them (regardless of
+ * their TTL)? */
 #define MAX_DNS_ENTRY_AGE (30*60)
-#endif
+#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? */
+#define MIN_DNS_TTL (60)
 
 /** How often do we rotate onion keys? */
 #define MIN_ONION_KEY_LIFETIME (7*24*60*60)
@@ -652,6 +654,8 @@ struct connection_t {
   char *address; /**< FQDN (or IP) of the guy on the other end.
                   * strdup into this, because free_connection frees it.
                   */
+  uint32_t address_ttl; /**< TTL for address-to-addr mapping on exit
+                         * connection.  Exit connections only. */
   char identity_digest[DIGEST_LEN]; /**< Hash of the public RSA key for
                                      * the other side's signing key. */
   char *nickname; /**< Nickname of OR on other side (if any). */
@@ -1927,6 +1931,7 @@ void dirserv_free_all(void);
 
 void dns_init(void);
 void dns_free_all(void);
+uint32_t dns_clip_ttl(uint32_t ttl);
 int connection_dns_finished_flushing(connection_t *conn);
 int connection_dns_reached_eof(connection_t *conn);
 int connection_dns_process_inbuf(connection_t *conn);