Merge remote-tracking branch 'public/bug19769_19025_029' into maint-0.2.9

7 files changed, 42 insertions, 62 deletions

diff --git a/changes/bug19025 b/changes/bug19025
new file mode 100644
index 0000000000..0f365f52ba
--- /dev/null
+++ b/changes/bug19025
@@ -0,0 +1,4 @@
+  o Major bugfixes (DNS):
+    - Fix a bug that prevented exit nodes from caching DNS records for more
+      than 60 seconds.
+      Fixes bug 19025; bugfix on 0.2.4.7-alpha.
diff --git a/changes/ticket19769 b/changes/ticket19769
new file mode 100644
index 0000000000..9fc05c3e9e
--- /dev/null
+++ b/changes/ticket19769
@@ -0,0 +1,7 @@
+  o Major features (security):
+    - Change the algorithm used to decide DNS TTLs on client and server side,
+      to better resist DNS-based correlation attacks like the DefecTor attack
+      of Greschbach, Pulls, Roberts, Winter, and Feamster).  Now
+      relays only return one of two possible DNS TTL values, and clients
+      are willing to believe DNS TTL values up to 3 hours long.
+      Closes ticket 19769.
diff --git a/src/or/dns.c b/src/or/dns.c
index 5f9813b912..e007a402f4 100644
--- a/src/or/dns.c
+++ b/src/or/dns.c
@@ -243,29 +243,19 @@ has_dns_init_failed(void)
 }
 
 /** Helper: Given a TTL from a DNS response, determine what TTL to give the
- * OP that asked us to resolve it. */
+ * OP that asked us to resolve it, and how long to cache that record
+ * ourselves. */
 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;
-}
-
-/** Helper: Given a TTL from a DNS response, determine how long to hold it in
- * our cache. */
-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;
+  /* This logic is a defense against "DefectTor" DNS-based traffic
+   * confirmation attacks, as in https://nymity.ch/tor-dns/tor-dns.pdf .
+   * We only give two values: a "low" value and a "high" value.
+   */
+  if (ttl < MIN_DNS_TTL_AT_EXIT)
+    return MIN_DNS_TTL_AT_EXIT;
   else
-    return ttl;
+    return MAX_DNS_TTL_AT_EXIT;
 }
 
 /** Helper: free storage held by an entry in the DNS cache. */
@@ -336,7 +326,7 @@ cached_resolve_add_answer(cached_resolve_t *resolve,
       resolve->result_ipv4.err_ipv4 = dns_result;
       resolve->res_status_ipv4 = RES_STATUS_DONE_ERR;
     }
-
+    resolve->ttl_ipv4 = ttl;
   } else if (query_type == DNS_IPv6_AAAA) {
     if (resolve->res_status_ipv6 != RES_STATUS_INFLIGHT)
       return;
@@ -351,6 +341,7 @@ cached_resolve_add_answer(cached_resolve_t *resolve,
       resolve->result_ipv6.err_ipv6 = dns_result;
       resolve->res_status_ipv6 = RES_STATUS_DONE_ERR;
     }
+    resolve->ttl_ipv6 = ttl;
   }
 }
 
@@ -1317,7 +1308,7 @@ make_pending_resolve_cached(cached_resolve_t *resolve)
         resolve->ttl_hostname < ttl)
       ttl = resolve->ttl_hostname;
 
-    set_expiry(new_resolve, time(NULL) + dns_get_expiry_ttl(ttl));
+    set_expiry(new_resolve, time(NULL) + dns_clip_ttl(ttl));
   }
 
   assert_cache_ok();
diff --git a/src/or/dns.h b/src/or/dns.h
index b14f7dd29c..951a2a3467 100644
--- a/src/or/dns.h
+++ b/src/or/dns.h
@@ -12,6 +12,18 @@
 #ifndef TOR_DNS_H
 #define TOR_DNS_H
 
+/** Lowest value for DNS ttl that a server will give. */
+#define MIN_DNS_TTL_AT_EXIT (5*60)
+/** Highest value for DNS ttl that a server will give. */
+#define MAX_DNS_TTL_AT_EXIT (60*60)
+
+/** How long do we keep DNS cache entries before purging them (regardless of
+ * their TTL)? */
+#define MAX_DNS_ENTRY_AGE (3*60*60)
+/** How long do we cache/tell clients to cache DNS records when no TTL is
+ * known? */
+#define DEFAULT_DNS_TTL (30*60)
+
 int dns_init(void);
 int has_dns_init_failed(void);
 void dns_free_all(void);
@@ -31,8 +43,6 @@ void dump_dns_mem_usage(int severity);
 #ifdef DNS_PRIVATE
 #include "dns_structs.h"
 
-STATIC uint32_t dns_get_expiry_ttl(uint32_t ttl);
-
 MOCK_DECL(STATIC int,dns_resolve_impl,(edge_connection_t *exitconn,
 int is_resolve,or_circuit_t *oncirc, char **hostname_out,
 int *made_connection_pending_out, cached_resolve_t **resolve_out));
diff --git a/src/or/or.h b/src/or/or.h
index b7c67baf93..33fe8b96c4 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -143,18 +143,6 @@
 /** Maximum size of a single extrainfo document, as above. */
 #define MAX_EXTRAINFO_UPLOAD_SIZE 50000
 
-/** How long do we keep DNS cache entries before purging them (regardless of
- * their TTL)? */
-#define MAX_DNS_ENTRY_AGE (30*60)
-/** How long do we cache/tell clients to cache DNS records when no TTL is
- * known? */
-#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?  Provides rudimentary
- * pinning. */
-#define MIN_DNS_TTL 60
-
 /** How often do we rotate onion keys? */
 #define MIN_ONION_KEY_LIFETIME (7*24*60*60)
 /** How often do we rotate TLS contexts? */
diff --git a/src/test/test_cell_formats.c b/src/test/test_cell_formats.c
index f839a5b72c..f429f4291d 100644
--- a/src/test/test_cell_formats.c
+++ b/src/test/test_cell_formats.c
@@ -345,9 +345,9 @@ test_cfmt_connected_cells(void *arg)
   memset(&cell, 0, sizeof(cell));
   tor_addr_parse(&addr, "30.40.50.60");
   rh.length = connected_cell_format_payload(cell.payload+RELAY_HEADER_SIZE,
-                                            &addr, 128);
+                                            &addr, 1024);
   tt_int_op(rh.length, OP_EQ, 8);
-  test_memeq_hex(cell.payload+RELAY_HEADER_SIZE, "1e28323c" "00000080");
+  test_memeq_hex(cell.payload+RELAY_HEADER_SIZE, "1e28323c" "00000e10");
 
   /* Try parsing it. */
   tor_addr_make_unspec(&addr);
@@ -355,7 +355,7 @@ test_cfmt_connected_cells(void *arg)
   tt_int_op(r, OP_EQ, 0);
   tt_int_op(tor_addr_family(&addr), OP_EQ, AF_INET);
   tt_str_op(fmt_addr(&addr), OP_EQ, "30.40.50.60");
-  tt_int_op(ttl, OP_EQ, 128);
+  tt_int_op(ttl, OP_EQ, 3600); /* not 1024, since we clipped to 3600 */
 
   /* Try an IPv6 address */
   memset(&rh, 0, sizeof(rh));
diff --git a/src/test/test_dns.c b/src/test/test_dns.c
index 8346c0a33f..6a8e92cb47 100644
--- a/src/test/test_dns.c
+++ b/src/test/test_dns.c
@@ -16,30 +16,11 @@ NS(test_main)(void *arg)
 {
   (void)arg;
 
-  uint32_t ttl_mid = MIN_DNS_TTL / 2 + MAX_DNS_TTL / 2;
+  uint32_t ttl_mid = MIN_DNS_TTL_AT_EXIT / 2 + MAX_DNS_TTL_AT_EXIT / 2;
 
-  tt_int_op(dns_clip_ttl(MIN_DNS_TTL - 1),==,MIN_DNS_TTL);
-  tt_int_op(dns_clip_ttl(ttl_mid),==,ttl_mid);
-  tt_int_op(dns_clip_ttl(MAX_DNS_TTL + 1),==,MAX_DNS_TTL);
-
-  done:
-  return;
-}
-
-#undef NS_SUBMODULE
-
-#define NS_SUBMODULE expiry_ttl
-
-static void
-NS(test_main)(void *arg)
-{
-  (void)arg;
-
-  uint32_t ttl_mid = MIN_DNS_TTL / 2 + MAX_DNS_ENTRY_AGE / 2;
-
-  tt_int_op(dns_get_expiry_ttl(MIN_DNS_TTL - 1),==,MIN_DNS_TTL);
-  tt_int_op(dns_get_expiry_ttl(ttl_mid),==,ttl_mid);
-  tt_int_op(dns_get_expiry_ttl(MAX_DNS_ENTRY_AGE + 1),==,MAX_DNS_ENTRY_AGE);
+  tt_int_op(dns_clip_ttl(MIN_DNS_TTL_AT_EXIT - 1),==,MIN_DNS_TTL_AT_EXIT);
+  tt_int_op(dns_clip_ttl(ttl_mid),==,MAX_DNS_TTL_AT_EXIT);
+  tt_int_op(dns_clip_ttl(MAX_DNS_TTL_AT_EXIT + 1),==,MAX_DNS_TTL_AT_EXIT);
 
   done:
   return;
@@ -749,7 +730,6 @@ NS(test_main)(void *arg)
 
 struct testcase_t dns_tests[] = {
    TEST_CASE(clip_ttl),
-   TEST_CASE(expiry_ttl),
    TEST_CASE(resolve),
    TEST_CASE_ASPECT(resolve_impl, addr_is_ip_no_need_to_resolve),
    TEST_CASE_ASPECT(resolve_impl, non_exit),