Opened 8 months ago

Closed 8 months ago

#23900 closed enhancement (implemented)

Let programs call tor_main with a preconstructed control socket

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-mobile, review-group-24, s8-api
Cc: darkk, brade, mcs, sbs, hellais, mtigas Actual Points:
Parent ID: #23684 Points:
Reviewer: Sponsor: Sponsor8

Description

At the ooni/tor/embedding meeting, we mentioned the idea of having a way for programs that want to call tor_main to pass a control socket to tor_main, so that they don't need to have tor listening on a control port at all.

Child Tickets

Change History (13)

comment:1 Changed 8 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: 0.3.3.x-final
Owner: set to nickm
Status: newaccepted

comment:2 Changed 8 months ago by nickm

Status: acceptedneeds_review

Specified in my torspec branch owning_control_fd.

Implementation in my tor branch owning_control_fd.

comment:3 Changed 8 months ago by nickm

Keywords: review-group-24 added

review-group-24 is now open.

comment:4 Changed 8 months ago by nickm

Sponsor: Sponsor8

comment:5 Changed 8 months ago by ahf

Status: needs_reviewmerge_ready

5bcd8dc5c482da4da71402cd06b0ecc709f05493, 4eb5753bd29b24fd5a523499add35a6214293cd9, f0daaf8d60be8bfcfaa99e3a878cd90967a84bb0, and f1bf9bf8198fcfaf078fdc12eb2ad5adf1901d29 All looks good to me.

comment:6 Changed 8 months ago by nickm

Okay. I'll wait a little bit to see if the mobile people have any comments.

comment:7 Changed 8 months ago by ahf

Cc: mtigas added

Adding Mike here as well in case he wants to check out if the interface is useful for him.

comment:8 Changed 8 months ago by ahf

Keywords: s8-api added

comment:9 Changed 8 months ago by hellais

sbs and I looked at this.

The torspec and tor diffs look good.

One thing to be careful about though is that when the socket is closed (as per TAKEOWNERSHIP behavior), when you "shut down" Tor, you must not call exit().
This may be differ from how OwningControllerProcess behaves.

Moreover lack of windows support may be an issue for us (as that is also a target platform for Measurement Kit), but I guess for the time being we can avoid using this option there.

Also as mentioned in https://trac.torproject.org/projects/tor/ticket/23845#comment:12, it would be useful to document that for non windows platforms it's recommended to set this option when constructing a tor_main_configuration_t to pass to tor_run_main.

comment:10 Changed 8 months ago by sbs

I was also wondering:

  1. assuming that tor_socket_t is similar to evutil_socket_t, whether we can pass a socket on Windows to obtain the same effect
  1. if this function to set an owning socket is thread safe (I don't know enough on Tor internals to say whether it's obviously thread safe or not)

comment:11 in reply to:  9 Changed 8 months ago by nickm

Replying to hellais:

sbs and I looked at this.

The torspec and tor diffs look good.

One thing to be careful about though is that when the socket is closed (as per TAKEOWNERSHIP behavior), when you "shut down" Tor, you must not call exit().
This may be differ from how OwningControllerProcess behaves.

Right! All of the exit handling is in a different ticket, #23848.

Moreover lack of windows support may be an issue for us (as that is also a target platform for Measurement Kit), but I guess for the time being we can avoid using this option there.

So, this actually _will_ work on windows, but not from the command line. You have to construct a socketpair instead, and you can't pass those around between processes as fd's... but in-process, it will work fine.

Also as mentioned in https://trac.torproject.org/projects/tor/ticket/23845#comment:12, it would be useful to document that for non windows platforms it's recommended to set this option when constructing a tor_main_configuration_t to pass to tor_run_main.

I'm waiting for the #23845 merge for that. I think, as a followup, it would make sense to actually have a C API to set this up without having to use the command line. Fortunately, tor_run_main() is extensible.

comment:12 in reply to:  10 Changed 8 months ago by nickm

Replying to sbs:

I was also wondering:

  1. assuming that tor_socket_t is similar to evutil_socket_t, whether we can pass a socket on Windows to obtain the same effect

They're similar, but it's going to be a little hacking to get this working on windows: Sockets aren't treated as fds in a nice inheritable way on Windows. But in-process, it should work fine, if we make a fake socketpair with the usual tricks.

  1. if this function to set an owning socket is thread safe (I don't know enough on Tor internals to say whether it's obviously thread safe or not)

It should only ever be called from the Tor main thread, so it should be safe.

comment:13 Changed 8 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

merged to master; torspec branch merged too.

Note: See TracTickets for help on using tickets.