Opened 6 months ago

Last modified 9 days ago

#30767 needs_review defect

Custom obfs4 bridge does not work on Tor Browser for Android

Reported by: gk Owned by: sisbell
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-parity, tbb-mobile, TorBrowserTeam201912R
Cc: phw, cohosh, arma, tbb-team Actual Points:
Parent ID: #31284 Points: 0.5
Reviewer: sysrqb Sponsor: Sponsor30-can

Description

We got a report on our blog post (https://blog.torproject.org/comment/282217#comment-282217) mentioning that custom obfs4 bridges don't work on Android while they do on desktop. I tested with a bridge provided and can reproduce the issue.

Child Tickets

Change History (22)

comment:1 Changed 6 months ago by gk

Keywords: tbb-paritiy added; tbb-partiy removed

comment:2 Changed 6 months ago by gk

Keywords: tbb-parity added; tbb-paritiy removed

comment:3 Changed 4 months ago by pili

Parent ID: #31284
Sponsor: Sponsor30-can

comment:4 Changed 3 months ago by arma

Cc: phw cohosh arma added

comment:5 Changed 3 months ago by sysrqb

Cc: sisbell added
Keywords: TorBrowserTeam201909 added; TorBrowserTeam201906 removed

Notice the missing ClientTransportPlugin line for obfs4proxy.

generic_x86:/ # cat /data/data/org.torproject.torbrowser_alpha/app_torservice/torrc
AutomapHostsOnResolve 1
ControlPortWriteToFile /data/user/0/org.torproject.torbrowser_alpha/app_torservice/lib/tor/control.txt
ControlPort auto
CookieAuthentication 1 
CookieAuthFile /data/user/0/org.torproject.torbrowser_alpha/app_torservice/lib/tor/control_auth_cookie
DisableNetwork 1
DNSPort 5400
HTTPTunnelPort 8218
ReducedConnectionPadding 1
RunAsDaemon 1
SafeSocks 0
SOCKSPort 9150 KeepAliveIsolateSOCKSAuth IPv6Traffic PreferIPv6
StrictNodes 0
TestSocks 0
TransPort 9140
VirtualAddrNetwork 10.192.0.0/10
Bridge obfs4 123.234.345.456:3322 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA cert=asdfasdf
UseBridges 1

It's because we only add the ClientTransportPlugin line for the default bridges.

comment:6 Changed 2 months ago by pili

Keywords: TorBrowserTeam201910 added

comment:7 Changed 2 months ago by pili

Keywords: TorBrowserTeam201909 removed

comment:8 Changed 2 months ago by sysrqb

Points: 0.5

comment:9 Changed 2 months ago by sisbell

This should pick up. It will take the list of bridges from preferences in the UI (settings). I'm thinking maybe the preferences are not getting set anymore after removing the old orbot code. I'll confirm if this is the case.

comment:10 in reply to:  5 ; Changed 2 months ago by arma

Replying to sysrqb:

It's because we only add the ClientTransportPlugin line for the default bridges.

In theory you should be able to put all the ClientTransportPlugin lines in there by default, even when there are no bridges set, and Tor won't launch any of them until it has a bridge that needs them.

comment:11 in reply to:  10 Changed 2 months ago by mcs

Replying to arma:

In theory you should be able to put all the ClientTransportPlugin lines in there by default, even when there are no bridges set, and Tor won't launch any of them until it has a bridge that needs them.

FWIW, Tor Browser on desktop has used this approach ever since we switched to Tor Launcher (i.e., for a long time now). The following gets appended to the torrc-defaults file which is always loaded by TB desktop's tor:

https://gitweb.torproject.org/builders/tor-browser-build.git/tree/projects/tor-browser/Bundle-Data/PTConfigs/linux/torrc-defaults-appendix

Last edited 2 months ago by mcs (previous) (diff)

comment:12 Changed 8 weeks ago by sisbell

Keywords: TorBrowserTeam201910R added; TorBrowserTeam201910 removed
Status: newneeds_review

I fixed this as part of fixing the bridge list. I took the approach suggested by arma, the plugin transport is added regardless of whether user specifies a bridge. This is a much cleaner approach.I added some unit tests for verify behavior.

https://github.com/sisbell/Tor_Onion_Proxy_Library/commit/ac9ad2295b3dcb312a9aff4f3f7b27343bc4b73d

comment:13 Changed 6 weeks ago by pili

Keywords: TorBrowserTeam201911 added

Moving tickets to November 2019

comment:14 Changed 6 weeks ago by gk

Keywords: TorBrowserTeam201911R added; TorBrowserTeam201910R removed

There is no way to do reviews in October 2019 anymore.

comment:15 Changed 6 weeks ago by gk

Keywords: TorBrowserTeam201911 removed

comment:16 Changed 4 weeks ago by sisbell

This is a very tight fix, only addressing the obfs4 bug and a couple of bridge bugs. No change required for tor-android-service since the API hasn't changed. This commit does not address all issues we have with settings.

Changes

  • Break apart the setting of transports and bridges. Transports will be set independent of whether user has configured bridges.
  • Removed shuffle of bridges (addresses previous review comment)
  • Removed max limit on number of bridges (addresses previous review comment)

https://github.com/sisbell/Tor_Onion_Proxy_Library/commit/39f6aad9257947d7c47181e04963857284a67a44

comment:17 Changed 3 weeks ago by sysrqb

Keywords: TorBrowserTeam201911 added; TorBrowserTeam201911R removed
Status: needs_reviewneeds_revision

Looks pretty good.

+    @SettingsConfig
+    public TorConfigBuilder bridgesFromSettings() {
+        List<String> supportedBridges = settings.getListOfSupportedBridges();
+        if (supportedBridges == null || supportedBridges.isEmpty()) {
+            return this;
+        }
+        String type = supportedBridges.contains("meek") ? "meek_lite" : "obfs4";

Do we still use both? obfs4 now has support for meek, so I assume we simply can do the same. As an example, I have this in my Tor Browser torrc:

ClientTransportPlugin meek_lite,obfs2,obfs3,obfs4,scramblesuit exec ./TorBrowser/Tor/PluggableTransports/obfs4proxy

-        if(hasAddedBridge) useBridges();

I don't see any other useBridges() calls, do we need an unconditional call now?

comment:18 in reply to:  17 Changed 3 weeks ago by sisbell

Replying to sysrqb:

Looks pretty good.

+    @SettingsConfig
+    public TorConfigBuilder bridgesFromSettings() {
+        List<String> supportedBridges = settings.getListOfSupportedBridges();
+        if (supportedBridges == null || supportedBridges.isEmpty()) {
+            return this;
+        }
+        String type = supportedBridges.contains("meek") ? "meek_lite" : "obfs4";

This is done to filter out obfs4 bridges is meek is asked for but based on your information, I don't think we need a filter.

Do we still use both? obfs4 now has support for meek, so I assume we simply can do the same. As an example, I have this in my Tor Browser torrc:

ClientTransportPlugin meek_lite,obfs2,obfs3,obfs4,scramblesuit exec ./TorBrowser/Tor/PluggableTransports/obfs4proxy

-        if(hasAddedBridge) useBridges();

I don't see any other useBridges() calls, do we need an unconditional call now?

I looked through the bridge settings code. Since UseBridges is 0 by default, the following methods needs to be changed to write bridges if enabled.

    @SettingsConfig
    public TorConfigBuilder useBridgesFromSettings() {
        return !settings.hasBridges() ? dontUseBridges() : this;
    }

to

    @SettingsConfig
    public TorConfigBuilder useBridgesFromSettings() {
        return settings.hasBridges() ? useBridges() : this;
    }

There is one other use of useBridges, but I believe I can remove that as well. This MAY cause a problem with Orbot configuration but that can be resolved later as we clean up the code.

I'll make a couple of changes and make a new commit.

Last edited 3 weeks ago by sisbell (previous) (diff)

comment:19 Changed 3 weeks ago by sisbell

I made the following changes:

  • We no longer filter bridges based on type
  • Write out UseBridges, only if explicitly set

The filter part was supporting a filter feature from orbot that I don't think was fully baked since it only supported an either/or meek/obfs option. Filtering is a useful option but we should open an issue for that and come up with a more general solution. This is a breaking change with Orbot.

https://github.com/sisbell/Tor_Onion_Proxy_Library/commit/d05797385127876889d6d7a8e8fc8cc45d8cccdd

comment:20 Changed 12 days ago by pili

Keywords: TorBrowserTeam201912 added; TorBrowserTeam201911 removed

Moving tickets to December

comment:21 Changed 9 days ago by sysrqb

Cc: tbb-team added; sisbell removed
Keywords: TorBrowserTeam201912R added; TorBrowserTeam201912 removed
Owner: changed from tbb-team to sisbell
Reviewer: sysrqb
Status: needs_revisionassigned

comment:22 Changed 9 days ago by sysrqb

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