[tor-commits] [tor/master] Don't consider a port "handled" by an isolated circuit.

[nickm at torproject.org]

commit 2bd91dbd346d6dcaa718ad7b952264cbfb713db6
Author: Arthur Edelstein <arthuredelstein at gmail.com>
Date:   Tue Dec 5 20:23:02 2017 -0800

    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(-)

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)) {