[tor-bugs] #18859 [Core Tor/Tor]: A new SOCKS connection should use a pre-built circuit for its first stream

[Tor Bug Tracker & Wiki]

#18859: A new SOCKS connection should use a pre-built circuit for its first stream
-------------------------------------------------+-------------------------
 Reporter:  arthuredelstein                      |          Owner:
                                                 |  arthuredelstein
     Type:  defect                               |         Status:
                                                 |  needs_review
 Priority:  Medium                               |      Milestone:  Tor:
                                                 |  0.3.2.x-final
Component:  Core Tor/Tor                         |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  tbb-performance tor-client needs-    |  Actual Points:
  diagnosis needs-analysis performance review-   |
  group-27                                       |
Parent ID:                                       |         Points:
 Reviewer:  nickm                                |        Sponsor:
-------------------------------------------------+-------------------------
Changes (by arthuredelstein):

 * status:  needs_revision => needs_review


Comment:

 Replying to [comment:19 nickm, comment 19]:

 > 1) How far do you think it makes sense to backport this?

 Tor Browser stable is currently using 0.3.1.x, so I think that would be
 useful.

 > 2) What if we allowed circuits with isolation flags set, so long as the
 isolation flags were compatible with the stream we wanted to attach?  In
 other words,

 Thanks for the suggestion. I added that check, plus a NULL check, because
 `conn` is set to NULL when the code path looks like:
 {{{
 #0  circuit_stream_is_being_handled (conn=0x0, port=443, min=2) at
 src/or/circuituse.c:993
 #1  0x00000453de87e352 in circuit_remove_handled_ports
 (needed_ports=0x453e056b6e0) at src/or/circuituse.c:974
 #2  0x00000453de867cfb in circuit_get_unhandled_ports (now=1512599788) at
 src/or/circuitbuild.c:1674
 #3  0x00000453de867d44 in circuit_all_predicted_ports_handled
 (now=1512599788, need_uptime=0x7ffe6698b060,
     need_capacity=0x7ffe6698b064) at src/or/circuitbuild.c:1690
 #4  0x00000453de87e7cf in needs_exit_circuits (now=1512599788,
 needs_uptime=0x7ffe6698b060, needs_capacity=0x7ffe6698b064)
     at src/or/circuituse.c:1082
 #5  0x00000453de87ea8c in circuit_predict_and_launch_new () at
 src/or/circuituse.c:1183
 #6  0x00000453de87ec98 in circuit_build_needed_circs (now=1512599788) at
 src/or/circuituse.c:1268
 #7  0x00000453de7b0505 in run_scheduled_events (now=1512599788) at
 src/or/main.c:1443
 }}}

 Replying to [comment:20 nickm, comment 20]:
 > (Other code review notes, which I'm totally willing to clean up if you
 don't feel like it: We make a changes file for every user-visible change
 in Tor, and we try to base backportable changes on the oldest maint-*
 branch that they should be backported to.)

 OK! I've added a changes file (feel free to fix that up as needed) and I
 applied the patch to main-0.3.1:
 https://github.com/arthuredelstein/tor/commit/18859+1

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18859#comment:21>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online