Opened 19 months ago

Closed 9 months ago

Last modified 5 months ago

#24204 closed defect (implemented)

Improve the in-process Tor API: create owning control port

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-mobile, s8-api, 034-triage-20180328, 035-roadmap-subtask
Cc: darkk, brade, mcs, sbs, mtigas, hellais Actual Points: 4
Parent ID: #25510 Points:
Reviewer: ahf Sponsor: Sponsor8-can

Description

It would be good to have an interface something like this for controller authors to use, if they're embedding Tor:

#ifdef _WIN32
#define CONTROL_SOCKET SOCKET
#else
#define CONTROL_SOCKET int
#endif

/**
 * Tells Tor to create a socket for an owning controller connection to the
 * Tor that will be launched.  Return the socket.
 */
CONTROL_SOCKET tor_main_configuration_setup_control_socket(
                                           tor_main_configuration_t *cfg);

#idef _WIN32
/** Windows-only; experimental. Tells Tor to create an anonymous pipe
 * (whatever that's called) as an owning connection for the he controller, and
 * and return that pipe. */
HANDLE tor_main_configuration_setup_control_pipe(
                                           tor_main_configuration_t *cfg);
#endif

We should make sure it's portable, and that it can be implemented both with libtor_runner and with in-process Tor.

Child Tickets

Change History (17)

comment:1 Changed 19 months ago by nickm

Sponsor: Sponsor8-can

comment:2 Changed 19 months ago by mtigas

Cc: mtigas added; mike@… removed

Don't have too much to comment on here, but just reporting back so folks know I saw it.

Since AFAIK all of the mobile implementations (including the in-process iOS "tor as thread" way of doing things) happily use a TCP or unix socket control port, this is a much lower priority for me (nice to have someday) than #24107 / #23847 (which fixes a platform-specific stability issue).

comment:3 Changed 18 months ago by nickm

Owner: set to nickm
Status: newaccepted

comment:4 Changed 14 months ago by nickm

Keywords: 034-triage-20180328 added

comment:5 Changed 14 months ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:6 Changed 14 months ago by nickm

Keywords: 034-removed-20180328 removed
Parent ID: #23684#25510
Priority: MediumVery Low

comment:7 Changed 13 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

I think this would be useful, but none of our current downstream developers seem so keen on it yet. I don't have time to finish it before the freeze for 0.3.4.

comment:8 Changed 11 months ago by nickm

Keywords: 035-roadmap-subtask added

comment:9 Changed 11 months ago by eighthave

On Android, I think it would be most useful to have the control port be a unix socket that is in the private directory of Orbot or Orfox (e.g. /data/data/org.torproject.android/files/control_port). Then, tor daemon should quit when the other side disconnects after a connection. That'll help with sorting things out when Android kills the app.

comment:10 Changed 10 months ago by hellais

I am now a believe in this solution.

The main advantages I see for a mobile app developer use-case, is the fact that:

  1. You can create out of this an event telling you that the Tor control port is ready to receive messages without relying on polling to see if teh port is open.

ex. since the function returns a socket handle, you can just write directly to the socket and when you receive a response back you know the control port is ready

  1. You can use it to signal tor to shutdown cleanly, by simply closing the socket.

As such this solution crossed out two of the items I listed in https://trac.torproject.org/projects/tor/ticket/25510#comment:7, namely:

  • Ability to know when the control port is ready
  • Ability to terminate tor cleanly without using the control port

comment:11 Changed 10 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.5.x-final

comment:12 Changed 10 months ago by nickm

Cc: hellais added
Priority: Very LowMedium
Status: acceptedneeds_review

Code in my tor_api_owning_control branch; PR at https://github.com/torproject/tor/pull/260

comment:13 in reply to:  12 Changed 10 months ago by hellais

Replying to nickm:

Code in my tor_api_owning_control branch; PR at https://github.com/torproject/tor/pull/260

Thanks for working on this!

I started doing some testing of this branch.

So far I have verified that:

  1. By closing the control socket tor shuts down
  1. I am able to use the control socket to send control port commands

Here is a snippet of the code I have used to test it: https://gist.github.com/hellais/f231c95e5a487ec7b2edd5c80f6291f8.

I think some more through testing is perhaps appropriate, but it for sure passes a smoke test.

comment:14 Changed 10 months ago by asn

Reviewer: ahf

comment:15 Changed 9 months ago by ahf

Status: needs_reviewmerge_ready

Left a very small nitpick on the PR. Otherwise looks good!

comment:16 Changed 9 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

tweaked and merged. thanks for the review!

comment:17 Changed 5 months ago by nickm

Actual Points: 4
Note: See TracTickets for help on using tickets.