From 462568674a2465e8eb9cbb1cda65fa3e736c99fd Mon Sep 17 00:00:00 2001 From: Robert Hogan Date: Fri, 6 Aug 2010 20:29:15 +0100 Subject: [PATCH] Ensure controller RESOLVE commands respect __LeaveStreamsUnattached https://trac.torproject.org/projects/tor/ticket/1525 "The codepath taken by the control port "RESOLVE" command to create a synthetic SOCKS resolve request isn't the same as the path taken by a real SOCKS request from 'tor-resolve'. This prevents controllers who set LeaveStreamsUnattached=1 from being able to attach RESOLVE streams to circuits of their choosing." Create a new function connection_ap_rewrite_and_attach_if_allowed() and call that when Tor needs to attach a stream to a circuit but needs to know if the controller permits it. No tests added. --- changes/bug1525 | 4 ++++ src/or/connection_edge.c | 37 ++++++++++++++++++++----------------- src/or/connection_edge.h | 3 +++ src/or/dnsserv.c | 22 ++++++++++++---------- 4 files changed, 39 insertions(+), 27 deletions(-) create mode 100644 changes/bug1525 diff --git a/changes/bug1525 b/changes/bug1525 new file mode 100644 index 0000000000..64f5b175d3 --- /dev/null +++ b/changes/bug1525 @@ -0,0 +1,4 @@ + o Minor bugfixes: + - Ensure DNS requests launched by "RESOLVE" commands from the controller + respect __LeaveStreamsUnattached. Bugfix on 0.2.2.14-alpha; + fixes bug 1525. diff --git a/src/or/connection_edge.c b/src/or/connection_edge.c index 7522368c56..365a7972e3 100644 --- a/src/or/connection_edge.c +++ b/src/or/connection_edge.c @@ -1408,6 +1408,23 @@ consider_plaintext_ports(edge_connection_t *conn, uint16_t port) * different one? */ #define TRACKHOSTEXITS_RETRIES 5 +/** Call connection_ap_handshake_rewrite_and_attach() unless a controller + * asked us to leave streams unattached. + */ +int +connection_ap_rewrite_and_attach_if_allowed(edge_connection_t *conn, + origin_circuit_t *circ, + crypt_path_t *cpath) +{ + or_options_t *options = get_options(); + + if (options->LeaveStreamsUnattached) { + conn->_base.state = AP_CONN_STATE_CONTROLLER_WAIT; + return 0; + } + return connection_ap_handshake_rewrite_and_attach(conn, circ, cpath); +} + /** Connection conn just finished its socks handshake, or the * controller asked us to take care of it. If circ is defined, * then that's where we'll want to attach it. Otherwise we have to @@ -1908,11 +1925,7 @@ connection_ap_handshake_process_socks(edge_connection_t *conn) else control_event_stream_status(conn, STREAM_EVENT_NEW_RESOLVE, 0); - if (options->LeaveStreamsUnattached) { - conn->_base.state = AP_CONN_STATE_CONTROLLER_WAIT; - return 0; - } - return connection_ap_handshake_rewrite_and_attach(conn, NULL, NULL); + return connection_ap_rewrite_and_attach_if_allowed(conn, NULL, NULL); } /** connection_init_accepted_conn() found a new trans AP conn. @@ -1926,7 +1939,6 @@ int connection_ap_process_transparent(edge_connection_t *conn) { socks_request_t *socks; - or_options_t *options = get_options(); tor_assert(conn); tor_assert(conn->_base.type == CONN_TYPE_AP); @@ -1950,11 +1962,7 @@ connection_ap_process_transparent(edge_connection_t *conn) control_event_stream_status(conn, STREAM_EVENT_NEW, 0); - if (options->LeaveStreamsUnattached) { - conn->_base.state = AP_CONN_STATE_CONTROLLER_WAIT; - return 0; - } - return connection_ap_handshake_rewrite_and_attach(conn, NULL, NULL); + return connection_ap_rewrite_and_attach_if_allowed(conn, NULL, NULL); } /** connection_edge_process_inbuf() found a conn in state natd_wait. See if @@ -1975,7 +1983,6 @@ connection_ap_process_natd(edge_connection_t *conn) size_t tlen = 30; int err, port_ok; socks_request_t *socks; - or_options_t *options = get_options(); tor_assert(conn); tor_assert(conn->_base.type == CONN_TYPE_AP); @@ -2031,13 +2038,9 @@ connection_ap_process_natd(edge_connection_t *conn) control_event_stream_status(conn, STREAM_EVENT_NEW, 0); - if (options->LeaveStreamsUnattached) { - conn->_base.state = AP_CONN_STATE_CONTROLLER_WAIT; - return 0; - } conn->_base.state = AP_CONN_STATE_CIRCUIT_WAIT; - return connection_ap_handshake_rewrite_and_attach(conn, NULL, NULL); + return connection_ap_rewrite_and_attach_if_allowed(conn, NULL, NULL); } /** Iterate over the two bytes of stream_id until we get one that is not diff --git a/src/or/connection_edge.h b/src/or/connection_edge.h index c3d6098c5a..85444503b8 100644 --- a/src/or/connection_edge.h +++ b/src/or/connection_edge.h @@ -79,6 +79,9 @@ void client_dns_set_addressmap(const char *address, uint32_t val, const char *addressmap_register_virtual_address(int type, char *new_address); void addressmap_get_mappings(smartlist_t *sl, time_t min_expires, time_t max_expires, int want_expiry); +int connection_ap_rewrite_and_attach_if_allowed(edge_connection_t *conn, + origin_circuit_t *circ, + crypt_path_t *cpath); int connection_ap_handshake_rewrite_and_attach(edge_connection_t *conn, origin_circuit_t *circ, crypt_path_t *cpath); diff --git a/src/or/dnsserv.c b/src/or/dnsserv.c index e231b655f4..ad4f4122bc 100644 --- a/src/or/dnsserv.c +++ b/src/or/dnsserv.c @@ -141,16 +141,17 @@ evdns_server_callback(struct evdns_server_request *req, void *_data) control_event_stream_status(conn, STREAM_EVENT_NEW, 0); - /* Now, throw the connection over to get rewritten (which will answer it - * immediately if it's in the cache, or completely bogus, or automapped), - * and then attached to a circuit. */ + /* Now, unless a controller asked us to leave streams unattached, + * throw the connection over to get rewritten (which will + * answer it immediately if it's in the cache, or completely bogus, or + * automapped), and then attached to a circuit. */ log_info(LD_APP, "Passing request for %s to rewrite_and_attach.", escaped_safe_str_client(q->name)); q_name = tor_strdup(q->name); /* q could be freed in rewrite_and_attach */ - connection_ap_handshake_rewrite_and_attach(conn, NULL, NULL); + connection_ap_rewrite_and_attach_if_allowed(conn, NULL, NULL); /* Now, the connection is marked if it was bad. */ - log_info(LD_APP, "Passed request for %s to rewrite_and_attach.", + log_info(LD_APP, "Passed request for %s to rewrite_and_attach_if_allowed.", escaped_safe_str_client(q_name)); tor_free(q_name); } @@ -186,16 +187,17 @@ dnsserv_launch_request(const char *name, int reverse) return -1; } - /* Now, throw the connection over to get rewritten (which will answer it - * immediately if it's in the cache, or completely bogus, or automapped), - * and then attached to a circuit. */ + /* Now, unless a controller asked us to leave streams unattached, + * throw the connection over to get rewritten (which will + * answer it immediately if it's in the cache, or completely bogus, or + * automapped), and then attached to a circuit. */ log_info(LD_APP, "Passing request for %s to rewrite_and_attach.", escaped_safe_str_client(name)); q_name = tor_strdup(name); /* q could be freed in rewrite_and_attach */ - connection_ap_handshake_rewrite_and_attach(conn, NULL, NULL); + connection_ap_rewrite_and_attach_if_allowed(conn, NULL, NULL); /* Now, the connection is marked if it was bad. */ - log_info(LD_APP, "Passed request for %s to rewrite_and_attach.", + log_info(LD_APP, "Passed request for %s to rewrite_and_attach_if_allowed.", escaped_safe_str_client(q_name)); tor_free(q_name); return 0;