Address review comments for Tor bridge support

bramble-android/src/androidTest/java/org/briarproject/bramble/plugin/tor/BridgeTest.java

@@ -1,16 +1,17 @@
 package org.briarproject.bramble.plugin.tor;
 
 import android.content.Context;
-import android.support.test.InstrumentationRegistry;
 import android.support.test.runner.AndroidJUnit4;
 
 import org.briarproject.bramble.DaggerIntegrationTestComponent;
 import org.briarproject.bramble.IntegrationTestComponent;
 import org.briarproject.bramble.api.event.EventBus;
 import org.briarproject.bramble.api.plugin.BackoffFactory;
+import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin;
 import org.briarproject.bramble.api.system.Clock;
 import org.briarproject.bramble.api.system.LocationUtils;
 import org.briarproject.bramble.test.BrambleTestCase;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
@@ -24,10 +25,10 @@ import java.util.logging.Logger;
 import javax.inject.Inject;
 import javax.net.SocketFactory;
 
+import static android.support.test.InstrumentationRegistry.getTargetContext;
 import static java.util.Collections.singletonList;
 import static java.util.concurrent.TimeUnit.SECONDS;
-import static org.briarproject.bramble.plugin.tor.TorNetworkMetadata.doBridgesWork;
+import static org.junit.Assert.assertNotNull;
-import static org.briarproject.bramble.plugin.tor.TorNetworkMetadata.isTorProbablyBlocked;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -35,7 +36,6 @@ import static org.junit.Assert.fail;
 @RunWith(AndroidJUnit4.class)
 public class BridgeTest extends BrambleTestCase {
 
-	private final static String BRIDGE_COUNTRY = "VE";
 	private final static long TIMEOUT = SECONDS.toMillis(23);
 
 	private final static Logger LOG =
@@ -48,42 +48,64 @@ public class BridgeTest extends BrambleTestCase {
 	@Inject
 	Clock clock;
 
-	private final TorPluginFactory factory;
+	private final Context appContext = getTargetContext();
-	private TorPlugin plugin;
+	private final CircumventionProvider circumventionProvider;
 	private final List<String> bridges;
-	private int currentBridge = 0;
+	private TorPluginFactory factory;
+	private volatile int currentBridge = 0;
 
 	public BridgeTest() {
+		super();
+		circumventionProvider = new CircumventionProvider() {
+			@Override
+			public boolean isTorProbablyBlocked(String countryCode) {
+				return true;
+			}
+
+			@Override
+			public boolean doBridgesWork(String countryCode) {
+				return true;
+			}
+
+			@Override
+			public List<String> getBridges() {
+				return singletonList(bridges.get(currentBridge));
+			}
+		};
+		bridges = new CircumventionProviderImpl(appContext).getBridges();
+	}
+
+	@Before
+	public void setUp() {
 		IntegrationTestComponent component =
 				DaggerIntegrationTestComponent.builder().build();
 		component.inject(this);
 
 		Executor ioExecutor = Executors.newCachedThreadPool();
 		ScheduledExecutorService scheduler = new ScheduledThreadPoolExecutor(1);
-		Context appContext = InstrumentationRegistry.getTargetContext();
+		LocationUtils locationUtils = () -> "US";
-		LocationUtils locationUtils = () -> BRIDGE_COUNTRY;
 		SocketFactory torSocketFactory = SocketFactory.getDefault();
-		bridges = new BridgeProviderImpl().getBridges(appContext);
+
-		BridgeProvider bridgeProvider =
-				context -> singletonList(bridges.get(currentBridge));
 		factory = new TorPluginFactory(ioExecutor, scheduler, appContext,
 				locationUtils, eventBus, torSocketFactory,
-				backoffFactory, bridgeProvider, clock);
+				backoffFactory, circumventionProvider, clock);
 	}
 
 	@Test
 	public void testBridges() throws Exception {
-		assertTrue(isTorProbablyBlocked(BRIDGE_COUNTRY));
-		assertTrue(doBridgesWork(BRIDGE_COUNTRY));
 		assertTrue(bridges.size() > 0);
 
 		for (int i = 0; i < bridges.size(); i++) {
-			plugin = (TorPlugin) factory.createPlugin(new TorPluginCallBack());
 			testBridge(i);
 		}
 	}
 
 	private void testBridge(int bridge) throws Exception {
+		DuplexPlugin duplexPlugin =
+				factory.createPlugin(new TorPluginCallBack());
+		assertNotNull(duplexPlugin);
+		TorPlugin plugin = (TorPlugin) duplexPlugin;
+
 		currentBridge = bridge;
 		LOG.warning("Testing " + bridges.get(currentBridge));
 		try {

bramble-android/src/main/java/org/briarproject/bramble/BrambleAndroidModule.java

@@ -1,7 +1,9 @@
 package org.briarproject.bramble;
 
-import org.briarproject.bramble.plugin.tor.BridgeProvider;
+import android.app.Application;
-import org.briarproject.bramble.plugin.tor.BridgeProviderImpl;
+
+import org.briarproject.bramble.plugin.tor.CircumventionProvider;
+import org.briarproject.bramble.plugin.tor.CircumventionProviderImpl;
 import org.briarproject.bramble.system.AndroidSystemModule;
 
 import javax.inject.Singleton;
@@ -16,8 +18,8 @@ public class BrambleAndroidModule {
 
 	@Provides
 	@Singleton
-	BridgeProvider provideBridgeProvider() {
+	CircumventionProvider provideCircumventionProvider(Application app) {
-		return new BridgeProviderImpl();
+		return new CircumventionProviderImpl(app.getApplicationContext());
 	}
 
 }

bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/BridgeProvider.java

@@ -1,14 +0,0 @@
-package org.briarproject.bramble.plugin.tor;
-
-import android.content.Context;
-
-import org.briarproject.bramble.api.lifecycle.IoExecutor;
-
-import java.util.List;
-
-public interface BridgeProvider {
-
-	@IoExecutor
-	List<String> getBridges(Context context);
-
-}

bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/BridgeProviderImpl.java

@@ -1,42 +0,0 @@
-package org.briarproject.bramble.plugin.tor;
-
-import android.content.Context;
-import android.content.res.Resources;
-
-import org.briarproject.bramble.api.lifecycle.IoExecutor;
-
-import java.io.InputStream;
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Scanner;
-
-import javax.annotation.Nullable;
-
-public class BridgeProviderImpl implements BridgeProvider {
-
-	private final static String BRIDGE_FILE_NAME = "bridges";
-
-	@Nullable
-	private volatile List<String> bridges = null;
-
-	@Override
-	@IoExecutor
-	public List<String> getBridges(Context context) {
-		if (this.bridges != null) return this.bridges;
-
-		Resources res = context.getResources();
-		int resId = res.getIdentifier(BRIDGE_FILE_NAME, "raw",
-				context.getPackageName());
-		InputStream is = context.getResources().openRawResource(resId);
-		Scanner scanner = new Scanner(is);
-
-		List<String> bridges = new ArrayList<>();
-		while (scanner.hasNextLine()) {
-			String line = scanner.nextLine();
-			if (!line.startsWith("#")) bridges.add(line);
-		}
-		this.bridges = bridges;
-		return bridges;
-	}
-
-}

bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/CircumventionProvider.java

@@ -0,0 +1,30 @@
+package org.briarproject.bramble.plugin.tor;
+
+import org.briarproject.bramble.api.lifecycle.IoExecutor;
+
+import java.util.List;
+
+public interface CircumventionProvider {
+
+	/**
+	 * Countries where Tor is blocked, i.e. vanilla Tor connection won't work.
+	 *
+	 * See https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2
+	 * and https://trac.torproject.org/projects/tor/wiki/doc/OONI/censorshipwiki
+	 */
+	String[] BLOCKED = {"CN", "IR", "EG", "BY", "TR", "SY", "VE"};
+
+	/**
+	 * Countries where vanilla bridge connection are likely to work.
+	 * Should be a subset of {@link #BLOCKED}.
+	 */
+	String[] BRIDGES = { "EG", "BY", "TR", "SY", "VE" };
+
+	boolean isTorProbablyBlocked(String countryCode);
+
+	boolean doBridgesWork(String countryCode);
+
+	@IoExecutor
+	List<String> getBridges();
+
+}

bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/CircumventionProviderImpl.java

@@ -0,0 +1,68 @@
+package org.briarproject.bramble.plugin.tor;
+
+import android.content.Context;
+import android.content.res.Resources;
+
+import org.briarproject.bramble.api.lifecycle.IoExecutor;
+
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Scanner;
+import java.util.Set;
+
+import javax.annotation.Nullable;
+import javax.inject.Inject;
+
+public class CircumventionProviderImpl implements CircumventionProvider {
+
+	private final static String BRIDGE_FILE_NAME = "bridges";
+
+	private final Context ctx;
+	@Nullable
+	private volatile List<String> bridges = null;
+
+	@Inject
+	public CircumventionProviderImpl(Context ctx) {
+		this.ctx = ctx;
+	}
+
+	private static final Set<String> BLOCKED_IN_COUNTRIES =
+			new HashSet<>(Arrays.asList(BLOCKED));
+	private static final Set<String> BRIDGES_WORK_IN_COUNTRIES =
+			new HashSet<>(Arrays.asList(BRIDGES));
+
+	@Override
+	public boolean isTorProbablyBlocked(String countryCode) {
+		return BLOCKED_IN_COUNTRIES.contains(countryCode);
+	}
+
+	@Override
+	public boolean doBridgesWork(String countryCode) {
+		return BRIDGES_WORK_IN_COUNTRIES.contains(countryCode);
+	}
+
+	@Override
+	@IoExecutor
+	public List<String> getBridges() {
+		if (this.bridges != null) return this.bridges;
+
+		Resources res = ctx.getResources();
+		int resId = res.getIdentifier(BRIDGE_FILE_NAME, "raw",
+				ctx.getPackageName());
+		InputStream is = ctx.getResources().openRawResource(resId);
+		Scanner scanner = new Scanner(is);
+
+		List<String> bridges = new ArrayList<>();
+		while (scanner.hasNextLine()) {
+			String line = scanner.nextLine();
+			if (!line.startsWith("#")) bridges.add(line);
+		}
+		scanner.close();
+		this.bridges = bridges;
+		return bridges;
+	}
+
+}

bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/TorNetworkMetadata.java

@@ -1,37 +0,0 @@
-package org.briarproject.bramble.plugin.tor;
-
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Set;
-
-class TorNetworkMetadata {
-
-	/**
-	 * Countries where Tor is blocked, i.e. vanilla Tor connection won't work.
-	 */
-	private static final String[] BLOCKED =
-			{"CN", "IR", "EG", "BY", "TR", "SY", "VE"};
-
-	/**
-	 * Countries where vanilla bridge connection are likely to work.
-	 * Should be a subset of {@link #BLOCKED}.
-	 */
-	private static final String[] BRIDGES = { "EG", "BY", "TR", "SY", "VE" };
-
-	// See https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2
-	// and https://trac.torproject.org/projects/tor/wiki/doc/OONI/censorshipwiki
-	// TODO: get a more complete list
-	private static final Set<String> BLOCKED_IN_COUNTRIES =
-			new HashSet<>(Arrays.asList(BLOCKED));
-	private static final Set<String> BRIDGES_WORK_IN_COUNTRIES =
-			new HashSet<>(Arrays.asList(BRIDGES));
-
-	static boolean isTorProbablyBlocked(String countryCode) {
-		return BLOCKED_IN_COUNTRIES.contains(countryCode);
-	}
-
-	static boolean doBridgesWork(String countryCode) {
-		return BRIDGES_WORK_IN_COUNTRIES.contains(countryCode);
-	}
-
-}

bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/TorPlugin.java

@@ -93,7 +93,6 @@ import static org.briarproject.bramble.api.plugin.TorConstants.PREF_TOR_NETWORK_
 import static org.briarproject.bramble.api.plugin.TorConstants.PREF_TOR_NETWORK_WIFI;
 import static org.briarproject.bramble.api.plugin.TorConstants.PREF_TOR_PORT;
 import static org.briarproject.bramble.api.plugin.TorConstants.PROP_ONION;
-import static org.briarproject.bramble.plugin.tor.TorNetworkMetadata.doBridgesWork;
 import static org.briarproject.bramble.util.LogUtils.logException;
 import static org.briarproject.bramble.util.PrivacyUtils.scrubOnion;
 
@@ -122,7 +121,7 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 	private final Backoff backoff;
 	private final DuplexPluginCallback callback;
 	private final String architecture;
-	private final BridgeProvider bridgeProvider;
+	private final CircumventionProvider circumventionProvider;
 	private final int maxLatency, maxIdleTime, socketTimeout;
 	private final ConnectionStatus connectionStatus;
 	private final File torDirectory, torFile, geoIpFile, configFile;
@@ -142,7 +141,7 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 			Context appContext, LocationUtils locationUtils,
 			SocketFactory torSocketFactory, Clock clock, Backoff backoff,
 			DuplexPluginCallback callback, String architecture,
-			BridgeProvider bridgeProvider, int maxLatency, int maxIdleTime) {
+			CircumventionProvider circumventionProvider, int maxLatency, int maxIdleTime) {
 		this.ioExecutor = ioExecutor;
 		this.scheduler = scheduler;
 		this.appContext = appContext;
@@ -152,7 +151,7 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 		this.backoff = backoff;
 		this.callback = callback;
 		this.architecture = architecture;
-		this.bridgeProvider = bridgeProvider;
+		this.circumventionProvider = circumventionProvider;
 		this.maxLatency = maxLatency;
 		this.maxIdleTime = maxIdleTime;
 		if (maxIdleTime > Integer.MAX_VALUE / 2)
@@ -507,7 +506,7 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 		if (enable) {
 			Collection<String> conf = new ArrayList<>();
 			conf.add("UseBridges 1");
-			conf.addAll(bridgeProvider.getBridges(appContext));
+			conf.addAll(circumventionProvider.getBridges());
 			controlConnection.setConf(conf);
 		} else {
 			controlConnection.setConf("UseBridges", "0");
@@ -678,8 +677,8 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 			boolean online = net != null && net.isConnected();
 			boolean wifi = online && net.getType() == TYPE_WIFI;
 			String country = locationUtils.getCurrentCountry();
-			boolean blocked = TorNetworkMetadata.isTorProbablyBlocked(
+			boolean blocked =
-					country);
+					circumventionProvider.isTorProbablyBlocked(country);
 			Settings s = callback.getSettings();
 			int network = s.getInt(PREF_TOR_NETWORK, PREF_TOR_NETWORK_ALWAYS);
 
@@ -693,8 +692,12 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 				if (!online) {
 					LOG.info("Disabling network, device is offline");
 					enableNetwork(false);
+				} else if (network == PREF_TOR_NETWORK_NEVER
+						|| (network == PREF_TOR_NETWORK_WIFI && !wifi)) {
+					LOG.info("Disabling network due to data setting");
+					enableNetwork(false);
 				} else if (blocked) {
-					if (doBridgesWork(country)) {
+					if (circumventionProvider.doBridgesWork(country)) {
 						LOG.info("Enabling network, using bridges");
 						enableBridges(true);
 						enableNetwork(true);
@@ -702,10 +705,6 @@ class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 						LOG.info("Disabling network, country is blocked");
 						enableNetwork(false);
 					}
-				} else if (network == PREF_TOR_NETWORK_NEVER
-						|| (network == PREF_TOR_NETWORK_WIFI && !wifi)) {
-					LOG.info("Disabling network due to data setting");
-					enableNetwork(false);
 				} else {
 					LOG.info("Enabling network");
 					enableBridges(false);

bramble-android/src/main/java/org/briarproject/bramble/plugin/tor/TorPluginFactory.java

@@ -43,14 +43,14 @@ public class TorPluginFactory implements DuplexPluginFactory {
 	private final EventBus eventBus;
 	private final SocketFactory torSocketFactory;
 	private final BackoffFactory backoffFactory;
-	private final BridgeProvider bridgeProvider;
+	private final CircumventionProvider circumventionProvider;
 	private final Clock clock;
 
 	public TorPluginFactory(Executor ioExecutor,
 			ScheduledExecutorService scheduler, Context appContext,
 			LocationUtils locationUtils, EventBus eventBus,
 			SocketFactory torSocketFactory, BackoffFactory backoffFactory,
-			BridgeProvider bridgeProvider,
+			CircumventionProvider circumventionProvider,
 			Clock clock) {
 		this.ioExecutor = ioExecutor;
 		this.scheduler = scheduler;
@@ -59,7 +59,7 @@ public class TorPluginFactory implements DuplexPluginFactory {
 		this.eventBus = eventBus;
 		this.torSocketFactory = torSocketFactory;
 		this.backoffFactory = backoffFactory;
-		this.bridgeProvider = bridgeProvider;
+		this.circumventionProvider = circumventionProvider;
 		this.clock = clock;
 	}
 
@@ -98,7 +98,7 @@ public class TorPluginFactory implements DuplexPluginFactory {
 				MAX_POLLING_INTERVAL, BACKOFF_BASE);
 		TorPlugin plugin = new TorPlugin(ioExecutor, scheduler, appContext,
 				locationUtils, torSocketFactory, clock, backoff, callback,
-				architecture, bridgeProvider, MAX_LATENCY, MAX_IDLE_TIME);
+				architecture, circumventionProvider, MAX_LATENCY, MAX_IDLE_TIME);
 		eventBus.addListener(plugin);
 		return plugin;
 	}

bramble-android/src/main/res/raw/bridges

@@ -1,4 +1,5 @@
 Bridge 131.252.210.150:8081 0E858AC201BF0F3FA3C462F64844CBFFC7297A42
-#Bridge 128.105.214.161:8081 1E326AAFB3FCB515015250D8FCCC8E37F91A153B
 Bridge 67.205.189.122:8443 12D64D5D44E20169585E7378580C0D33A872AD98
-Bridge 45.32.148.146:8443 0CE016FB2462D8BF179AE71F7D702D09DEAC3F1D
+Bridge 45.32.148.146:8443 0CE016FB2462D8BF179AE71F7D702D09DEAC3F1D
+Bridge 148.251.90.59:7510 019F727CA6DCA6CA5C90B55E477B7D87981E75BC
+#Bridge 128.105.214.161:8081 1E326AAFB3FCB515015250D8FCCC8E37F91A153B

briar-android/src/main/java/org/briarproject/briar/android/AppModule.java

@@ -25,7 +25,7 @@ import org.briarproject.bramble.api.system.LocationUtils;
 import org.briarproject.bramble.api.system.Scheduler;
 import org.briarproject.bramble.plugin.bluetooth.AndroidBluetoothPluginFactory;
 import org.briarproject.bramble.plugin.tcp.AndroidLanTcpPluginFactory;
-import org.briarproject.bramble.plugin.tor.BridgeProvider;
+import org.briarproject.bramble.plugin.tor.CircumventionProvider;
 import org.briarproject.bramble.plugin.tor.TorPluginFactory;
 import org.briarproject.bramble.util.AndroidUtils;
 import org.briarproject.bramble.util.StringUtils;
@@ -100,14 +100,14 @@ public class AppModule {
 			AndroidExecutor androidExecutor, SecureRandom random,
 			SocketFactory torSocketFactory, BackoffFactory backoffFactory,
 			Application app, LocationUtils locationUtils, EventBus eventBus,
-			BridgeProvider bridgeProvider, Clock clock) {
+			CircumventionProvider circumventionProvider, Clock clock) {
 		Context appContext = app.getApplicationContext();
 		DuplexPluginFactory bluetooth =
 				new AndroidBluetoothPluginFactory(ioExecutor, androidExecutor,
 						appContext, random, eventBus, backoffFactory);
 		DuplexPluginFactory tor = new TorPluginFactory(ioExecutor, scheduler,
 				appContext, locationUtils, eventBus, torSocketFactory,
-				backoffFactory, bridgeProvider, clock);
+				backoffFactory, circumventionProvider, clock);
 		DuplexPluginFactory lan = new AndroidLanTcpPluginFactory(ioExecutor,
 				scheduler, backoffFactory, appContext);
 		Collection<DuplexPluginFactory> duplex = asList(bluetooth, tor, lan);