Merge remote-tracking branch 'dgoulet/ticket18693_029_01'

4 files changed, 210 insertions, 28 deletions

diff --git a/src/or/config.c b/src/or/config.c
index 98b32708c6..9c5514f1da 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -6054,6 +6054,8 @@ port_cfg_new(size_t namelen)
   tor_assert(namelen <= SIZE_T_CEILING - sizeof(port_cfg_t) - 1);
   port_cfg_t *cfg = tor_malloc_zero(sizeof(port_cfg_t) + namelen + 1);
   cfg->entry_cfg.ipv4_traffic = 1;
+  cfg->entry_cfg.dns_request = 1;
+  cfg->entry_cfg.onion_traffic = 1;
   cfg->entry_cfg.cache_ipv4_answers = 1;
   cfg->entry_cfg.prefer_ipv6_virtaddr = 1;
   return cfg;
@@ -6324,8 +6326,7 @@ parse_port_config(smartlist_t *out,
       tor_addr_make_unspec(&cfg->addr); /* Server ports default to 0.0.0.0 */
       cfg->server_cfg.no_listen = 1;
       cfg->server_cfg.bind_ipv4_only = 1;
-      cfg->entry_cfg.ipv4_traffic = 1;
-      cfg->entry_cfg.prefer_ipv6_virtaddr = 1;
+      /* cfg->entry_cfg defaults are already set by port_cfg_new */
       smartlist_add(out, cfg);
     }
 
@@ -6396,9 +6397,11 @@ parse_port_config(smartlist_t *out,
     char *addrport;
     uint16_t ptmp=0;
     int ok;
+    /* This must be kept in sync with port_cfg_new's defaults */
     int no_listen = 0, no_advertise = 0, all_addrs = 0,
       bind_ipv4_only = 0, bind_ipv6_only = 0,
-      ipv4_traffic = 1, ipv6_traffic = 0, prefer_ipv6 = 0,
+      ipv4_traffic = 1, ipv6_traffic = 0, prefer_ipv6 = 0, dns_request = 1,
+      onion_traffic = 1,
       cache_ipv4 = 1, use_cached_ipv4 = 0,
       cache_ipv6 = 0, use_cached_ipv6 = 0,
       prefer_ipv6_automap = 1, world_writable = 0, group_writable = 0,
@@ -6584,6 +6587,24 @@ parse_port_config(smartlist_t *out,
           } else if (!strcasecmp(elt, "PreferIPv6")) {
             prefer_ipv6 = ! no;
             continue;
+          } else if (!strcasecmp(elt, "DNSRequest")) {
+            dns_request = ! no;
+            continue;
+          } else if (!strcasecmp(elt, "OnionTraffic")) {
+            onion_traffic = ! no;
+            continue;
+          } else if (!strcasecmp(elt, "OnionTrafficOnly")) {
+            /* Only connect to .onion addresses.  Equivalent to
+             * NoDNSRequest, NoIPv4Traffic, NoIPv6Traffic. The option
+             * NoOnionTrafficOnly is not supported, it's too confusing. */
+            if (no) {
+              log_warn(LD_CONFIG, "Unsupported %sPort option 'No%s'. Use "
+                       "DNSRequest, IPv4Traffic, and/or IPv6Traffic instead.",
+                       portname, escaped(elt));
+            } else {
+              ipv4_traffic = ipv6_traffic = dns_request = 0;
+            }
+            continue;
           }
         }
         if (!strcasecmp(elt, "CacheIPv4DNS")) {
@@ -6652,9 +6673,24 @@ parse_port_config(smartlist_t *out,
     else
       got_zero_port = 1;
 
-    if (ipv4_traffic == 0 && ipv6_traffic == 0) {
-      log_warn(LD_CONFIG, "You have a %sPort entry with both IPv4 and "
-               "IPv6 disabled; that won't work.", portname);
+    if (dns_request == 0 && listener_type == CONN_TYPE_AP_DNS_LISTENER) {
+      log_warn(LD_CONFIG, "You have a %sPort entry with DNS disabled; that "
+               "won't work.", portname);
+      goto err;
+    }
+
+    if (ipv4_traffic == 0 && ipv6_traffic == 0 && onion_traffic == 0
+        && listener_type != CONN_TYPE_AP_DNS_LISTENER) {
+      log_warn(LD_CONFIG, "You have a %sPort entry with all of IPv4 and "
+               "IPv6 and .onion disabled; that won't work.", portname);
+      goto err;
+    }
+
+    if (dns_request == 1 && ipv4_traffic == 0 && ipv6_traffic == 0
+        && listener_type != CONN_TYPE_AP_DNS_LISTENER) {
+      log_warn(LD_CONFIG, "You have a %sPort entry with DNSRequest enabled, "
+               "but IPv4 and IPv6 disabled; DNS-based sites won't work.",
+               portname);
       goto err;
     }
 
@@ -6698,6 +6734,8 @@ parse_port_config(smartlist_t *out,
       cfg->entry_cfg.ipv4_traffic = ipv4_traffic;
       cfg->entry_cfg.ipv6_traffic = ipv6_traffic;
       cfg->entry_cfg.prefer_ipv6 = prefer_ipv6;
+      cfg->entry_cfg.dns_request = dns_request;
+      cfg->entry_cfg.onion_traffic = onion_traffic;
       cfg->entry_cfg.cache_ipv4_answers = cache_ipv4;
       cfg->entry_cfg.cache_ipv6_answers = cache_ipv6;
       cfg->entry_cfg.use_cached_ipv4_answers = use_cached_ipv4;
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index dc6b0930ca..4d615e8e2b 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -1227,7 +1227,7 @@ connection_ap_handshake_rewrite(entry_connection_t *conn,
     }
 
     /* Hang on, did we find an answer saying that this is a reverse lookup for
-     * an internal address?  If so, we should reject it if we're condigured to
+     * an internal address?  If so, we should reject it if we're configured to
      * do so. */
     if (options->ClientDNSRejectInternalAddresses) {
       /* Don't let people try to do a reverse lookup on 10.0.0.1. */
@@ -1466,14 +1466,61 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,
     /* If we're running in Tor2webMode, we don't allow anything BUT .onion
      * addresses. */
     if (options->Tor2webMode) {
-      log_warn(LD_APP, "Refusing to connect to non-hidden-service hostname %s "
-               "because tor2web mode is enabled.",
+      log_warn(LD_APP, "Refusing to connect to non-hidden-service hostname "
+               "or IP address %s because tor2web mode is enabled.",
                safe_str_client(socks->address));
       connection_mark_unattached_ap(conn, END_STREAM_REASON_ENTRYPOLICY);
       return -1;
     }
 #endif
 
+    /* socks->address is a non-onion hostname or IP address.
+     * If we can't do any non-onion requests, refuse the connection.
+     * If we have a hostname but can't do DNS, refuse the connection.
+     * If we have an IP address, but we can't use that address family,
+     * refuse the connection.
+     *
+     * If we can do DNS requests, and we can use at least one address family,
+     * then we have to resolve the address first. Then we'll know if it
+     * resolves to a usable address family. */
+
+    /* First, check if all non-onion traffic is disabled */
+    if (!conn->entry_cfg.dns_request && !conn->entry_cfg.ipv4_traffic
+        && !conn->entry_cfg.ipv6_traffic) {
+        log_warn(LD_APP, "Refusing to connect to non-hidden-service hostname "
+                 "or IP address %s because Port has OnionTrafficOnly set (or "
+                 "NoDNSRequest, NoIPv4Traffic, and NoIPv6Traffic).",
+                 safe_str_client(socks->address));
+        connection_mark_unattached_ap(conn, END_STREAM_REASON_ENTRYPOLICY);
+        return -1;
+    }
+
+    /* Then check if we have a hostname or IP address, and whether DNS or
+     * the IP address family are permitted */
+    tor_addr_t dummy_addr;
+    int socks_family = tor_addr_parse(&dummy_addr, socks->address);
+    /* family will be -1 for a non-onion hostname that's not an IP */
+    if (socks_family == -1 && !conn->entry_cfg.dns_request) {
+      log_warn(LD_APP, "Refusing to connect to hostname %s "
+               "because Port has NoDNSRequest set.",
+               safe_str_client(socks->address));
+      connection_mark_unattached_ap(conn, END_STREAM_REASON_ENTRYPOLICY);
+      return -1;
+    } else if (socks_family == AF_INET && !conn->entry_cfg.ipv4_traffic) {
+      log_warn(LD_APP, "Refusing to connect to IPv4 address %s because "
+               "Port has NoIPv4Traffic set.",
+               safe_str_client(socks->address));
+      connection_mark_unattached_ap(conn, END_STREAM_REASON_ENTRYPOLICY);
+      return -1;
+    } else if (socks_family == AF_INET6 && !conn->entry_cfg.ipv6_traffic) {
+      log_warn(LD_APP, "Refusing to connect to IPv6 address %s because "
+               "Port has NoIPv6Traffic set.",
+               safe_str_client(socks->address));
+      connection_mark_unattached_ap(conn, END_STREAM_REASON_ENTRYPOLICY);
+      return -1;
+    }
+    /* No else, we've covered all possible returned value. */
+
     /* See if this is a hostname lookup that we can answer immediately.
      * (For example, an attempt to look up the IP address for an IP address.)
      */
@@ -1661,6 +1708,14 @@ connection_ap_handshake_rewrite_and_attach(entry_connection_t *conn,
     /* If we get here, it's a request for a .onion address! */
     tor_assert(!automap);
 
+    /* If .onion address requests are disabled, refuse the request */
+    if (!conn->entry_cfg.onion_traffic) {
+      log_warn(LD_APP, "Onion address %s requested from a port with .onion "
+                       "disabled", safe_str_client(socks->address));
+      connection_mark_unattached_ap(conn, END_STREAM_REASON_ENTRYPOLICY);
+      return -1;
+    }
+
     /* Check whether it's RESOLVE or RESOLVE_PTR.  We don't handle those
      * for hidden service addresses. */
     if (SOCKS_COMMAND_IS_RESOLVE(socks->command)) {
diff --git a/src/or/or.h b/src/or/or.h
index 34089ad994..5b9b007ac1 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1151,6 +1151,8 @@ typedef struct entry_port_cfg_t {
   unsigned int ipv4_traffic : 1;
   unsigned int ipv6_traffic : 1;
   unsigned int prefer_ipv6 : 1;
+  unsigned int dns_request : 1;
+  unsigned int onion_traffic : 1;
 
   /** For a socks listener: should we cache IPv4/IPv6 DNS information that
    * exit nodes tell us?
diff --git a/src/test/test_config.c b/src/test/test_config.c
index 1e6d2adf57..80a172789b 100644
--- a/src/test/test_config.c
+++ b/src/test/test_config.c
@@ -3954,7 +3954,7 @@ test_config_parse_port_config__ports__ports_given(void *data)
   tt_int_op(ret, OP_EQ, -1);
 
   // Test error when encounters a unix domain specification but the listener
-  // doesnt support domain sockets
+  // doesn't support domain sockets
   config_port_valid = mock_config_line("DNSPort", "unix:/tmp/foo/bar");
   ret = parse_port_config(NULL, config_port_valid, NULL, "DNS",
                           CONN_TYPE_AP_DNS_LISTENER, NULL, 0, 0);
@@ -3963,7 +3963,7 @@ test_config_parse_port_config__ports__ports_given(void *data)
   // Test valid unix domain
   SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
   smartlist_clear(slout);
-  ret = parse_port_config(slout, config_port_valid, NULL, "DNS",
+  ret = parse_port_config(slout, config_port_valid, NULL, "SOCKS",
                           CONN_TYPE_AP_LISTENER, NULL, 0, 0);
 #ifdef _WIN32
   tt_int_op(ret, OP_EQ, -1);
@@ -3974,26 +3974,114 @@ test_config_parse_port_config__ports__ports_given(void *data)
   tt_int_op(port_cfg->port, OP_EQ, 0);
   tt_int_op(port_cfg->is_unix_addr, OP_EQ, 1);
   tt_str_op(port_cfg->unix_addr, OP_EQ, "/tmp/foo/bar");
+  /* Test entry port defaults as initialised in parse_port_config */
+  tt_int_op(port_cfg->entry_cfg.dns_request, OP_EQ, 1);
+  tt_int_op(port_cfg->entry_cfg.ipv4_traffic, OP_EQ, 1);
+  tt_int_op(port_cfg->entry_cfg.onion_traffic, OP_EQ, 1);
+  tt_int_op(port_cfg->entry_cfg.cache_ipv4_answers, OP_EQ, 1);
+  tt_int_op(port_cfg->entry_cfg.prefer_ipv6_virtaddr, OP_EQ, 1);
 #endif
 
-  // Test failure if we have no ipv4 and no ipv6 (for unix domain sockets,
-  // this makes no sense - it should be fixed)
+  // Test failure if we have no ipv4 and no ipv6 and no onion (DNS only)
+  config_free_lines(config_port_invalid); config_port_invalid = NULL;
+  config_port_invalid = mock_config_line("SOCKSPort",
+                                         "unix:/tmp/foo/bar NoIPv4Traffic "
+                                         "NoOnionTraffic");
+  ret = parse_port_config(NULL, config_port_invalid, NULL, "SOCKS",
+                          CONN_TYPE_AP_LISTENER, NULL, 0,
+                          CL_PORT_TAKES_HOSTNAMES);
+  tt_int_op(ret, OP_EQ, -1);
+
+  // Test failure if we have no DNS and we're a DNSPort
   config_free_lines(config_port_invalid); config_port_invalid = NULL;
   config_port_invalid = mock_config_line("DNSPort",
-                                         "unix:/tmp/foo/bar NoIPv4Traffic");
+                                         "127.0.0.1:80 NoDNSRequest");
   ret = parse_port_config(NULL, config_port_invalid, NULL, "DNS",
+                          CONN_TYPE_AP_DNS_LISTENER, NULL, 0,
+                          CL_PORT_TAKES_HOSTNAMES);
+  tt_int_op(ret, OP_EQ, -1);
+
+  // If we're a DNSPort, DNS only is ok
+  // Use a port because DNSPort doesn't support sockets
+  config_free_lines(config_port_valid); config_port_valid = NULL;
+  SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
+  smartlist_clear(slout);
+  config_port_valid = mock_config_line("DNSPort", "127.0.0.1:80 "
+                                       "NoIPv4Traffic NoOnionTraffic");
+  ret = parse_port_config(slout, config_port_valid, NULL, "DNS",
+                          CONN_TYPE_AP_DNS_LISTENER, NULL, 0,
+                          CL_PORT_TAKES_HOSTNAMES);
+#ifdef _WIN32
+  tt_int_op(ret, OP_EQ, -1);
+#else
+  tt_int_op(ret, OP_EQ, 0);
+  tt_int_op(smartlist_len(slout), OP_EQ, 1);
+  port_cfg = (port_cfg_t *)smartlist_get(slout, 0);
+  tt_int_op(port_cfg->entry_cfg.dns_request, OP_EQ, 1);
+  tt_int_op(port_cfg->entry_cfg.ipv4_traffic, OP_EQ, 0);
+  tt_int_op(port_cfg->entry_cfg.ipv6_traffic, OP_EQ, 0);
+  tt_int_op(port_cfg->entry_cfg.onion_traffic, OP_EQ, 0);
+#endif
+
+  // Test failure if we have DNS but no ipv4 and no ipv6
+  config_free_lines(config_port_invalid); config_port_invalid = NULL;
+  config_port_invalid = mock_config_line("SOCKSPort",
+                                         "unix:/tmp/foo/bar NoIPv4Traffic");
+  ret = parse_port_config(NULL, config_port_invalid, NULL, "SOCKS",
                           CONN_TYPE_AP_LISTENER, NULL, 0,
                           CL_PORT_TAKES_HOSTNAMES);
   tt_int_op(ret, OP_EQ, -1);
 
-  // Test success with no ipv4 but take ipv6 (for unix domain sockets, this
-  // makes no sense - it should be fixed)
+  // Test success with no DNS, no ipv4, no ipv6 (only onion, using separate
+  // options)
   config_free_lines(config_port_valid); config_port_valid = NULL;
   SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
   smartlist_clear(slout);
-  config_port_valid = mock_config_line("DNSPort", "unix:/tmp/foo/bar "
+  config_port_valid = mock_config_line("SOCKSPort", "unix:/tmp/foo/bar "
+                                       "NoDNSRequest NoIPv4Traffic");
+  ret = parse_port_config(slout, config_port_valid, NULL, "SOCKS",
+                          CONN_TYPE_AP_LISTENER, NULL, 0,
+                          CL_PORT_TAKES_HOSTNAMES);
+#ifdef _WIN32
+  tt_int_op(ret, OP_EQ, -1);
+#else
+  tt_int_op(ret, OP_EQ, 0);
+  tt_int_op(smartlist_len(slout), OP_EQ, 1);
+  port_cfg = (port_cfg_t *)smartlist_get(slout, 0);
+  tt_int_op(port_cfg->entry_cfg.dns_request, OP_EQ, 0);
+  tt_int_op(port_cfg->entry_cfg.ipv4_traffic, OP_EQ, 0);
+  tt_int_op(port_cfg->entry_cfg.ipv6_traffic, OP_EQ, 0);
+  tt_int_op(port_cfg->entry_cfg.onion_traffic, OP_EQ, 1);
+#endif
+
+  // Test success with OnionTrafficOnly (no DNS, no ipv4, no ipv6)
+  config_free_lines(config_port_valid); config_port_valid = NULL;
+  SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
+  smartlist_clear(slout);
+  config_port_valid = mock_config_line("SOCKSPort", "unix:/tmp/foo/bar "
+                                       "OnionTrafficOnly");
+  ret = parse_port_config(slout, config_port_valid, NULL, "SOCKS",
+                          CONN_TYPE_AP_LISTENER, NULL, 0,
+                          CL_PORT_TAKES_HOSTNAMES);
+#ifdef _WIN32
+  tt_int_op(ret, OP_EQ, -1);
+#else
+  tt_int_op(ret, OP_EQ, 0);
+  tt_int_op(smartlist_len(slout), OP_EQ, 1);
+  port_cfg = (port_cfg_t *)smartlist_get(slout, 0);
+  tt_int_op(port_cfg->entry_cfg.dns_request, OP_EQ, 0);
+  tt_int_op(port_cfg->entry_cfg.ipv4_traffic, OP_EQ, 0);
+  tt_int_op(port_cfg->entry_cfg.ipv6_traffic, OP_EQ, 0);
+  tt_int_op(port_cfg->entry_cfg.onion_traffic, OP_EQ, 1);
+#endif
+
+  // Test success with no ipv4 but take ipv6
+  config_free_lines(config_port_valid); config_port_valid = NULL;
+  SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
+  smartlist_clear(slout);
+  config_port_valid = mock_config_line("SOCKSPort", "unix:/tmp/foo/bar "
                                        "NoIPv4Traffic IPv6Traffic");
-  ret = parse_port_config(slout, config_port_valid, NULL, "DNS",
+  ret = parse_port_config(slout, config_port_valid, NULL, "SOCKS",
                           CONN_TYPE_AP_LISTENER, NULL, 0,
                           CL_PORT_TAKES_HOSTNAMES);
 #ifdef _WIN32
@@ -4006,14 +4094,13 @@ test_config_parse_port_config__ports__ports_given(void *data)
   tt_int_op(port_cfg->entry_cfg.ipv6_traffic, OP_EQ, 1);
 #endif
 
-  // Test success with both ipv4 and ipv6 (for unix domain sockets,
-  // this makes no sense - it should be fixed)
+  // Test success with both ipv4 and ipv6
   config_free_lines(config_port_valid); config_port_valid = NULL;
   SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
   smartlist_clear(slout);
-  config_port_valid = mock_config_line("DNSPort", "unix:/tmp/foo/bar "
+  config_port_valid = mock_config_line("SOCKSPort", "unix:/tmp/foo/bar "
                                        "IPv4Traffic IPv6Traffic");
-  ret = parse_port_config(slout, config_port_valid, NULL, "DNS",
+  ret = parse_port_config(slout, config_port_valid, NULL, "SOCKS",
                           CONN_TYPE_AP_LISTENER, NULL, 0,
                           CL_PORT_TAKES_HOSTNAMES);
 #ifdef _WIN32
@@ -4155,8 +4242,8 @@ test_config_parse_port_config__ports__ports_given(void *data)
   config_free_lines(config_port_valid); config_port_valid = NULL;
   SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
   smartlist_clear(slout);
-  config_port_valid = mock_config_line("DNSPort", "42 IPv6Traffic PreferIPv6");
-  ret = parse_port_config(slout, config_port_valid, NULL, "DNS",
+  config_port_valid = mock_config_line("SOCKSPort", "42 IPv6Traffic PreferIPv6");
+  ret = parse_port_config(slout, config_port_valid, NULL, "SOCKS",
                           CONN_TYPE_AP_LISTENER, "127.0.0.42", 0,
                           CL_PORT_TAKES_HOSTNAMES);
   tt_int_op(ret, OP_EQ, 0);
@@ -4294,7 +4381,7 @@ test_config_parse_port_config__ports__ports_given(void *data)
   // Test success with warn non-local control
   SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
   smartlist_clear(slout);
-  ret = parse_port_config(slout, config_port_valid, NULL, "DNS",
+  ret = parse_port_config(slout, config_port_valid, NULL, "Control",
                           CONN_TYPE_CONTROL_LISTENER, "127.0.0.42", 0,
                           CL_PORT_WARN_NONLOCAL);
   tt_int_op(ret, OP_EQ, 0);
@@ -4302,7 +4389,7 @@ test_config_parse_port_config__ports__ports_given(void *data)
   // Test success with warn non-local listener
   SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
   smartlist_clear(slout);
-  ret = parse_port_config(slout, config_port_valid, NULL, "DNS",
+  ret = parse_port_config(slout, config_port_valid, NULL, "ExtOR",
                           CONN_TYPE_EXT_OR_LISTENER, "127.0.0.42", 0,
                           CL_PORT_WARN_NONLOCAL);
   tt_int_op(ret, OP_EQ, 0);
@@ -4474,8 +4561,8 @@ test_config_parse_port_config__ports__ports_given(void *data)
   config_free_lines(config_port_valid); config_port_valid = NULL;
   SMARTLIST_FOREACH(slout,port_cfg_t *,pf,port_cfg_free(pf));
   smartlist_clear(slout);
-  config_port_valid = mock_config_line("DNSPort", "unix:/tmp/somewhere");
-  ret = parse_port_config(slout, config_port_valid, NULL, "DNS",
+  config_port_valid = mock_config_line("SOCKSPort", "unix:/tmp/somewhere");
+  ret = parse_port_config(slout, config_port_valid, NULL, "SOCKS",
                           CONN_TYPE_AP_LISTENER, "127.0.0.46", 0,
                           CL_PORT_DFLT_GROUP_WRITABLE);
 #ifdef _WIN32