From 2bd91dbd346d6dcaa718ad7b952264cbfb713db6 Mon Sep 17 00:00:00 2001 From: Arthur Edelstein Date: Tue, 5 Dec 2017 20:23:02 -0800 Subject: [PATCH 1/2] Don't consider a port "handled" by an isolated circuit. Previously, circuit_stream_is_being_handled incorrectly reported that (1) an exit port was "handled" by a circuit regardless of whether the circuit was already isolated in some way, and (2) that a stream could be "handled" by a circuit even if their isolation settings were incompatible. As a result of (1), in Tor Browser, circuit_get_unhandled_ports was reporting that all ports were handled even though all non-internal circuits had already been isolated by a SOCKS username+password. Therefore, circuit_predict_and_launch_new was declining to launch new exit circuits. Then, when the user visited a new site in Tor Browser, a stream with new SOCKS credentials would be initiated, and the stream would have to wait while a new circuit with those credentials could be built. That wait was making the time-to-first-byte longer than it needed to be. Now, clean, not-yet-isolated circuit(s) will be automatically launched ahead of time and be ready for use whenever a new stream with new SOCKS credentials (or other isolation criteria) is initiated. Fixes bug 18859. Thanks to Nick Mathewson for improvements. --- changes/bug18859 | 5 +++++ src/or/circuituse.c | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 changes/bug18859 diff --git a/changes/bug18859 b/changes/bug18859 new file mode 100644 index 000000000..993074079 --- /dev/null +++ b/changes/bug18859 @@ -0,0 +1,5 @@ + o Minor bugfixes (circuit prediction): + - Fix circuit_stream_is_being_handled so it correctly reports on circuits + with isolation settings. Ports must not be said to be "handled" by + already-isolated circuits, and a stream can only be handled by a circuit + if their isolation settings are compatible. Fixes bug 18859. diff --git a/src/or/circuituse.c b/src/or/circuituse.c index 9f9d3abf7..6378cb8ac 100644 --- a/src/or/circuituse.c +++ b/src/or/circuituse.c @@ -973,7 +973,7 @@ circuit_remove_handled_ports(smartlist_t *needed_ports) tor_assert(*port); if (circuit_stream_is_being_handled(NULL, *port, MIN_CIRCUITS_HANDLING_STREAM)) { -// log_debug(LD_CIRC,"Port %d is already being handled; removing.", port); + log_debug(LD_CIRC,"Port %d is already being handled; removing.", *port); smartlist_del(needed_ports, i--); tor_free(port); } else { @@ -1010,6 +1010,10 @@ circuit_stream_is_being_handled(entry_connection_t *conn, continue; if (origin_circ->unusable_for_new_conns) continue; + if (origin_circ->isolation_values_set && + (conn == NULL || + !connection_edge_compatible_with_circuit(conn, origin_circ))) + continue; exitnode = build_state_get_exit_node(build_state); if (exitnode && (!need_uptime || build_state->need_uptime)) { From 13049a9866311b30d43b6b8268e57b9631b591f3 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 7 Dec 2017 08:41:00 -0500 Subject: [PATCH 2/2] Rewrite 18859 changes file from user POV. --- changes/bug18859 | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/changes/bug18859 b/changes/bug18859 index 993074079..1fe5bc210 100644 --- a/changes/bug18859 +++ b/changes/bug18859 @@ -1,5 +1,7 @@ - o Minor bugfixes (circuit prediction): - - Fix circuit_stream_is_being_handled so it correctly reports on circuits - with isolation settings. Ports must not be said to be "handled" by - already-isolated circuits, and a stream can only be handled by a circuit - if their isolation settings are compatible. Fixes bug 18859. + o Major bugfixes (circuit prediction): + - Fix circuit prediction logic so that a client doesn't treat a stream as + being "handled" by a circuit if that circuit already has isolation + settings on it that might make it incompatible with the stream. This + change should make Tor clients more responsive by improving their + chances of having a pre-created circuit ready for use when a new client + request arrives. Fixes bug 18859; bugfix on 0.2.3.3-alpha.