[tor-bugs] #32190 [Core Tor/Tor]: Send a control port event when a stream enters the controller_wait state

[Tor Bug Tracker & Wiki]

#32190: Send a control port event when a stream enters the controller_wait state
--------------------------+------------------------
 Reporter:  irl           |          Owner:  (none)
     Type:  enhancement   |         Status:  new
 Priority:  Medium        |      Milestone:
Component:  Core Tor/Tor  |        Version:
 Severity:  Normal        |     Resolution:
 Keywords:                |  Actual Points:
Parent ID:                |         Points:
 Reviewer:                |        Sponsor:
--------------------------+------------------------

Comment (by arma):

 It's actually worse than that: trying to attach and checking for a 555
 error is risky: Tor will give you a 555 error if:
 {{{
   if (ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_CONTROLLER_WAIT &&
       ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_CONNECT_WAIT &&
       ENTRY_TO_CONN(ap_conn)->state != AP_CONN_STATE_RESOLVE_WAIT) {
     connection_write_str_to_buf(
                     "555 Connection is not managed by controller.\r\n",
 }}}
 but if it's in, say, CONNECT_WAIT because it already sent its begin cell,
 then doing an attachstream attempt will cause it to send an end cell,
 detach from that circuit, and try to obey you.

 So you really ought to only streamattach to circuits that you know are
 yours.

 I believe in irl's use case (exitmap scanner), if he listens for STREAM
 NEW events and checks PURPOSE=USER, he will get what he wants. He should
 just avoid trying to attachstream any streams with other purposes or
 states.

 That said, it can't hurt to add another event here. And it would actually
 be easy: there are only three places where we set {{{state =
 AP_CONN_STATE_CONTROLLER_WAIT}}}:

 * In handle_control_attachstream(), if you just tried to call attachstream
 on the stream and it was doing something else but it attached and became
 yours. I don't think we need to emit an event here, because the controller
 knows it just did that.

 * In connection_ap_rewrite_and_attach_if_allowed(), which receives NEW
 streams and intercepts them if options->LeaveStreamsUnattached. Easy to
 do.

 * In connection_ap_detach_retriable(), where a stream just timed out, and
 we're about to send it back into the "go find a circuit" logic. We also
 intercept them if options->LeaveStreamsUnattached.

 So we could pull the three lines that are duplicate code in these last two
 cases:
 {{{
     CONNECTION_AP_EXPECT_NONPENDING(conn);
     ENTRY_TO_CONN(conn)->state = AP_CONN_STATE_CONTROLLER_WAIT;
     circuit_detach_stream(TO_CIRCUIT(circ),ENTRY_TO_EDGE_CONN(conn));
 }}}
 and make a new function out of them, and add a fourth line which is an
 event.

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