r11938@Kushana: nickm | 2007-01-11 11:02:28 -0500

 Check addresses for rfc953-saneness at exit too, and give a PROTOCOL_WARN when they fail.  Also provide a mechanism to override this, so blossom can have its @@##$$^.whatever.exit hostnames if it wants.


svn:r9336

7 files changed, 47 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index 93f6284cb8..12873fd794 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,10 @@
 Changes in version 0.1.2.7-alpha - 2007-??-??
+
+ o Minor features:
+    - Check for addresses with invalid characters at the exit as well as at
+      the client, and warn less verbosely when they fail.  You can override
+      this by setting ServerDNSAllowNonRFC953Addresses to 1.
+
  o Major bugfixes:
     - Fix a crash bug in the presence of DNS hijacking  (reported by Andrew
       Del Vecchio).
diff --git a/doc/tor.1.in b/doc/tor.1.in
index 21fd64be8a..18e6dfd871 100644
--- a/doc/tor.1.in
+++ b/doc/tor.1.in
@@ -501,7 +501,7 @@ When a controller asks for a virtual (unused) address with the
 .LP
 .TP
 \fBAllowNonRFC953Hostnames \fR\fB0\fR|\fB1\fR\fP
-When this option is enabled, Tor blocks hostnames containing illegal
+When this option is disabled, Tor blocks hostnames containing illegal
 characters (like @ and :) rather than sending them to an exit node to be
 resolved.  This helps trap accidental attempts to resolve URLs and so on.
 (Default: 0)
@@ -717,6 +717,12 @@ addresses aren't getting redirected.  If they are, then our DNS is
 completely useless, and we'll reset our exit policy to "reject *:*".
 (Defaults to "www.google.com, www.mit.edu, www.yahoo.com,
 www.slashdot.org".)
+\fBServerDNSAllowNonRFC953Hostnames \fR\fB0\fR|\fB1\fR\fP
+When this option is disabled, Tor does not try to resolve hostnames
+containing illegal characters (like @ and :) rather than sending them to an
+exit node to be resolved.  This helps trap accidental attempts to resolve
+URLs and so on.
+(Default: 0)
 
 .SH DIRECTORY SERVER OPTIONS
 .PP
diff --git a/src/or/config.c b/src/or/config.c
index 2378aa2946..70b981bd76 100644
--- a/src/or/config.c
+++ b/src/or/config.c
@@ -230,6 +230,8 @@ static config_var_t _option_vars[] = {
   VAR("RunTesting",          BOOL,     RunTesting,           "0"),
   VAR("SafeLogging",         BOOL,     SafeLogging,          "1"),
   VAR("SafeSocks",           BOOL,     SafeSocks,            "0"),
+  VAR("ServerDNSAllowNonRFC953Hostnames", BOOL,
+                                         ServerDNSAllowNonRFC953Hostnames, "0"),
   VAR("ServerDNSDetectHijacking",BOOL,   ServerDNSDetectHijacking,"1"),
   VAR("ServerDNSResolvConfFile", STRING, ServerDNSResolvConfFile, NULL),
   VAR("ServerDNSSearchDomains",  BOOL,   ServerDNSSearchDomains,  "0"),
@@ -3116,7 +3118,7 @@ config_register_addressmaps(or_options_t *options)
     if (smartlist_len(elts) >= 2) {
       from = smartlist_get(elts,0);
       to = smartlist_get(elts,1);
-      if (address_is_invalid_destination(to)) {
+      if (address_is_invalid_destination(to, 1)) {
         log_warn(LD_CONFIG,
                  "Skipping invalid argument '%s' to MapAddress", to);
       } else {
diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c
index e183512ecf..d96aa5d232 100644
--- a/src/or/connection_edge.c
+++ b/src/or/connection_edge.c
@@ -1082,14 +1082,21 @@ addressmap_register_virtual_address(int type, char *new_address)
   return *addrp;
 }
 
-/** Return 1 if <b>address</b> has funny characters in it like
- * colons. Return 0 if it's fine.
+/** Return 1 if <b>address</b> has funny characters in it like colons. Return
+ * 0 if it's fine, or if we're configured to allow it anyway.  <b>client</b>
+ * should be true if we're using this address as a client; false if we're
+ * using it as a server.
  */
 int
-address_is_invalid_destination(const char *address)
+address_is_invalid_destination(const char *address, int client)
 {
-  if (get_options()->AllowNonRFC953Hostnames)
-    return 0;
+  if (client) {
+    if (get_options()->AllowNonRFC953Hostnames)
+      return 0;
+  } else {
+    if (get_options()->ServerDNSAllowNonRFC953Hostnames)
+      return 0;
+  }
 
   while (*address) {
     if (TOR_ISALNUM(*address) ||
@@ -1234,7 +1241,7 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
   if (addresstype != ONION_HOSTNAME) {
     /* not a hidden-service request (i.e. normal or .exit) */
 
-    if (address_is_invalid_destination(socks->address)) {
+    if (address_is_invalid_destination(socks->address, 1)) {
       log_warn(LD_APP,
                "Destination '%s' seems to be an invalid hostname. Failing.",
                safe_str(socks->address));
diff --git a/src/or/control.c b/src/or/control.c
index ea272b8686..1eca654bae 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -1350,7 +1350,7 @@ handle_control_mapaddress(control_connection_t *conn, uint32_t len,
       const char *to = smartlist_get(elts,1);
       size_t anslen = strlen(line)+512;
       char *ans = tor_malloc(anslen);
-      if (address_is_invalid_destination(to)) {
+      if (address_is_invalid_destination(to, 1)) {
         if (!v0) {
           tor_snprintf(ans, anslen,
             "512-syntax error: invalid address '%s'", to);
diff --git a/src/or/dns.c b/src/or/dns.c
index d5f5ef5829..7897075356 100644
--- a/src/or/dns.c
+++ b/src/or/dns.c
@@ -584,6 +584,20 @@ dns_resolve(edge_connection_t *exitconn, or_circuit_t *oncirc)
       send_resolved_cell(exitconn, oncirc, RESOLVED_TYPE_IPV4);
     return 1;
   }
+  if (address_is_invalid_destination(exitconn->_base.address, 0)) {
+    log(LOG_PROTOCOL_WARN, LD_EXIT,
+        "Rejecting invalid destination address %s",
+        escaped_safe_str(exitconn->_base.address));
+    if (is_resolve)
+      send_resolved_cell(exitconn, oncirc, RESOLVED_TYPE_ERROR);
+    /* XXXX012 send error in connect case? -NM */
+    circ = circuit_get_by_edge_conn(exitconn);
+    if (circ)
+      circuit_detach_stream(circ, exitconn);
+    if (!exitconn->_base.marked_for_close)
+      connection_free(TO_CONN(exitconn));
+    return -1;
+  }
 
   /* then take this opportunity to see if there are any expired
    * resolves in the hash table. */
diff --git a/src/or/or.h b/src/or/or.h
index 46d1f6c3d6..77d94f1b48 100644
--- a/src/or/or.h
+++ b/src/or/or.h
@@ -1671,6 +1671,8 @@ typedef struct {
                                * support BEGIN_DIR, when possible. */
   int AllowNonRFC953Hostnames; /**< If true, we allow connections to hostnames
                                 * with weird characters. */
+ /** If true, we try resolving hostnames with weird characters. */
+  int ServerDNSAllowNonRFC953Hostnames;
 } or_options_t;
 
 /** Persistent state for an onion router, as saved to disk. */
@@ -2094,7 +2096,7 @@ int connection_ap_detach_retriable(edge_connection_t *conn,
                                    int reason);
 int connection_ap_process_transparent(edge_connection_t *conn);
 
-int address_is_invalid_destination(const char *address);
+int address_is_invalid_destination(const char *address, int client);
 
 void addressmap_init(void);
 void addressmap_clean(time_t now);