Opened 3 weeks ago

Last modified 2 weeks ago

#32190 new enhancement

Send a control port event when a stream enters the controller_wait state

Reported by: irl Owned by:
Priority: Medium Milestone:
Component: Core Tor/Tor Version:
Severity: Normal Keywords: exitscanner
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Currently the only way to find out whether a stream was waiting for you to attach it is to try to attach it, and then if you get a 555 error back then you know it wasn't waiting for you.

The control protocol could be extended to let controllers know when a stream is ready for the controller to attach it.

Child Tickets

Change History (2)

comment:1 Changed 3 weeks ago 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 two 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;

and make a new function out of them, and add a third line which is an event.

Last edited 3 weeks ago by arma (previous) (diff)

comment:2 Changed 2 weeks ago by gaba

Keywords: exitscanner added
Note: See TracTickets for help on using tickets.