Opened 6 months ago

Last modified 3 days ago

#25510 assigned project

Collect feedback on mobile embedding API; resolve issues.

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.6.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: roadmap, 035-roadmap-master, 034-triage-20180328, 034-included-20180328, 035-triaged-in-20180711
Cc: dmr Actual Points:
Parent ID: Points:
Reviewer: Sponsor: Sponsor8-must

Description

This is a master ticket for improving the design and reliability of the embedding API in tor_api.h. We already have a few projects starting to try this code, but we need more feedback about it in order to know where the developer-facing issues are. As we learn more about such issues, we should fix them.

Child Tickets

TicketStatusOwnerSummaryComponent
#21818newtor's handling of SIGHUP considered harmfulCore Tor/Tor
#23573needs_revisionDo we want to close all connections when tor closes?Core Tor/Tor
#23846closednickmOption to build Tor with -fPIC (Use libtool for building shared library?)Core Tor/Tor
#24204closednickmImprove the in-process Tor API: create owning control portCore Tor/Tor
#24586newAudit all state for stuff we need to reset on exit to make tor restartable.Core Tor/Tor
#25493newImprove patterns for cleaning up static variables on exit/restartCore Tor/Tor
#25512closednickmTor in-process restart fails to write auth cookieCore Tor/Tor
#25607assignednickmOn restart-in-process, do the right thing with thread-local storageCore Tor/Tor
#26651closedCalling event_enable_debug_mode() a second time exits the processCore Tor/Tor
#26947closednickmAdd function for reporting the tor version in tor_api.hCore Tor/Tor
#26948closednickmtor_run_main crashes when called a second time with --versionCore Tor/Tor
#26997newtor_api: Do we want an easier way to set options?Core Tor/Tor
#26999newtor_api: Some way to capture stdout/stderrCore Tor/Tor
#27801newtor_api: CreateConnection() interfaceCore Tor/Tor

Change History (11)

comment:1 Changed 6 months ago by nickm

Keywords: 034-roadmap-master added

comment:2 Changed 6 months ago by nickm

Keywords: 034-triage-20180328 added

comment:3 Changed 6 months ago by nickm

Keywords: 034-included-20180328 added

comment:4 Changed 4 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: 0.3.5.x-final

comment:5 Changed 3 months ago by nickm

Keywords: 035-roadmap-master added; 034-roadmap-master removed
Sponsor: Sponsor8-must

comment:6 Changed 2 months ago by nickm

Keywords: 035-triaged-in-20180711 added

comment:7 Changed 2 months ago by hellais

I started the new tor_api.h and have been chatting with ahf on IRC about this.

Here is how far I got with testing it.

Setup

First, to setup an environment for testing I needed to build it as a shared library.

This was done by building tor with make V=1 CFLAGS=-fPIC.

I then deleted src/app/tor, re-ran make V=1 CFLAGS=-fPIC and reran that line adding -shared to it. That lead me to having a dylib (or .so on linux) in src/app/tor.

This artifact can now be used for testing libtor.

Testing

I wrote a testing harness that uses python and cffi, to implement some basic smoke tests for libtor. This can be found here: https://gist.github.com/hellais/b56043d57eb5be885958e80b3665bfe2 (to run it do pip install cffi and change the LIB_PATH to the correct path).

In particular by adding the command line flag --version and starting tor, I am unable to re-start it due to the exception listed in the above gist.

Feedback

WRT API design I have the following thoughts on possible areas for improvement. I have for the time being only created tickets for the things I think should be part of an MVP, while the others can possibly be deferred to a future version and if you think they are good ideas I will proceed with creating tickets for them.
I may also be missing some best practice here.

  • Ability to know when the control port is ready

Currently it's not possible for a user of the API to know when the control port is ready if not by looking at the standard output and/or polling the port/socket where the control port is supposed to be at. It would be great if there were some function (or some documented approach) to doing this.

  • Ability to get the output of tor_run_main() without having to mock standard out

There is a wealth of information inside of the log output of tor, which would be nice if it were exposed in a more API friendly way (maybe some sort of callback function?) so that a user of the library doesn't have to mock stdout/stderr.

  • Ability to terminate tor cleanly without using the control port

I am aware of the fact that there is a control port command to stop tor, but it would be great if there were also an API call to be used to tell tor to shutdown cleanly (similarly to calling kill -SIGTERM tor).

  • Add API calls for making it easier to setup the configuration for tor

This is more of usability issue, but it would be great if the tor_api.h has some function for setting configuration options for Tor, without having to rely on passing them as command line arguments.

comment:8 Changed 8 weeks ago by dmr

Cc: dmr added

comment:9 in reply to:  7 Changed 8 weeks ago by nickm

Replying to hellais:

First, to setup an environment for testing I needed to build it as a shared library.

This was done by building tor with make V=1 CFLAGS=-fPIC.

Making this easier is now the focus of #23846.

In particular by adding the command line flag --version and starting tor, I am unable to re-start it due to the exception listed in the above gist.

Hellais opened #26948 for this.

Feedback

Now in needs_review.

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

These two are covered by #24204.

  • Ability to get the output of tor_run_main() without having to mock standard out

Opened #26999 for this.

  • Add API calls for making it easier to setup the configuration for tor

Opened #26997 for this.

comment:10 Changed 11 days ago by nickm

Milestone: Tor: 0.3.5.x-finalTor: 0.3.6.x-final

These are not on track for 0.3.5, given the amount of time remaining before freeze.

comment:11 Changed 3 days ago by sysrqb

I finally took a look at the API. I really like it. I created some example programs while I was looking at it and thinking about whether anything was missing. I documented some of that on #26653. They're in my public tor repo on branch testing_26653.

#24204 was a big win.

I think the API can use a little more documentation, and I created #27801 as a maybe-this-will-be-useful-for-someone.

Overall, I don't think we need anything else for TBA. Thanks!

Note: See TracTickets for help on using tickets.