Merge branch 'bug2279' into maint-0.2.2

changes/bug2279

@@ -0,0 +1,15 @@
+  o Minor bugfixes
+    - Avoid a double mark-for-free warning when failing to attach a
+      transparent proxy connection.  Fixes bug 2279.  Bugfix on
+      Tor 0.1.2.1 alpha.
+
+  o Minor features
+    - Detect attempts at the client side to open connections to private
+      IP addresses (like 127.0.0.1, 10.0.0.1, and so on) with a randomly
+      chosen exit node.  Attempts to do so are always ill-defined, generally
+      prevented by exit policies, and usually in error.  This will also
+      help to detect loops in transparent proxy configurations.  You can
+      disable this feature by setting "ClientRejectInternalAddresses 0"
+      in your torrc.
+
+

doc/spec/control-spec.txt

@@ -1070,7 +1070,8 @@
       Reason = "MISC" / "RESOLVEFAILED" / "CONNECTREFUSED" /
                "EXITPOLICY" / "DESTROY" / "DONE" / "TIMEOUT" /
                "NOROUTE" / "HIBERNATING" / "INTERNAL"/ "RESOURCELIMIT" /
-               "CONNRESET" / "TORPROTOCOL" / "NOTDIRECTORY" / "END"
+               "CONNRESET" / "TORPROTOCOL" / "NOTDIRECTORY" / "END" /
+               "PRIVATE_ADDR"
 
    The "REASON" field is provided only for FAILED, CLOSED, and DETACHED
    events, and only if extended events are enabled (see 3.19).  Clients MUST
@@ -1079,7 +1080,10 @@
 
       END          (We received a RELAY_END cell from the other side of this
                     stream.)
+      PRIVATE_ADDR (The client tried to connect to a private address like
+                    127.0.0.1 or 10.0.0.1 over Tor.)
       [XXXX document more. -NM]
+      
 
    The "REMOTE_REASON" field is provided only when we receive a RELAY_END
    cell, and only if extended events are enabled.  It contains the actual

doc/tor.1.txt

@@ -654,8 +654,9 @@ The following options are useful only for clients (that is, if
     can leak your location to attackers. (Default: 1)
 
 **VirtualAddrNetwork** __Address__/__bits__::
-    When a controller asks for a virtual (unused) address with the MAPADDRESS
-    command, Tor picks an unassigned address from this range. (Default:
+    When Tor needs to assign a virtual (unused) address because of a MAPADDRESS
+    command from the controller or the AutpmapHostsOnResolve feature, Tor
+    picks an unassigned address from this range. (Default:
     127.192.0.0/10) +
  +
     When providing proxy server service to a network of computers using a tool
@@ -731,6 +732,12 @@ The following options are useful only for clients (that is, if
     192.168.0.1). This option prevents certain browser-based attacks; don't
     turn it off unless you know what you're doing. (Default: 1).
 
+**ClientRejectInternalAddresses** **0**|**1**::
+    If true, Tor does not try to fulfill requests to connect to an internal
+    address (like 127.0.0.1 or 192.168.0.1) __unless a exit node is
+    specifically requested__ (for example, via a .exit hostname, or a
+    controller request).  (Default: 1).
+
 **DownloadExtraInfo** **0**|**1**::
     If true, Tor downloads and caches "extra-info" documents. These documents
     contain information about servers other than the information in their
@@ -1269,6 +1276,7 @@ The following options are used for running a testing Tor network.
        AuthDirMaxServersPerAddr 0
        AuthDirMaxServersPerAuthAddr 0
        ClientDNSRejectInternalAddresses 0
+       ClientRejectInternalAddresses 0
        ExitPolicyRejectPrivate 0
        V3AuthVotingInterval 5 minutes
        V3AuthVoteDelay 20 seconds

src/or/config.c

@@ -195,6 +195,7 @@ static config_var_t _option_vars[] = {
   V(CircuitStreamTimeout,        INTERVAL, "0"),
   V(CircuitPriorityHalflife,     DOUBLE,  "-100.0"), /*negative:'Use default'*/
   V(ClientDNSRejectInternalAddresses, BOOL,"1"),
+  V(ClientRejectInternalAddresses, BOOL,   "1"),
   V(ClientOnly,                  BOOL,     "0"),
   V(ConsensusParams,             STRING,   NULL),
   V(ConnLimit,                   UINT,     "1000"),
@@ -405,6 +406,7 @@ static config_var_t testing_tor_network_defaults[] = {
   V(AuthDirMaxServersPerAddr,    UINT,     "0"),
   V(AuthDirMaxServersPerAuthAddr,UINT,     "0"),
   V(ClientDNSRejectInternalAddresses, BOOL,"0"),
+  V(ClientRejectInternalAddresses, BOOL,   "0"),
   V(ExitPolicyRejectPrivate,     BOOL,     "0"),
   V(V3AuthVotingInterval,        INTERVAL, "5 minutes"),
   V(V3AuthVoteDelay,             INTERVAL, "20 seconds"),

src/or/connection.c

@@ -1178,7 +1178,8 @@ connection_handle_listener_read(connection_t *conn, int new_type)
   }
 
   if (connection_init_accepted_conn(newconn, conn->type) < 0) {
-    connection_mark_for_close(newconn);
+    if (! conn->marked_for_close)
+      connection_mark_for_close(newconn);
     return 0;
   }
   return 0;
@@ -1204,9 +1205,11 @@ connection_init_accepted_conn(connection_t *conn, uint8_t listener_type)
           conn->state = AP_CONN_STATE_SOCKS_WAIT;
           break;
         case CONN_TYPE_AP_TRANS_LISTENER:
+          TO_EDGE_CONN(conn)->is_transparent_ap = 1;
           conn->state = AP_CONN_STATE_CIRCUIT_WAIT;
           return connection_ap_process_transparent(TO_EDGE_CONN(conn));
         case CONN_TYPE_AP_NATD_LISTENER:
+          TO_EDGE_CONN(conn)->is_transparent_ap = 1;
           conn->state = AP_CONN_STATE_NATD_WAIT;
           break;
       }

src/or/connection_edge.c

@@ -1659,6 +1659,28 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn,
         connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL);
         return -1;
       }
+      if (options->ClientRejectInternalAddresses &&
+          !conn->use_begindir && !conn->chosen_exit_name && !circ) {
+        tor_addr_t addr;
+        if (tor_addr_from_str(&addr, socks->address) >= 0 &&
+            tor_addr_is_internal(&addr, 0)) {
+          /* If this is an explicit private address with no chosen exit node,
+           * then we really don't want to try to connect to it.  That's
+           * probably an error. */
+          if (conn->is_transparent_ap) {
+            log_warn(LD_NET,
+                     "Rejecting request for anonymous connection to private "
+                     "address %s on a TransPort or NATDPort.  Possible loop "
+                     "in your NAT rules?", safe_str_client(socks->address));
+          } else {
+            log_warn(LD_NET,
+                     "Rejecting SOCKS request for anonymous connection to "
+                     "private address %s", safe_str_client(socks->address));
+          }
+          connection_mark_unattached_ap(conn, END_STREAM_REASON_PRIVATE_ADDR);
+          return -1;
+        }
+      }
 
       if (!conn->use_begindir && !conn->chosen_exit_name && !circ) {
         /* see if we can find a suitable enclave exit */

src/or/or.h

@@ -583,6 +583,9 @@ typedef enum {
 /** This is a connection on the NATD port, and the destination IP:Port was
  * either ill-formed or out-of-range. */
 #define END_STREAM_REASON_INVALID_NATD_DEST 261
+/** The target address is in a private network (like 127.0.0.1 or 10.0.0.1);
+ * you don't want to do that over a randomly chosen exit */
+#define END_STREAM_REASON_PRIVATE_ADDR 262
 
 /** Bitwise-and this value with endreason to mask out all flags. */
 #define END_STREAM_REASON_MASK 511
@@ -1170,6 +1173,10 @@ typedef struct edge_connection_t {
    * zero, abandon the associated mapaddress. */
   unsigned int chosen_exit_retries:3;
 
+  /** True iff this is an AP connection that came from a transparent or
+   * NATd connection */
+  unsigned int is_transparent_ap:1;
+
   /** If this is a DNSPort connection, this field holds the pending DNS
    * request that we're going to try to answer.  */
   struct evdns_server_request *dns_server_request;
@@ -2749,6 +2756,10 @@ typedef struct {
    * Helps avoid some cross-site attacks. */
   int ClientDNSRejectInternalAddresses;
 
+  /** If true, do not accept any requests to connect to internal addresses
+   * over randomly chosen exits. */
+  int ClientRejectInternalAddresses;
+
   /** The length of time that we think a consensus should be fresh. */
   int V3AuthVotingInterval;
   /** The length of time we think it will take to distribute votes. */

src/or/reasons.c

@@ -40,6 +40,8 @@ stream_end_reason_to_control_string(int reason)
     case END_STREAM_REASON_NET_UNREACHABLE: return "NET_UNREACHABLE";
     case END_STREAM_REASON_SOCKSPROTOCOL: return "SOCKS_PROTOCOL";
 
+    case END_STREAM_REASON_PRIVATE_ADDR: return "PRIVATE_ADDR";
+
     default: return NULL;
   }
 }
@@ -125,6 +127,9 @@ stream_end_reason_to_socks5_response(int reason)
       return SOCKS5_NET_UNREACHABLE;
     case END_STREAM_REASON_SOCKSPROTOCOL:
       return SOCKS5_GENERAL_ERROR;
+    case END_STREAM_REASON_PRIVATE_ADDR:
+      return SOCKS5_GENERAL_ERROR;
+
     default:
       log_fn(LOG_PROTOCOL_WARN, LD_PROTOCOL,
              "Reason for ending (%d) not recognized; "