Merge branch 'maint-0.4.7'

changes/ticket40674

@@ -0,0 +1,3 @@
+  o Major bugfixes (relay):
+    - Improve security of our DNS cache by randomly clipping the TTL value.
+      TROVE-2021-009. Fixes bug 40674; bugfix on 0.3.5.1-alpha.

src/core/or/connection_edge.c

@@ -102,6 +102,7 @@
 #include "feature/stats/predict_ports.h"
 #include "feature/stats/rephist.h"
 #include "lib/buf/buffers.h"
+#include "lib/crypt_ops/crypto_rand.h"
 #include "lib/crypt_ops/crypto_util.h"
 
 #include "core/or/cell_st.h"
@@ -484,6 +485,21 @@ clip_dns_ttl(uint32_t ttl)
     return MAX_DNS_TTL;
 }
 
+/** Given a TTL (in seconds), determine what TTL an exit relay should use by
+ * first clipping as usual and then adding some randomness which is sampled
+ * uniformly at random from [-FUZZY_DNS_TTL, FUZZY_DNS_TTL].  This facilitates
+ * fuzzy TTLs, which makes it harder to infer when a website was visited via
+ * side-channels like DNS (see "Website Fingerprinting with Website Oracles").
+ *
+ * Note that this can't underflow because FUZZY_DNS_TTL < MIN_DNS_TTL.
+ */
+uint32_t
+clip_dns_fuzzy_ttl(uint32_t ttl)
+{
+  return clip_dns_ttl(ttl) +
+    crypto_rand_uint(1 + 2*FUZZY_DNS_TTL) - FUZZY_DNS_TTL;
+}
+
 /** Send a relay end cell from stream <b>conn</b> down conn's circuit, and
  * remember that we've done so.  If this is not a client connection, set the
  * relay end cell's reason for closing as <b>reason</b>.
@@ -532,7 +548,7 @@ connection_edge_end(edge_connection_t *conn, uint8_t reason)
       memcpy(payload+1, tor_addr_to_in6_addr8(&conn->base_.addr), 16);
       addrlen = 16;
     }
-    set_uint32(payload+1+addrlen, htonl(clip_dns_ttl(conn->address_ttl)));
+    set_uint32(payload+1+addrlen, htonl(conn->address_ttl));
     payload_len += 4+addrlen;
   }
 
@@ -926,7 +942,7 @@ connected_cell_format_payload(uint8_t *payload_out,
     return -1;
   }
 
-  set_uint32(payload_out + connected_payload_len, htonl(clip_dns_ttl(ttl)));
+  set_uint32(payload_out + connected_payload_len, htonl(ttl));
   connected_payload_len += 4;
 
   tor_assert(connected_payload_len <= MAX_CONNECTED_CELL_PAYLOAD_LEN);

src/core/or/connection_edge.h

@@ -187,11 +187,9 @@ void connection_ap_warn_and_unmark_if_pending_circ(
                                              entry_connection_t *entry_conn,
                                              const char *where);
 
-/** Lowest value for DNS ttl that a server should give or a client should
- * believe. */
+/** Lowest value for DNS ttl clipping excluding the random addition. */
 #define MIN_DNS_TTL (5*60)
-/** Highest value for DNS ttl that a server should give or a client should
- * believe. */
+/** Highest value for DNS ttl clipping excluding the random addition. */
 #define MAX_DNS_TTL (60*60)
 /** How long do we keep DNS cache entries before purging them (regardless of
  * their TTL)? */
@@ -199,8 +197,16 @@ void connection_ap_warn_and_unmark_if_pending_circ(
 /** How long do we cache/tell clients to cache DNS records when no TTL is
  * known? */
 #define DEFAULT_DNS_TTL (30*60)
+/** How much should we +- each TTL to make it fuzzy with uniform sampling at
+ * exits?  The value 4 minutes was chosen so that the lowest possible clip is
+ * 60s.  Such low clips were used in the past for all TTLs due to a bug in Tor,
+ * see "The effect of DNS on Tor's Anonymity" by Greschbach et al.  In other
+ * words, sampling such low clips is unlikely to cause any breakage at exits.
+ */
+#define FUZZY_DNS_TTL (4*60)
 
 uint32_t clip_dns_ttl(uint32_t ttl);
+uint32_t clip_dns_fuzzy_ttl(uint32_t ttl);
 
 int connection_half_edge_is_valid_data(const smartlist_t *half_conns,
                                        streamid_t stream_id);

src/feature/relay/dns.c

@@ -512,7 +512,7 @@ send_resolved_cell,(edge_connection_t *conn, uint8_t answer_type,
   uint32_t ttl;
 
   buf[0] = answer_type;
-  ttl = clip_dns_ttl(conn->address_ttl);
+  ttl = conn->address_ttl;
 
   switch (answer_type)
     {
@@ -584,7 +584,7 @@ send_resolved_hostname_cell,(edge_connection_t *conn,
   size_t namelen = strlen(hostname);
 
   tor_assert(namelen < 256);
-  ttl = clip_dns_ttl(conn->address_ttl);
+  ttl = conn->address_ttl;
 
   buf[0] = RESOLVED_TYPE_HOSTNAME;
   buf[1] = (uint8_t)namelen;
@@ -1310,7 +1310,7 @@ make_pending_resolve_cached(cached_resolve_t *resolve)
         resolve->ttl_hostname < ttl)
       ttl = resolve->ttl_hostname;
 
-    set_expiry(new_resolve, time(NULL) + clip_dns_ttl(ttl));
+    set_expiry(new_resolve, time(NULL) + ttl);
   }
 
   assert_cache_ok();
@@ -1725,7 +1725,7 @@ evdns_callback(int result, char type, int count, int ttl, void *addresses,
   }
   if (result != DNS_ERR_SHUTDOWN)
     dns_found_answer(string_address, orig_query_type,
-                     result, &addr, hostname, ttl);
+                     result, &addr, hostname, clip_dns_fuzzy_ttl(ttl));
 
   /* The result can be changed within this function thus why we note the result
    * at the end. */

src/test/test_cell_formats.c

@@ -354,7 +354,7 @@ test_cfmt_connected_cells(void *arg)
   rh.length = connected_cell_format_payload(cell.payload+RELAY_HEADER_SIZE,
                                             &addr, 1024);
   tt_int_op(rh.length, OP_EQ, 8);
-  test_memeq_hex(cell.payload+RELAY_HEADER_SIZE, "1e28323c" "00000e10");
+  test_memeq_hex(cell.payload+RELAY_HEADER_SIZE, "1e28323c" "00000400");
 
   /* Try parsing it. */
   tor_addr_make_unspec(&addr);
@@ -362,7 +362,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, 3600); /* not 1024, since we clipped to 3600 */
+  tt_int_op(ttl, OP_EQ, 1024);
 
   /* Try an IPv6 address */
   memset(&rh, 0, sizeof(rh));

src/test/test_dns.c

@@ -90,6 +90,36 @@ test_dns_clip_ttl(void *arg)
   return;
 }
 
+static void
+test_dns_clip_fuzzy_ttl(void *arg)
+{
+  (void)arg;
+
+  /* Case 0: check that the fuzzy TTL constant is valid
+   */
+  tt_int_op(FUZZY_DNS_TTL, OP_LE, MIN_DNS_TTL);
+  tt_int_op(FUZZY_DNS_TTL, OP_LE, MAX_DNS_TTL);
+
+  /* Case 1: low clips
+   */
+  for (int i = 0; i < 1024; i++) {
+    int fuzzy_ttl = clip_dns_fuzzy_ttl(MIN_DNS_TTL - 1);
+    tt_int_op(fuzzy_ttl, OP_GE, MIN_DNS_TTL-FUZZY_DNS_TTL);
+    tt_int_op(fuzzy_ttl, OP_LE, MIN_DNS_TTL+FUZZY_DNS_TTL);
+  }
+
+  /* Case 2: high clips
+   */
+  for (int i = 0; i < 1024; i++) {
+    int fuzzy_ttl = clip_dns_fuzzy_ttl(MIN_DNS_TTL);
+    tt_int_op(fuzzy_ttl, OP_GE, MAX_DNS_TTL-FUZZY_DNS_TTL);
+    tt_int_op(fuzzy_ttl, OP_LE, MAX_DNS_TTL+FUZZY_DNS_TTL);
+  }
+
+  done:
+  return;
+}
+
 static int resolve_retval = 0;
 static int resolve_made_conn_pending = 0;
 static char *resolved_name = NULL;
@@ -779,6 +809,7 @@ struct testcase_t dns_tests[] = {
      TT_FORK, NULL, NULL },
 #endif
    { "clip_ttl", test_dns_clip_ttl, TT_FORK, NULL, NULL },
+   { "clip_fuzzy_ttl", test_dns_clip_fuzzy_ttl, TT_FORK, NULL, NULL },
    { "resolve", test_dns_resolve, TT_FORK, NULL, NULL },
    { "impl_addr_is_ip", test_dns_impl_addr_is_ip, TT_FORK, NULL, NULL },
    { "impl_non_exit", test_dns_impl_non_exit, TT_FORK, NULL, NULL },