Opened 2 months ago

Last modified 8 days ago

#29080 needs_review defect

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, TorBrowserTeam201903R, tbb-8.5
Cc: igt0, sisbell, sysrqb, n8fr8, hans@… Actual Points:
Parent ID: #27609 Points:
Reviewer: Sponsor:

Description


Child Tickets

Change History (23)

comment:1 Changed 2 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 8 weeks 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 7 weeks 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 7 weeks 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 6 weeks ago by gk

Keywords: TorBrowserTeam201902 added; TorBrowserTeam201901 removed

Moving tickets to February.

comment:6 Changed 4 weeks ago by sysrqb

Cc: n8fr8 added

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

comment:8 Changed 3 weeks ago by sisbell

Status: newneeds_review

comment:9 Changed 3 weeks ago by gk

Keywords: TorBrowserTeam201902R added; TorBrowserTeam201902 removed

Setting the "R" keyword

comment:10 Changed 3 weeks 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 weeks 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 weeks 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 weeks 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 weeks 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 weeks 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 weeks 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 weeks 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 weeks 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 weeks ago by eighthave

Cc: hans@… added

comment:20 Changed 3 weeks 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 2 weeks ago by gk

Keywords: TorBrowserTeam201903R added; TorBrowserTeam201902R removed

February is gone.

comment:22 Changed 12 days ago by gk

Keywords: tbb-8.5 added

Tickets on our radar for 8.5

comment:23 Changed 8 days ago by gk

Status: needs_informationneeds_review
Note: See TracTickets for help on using tickets.