Opened 4 months ago

Closed 6 weeks ago

#29080 closed defect (fixed)

Merge OrbotService and TOPL

Reported by: sisbell Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-mobile, TBA-a3, tbb-8.5-must-alpha, TorBrowserTeam201904R
Cc: igt0, sisbell, sysrqb, n8fr8, hans@… Actual Points:
Parent ID: #27609 Points:
Reviewer: Sponsor:

Description


Child Tickets

Change History (35)

comment:1 Changed 4 months ago by sisbell

I've done an initial (ugly) merging of TOPL and orbotservice so have enough to identify the specifics.

Changes to TOPL

  • The OnionProxyManager should be able to set the EventHandler, rather than use the default one. Need to add a field to the OnionProxyManager allowing us to pass in the custom EventHandler that we will use.
  • Currently only supports configuring one hidden service. Add support for multiple hidden services through OnionProxyManager.publishHiddenService
  • Add fields to the TorConfig builder: ServerDNSResolvConfFile, ExitNodes, Bridge, NEWNYM, Socks5ProxyUsername, Socks5ProxyPassword, ProxyAuthenticator, ClientTransportPlugin, ReachableAddresses, ExcludeNodes, EntryNodes, AutomapHostsOnResolve, DNSPort, HTTPTunnelPort
  • Add support for writing DNS File (resolv.conf)
  • Add support for loading bridges from resource file

Changes to OrbotService

  • TorEventHandler - Orbot delegates sending of Android broadcasts to the TorService. We can decouple this by sending the broadcasts directly. This interface integrates with the same one that TOPL uses and can be configured through the OnionProxyManager with the fix defined above. [Easy fix]
  • TorService - this is a 2K line uber class.
    • Dozen or so broadcast messages that the main app will display to user. Need to replicate these where is makes sense
    • HiddenService Content DB - writes out to torrc. Break into its own class (possibly auto-generate)
    • Client Cookie DB - writes out to torrc. Break into its own class (possibly auto-generate)
    • VPN - we can port this directly
    • Clean circuits - a pending intent through a notification on service startup (NEWNYM)
    • Create Notifications for Network connectivity - this exists in TOPL but does not create notif
    • Start/Stop tor - we can use TOPL OnionProxyManager.start and OnionProxyManager.stop method to replace this feature
    • Tor Installer - we can use the one from TOPL
    • Handle events sent to service: ACTION_START, ACTION_STATUS, CMD_SIGNAL_HUP, CMD_NEWNYM, CMD_VPN, CMD_VPN_CLEAR, CMD_SET_EXIT
    • Various torrc config params that mirror TOPL
    • Loads Bridges from resource file

Most of the work is simply taking everything from the Content DB and prefs and adding fields to torrc. The DB need to be rewritten. This should all be done in a separate package/set of classes, not in the TorService class.

The new TorService will:

  • delegate to TorConfig package when generating torrc from prefs/DB
  • delegate to the OnionProxyManager for the start/stop of tor.
  • delegate to the AndroidResourceInstaller (TOPL) for installing tor
  • delegate to TOPL Config for loading bridges
  • handle incoming events AND outgoing notifications.
  • handle VPN configuration

Integration:
TOPL will be an android library. OrbotService will use TOPL. We can create patches for both projects and then integrate into tor-browser-build. The main Orbot app shouldn't require any changes as we can support the existing interfaces and broadcasts.

comment:2 Changed 4 months ago by sisbell

I've made the following changes to TOPL

  • Add custom torrc location through OnionProxyContext
  • Added an EventBroadcaster. Orbot will pass in an implementation so that TOPL broadcasts the same events as Orbot currently broadcasts.
  • Added the following methods to OnionProxyManager: killTorProcess, reloadTorConfig, getTorPid, newIdentity, disableNetwork, setExitNodes
  • Added resolvConf to TorConfig class
  • Added a new TorConfigBuilder class which included parsing and loading of bridges from file

These changes make TOPL support the base features that Orbot needs.

Changes to Orbot:

  • Moved over functionality to TOPL as part of integration (see above)
  • Use TOPL for tor installation and bootstrapping.
  • Created EventBroadcaster implementation (previously from code in TorService)
  • Rewrite DBs (in progress)

TorService was 2K file, now its below 500 lines so a lot more manageable.

comment:3 Changed 4 months ago by sisbell

I added 10 issues to the TOPL issue tracker that outline the specific changes we need (issues #49-58)

https://github.com/thaliproject/Tor_Onion_Proxy_Library/issues

With the exception of #56,#57,I have these implemented and verified working with a modified version of OrbotService (main app will not need changes). Next step is to break the current work into each of their respective bug/commits. Then I will submit a pull request to TOPL.

comment:4 Changed 4 months ago by sisbell

I have everything integrated. There are four parts of the project currently.

1) https://github.com/sisbell/Tor_Onion_Proxy_Library
This is a fork of TOPL with the changes we need for integration with OrbotService. I've submitted a pull request. Yaron has some requested changes, but they are relatively minor so I'll get this resubmitted this week.

2) https://github.com/sisbell/Tor_Onion_Proxy_Library_Android
This contains the TOPL android code. Once the changes are merged from (1) we can do away with this repo. This project is dependent on (1) and pulls it down as part of the build through jitpack

3) https://github.com/sisbell/tor-android-service
This contains the merged TOPL and OrbotService code. This project is dependent on (2) and pulls it down as part of the build.

4) https://github.com/sisbell/orbot
This is a fork of Orbot. I removed the orbotservice code and added the dependency on (3). There are no code changes so tor-android-service is 100% compatible with the current Orbot UI/app implementation.

    implementation 'com.github.sisbell:tor-android-service:d4deab617b450401b9d5a3aa811ee6cdacaee303'

This repo is just for testing. We will use whatever is going to be included in the browser.

For testing, you can just checkout (4) and import into Android Studio. All dependencies are already handled.

I still need to

  1. Fix changes suggested in the pull request
  2. Add back in code for cookies and hidden services databases.
  3. Do more integration testing to make sure I didn't miss anything
  4. Start tor-browser-build

comment:5 Changed 4 months ago by gk

Keywords: TorBrowserTeam201902 added; TorBrowserTeam201901 removed

Moving tickets to February.

comment:6 Changed 3 months ago by sysrqb

Cc: n8fr8 added

Adding nathan, in case he wants to burn some cycles reviewing some of this.

comment:8 Changed 3 months ago by sisbell

Status: newneeds_review

comment:9 Changed 3 months ago by gk

Keywords: TorBrowserTeam201902R added; TorBrowserTeam201902 removed

Setting the "R" keyword

comment:10 Changed 3 months ago by gk

Status: needs_reviewneeds_information

sisbell: What exactly should we review here? Could you point me/us to the relevant branches/commits?

comment:11 in reply to:  10 ; Changed 3 months ago by sisbell

Replying to gk:

sisbell: What exactly should we review here? Could you point me/us to the relevant branches/commits?

The pull request has been merged to TOPL. I was thinking if everyone is in agreement we can close out this ticket.

sysrqb has commented on the merge commits in the parent ticket (see #27609).

comment:12 in reply to:  11 ; Changed 3 months ago by gk

Replying to sisbell:

Replying to gk:

sisbell: What exactly should we review here? Could you point me/us to the relevant branches/commits?

The pull request has been merged to TOPL. I was thinking if everyone is in agreement we can close out this ticket.

sysrqb has commented on the merge commits in the parent ticket (see #27609).

Okay. How does that relate to the changes mentioned in comment:4? Are they not needed anymore or are they prepared for review somewhere else? Or...?

comment:13 in reply to:  12 Changed 3 months ago by sisbell

Replying to gk:

Replying to sisbell:

Replying to gk:

sisbell: What exactly should we review here? Could you point me/us to the relevant branches/commits?

The pull request has been merged to TOPL. I was thinking if everyone is in agreement we can close out this ticket.

sysrqb has commented on the merge commits in the parent ticket (see #27609).

Okay. How does that relate to the changes mentioned in comment:4? Are they not needed anymore or are they prepared for review somewhere else? Or...?

sysrqb made some additional comments after the merge, which we can make into new PR as needed.
https://trac.torproject.org/projects/tor/ticket/27609#comment:28

  • tor-android-service is still needed. I'm maintaining this on github, we can move over to new repo once we decide on location.
  • My fork of orbot was just used for testing and can be tossed

I've opened tickets related to creating tor-browser-build for TOPL, tor-android-service and integration into Orbot and Firefox projects.

comment:14 Changed 3 months ago by sysrqb

Shane, can you expand on why this is better? https://github.com/sisbell/tor-android-service/issues/12

comment:15 in reply to:  14 ; Changed 3 months ago by sisbell

Replying to sysrqb:

Shane, can you expand on why this is better? https://github.com/sisbell/tor-android-service/issues/12

In tor-browser-build, we will replace these jars/libs with the ones that are generated as part of the build. The developer who wants to use tor-android-service in their own project will have these libraries already provided in the libs folder. This keeps a consistent approach. I'm still exploring whether these dependent libraries should be treated as provided, meaning they won't be packaged in the resulting aar.

Ideally, in the future, I'd like to get the TOPL artifacts officially deployed to a maven repo and pull these down as dependencies during a regular developer build. The tor-browser-build would pull down the dependencies as part of the build process as well.

comment:16 in reply to:  15 Changed 3 months ago by sysrqb

Replying to sisbell:

Replying to sysrqb:

Shane, can you expand on why this is better? https://github.com/sisbell/tor-android-service/issues/12

In tor-browser-build, we will replace these jars/libs with the ones that are generated as part of the build. The developer who wants to use tor-android-service in their own project will have these libraries already provided in the libs folder. This keeps a consistent approach. I'm still exploring whether these dependent libraries should be treated as provided, meaning they won't be packaged in the resulting aar.

Ideally, in the future, I'd like to get the TOPL artifacts officially deployed to a maven repo and pull these down as dependencies during a regular developer build. The tor-browser-build would pull down the dependencies as part of the build process as well.

Ah, so this is simply a stop-gap until they're available from a maven repo? That's fine. And if tor-browser-build will build these independently and (presumably) overwrite the vendored files, then I'm okay with that too, thanks for explaining.

comment:17 Changed 3 months ago by sysrqb

Have you seen these build errors. They're obviously coming form proguard, but i haven't started comparing Orbot's proguard file(s) with tor-android-service's yet.

 1:10.37 Note: the configuration refers to the unknown class 'com.google.android.gms.common.annotation.KeepName'
 1:10.37 Note: the configuration refers to the unknown class 'com.google.android.gms.common.annotation.KeepName'
 1:10.38 Note: the configuration refers to the unknown class 'okhttp3.internal.publicsuffix.PublicSuffixDatabase'
[...]
 1:10.64 Warning: com.msopentech.thali.toronionproxy.OnionProxyManagerEventHandler: can't find superclass or interface net.freehaven.tor.control.EventHandler
 1:10.64 Warning: org.torproject.android.service.TorEventHandler: can't find superclass or interface net.freehaven.tor.control.EventHandler
 1:10.94 Warning: com.msopentech.thali.toronionproxy.BaseEventBroadcaster: can't find referenced class org.slf4j.Logger
 1:10.94 Warning: com.msopentech.thali.toronionproxy.BaseEventBroadcaster: can't find referenced class org.slf4j.Logger
[...]
 1:10.95 Warning: com.msopentech.thali.toronionproxy.OnionProxyContext: can't find referenced class org.slf4j.Logger
 1:10.95 Warning: com.msopentech.thali.toronionproxy.OnionProxyContext: can't find referenced class org.slf4j.LoggerFactory
[...]
 1:11.22 Warning: org.torproject.android.service.TorService: can't find referenced class com.msopentech.thali.android.toronionproxy.AndroidOnionProxyManager
 1:11.22 Warning: org.torproject.android.service.TorService: can't find referenced class com.msopentech.thali.android.toronionproxy.AndroidOnionProxyManager
 1:11.22 Warning: org.torproject.android.service.TorService: can't find referenced class com.msopentech.thali.android.toronionproxy.AndroidOnionProxyManager
[...]
 1:11.40 Note: com.google.android.exoplayer2.DefaultRenderersFactory: can't find dynamically referenced class com.google.android.exoplayer2.ext.vp9.LibvpxVideoRenderer
[...]
 1:12.84 Note: there were 16 references to unknown classes.
 1:12.84       You should check your configuration for typos.
 1:12.84       (http://proguard.sourceforge.net/manual/troubleshooting.html#unknownclass)
 1:12.84 Note: there were 11 unkept descriptor classes in kept class members.
 1:12.84       You should consider explicitly keeping the mentioned classes
 1:12.84       (using '-keep').
 1:12.84       (http://proguard.sourceforge.net/manual/troubleshooting.html#descriptorclass)
 1:12.84 Note: there were 1 library classes explicitly being kept.
 1:12.84       You don't need to keep library classes; they are already left unchanged.
 1:12.85       (http://proguard.sourceforge.net/manual/troubleshooting.html#libraryclass)
 1:12.85 Note: there were 12 unresolved dynamic references to classes or interfaces.
 1:12.85       You should check if you need to specify additional program jars.
 1:12.85       (http://proguard.sourceforge.net/manual/troubleshooting.html#dynamicalclass)
 1:12.85 Warning: there were 83 unresolved references to classes or interfaces.
 1:12.87          You may need to add missing library jars or update their versions.
 1:12.87          If your code works fine without the missing classes, you can suppress
 1:12.87          the warnings with '-dontwarn' options.

comment:18 in reply to:  17 Changed 3 months ago by sysrqb

Replying to sysrqb:

 1:10.64 Warning: com.msopentech.thali.toronionproxy.OnionProxyManagerEventHandler: can't find superclass or interface net.freehaven.tor.control.EventHandler
 1:10.64 Warning: org.torproject.android.service.TorEventHandler: can't find superclass or interface net.freehaven.tor.control.EventHandler
 1:10.94 Warning: com.msopentech.thali.toronionproxy.BaseEventBroadcaster: can't find referenced class org.slf4j.Logger
 1:10.94 Warning: com.msopentech.thali.toronionproxy.BaseEventBroadcaster: can't find referenced class org.slf4j.Logger
[...]
 1:10.95 Warning: com.msopentech.thali.toronionproxy.OnionProxyContext: can't find referenced class org.slf4j.Logger
 1:10.95 Warning: com.msopentech.thali.toronionproxy.OnionProxyContext: can't find referenced class org.slf4j.LoggerFactory
[...]
 1:11.22 Warning: org.torproject.android.service.TorService: can't find referenced class com.msopentech.thali.android.toronionproxy.AndroidOnionProxyManager
 1:11.22 Warning: org.torproject.android.service.TorService: can't find referenced class com.msopentech.thali.android.toronionproxy.AndroidOnionProxyManager
 1:11.22 Warning: org.torproject.android.service.TorService: can't find referenced class com.msopentech.thali.android.toronionproxy.AndroidOnionProxyManager
[...]
 1:12.85 Warning: there were 83 unresolved references to classes or interfaces.
 1:12.87          You may need to add missing library jars or update their versions.
 1:12.87          If your code works fine without the missing classes, you can suppress
 1:12.87          the warnings with '-dontwarn' options.

To be clear, it is the latter Warnings causing the build failure (83 unresolved references to classes).

Warning: Exception while processing task java.io.IOException: Please correct the above warnings first.
:app:transformClassesAndResourcesWithProguardForOfficialWithGeckoBinariesNoMinApiPhotonDebug FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:transformClassesAndResourcesWithProguardForOfficialWithGeckoBinariesNoMinApiPhotonDebug'.
> Job failed, see logs for details

comment:19 Changed 3 months ago by eighthave

Cc: hans@… added

comment:20 Changed 3 months ago by gk

sysrqb: how are you building tor-android-service? I can successfully build it using sisbell's android-0224 branch.

comment:21 Changed 3 months ago by gk

Keywords: TorBrowserTeam201903R added; TorBrowserTeam201902R removed

February is gone.

comment:22 Changed 3 months ago by gk

Keywords: tbb-8.5 added

Tickets on our radar for 8.5

comment:23 Changed 2 months ago by gk

Status: needs_informationneeds_review

comment:24 Changed 2 months ago by eighthave

One small but important thing to do with this merging is to make TorService be an actual Android Service. That will help with the lifecycle issues a lot, and is the official Android interface for doing something like a UNIX daemon. I did some quick prototyping, this basically means writing a thin Java and JNI shim layer where the Java code defines the subclass of android.app.Service and the JNI layer maps the tor daemon start/stop to the Java class. We have to do something like this for Android Q anyway, since running daemons is now fully blocked:

I have some cycles allocated from Guardian Project to work on this kind of thing, so I'm wondering how best to integrate into sisbell's work.

comment:25 in reply to:  24 Changed 2 months ago by sisbell

Replying to eighthave:

One small but important thing to do with this merging is to make TorService be an actual Android Service. That will help with the lifecycle issues a lot, and is the official Android interface for doing something like a UNIX daemon. I did some quick prototyping, this basically means writing a thin Java and JNI shim layer where the Java code defines the subclass of android.app.Service and the JNI layer maps the tor daemon start/stop to the Java class.

I think this work wouldn't be done in TorService directly. It would be done in

https://github.com/thaliproject/Tor_Onion_Proxy_Library

The AndroidOnionProxyManager (extends OnionProxyManager) would be where the control of Tor is done. It manages the lifecycle events of the tor process. Stop is handled through the control connection, while start does all of the exec commands that we want to eliminate. So you can just fork TOPL and submit a PR. If there are any changes that need to bubble up, we can make those in TorService (tor-android-service)

We have to do something like this for Android Q anyway, since running daemons is now fully blocked:

It looks like this would also hit tor-android-binary dependency that we are using.

I have some cycles allocated from Guardian Project to work on this kind of thing, so I'm wondering how best to integrate into sisbell's work.

comment:26 Changed 2 months ago by eighthave

Turns out I was wrong about Android Q fully blocking daemons. It is just they can't be executed from the /data partition. They can be named with .so and included as a library, then directly executed from the system-managed unpack location.

comment:27 in reply to:  26 Changed 2 months ago by sisbell

Replying to eighthave:

Turns out I was wrong about Android Q fully blocking daemons. It is just they can't be executed from the /data partition. They can be named with .so and included as a library, then directly executed from the system-managed unpack location.

I gathered this from the links you posted. It looks like we can't copy libraries from the apk to the writable /data/data directory anymore. We will need to have the system unpack the libraries for us with the extractNativeLibs=true property in the manifest and then execute from the read-only /data/app directory.

comment:28 Changed 2 months ago by sisbell

I submitted a new issue with TOPL, since this has an impact:

https://github.com/thaliproject/Tor_Onion_Proxy_Library/issues/85

comment:29 Changed 2 months ago by n8fr8

We have an initial release out for testing that hopefully addresses the Android Q issue:

https://github.com/n8fr8/tor-android/releases/tag/tor-android-binary-tor-0.3.5.8-rc

and a build of Orbot, as well, based on this:
https://github.com/n8fr8/orbot/releases/tag/16.0.6-BETA-1-tor-0.3.5.8

There is a new gradle dependency published as well:
org.torproject:tor-android-binary:0.3.5.8-rc

comment:30 Changed 2 months ago by sisbell

I have a commit that breaks apart the install and config directories. I upgraded to use org.torproject:tor-android-binary:0.3.5.8-rc

https://github.com/sisbell/Tor_Onion_Proxy_Library/commit/575d9fd9f9727bf947ddac671034ae517799725b

I know there is some discussion around which version of tor-android-binary to use. If we need to drop to earlier version of tor for tor-browser-build, I think we can handle this through a patch, while still maintaining the newer support for Android Q.

comment:31 in reply to:  30 Changed 2 months ago by gk

Replying to sisbell:

I have a commit that breaks apart the install and config directories. I upgraded to use org.torproject:tor-android-binary:0.3.5.8-rc

https://github.com/sisbell/Tor_Onion_Proxy_Library/commit/575d9fd9f9727bf947ddac671034ae517799725b

I know there is some discussion around which version of tor-android-binary to use. If we need to drop to earlier version of tor for tor-browser-build, I think we can handle this through a patch, while still maintaining the newer support for Android Q.

0.3.5.8 is fine. We should give it a try in the next alpha (I was just not convinced that we should use a tor -rc in our browser instead of an actual stable release).

comment:32 Changed 2 months ago by gk

Keywords: tbb-8.5-must added; tbb-8.5 removed

Marking blockers for Tor Browser 8.5.

comment:33 Changed 2 months ago by gk

Keywords: tbb-8.5-must-alpha added; tbb-8.5-must removed

Tickets that block the next 8.5 alpha.

comment:34 Changed 8 weeks ago by gk

Keywords: TorBrowserTeam201904R added; TorBrowserTeam201903R removed

Moving review tickets to April.

comment:35 Changed 6 weeks ago by gk

Resolution: fixed
Status: needs_reviewclosed

I think we are done here. We can open follow-up issues if needed.

Note: See TracTickets for help on using tickets.