Prevent concurrent calls to Tor API

build.gradle

@@ -5,7 +5,7 @@ buildscript {
         google()
     }
     dependencies {
-        classpath 'com.android.tools.build:gradle:3.3.2'
+        classpath 'com.android.tools.build:gradle:3.3.3'
     }
 }
 

sampletorapp/src/main/AndroidManifest.xml

@@ -13,7 +13,8 @@
             android:supportsRtl="true"
             android:theme="@style/AppTheme"
             tools:ignore="GoogleAppIndexingWarning">
-        <activity android:name=".MainActivity">
+        <activity android:name=".MainActivity"
+            android:exported="true">
             <intent-filter>
                 <action android:name="android.intent.action.MAIN"/>
 

tor-android-binary/build.gradle

@@ -47,9 +47,10 @@ dependencies {
     api 'androidx.localbroadcastmanager:localbroadcastmanager:1.0.0'
     api 'info.guardianproject:jtorctl:0.4.2.5'
 
-    androidTestImplementation 'androidx.test:core:1.2.0'
-    androidTestImplementation 'androidx.test:runner:1.2.0'
-    androidTestImplementation 'androidx.test:rules:1.2.0'
+    androidTestImplementation 'androidx.test:core:1.4.0'
+    androidTestImplementation 'androidx.test:runner:1.4.0'
+    androidTestImplementation 'androidx.test:rules:1.4.0'
+    androidTestImplementation 'androidx.test.ext:junit:1.1.3'
     androidTestImplementation 'info.guardianproject.netcipher:netcipher:2.1.0'
     androidTestImplementation 'commons-io:commons-io:2.6'
     androidTestImplementation 'commons-net:commons-net:3.6'

tor-android-binary/src/androidTest/java/org/torproject/jni/TorServiceDoubleUnbindTest.java

@@ -0,0 +1,89 @@
+package org.torproject.jni;
+
+import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
+import static org.torproject.jni.TorService.ACTION_STATUS;
+import static org.torproject.jni.TorService.EXTRA_STATUS;
+import static org.torproject.jni.TorService.STATUS_OFF;
+import static org.torproject.jni.TorService.STATUS_ON;
+
+import android.content.BroadcastReceiver;
+import android.content.Context;
+import android.content.Intent;
+import android.content.IntentFilter;
+import android.content.ServiceConnection;
+import android.os.Handler;
+import android.os.HandlerThread;
+import android.os.IBinder;
+import android.os.Looper;
+import android.util.Log;
+
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+import androidx.test.rule.ServiceTestRule;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+@RunWith(AndroidJUnit4.class)
+public class TorServiceDoubleUnbindTest {
+
+    public static final String TAG = "TorServiceTest";
+
+    @Rule
+    public final ServiceTestRule serviceRule = ServiceTestRule.withTimeout(120L, TimeUnit.SECONDS);
+
+    private Context context;
+
+    @Before
+    public void setUp() {
+        context = getInstrumentation().getTargetContext();
+    }
+
+    /**
+     * Test using {@link ServiceTestRule#bindService(Intent, ServiceConnection, int)}
+     * for reliable start/stop when testing.
+     */
+    @Test
+    public void testBindService() throws Exception {
+        startAndUnbind();
+        startAndUnbind();
+    }
+
+    private void startAndUnbind() throws Exception {
+        final CountDownLatch startedLatch = new CountDownLatch(1);
+        final CountDownLatch stoppedLatch = new CountDownLatch(1);
+        BroadcastReceiver receiver = new BroadcastReceiver() {
+            @Override
+            public void onReceive(Context context, Intent intent) {
+                String status = intent.getStringExtra(EXTRA_STATUS);
+                Log.i(TAG, "receiver.onReceive: " + status + " " + intent);
+                if (STATUS_ON.equals(status)) {
+                    startedLatch.countDown();
+                } else if (STATUS_OFF.equals(status)) {
+                    stoppedLatch.countDown();
+                }
+            }
+        };
+        // run the BroadcastReceiver in its own thread
+        HandlerThread handlerThread = new HandlerThread(receiver.getClass().getSimpleName());
+        handlerThread.start();
+        Looper looper = handlerThread.getLooper();
+        Handler handler = new Handler(looper);
+        context.registerReceiver(receiver, new IntentFilter(ACTION_STATUS), null, handler);
+
+        Intent serviceIntent = new Intent(context, TorService.class);
+        IBinder binder = serviceRule.bindService(serviceIntent);
+        TorService torService = ((TorService.LocalBinder) binder).getService();
+        startedLatch.await();
+
+        serviceRule.unbindService();
+        stoppedLatch.await();
+
+        context.unregisterReceiver(receiver);
+    }
+
+}

tor-android-binary/src/androidTest/java/org/torproject/jni/TorServiceTest.java

@@ -155,7 +155,7 @@ public void onReceive(Context context, Intent intent) {
         assertTrue("NetCipher.getHttpURLConnection should use Tor",
                 NetCipher.isNetCipherGetHttpURLConnectionUsingTor());
 
-        URLConnection c = NetCipher.getHttpsURLConnection("https://www.nytimes3xbfgragh.onion/");
+        URLConnection c = NetCipher.getHttpsURLConnection("https://www.nytimesn7cgmftshazwhfgzm37qxb44r64ytbb2dj3x62d2lljsciiyd.onion/");
         Log.i(TAG, "Content-Length: " + c.getContentLength());
         Log.i(TAG, "CONTENTS: " + new String(IOUtils.readFully(c.getInputStream(), 100)));
 
@@ -174,6 +174,7 @@ public void testOverridingDefaultsTorrc() throws TimeoutException, InterruptedEx
         FileUtils.write(torrc, dnsPort + " " + testValue + "\n");
 
         final CountDownLatch startedLatch = new CountDownLatch(1);
+        final CountDownLatch stoppedLatch = new CountDownLatch(1);
         BroadcastReceiver receiver = new BroadcastReceiver() {
             @Override
             public void onReceive(Context context, Intent intent) {
@@ -185,6 +186,8 @@ public void onReceive(Context context, Intent intent) {
                 Log.i(TAG, "receiver.onReceive: " + status + " " + intent);
                 if (TorService.STATUS_ON.equals(status)) {
                     startedLatch.countDown();
+                } else if (TorService.STATUS_OFF.equals(status)) {
+                    stoppedLatch.countDown();
                 }
             }
         };
@@ -203,12 +206,14 @@ public void onReceive(Context context, Intent intent) {
         assertEquals(testValue, getConf(torService.getTorControlConnection(), dnsPort));
 
         serviceRule.unbindService();
+        stoppedLatch.await();
     }
 
     @Test
     public void testDownloadingLargeFile() throws TimeoutException, InterruptedException, IOException {
         Assume.assumeTrue("Only works on Android 7.1.2 or higher", Build.VERSION.SDK_INT >= 24);
         final CountDownLatch startedLatch = new CountDownLatch(1);
+        final CountDownLatch stoppedLatch = new CountDownLatch(1);
         BroadcastReceiver receiver = new BroadcastReceiver() {
             @Override
             public void onReceive(Context context, Intent intent) {
@@ -220,6 +225,8 @@ public void onReceive(Context context, Intent intent) {
                 Log.i(TAG, "receiver.onReceive: " + status + " " + intent);
                 if (TorService.STATUS_ON.equals(status)) {
                     startedLatch.countDown();
+                } else if (TorService.STATUS_OFF.equals(status)) {
+                    stoppedLatch.countDown();
                 }
             }
         };
@@ -242,7 +249,7 @@ public void onReceive(Context context, Intent intent) {
         // ~350MB
         //URL url = new URL("http://dl.google.com/android/ndk/android-ndk-r9b-linux-x86_64.tar.bz2");
         // ~3MB
-        URL url = new URL("http://dl.google.com/android/repository/platform-tools_r24-linux.zip");
+        URL url = new URL("https://dl.google.com/android/repository/platform-tools_r24-linux.zip");
         // 55KB
         //URL url = new URL("https://jcenter.bintray.com/com/android/tools/build/gradle/2.2.3/gradle-2.2.3.jar");
         HttpURLConnection connection = NetCipher.getHttpURLConnection(url);
@@ -252,6 +259,7 @@ public void onReceive(Context context, Intent intent) {
         IOUtils.copy(connection.getInputStream(), new FileWriter(new File("/dev/null")));
 
         serviceRule.unbindService();
+        stoppedLatch.await();
     }
 
     private static boolean canConnectToSocket(String host, int port) {

tor-android-binary/src/main/java/org/torproject/jni/TorService.java

@@ -7,6 +7,7 @@
 import android.os.FileObserver;
 import android.os.IBinder;
 import android.os.Process;
+import android.util.Log;
 
 import net.freehaven.tor.control.RawEventListener;
 import net.freehaven.tor.control.TorControlCommands;
@@ -24,6 +25,7 @@
 import java.util.Arrays;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.ReentrantLock;
 
 import androidx.annotation.Nullable;
 import androidx.localbroadcastmanager.content.LocalBroadcastManager;
@@ -167,6 +169,12 @@ private static File getAppTorServiceDataDir(Context context) {
 
     private TorControlConnection torControlConnection;
 
+    /**
+     * This lock must be acquired before calling createTorConfiguration() and
+     * held until mainConfigurationFree() has been called.
+     */
+    private static final ReentrantLock runLock = new ReentrantLock();
+
     private native String apiGetProviderVersion();
 
     private native boolean createTorConfiguration();
@@ -314,50 +322,61 @@ public void run() {
                         httpTunnelPort = Integer.toString(8118);
                     }
 
-                    createTorConfiguration();
-                    ArrayList<String> lines = new ArrayList<>(Arrays.asList("tor", "--verify-config", // must always be here
-                            "--RunAsDaemon", "0",
-                            "-f", getTorrc(context).getAbsolutePath(),
-                            "--defaults-torrc", getDefaultsTorrc(context).getAbsolutePath(),
-                            "--ignore-missing-torrc",
-                            "--SyslogIdentityTag", TAG,
-                            "--CacheDirectory", new File(getCacheDir(), TAG).getAbsolutePath(),
-                            "--DataDirectory", getAppTorServiceDataDir(context).getAbsolutePath(),
-                            "--ControlSocket", getControlSocket(context).getAbsolutePath(),
-                            "--CookieAuthentication", "0",
-                            "--SOCKSPort", socksPort,
-                            "--HTTPTunnelPort", httpTunnelPort,
-
-                            // can be moved to ControlPort messages
-                            "--LogMessageDomains", "1",
-                            "--TruncateLogFile", "1"
-                    ));
-                    String[] verifyLines = lines.toArray(new String[0]);
-                    if (!mainConfigurationSetCommandLine(verifyLines)) {
-                        throw new IllegalArgumentException("Setting command line failed: " + Arrays.toString(verifyLines));
-                    }
-                    int result = runMain(); // run verify
-                    if (result != 0) {
-                        throw new IllegalArgumentException("Bad command flags: " + Arrays.toString(verifyLines));
+                    if (runLock.isLocked()) {
+                        Log.i(TAG, "Waiting for lock");
                     }
+                    runLock.lock();
+                    Log.i(TAG, "Acquired lock");
+                    try {
+                        createTorConfiguration();
+                        ArrayList<String> lines = new ArrayList<>(Arrays.asList("tor", "--verify-config", // must always be here
+                                "--RunAsDaemon", "0",
+                                "-f", getTorrc(context).getAbsolutePath(),
+                                "--defaults-torrc", getDefaultsTorrc(context).getAbsolutePath(),
+                                "--ignore-missing-torrc",
+                                "--SyslogIdentityTag", TAG,
+                                "--CacheDirectory", new File(getCacheDir(), TAG).getAbsolutePath(),
+                                "--DataDirectory", getAppTorServiceDataDir(context).getAbsolutePath(),
+                                "--ControlSocket", getControlSocket(context).getAbsolutePath(),
+                                "--CookieAuthentication", "0",
+                                "--SOCKSPort", socksPort,
+                                "--HTTPTunnelPort", httpTunnelPort,
+
+                                // can be moved to ControlPort messages
+                                "--LogMessageDomains", "1",
+                                "--TruncateLogFile", "1"
+                        ));
+                        String[] verifyLines = lines.toArray(new String[0]);
+                        if (!mainConfigurationSetCommandLine(verifyLines)) {
+                            throw new IllegalArgumentException("Setting command line failed: " + Arrays.toString(verifyLines));
+                        }
+                        int result = runMain(); // run verify
+                        if (result != 0) {
+                            throw new IllegalArgumentException("Bad command flags: " + Arrays.toString(verifyLines));
+                        }
 
-                    controlPortThreadStarted = new CountDownLatch(1);
-                    controlPortThread.start();
-                    controlPortThreadStarted.await();
+                        controlPortThreadStarted = new CountDownLatch(1);
+                        controlPortThread.start();
+                        controlPortThreadStarted.await();
 
-                    String[] runLines = new String[lines.size() - 1];
-                    runLines[0] = "tor";
-                    for (int i = 2; i < lines.size(); i++) {
-                        runLines[i - 1] = lines.get(i);
-                    }
-                    if (!mainConfigurationSetCommandLine(runLines)) {
-                        throw new IllegalArgumentException("Setting command line failed: " + Arrays.toString(runLines));
-                    }
-                    if (!mainConfigurationSetupControlSocket()) {
-                        throw new IllegalStateException("Setting up ControlPort failed!");
-                    }
-                    if (runMain() != 0) {
-                        throw new IllegalStateException("Tor could not start!");
+                        String[] runLines = new String[lines.size() - 1];
+                        runLines[0] = "tor";
+                        for (int i = 2; i < lines.size(); i++) {
+                            runLines[i - 1] = lines.get(i);
+                        }
+                        if (!mainConfigurationSetCommandLine(runLines)) {
+                            throw new IllegalArgumentException("Setting command line failed: " + Arrays.toString(runLines));
+                        }
+                        if (!mainConfigurationSetupControlSocket()) {
+                            throw new IllegalStateException("Setting up ControlPort failed!");
+                        }
+                        if (runMain() != 0) {
+                            throw new IllegalStateException("Tor could not start!");
+                        }
+                    } finally {
+                        mainConfigurationFree();
+                        Log.i(TAG, "Releasing lock");
+                        runLock.unlock();
                     }
 
                 } catch (IllegalStateException | IllegalArgumentException | InterruptedException e) {
@@ -377,7 +396,6 @@ public void onDestroy() {
             torControlConnection.removeRawEventListener(startedEventListener);
         }
         shutdownTor(TorControlCommands.SIGNAL_SHUTDOWN);
-        mainConfigurationFree();
         broadcastStatus(TorService.this, STATUS_OFF);
     }