From 85da676108f0de765301f961bc58aebd139a5564 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 25 Jan 2011 19:07:03 -0500 Subject: [PATCH 1/5] Fix double-mark bug when failing to init transparent connection Fixes part of bug 2279. Bugfix on 0.1.2.1-alpha. --- changes/bug2279 | 5 +++++ src/or/connection.c | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 changes/bug2279 diff --git a/changes/bug2279 b/changes/bug2279 new file mode 100644 index 0000000000..b796cda761 --- /dev/null +++ b/changes/bug2279 @@ -0,0 +1,5 @@ + 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. + diff --git a/src/or/connection.c b/src/or/connection.c index 8a21d81c58..55a9557ef6 100644 --- a/src/or/connection.c +++ b/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; From 411ec3c0f8cd4786233a3bc274cb2b766d4bfe7c Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 25 Jan 2011 20:39:44 -0500 Subject: [PATCH 2/5] Add client code to detect attempts to connect to 127.0.0.1 etc We detect and reject said attempts if there is no chosen exit node or circuit: connecting to a private addr via a randomly chosen exit node will usually fail (if all exits reject private addresses), is always ill-defined (you're not asking for any particular host or service), and usually an error (you've configured all requests to go over Tor when you really wanted to configure all _remote_ requests to go over Tor). This can also help detect forwarding loop requests. Found as part of bug2279. --- changes/bug2279 | 8 ++++++++ doc/spec/control-spec.txt | 6 +++++- src/or/connection.c | 2 ++ src/or/connection_edge.c | 21 +++++++++++++++++++++ src/or/or.h | 7 +++++++ src/or/reasons.c | 5 +++++ 6 files changed, 48 insertions(+), 1 deletion(-) diff --git a/changes/bug2279 b/changes/bug2279 index b796cda761..e0c23b3604 100644 --- a/changes/bug2279 +++ b/changes/bug2279 @@ -3,3 +3,11 @@ 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. + + diff --git a/doc/spec/control-spec.txt b/doc/spec/control-spec.txt index 255adf00a4..1096245571 100644 --- a/doc/spec/control-spec.txt +++ b/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 diff --git a/src/or/connection.c b/src/or/connection.c index 55a9557ef6..fd30ac8cb6 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -1205,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; } diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 73ed9fb5c3..a85943f69f 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1659,6 +1659,27 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn, connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); return -1; } + if (!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 */ diff --git a/src/or/or.h b/src/or/or.h index 22c8498b66..a3ec71a927 100644 --- a/src/or/or.h +++ b/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; diff --git a/src/or/reasons.c b/src/or/reasons.c index 1401552223..304ea9fcfa 100644 --- a/src/or/reasons.c +++ b/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; " From d92a415bedd5220be05f3556007bf29ef18bd2f5 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2011 11:35:24 -0500 Subject: [PATCH 3/5] Add an option to disable the block-private-addresses feature Suggested by rransom. Probably necessary for testing network mode. --- changes/bug2279 | 4 +++- src/or/config.c | 2 ++ src/or/connection_edge.c | 3 ++- src/or/or.h | 4 ++++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/changes/bug2279 b/changes/bug2279 index e0c23b3604..d31300978e 100644 --- a/changes/bug2279 +++ b/changes/bug2279 @@ -8,6 +8,8 @@ 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. + help to detect loops in transparent proxy configurations. You can + disable this feature by setting "ClientRejectInternalAddresses 0" + in your torrc. diff --git a/src/or/config.c b/src/or/config.c index 8c1205de47..5aca2256ff 100644 --- a/src/or/config.c +++ b/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"), diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index a85943f69f..47e9035e90 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1659,7 +1659,8 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn, connection_mark_unattached_ap(conn, END_STREAM_REASON_TORPROTOCOL); return -1; } - if (!conn->use_begindir && !conn->chosen_exit_name && !circ) { + 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)) { diff --git a/src/or/or.h b/src/or/or.h index a3ec71a927..752de219ef 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -2756,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. */ From 30111a3a01993c7591eee1720b8434cb1b25c5b1 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 26 Jan 2011 12:08:52 -0500 Subject: [PATCH 4/5] add documentation for ClientRejectInternalAddresses --- doc/tor.1.txt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index c8608eb845..685e788d5e 100644 --- a/doc/tor.1.txt +++ b/doc/tor.1.txt @@ -731,6 +731,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 fulfil 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, 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 @@ -1267,6 +1273,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 From e854e01d5713108cf28e99ef9bc15ea27cb30759 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 7 Feb 2011 12:40:43 -0500 Subject: [PATCH 5/5] Some cleanups to bug2279 messages/docs from rransom --- doc/tor.1.txt | 11 ++++++----- src/or/connection_edge.c | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/doc/tor.1.txt b/doc/tor.1.txt index 685e788d5e..8909d8280a 100644 --- a/doc/tor.1.txt +++ b/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 @@ -732,10 +733,10 @@ The following options are useful only for clients (that is, if turn it off unless you know what you're doing. (Default: 1). **ClientRejectInternalAddresses** **0**|**1**:: - If true, Tor does not try to fulfil requests to connect to an internal + 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, a controller - request). (Default: 1). + 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 diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 47e9035e90..f02479fd59 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1670,7 +1670,7 @@ connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn, 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 " + "address %s on a TransPort or NATDPort. Possible loop " "in your NAT rules?", safe_str_client(socks->address)); } else { log_warn(LD_NET,