Ensure tor state is set to OFF after snowflake completely stopped.

5 files changed, 98 insertions, 46 deletions

diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderAPI.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderAPI.java
index c8cc786a..4afeb26e 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderAPI.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderAPI.java
@@ -154,7 +154,7 @@ public class ProviderAPI extends JobIntentService implements ProviderApiManagerB
     }
 
     @Override
-    public void startTorService() {
+    public void startTorService() throws InterruptedException, IllegalStateException {
         initTorServiceConnection(this);
         Intent torServiceIntent = new Intent(this, TorService.class);
 
@@ -171,12 +171,16 @@ public class ProviderAPI extends JobIntentService implements ProviderApiManagerB
 
     @Override
     public int getTorHttpTunnelPort() {
-        initTorServiceConnection(this);
-        if (torServiceConnection != null) {
-            int tunnelPort = torServiceConnection.torService.getHttpTunnelPort();
-            torServiceConnection.close();
-            torServiceConnection = null;
-            return tunnelPort;
+        try {
+            initTorServiceConnection(this);
+            if (torServiceConnection != null) {
+                int tunnelPort = torServiceConnection.torService.getHttpTunnelPort();
+                torServiceConnection.close();
+                torServiceConnection = null;
+                return tunnelPort;
+            }
+        } catch (InterruptedException | IllegalStateException e) {
+            e.printStackTrace();
         }
 
         return -1;
@@ -195,18 +199,14 @@ public class ProviderAPI extends JobIntentService implements ProviderApiManagerB
      * @throws InterruptedException  thrown if thread gets interrupted
      * @throws IllegalStateException thrown if this method was not called from a background thread
      */
-    private void initTorServiceConnection(Context context) {
+    private void initTorServiceConnection(Context context) throws InterruptedException, IllegalStateException {
         if (PreferenceHelper.getUseBridges(context)) {
-            try {
-                if (torServiceConnection == null) {
-                    Log.d(TAG, "serviceConnection is still null");
-                    if (!TorService.hasClientTransportPlugin()) {
-                        TorService.setClientTransportPlugin(new ClientTransportPlugin(context.getApplicationContext()));
-                    }
-                    torServiceConnection = new TorServiceConnection(context);
+            if (torServiceConnection == null) {
+                Log.d(TAG, "serviceConnection is still null");
+                if (!TorService.hasClientTransportPlugin()) {
+                    TorService.setClientTransportPlugin(new ClientTransportPlugin(context.getApplicationContext()));
                 }
-            } catch (InterruptedException | IllegalStateException e) {
-                e.printStackTrace();
+                torServiceConnection = new TorServiceConnection(context);
             }
         }
     }
diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
index d646b4bb..074cc121 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
@@ -48,11 +48,7 @@ import java.security.interfaces.RSAPrivateKey;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.NoSuchElementException;
-import java.util.Observer;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.net.ssl.SSLHandshakeException;
 import javax.net.ssl.SSLPeerUnverifiedException;
@@ -150,7 +146,7 @@ public abstract class ProviderApiManagerBase {
 
     public interface ProviderApiServiceCallback {
         void broadcastEvent(Intent intent);
-        void startTorService();
+        void startTorService() throws InterruptedException, IllegalStateException;
         int getTorHttpTunnelPort();
         boolean isConnectedToWifi();
     }
@@ -190,10 +186,9 @@ public abstract class ProviderApiManagerBase {
             return;
         }
 
-        // uncomment for testing --v
          try {
             startTorProxy();
-        } catch (InterruptedException | TimeoutException e) {
+        } catch (InterruptedException | IllegalStateException | TimeoutException e) {
             e.printStackTrace();
             return;
         }
@@ -295,7 +290,7 @@ public abstract class ProviderApiManagerBase {
         }
     }
 
-    protected boolean startTorProxy() throws InterruptedException, TimeoutException {
+    protected boolean startTorProxy() throws InterruptedException, IllegalStateException, TimeoutException {
         if (PreferenceHelper.getUseBridges(preferences) &&
                 EipStatus.getInstance().isDisconnected() &&
                 serviceCallback.isConnectedToWifi()
@@ -303,7 +298,7 @@ public abstract class ProviderApiManagerBase {
             serviceCallback.startTorService();
             waitForTorCircuits();
             if (TorStatusObservable.isCancelled()) {
-                throw new InterruptedException("cancelled Tor setup");
+                throw new InterruptedException("Cancelled Tor setup.");
             }
             int port = serviceCallback.getTorHttpTunnelPort();
             TorStatusObservable.setProxyPort(port);
@@ -316,20 +311,11 @@ public abstract class ProviderApiManagerBase {
         if (TorStatusObservable.getStatus() == ON) {
             return;
         }
-        CountDownLatch countDownLatch = new CountDownLatch(1);
-        AtomicBoolean stopWaiting = new AtomicBoolean(false);
-        Observer observer = (o, arg) -> {
-            if (TorStatusObservable.getStatus() == ON || TorStatusObservable.isCancelled()) {
-                stopWaiting.set(true);
-                countDownLatch.countDown();
-            }
-        };
-        TorStatusObservable.getInstance().addObserver(observer);
-        countDownLatch.await(180, TimeUnit.SECONDS);
-        TorStatusObservable.getInstance().deleteObserver(observer);
-        if (!stopWaiting.get()) {
-            throw new TimeoutException("Timeout reached");
-        }
+        TorStatusObservable.waitUntil(this::isTorOnOrCancelled, 180);
+    }
+
+    private boolean isTorOnOrCancelled() {
+        return TorStatusObservable.getStatus() == ON || TorStatusObservable.isCancelled();
     }
 
     void resetProviderDetails(Provider provider) {
diff --git a/app/src/main/java/se/leap/bitmaskclient/tor/ClientTransportPlugin.java b/app/src/main/java/se/leap/bitmaskclient/tor/ClientTransportPlugin.java
index 5fc604e5..5707cde0 100644
--- a/app/src/main/java/se/leap/bitmaskclient/tor/ClientTransportPlugin.java
+++ b/app/src/main/java/se/leap/bitmaskclient/tor/ClientTransportPlugin.java
@@ -4,6 +4,7 @@ import android.content.Context;
 import android.os.FileObserver;
 import android.util.Log;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
 import org.torproject.jni.ClientTransportPluginInterface;
@@ -18,6 +19,7 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.Scanner;
 import java.util.Vector;
+import java.util.concurrent.TimeoutException;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -96,10 +98,19 @@ public class ClientTransportPlugin implements ClientTransportPluginInterface {
     @Override
     public void stop() {
         IPtProxy.stopSnowflake();
+         try {
+            TorStatusObservable.waitUntil(this::isSnowflakeOff, 10);
+        } catch (InterruptedException | TimeoutException e) {
+            e.printStackTrace();
+        }
         snowflakePort = -1;
         logFileObserver.stopWatching();
     }
 
+    private boolean isSnowflakeOff() {
+        return TorStatusObservable.getSnowflakeStatus() == TorStatusObservable.SnowflakeStatus.OFF;
+    }
+
     @Override
     public String getTorrc() {
         return "UseBridges 1\n" +
diff --git a/app/src/main/java/se/leap/bitmaskclient/tor/TorStatusObservable.java b/app/src/main/java/se/leap/bitmaskclient/tor/TorStatusObservable.java
index f4f0ff11..eca0f555 100644
--- a/app/src/main/java/se/leap/bitmaskclient/tor/TorStatusObservable.java
+++ b/app/src/main/java/se/leap/bitmaskclient/tor/TorStatusObservable.java
@@ -9,7 +9,12 @@ import androidx.annotation.Nullable;
 import org.torproject.jni.TorService;
 
 import java.util.Observable;
+import java.util.Observer;
 import java.util.Vector;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import se.leap.bitmaskclient.R;
 
@@ -17,21 +22,34 @@ public class TorStatusObservable extends Observable {
 
     private static final String TAG = TorStatusObservable.class.getSimpleName();
 
+    public interface StatusCondition {
+        boolean met();
+    }
+
     public enum TorStatus {
         ON,
         OFF,
         STARTING,
-        STOPPING,
-       // UNKOWN
+        STOPPING
+    }
+
+    public enum SnowflakeStatus {
+        ON,
+        OFF
     }
 
+    // indicates if the user has cancelled Tor, the actual TorStatus can still be different until
+    // the TorService has sent the shutdown signal
     private boolean cancelled = false;
 
     public static final String LOG_TAG_TOR = "[TOR]";
     public static final String LOG_TAG_SNOWFLAKE = "[SNOWFLAKE]";
+    public static final String SNOWFLAKE_STARTED = "--- Starting Snowflake Client ---";
+    public static final String SNOWFLAKE_STOPPED = "---- SnowflakeConn: end collecting snowflakes ---";
 
     private static TorStatusObservable instance;
     private TorStatus status = TorStatus.OFF;
+    private SnowflakeStatus snowflakeStatus = SnowflakeStatus.OFF;
     private final TorNotificationManager torNotificationManager;
     private String lastError;
     private String lastTorLog;
@@ -55,13 +73,50 @@ public class TorStatusObservable extends Observable {
         return getInstance().status;
     }
 
+    public static SnowflakeStatus getSnowflakeStatus() {
+        return getInstance().snowflakeStatus;
+    }
+
+    /**
+     * Waits on the current Thread until a certain tor/snowflake status has been reached
+     * @param condition defines when wait should be interrupted
+     * @param timeout Timout in seconds
+     * @throws InterruptedException if thread was interrupted while waiting
+     * @throws TimeoutException thrown if timeout was reached
+     */
+    public static void waitUntil(StatusCondition condition, int timeout) throws InterruptedException, TimeoutException {
+        CountDownLatch countDownLatch = new CountDownLatch(1);
+        final AtomicBoolean conditionMet = new AtomicBoolean(false);
+        Observer observer = (o, arg) -> {
+              if (condition.met()) {
+                  countDownLatch.countDown();
+                  conditionMet.set(true);
+              }
+        };
+        if (condition.met()) {
+            // no need to wait
+            return;
+        }
+        getInstance().addObserver(observer);
+        countDownLatch.await(timeout, TimeUnit.SECONDS);
+        getInstance().deleteObserver(observer);
+        if (!conditionMet.get()) {
+            throw new TimeoutException("Status condition not met within " + timeout + "s.");
+        }
+    }
+
     public static void logSnowflakeMessage(Context context, String message) {
-        Log.d(LOG_TAG_SNOWFLAKE, message);
         addLog(message);
         getInstance().lastSnowflakeLog = message;
         if (getInstance().status != TorStatus.OFF) {
             getInstance().torNotificationManager.buildTorNotification(context, getStringForCurrentStatus(context), getNotificationLog(), getBootstrapProgress());
         }
+        //TODO: implement proper state signalling in IPtProxy
+        if (SNOWFLAKE_STARTED.equals(message)) {
+            getInstance().snowflakeStatus = SnowflakeStatus.ON;
+        } else if (SNOWFLAKE_STOPPED.equals(message)) {
+            getInstance().snowflakeStatus = SnowflakeStatus.OFF;
+        }
         instance.setChanged();
         instance.notifyObservers();
     }
@@ -206,7 +261,7 @@ public class TorStatusObservable extends Observable {
         getInstance().notifyObservers();
 
         Intent intent = new Intent(context, TorService.class);
-        boolean stopped = context.stopService(intent);
+        context.stopService(intent);
     }
 
     public static boolean isCancelled() {
diff --git a/app/src/production/java/se/leap/bitmaskclient/providersetup/ProviderApiManager.java b/app/src/production/java/se/leap/bitmaskclient/providersetup/ProviderApiManager.java
index c60e21dc..ceb3be3e 100644
--- a/app/src/production/java/se/leap/bitmaskclient/providersetup/ProviderApiManager.java
+++ b/app/src/production/java/se/leap/bitmaskclient/providersetup/ProviderApiManager.java
@@ -336,7 +336,7 @@ public class ProviderApiManager extends ProviderApiManagerBase {
             ) {
                 return downloadWithCommercialCA(stringUrl, provider, 1);
             }
-        } catch (InterruptedException | TimeoutException e) {
+        } catch (InterruptedException | IllegalStateException | TimeoutException e) {
             e.printStackTrace();
         }
         return responseString;
@@ -380,7 +380,7 @@ public class ProviderApiManager extends ProviderApiManagerBase {
             ) {
                 return downloadFromUrlWithProviderCA(urlString, provider, 1);
             }
-        } catch (InterruptedException | TimeoutException e) {
+        } catch (InterruptedException | IllegalStateException | TimeoutException e) {
             e.printStackTrace();
         }