implement tiomeout error handling + UI if tor doesn't start within 3 minutes

7 files changed, 85 insertions, 20 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 4afeb26e..65810861 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderAPI.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderAPI.java
@@ -79,6 +79,7 @@ public class ProviderAPI extends JobIntentService implements ProviderApiManagerB
             RECEIVER_KEY = "receiver",
             ERRORS = "errors",
             ERRORID = "errorId",
+            INITIAL_ACTION = "initalAction",
             BACKEND_ERROR_KEY = "error",
             BACKEND_ERROR_MESSAGE = "message",
             USER_MESSAGE = "userMessage",
@@ -100,7 +101,8 @@ public class ProviderAPI extends JobIntentService implements ProviderApiManagerB
             CORRECTLY_UPDATED_INVALID_VPN_CERTIFICATE = 15,
             INCORRECTLY_UPDATED_INVALID_VPN_CERTIFICATE = 16,
             CORRECTLY_DOWNLOADED_GEOIP_JSON = 17,
-            INCORRECTLY_DOWNLOADED_GEOIP_JSON = 18;
+            INCORRECTLY_DOWNLOADED_GEOIP_JSON = 18,
+            TOR_TIMEOUT = 19;
 
     ProviderApiManager providerApiManager;
     private volatile TorServiceConnection torServiceConnection;
@@ -137,6 +139,10 @@ public class ProviderAPI extends JobIntentService implements ProviderApiManagerB
     @Override
     public void onDestroy() {
         super.onDestroy();
+        closeTorServiceConnection();
+    }
+
+    private void closeTorServiceConnection() {
         if (torServiceConnection != null) {
             torServiceConnection.close();
             torServiceConnection = null;
@@ -168,6 +174,12 @@ public class ProviderAPI extends JobIntentService implements ProviderApiManagerB
         }
     }
 
+    @Override
+    public void stopTorService() throws IllegalStateException {
+        closeTorServiceConnection();
+        Intent stopIntent = new Intent(this, TorService.class);
+        stopService(stopIntent);
+    }
 
     @Override
     public int getTorHttpTunnelPort() {
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 074cc121..555eb21d 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiManagerBase.java
@@ -114,6 +114,7 @@ import static se.leap.bitmaskclient.providersetup.ProviderAPI.INCORRECTLY_DOWNLO
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.INCORRECTLY_DOWNLOADED_GEOIP_JSON;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.INCORRECTLY_DOWNLOADED_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.INCORRECTLY_UPDATED_INVALID_VPN_CERTIFICATE;
+import static se.leap.bitmaskclient.providersetup.ProviderAPI.INITIAL_ACTION;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.LOGOUT_FAILED;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.LOG_IN;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.LOG_OUT;
@@ -126,12 +127,14 @@ import static se.leap.bitmaskclient.providersetup.ProviderAPI.SIGN_UP;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.SUCCESSFUL_LOGIN;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.SUCCESSFUL_LOGOUT;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.SUCCESSFUL_SIGNUP;
+import static se.leap.bitmaskclient.providersetup.ProviderAPI.TOR_TIMEOUT;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.UPDATE_INVALID_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.UPDATE_PROVIDER_DETAILS;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.USER_MESSAGE;
 import static se.leap.bitmaskclient.providersetup.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.ERROR_CERTIFICATE_PINNING;
 import static se.leap.bitmaskclient.providersetup.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.ERROR_CORRUPTED_PROVIDER_JSON;
 import static se.leap.bitmaskclient.providersetup.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.ERROR_INVALID_CERTIFICATE;
+import static se.leap.bitmaskclient.providersetup.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.ERROR_TOR_TIMEOUT;
 import static se.leap.bitmaskclient.tor.TorStatusObservable.TorStatus.ON;
 import static se.leap.bitmaskclient.tor.TorStatusObservable.getProxyPort;
 
@@ -147,6 +150,7 @@ public abstract class ProviderApiManagerBase {
     public interface ProviderApiServiceCallback {
         void broadcastEvent(Intent intent);
         void startTorService() throws InterruptedException, IllegalStateException;
+        void stopTorService() throws IllegalStateException;
         int getTorHttpTunnelPort();
         boolean isConnectedToWifi();
     }
@@ -188,10 +192,16 @@ public abstract class ProviderApiManagerBase {
 
          try {
             startTorProxy();
-        } catch (InterruptedException | IllegalStateException | TimeoutException e) {
+        } catch (InterruptedException | IllegalStateException e) {
             e.printStackTrace();
             return;
-        }
+        } catch (TimeoutException e) {
+             serviceCallback.stopTorService();
+             Bundle result = new Bundle();
+             setErrorResult(result, action, R.string.error_tor_timeout, ERROR_TOR_TIMEOUT.toString());
+             sendToReceiverOrBroadcast(receiver, TOR_TIMEOUT, result, provider);
+             return;
+         }
 
         Bundle result = new Bundle();
         switch (action) {
@@ -348,10 +358,11 @@ public abstract class ProviderApiManagerBase {
         }
     }
 
-    private void addErrorMessageToJson(JSONObject jsonObject, String errorMessage, String errorId) {
+    private void addErrorMessageToJson(JSONObject jsonObject, String initialAction, String errorMessage, String errorId) {
         try {
             jsonObject.put(ERRORS, errorMessage);
-            jsonObject.put(ERRORID, errorId);
+            jsonObject.putOpt(ERRORID, errorId);
+            jsonObject.putOpt(INITIAL_ACTION, initialAction);
         } catch (JSONException e) {
             e.printStackTrace();
         }
@@ -908,13 +919,13 @@ public abstract class ProviderApiManagerBase {
     }
 
     Bundle setErrorResult(Bundle result, int errorMessageId, String errorId) {
+        return setErrorResult(result, null, errorMessageId, errorId);
+    }
+
+    Bundle setErrorResult(Bundle result, String initialAction, int errorMessageId, String errorId) {
         JSONObject errorJson = new JSONObject();
         String errorMessage = getProviderFormattedString(resources, errorMessageId);
-        if (errorId != null) {
-            addErrorMessageToJson(errorJson, errorMessage, errorId);
-        } else {
-            addErrorMessageToJson(errorJson, errorMessage);
-        }
+        addErrorMessageToJson(errorJson, initialAction, errorMessage, errorId);
         VpnStatus.logWarning("[API] error: " + errorMessage);
         result.putString(ERRORS, errorJson.toString());
         result.putBoolean(BROADCAST_RESULT_KEY, false);
diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiSetupBroadcastReceiver.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiSetupBroadcastReceiver.java
index 710aee0f..712ee172 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiSetupBroadcastReceiver.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderApiSetupBroadcastReceiver.java
@@ -67,8 +67,9 @@ public class ProviderApiSetupBroadcastReceiver extends BroadcastReceiver {
                     case ProviderAPI.PROVIDER_OK:
                         setupInterface.handleProviderSetUp(handledProvider);
                         break;
+                    case ProviderAPI.TOR_TIMEOUT:
                     case ProviderAPI.PROVIDER_NOK:
-                        setupInterface.handleProviderSetupFailed(resultData);
+                        setupInterface.handleError(resultData);
                         break;
                     case ProviderAPI.CORRECTLY_DOWNLOADED_VPN_CERTIFICATE:
                         setupInterface.handleCorrectlyDownloadedCertificate(handledProvider);
diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderSetupFailedDialog.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderSetupFailedDialog.java
index 947d1182..50c063b2 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderSetupFailedDialog.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderSetupFailedDialog.java
@@ -21,19 +21,25 @@ import android.app.Dialog;
 import android.content.Context;
 import android.content.DialogInterface;
 import android.os.Bundle;
+
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.fragment.app.DialogFragment;
 
 import org.json.JSONObject;
 
-import se.leap.bitmaskclient.base.models.Provider;
 import se.leap.bitmaskclient.R;
+import se.leap.bitmaskclient.base.models.Provider;
+import se.leap.bitmaskclient.base.utils.PreferenceHelper;
 
-import static se.leap.bitmaskclient.providersetup.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.DEFAULT;
-import static se.leap.bitmaskclient.providersetup.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.valueOf;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.ERRORID;
 import static se.leap.bitmaskclient.providersetup.ProviderAPI.ERRORS;
+import static se.leap.bitmaskclient.providersetup.ProviderAPI.INITIAL_ACTION;
+import static se.leap.bitmaskclient.providersetup.ProviderAPI.SET_UP_PROVIDER;
+import static se.leap.bitmaskclient.providersetup.ProviderAPI.UPDATE_INVALID_VPN_CERTIFICATE;
+import static se.leap.bitmaskclient.providersetup.ProviderAPI.UPDATE_PROVIDER_DETAILS;
+import static se.leap.bitmaskclient.providersetup.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.DEFAULT;
+import static se.leap.bitmaskclient.providersetup.ProviderSetupFailedDialog.DOWNLOAD_ERRORS.valueOf;
 
 /**
  * Implements a dialog to show why a download failed.
@@ -43,11 +49,13 @@ import static se.leap.bitmaskclient.providersetup.ProviderAPI.ERRORS;
 public class ProviderSetupFailedDialog extends DialogFragment {
 
     public static String TAG = "downloaded_failed_dialog";
-    private final static String KEY_PROVIDER = "key provider";
-    private final static String KEY_REASON_TO_FAIL = "key reason to fail";
-    private final static String KEY_DOWNLOAD_ERROR = "key download error";
+    private final static String KEY_PROVIDER = "key_provider";
+    private final static String KEY_REASON_TO_FAIL = "key_reason_to_fail";
+    private final static String KEY_DOWNLOAD_ERROR = "key_download_error";
+    private final static String KEY_INITAL_ACTION = "key_inital_action";
     private String reasonToFail;
     private DOWNLOAD_ERRORS downloadError = DEFAULT;
+    private String initialAction;
 
     private Provider provider;
 
@@ -59,7 +67,8 @@ public class ProviderSetupFailedDialog extends DialogFragment {
         ERROR_CORRUPTED_PROVIDER_JSON,
         ERROR_INVALID_CERTIFICATE,
         ERROR_CERTIFICATE_PINNING,
-        ERROR_NEW_URL_NO_VPN_PROVIDER
+        ERROR_NEW_URL_NO_VPN_PROVIDER,
+        ERROR_TOR_TIMEOUT
     }
 
     /**
@@ -91,6 +100,10 @@ public class ProviderSetupFailedDialog extends DialogFragment {
             } else if (testNewURL) {
                 dialogFragment.downloadError = DOWNLOAD_ERRORS.ERROR_NEW_URL_NO_VPN_PROVIDER;
             }
+
+            if (errorJson.has(INITIAL_ACTION)) {
+                dialogFragment.initialAction = errorJson.getString(INITIAL_ACTION);
+            }
         } catch (Exception e) {
             e.printStackTrace();
             dialogFragment.reasonToFail = dialogFragment.getString(R.string.error_io_exception_user_message);
@@ -125,6 +138,14 @@ public class ProviderSetupFailedDialog extends DialogFragment {
                 builder.setPositiveButton(R.string.retry, (dialog, id)
                         -> interfaceWithConfigurationWizard.addAndSelectNewProvider(provider.getMainUrlString()));
                 break;
+            case ERROR_TOR_TIMEOUT:
+                builder.setPositiveButton(R.string.retry, (dialog, id) -> {
+                    handleTorTimeoutError();
+                });
+                builder.setNeutralButton(R.string.retry_unobfuscated, ((dialog, id) -> {
+                    PreferenceHelper.useBridges(getContext(), false);
+                    handleTorTimeoutError();
+                }));
             default:
                 builder.setPositiveButton(R.string.retry, (dialog, id)
                         -> interfaceWithConfigurationWizard.retrySetUpProvider(provider));
@@ -135,6 +156,20 @@ public class ProviderSetupFailedDialog extends DialogFragment {
         return builder.create();
     }
 
+    private void handleTorTimeoutError() {
+        switch (initialAction) {
+            case SET_UP_PROVIDER:
+            case UPDATE_PROVIDER_DETAILS:
+                interfaceWithConfigurationWizard.retrySetUpProvider(provider);
+                break;
+            case UPDATE_INVALID_VPN_CERTIFICATE:
+                ProviderAPICommand.execute(getContext(), UPDATE_INVALID_VPN_CERTIFICATE, provider);
+                break;
+            default:
+                break;
+        }
+    }
+
     public interface DownloadFailedDialogInterface {
         void retrySetUpProvider(@NonNull Provider provider);
 
@@ -170,6 +205,7 @@ public class ProviderSetupFailedDialog extends DialogFragment {
         outState.putParcelable(KEY_PROVIDER, provider);
         outState.putString(KEY_REASON_TO_FAIL, reasonToFail);
         outState.putString(KEY_DOWNLOAD_ERROR, downloadError.toString());
+        outState.putString(KEY_INITAL_ACTION, initialAction);
     }
 
     private void restoreFromSavedInstance(Bundle savedInstanceState) {
@@ -185,5 +221,8 @@ public class ProviderSetupFailedDialog extends DialogFragment {
         if (savedInstanceState.containsKey(KEY_DOWNLOAD_ERROR)) {
             this.downloadError = valueOf(savedInstanceState.getString(KEY_DOWNLOAD_ERROR));
         }
+        if (savedInstanceState.containsKey(KEY_INITAL_ACTION)) {
+            this.initialAction = savedInstanceState.getString(KEY_INITAL_ACTION);
+        }
     }
 }
diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderSetupInterface.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderSetupInterface.java
index 5b5c94b4..0c60a3ce 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderSetupInterface.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/ProviderSetupInterface.java
@@ -35,7 +35,7 @@ public interface ProviderSetupInterface {
     }
 
     void handleProviderSetUp(Provider provider);
-    void handleProviderSetupFailed(Bundle resultData);
+    void handleError(Bundle resultData);
     void handleCorrectlyDownloadedCertificate(Provider provider);
     void handleIncorrectlyDownloadedCertificate();
     Provider getProvider();
diff --git a/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/ProviderSetupBaseActivity.java b/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/ProviderSetupBaseActivity.java
index 6395c7ae..7628dfcb 100644
--- a/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/ProviderSetupBaseActivity.java
+++ b/app/src/main/java/se/leap/bitmaskclient/providersetup/activities/ProviderSetupBaseActivity.java
@@ -145,7 +145,7 @@ public abstract class ProviderSetupBaseActivity extends ConfigWizardBaseActivity
     }
 
     @Override
-    public void handleProviderSetupFailed(Bundle resultData) {
+    public void handleError(Bundle resultData) {
         reasonToFail = resultData.getString(ERRORS);
         showDownloadFailedDialog();
     }
diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml
index dbaf0f0b..dbe386b3 100644
--- a/app/src/main/res/values/strings.xml
+++ b/app/src/main/res/values/strings.xml
@@ -176,6 +176,8 @@
   <string name="log_done">Running</string>
   <string name="channel_name_tor_service">%s Bridges Service</string>
   <string name="channel_description_tor_service">Informs about usage of bridges while configuring %s.</string>
+  <string name="error_tor_timeout">Starting bridges failed. Do you want to retry or continue with an unobfuscated secure connection to configure %s?</string>
+  <string name="retry_unobfuscated">Retry unobfuscated</string>
 
 
 </resources>