Clean up extra for VPN start intents and try to avoid replacing VPN on autostarts

13 files changed, 71 insertions, 46 deletions

diff --git a/main/src/main/java/de/blinkt/openvpn/LaunchVPN.java b/main/src/main/java/de/blinkt/openvpn/LaunchVPN.java
index 90ea053a..a4a4cc5f 100644
--- a/main/src/main/java/de/blinkt/openvpn/LaunchVPN.java
+++ b/main/src/main/java/de/blinkt/openvpn/LaunchVPN.java
@@ -5,6 +5,8 @@
 
 package de.blinkt.openvpn;
 
+import static de.blinkt.openvpn.core.OpenVPNService.EXTRA_START_REASON;
+
 import android.annotation.SuppressLint;
 import android.annotation.TargetApi;
 import android.app.Activity;
@@ -73,7 +75,7 @@ public class LaunchVPN extends Activity {
     public static final String EXTRA_KEY = "de.blinkt.openvpn.shortcutProfileUUID";
     public static final String EXTRA_NAME = "de.blinkt.openvpn.shortcutProfileName";
     public static final String EXTRA_HIDELOG = "de.blinkt.openvpn.showNoLogWindow";
-    public static final String EXTRA_START_REASON = "de.blinkt.openvpn.start_reason";
+
     public static final String CLEARLOG = "clearlogconnect";
 
 
@@ -255,7 +257,7 @@ public class LaunchVPN extends Activity {
                     if (!mhideLog && showLogWindow)
                         showLogWindow();
                     ProfileManager.updateLRU(this, mSelectedProfile);
-                    VPNLaunchHelper.startOpenVpn(mSelectedProfile, getBaseContext(), mSelectedProfileReason);
+                    VPNLaunchHelper.startOpenVpn(mSelectedProfile, getBaseContext(), mSelectedProfileReason, true);
                     finish();
                 }
             } else if (resultCode == Activity.RESULT_CANCELED) {
diff --git a/main/src/main/java/de/blinkt/openvpn/OnBootReceiver.java b/main/src/main/java/de/blinkt/openvpn/OnBootReceiver.java
index 58c954c9..7033664b 100644
--- a/main/src/main/java/de/blinkt/openvpn/OnBootReceiver.java
+++ b/main/src/main/java/de/blinkt/openvpn/OnBootReceiver.java
@@ -36,6 +36,6 @@ public class OnBootReceiver extends BroadcastReceiver {
 	}
 
 	void launchVPN(VpnProfile profile, Context context) {
-		VPNLaunchHelper.startOpenVpn(profile, context.getApplicationContext(), "on Boot receiver");
+		VPNLaunchHelper.startOpenVpn(profile, context.getApplicationContext(), "on Boot receiver", false);
 	}
 }
diff --git a/main/src/main/java/de/blinkt/openvpn/VpnProfile.java b/main/src/main/java/de/blinkt/openvpn/VpnProfile.java
index 0da09eb0..f105dd56 100644
--- a/main/src/main/java/de/blinkt/openvpn/VpnProfile.java
+++ b/main/src/main/java/de/blinkt/openvpn/VpnProfile.java
@@ -5,6 +5,8 @@
 
 package de.blinkt.openvpn;
 
+import static de.blinkt.openvpn.core.OpenVPNService.EXTRA_DO_NOT_REPLACE_RUNNING_VPN;
+
 import android.annotation.SuppressLint;
 import android.content.Context;
 import android.content.Intent;
@@ -31,20 +33,16 @@ import org.spongycastle.util.io.pem.PemWriter;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.FileReader;
-import java.io.FileWriter;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.Serializable;
 import java.io.StringWriter;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
 import java.security.*;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import java.security.interfaces.RSAPrivateKey;
-import java.security.interfaces.RSAPublicKey;
 import java.security.spec.MGF1ParameterSpec;
 import java.security.spec.PSSParameterSpec;
 import java.util.Collection;
@@ -67,6 +65,8 @@ public class VpnProfile implements Serializable, Cloneable {
     transient public static final long MAX_EMBED_FILE_SIZE = 2048 * 1024; // 2048kB
     // Don't change this, not all parts of the program use this constant
     public static final String EXTRA_PROFILEUUID = "de.blinkt.openvpn.profileUUID";
+    public static final String EXTRA_PROFILE_VERSION = "de.blinkt.openvpn.profileVersion";
+
     public static final String INLINE_TAG = "[[INLINE]]";
     public static final String DISPLAYNAME_TAG = "[[NAME]]";
     public static final int MAXLOGLEVEL = 4;
@@ -816,14 +816,14 @@ public class VpnProfile implements Serializable, Cloneable {
         cfg.close();
     }
 
-    public Intent getStartServiceIntent(Context context, String startReason) {
-        String prefix = context.getPackageName();
-
+    public Intent getStartServiceIntent(Context context, String startReason, boolean replace_running_vpn) {
         Intent intent = new Intent(context, OpenVPNService.class);
-        intent.putExtra(prefix + ".profileUUID", mUuid.toString());
-        intent.putExtra(prefix + ".profileVersion", mVersion);
+        intent.putExtra(EXTRA_PROFILEUUID, mUuid.toString());
+        intent.putExtra(EXTRA_PROFILE_VERSION, mVersion);
         if (startReason != null)
-            intent.putExtra(prefix + ".startReason", startReason);
+            intent.putExtra(OpenVPNService.EXTRA_START_REASON, startReason);
+        if (!replace_running_vpn)
+            intent.putExtra(EXTRA_DO_NOT_REPLACE_RUNNING_VPN, true);
         return intent;
     }
 
diff --git a/main/src/main/java/de/blinkt/openvpn/activities/DisconnectVPN.java b/main/src/main/java/de/blinkt/openvpn/activities/DisconnectVPN.java
index 6cc170fa..4ccf5bd3 100644
--- a/main/src/main/java/de/blinkt/openvpn/activities/DisconnectVPN.java
+++ b/main/src/main/java/de/blinkt/openvpn/activities/DisconnectVPN.java
@@ -86,7 +86,7 @@ public class DisconnectVPN extends Activity implements DialogInterface.OnClickLi
         } else if (which == DialogInterface.BUTTON_NEUTRAL) {
             Intent intent = new Intent(this, LaunchVPN.class);
             intent.putExtra(LaunchVPN.EXTRA_KEY, VpnStatus.getLastConnectedVPNProfile());
-            intent.putExtra(LaunchVPN.EXTRA_START_REASON, "Reconnect button pressed.");
+            intent.putExtra(OpenVPNService.EXTRA_START_REASON, "Reconnect button pressed.");
             intent.setAction(Intent.ACTION_MAIN);
             startActivity(intent);
         }
diff --git a/main/src/main/java/de/blinkt/openvpn/api/ExternalOpenVPNService.java b/main/src/main/java/de/blinkt/openvpn/api/ExternalOpenVPNService.java
index ab71f00b..2f1f0788 100644
--- a/main/src/main/java/de/blinkt/openvpn/api/ExternalOpenVPNService.java
+++ b/main/src/main/java/de/blinkt/openvpn/api/ExternalOpenVPNService.java
@@ -6,7 +6,6 @@
 package de.blinkt.openvpn.api;

 

 import android.annotation.TargetApi;

-import android.app.PendingIntent;

 import android.app.Service;

 import android.content.BroadcastReceiver;

 import android.content.ComponentName;

@@ -14,9 +13,6 @@ import android.content.Context;
 import android.content.Intent;

 import android.content.IntentFilter;

 import android.content.ServiceConnection;

-import android.content.pm.ApplicationInfo;

-import android.content.pm.PackageManager;

-import android.content.pm.PackageManager.NameNotFoundException;

 import android.net.VpnService;

 import android.os.Binder;

 import android.os.Build;

@@ -146,11 +142,11 @@ public class ExternalOpenVPNService extends Service implements StateListener {
                 shortVPNIntent.setClass(getBaseContext(), de.blinkt.openvpn.LaunchVPN.class);

                 shortVPNIntent.putExtra(de.blinkt.openvpn.LaunchVPN.EXTRA_KEY, vp.getUUIDString());

                 shortVPNIntent.putExtra(de.blinkt.openvpn.LaunchVPN.EXTRA_HIDELOG, true);

-                shortVPNIntent.putExtra(de.blinkt.openvpn.LaunchVPN.EXTRA_START_REASON, startReason);

+                shortVPNIntent.putExtra(de.blinkt.openvpn.core.OpenVPNService.EXTRA_START_REASON, startReason);

                 shortVPNIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);

                 startActivity(shortVPNIntent);

             } else {

-                VPNLaunchHelper.startOpenVpn(vp, getBaseContext(), startReason);

+                VPNLaunchHelper.startOpenVpn(vp, getBaseContext(), startReason, true);

             }

 

         }

diff --git a/main/src/main/java/de/blinkt/openvpn/api/RemoteAction.java b/main/src/main/java/de/blinkt/openvpn/api/RemoteAction.java
index 22110ad0..55322e7f 100644
--- a/main/src/main/java/de/blinkt/openvpn/api/RemoteAction.java
+++ b/main/src/main/java/de/blinkt/openvpn/api/RemoteAction.java
@@ -93,7 +93,7 @@ public class RemoteAction extends Activity {
                 } else {
                     Intent startVPN = new Intent(this, LaunchVPN.class);
                     startVPN.putExtra(LaunchVPN.EXTRA_KEY, profile.getUUID().toString());
-                    startVPN.putExtra(LaunchVPN.EXTRA_START_REASON, ".api.ConnectVPN call");
+                    startVPN.putExtra(OpenVPNService.EXTRA_START_REASON, ".api.ConnectVPN call");
                     startVPN.setAction(Intent.ACTION_MAIN);
                     startActivity(startVPN);
                 }
diff --git a/main/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java b/main/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
index 983b63f3..c555fd27 100644
--- a/main/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
+++ b/main/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
@@ -6,6 +6,8 @@
 package de.blinkt.openvpn.core;
 
 import static android.content.Intent.FLAG_ACTIVITY_NEW_TASK;
+import static de.blinkt.openvpn.VpnProfile.EXTRA_PROFILEUUID;
+import static de.blinkt.openvpn.VpnProfile.EXTRA_PROFILE_VERSION;
 import static de.blinkt.openvpn.core.ConnectionStatus.LEVEL_CONNECTED;
 import static de.blinkt.openvpn.core.ConnectionStatus.LEVEL_WAITING_FOR_USER_INPUT;
 import static de.blinkt.openvpn.core.NetworkSpace.IpAddress;
@@ -70,6 +72,11 @@ public class OpenVPNService extends VpnService implements StateListener, Callbac
     public static final String START_SERVICE = "de.blinkt.openvpn.START_SERVICE";
     public static final String START_SERVICE_STICKY = "de.blinkt.openvpn.START_SERVICE_STICKY";
     public static final String ALWAYS_SHOW_NOTIFICATION = "de.blinkt.openvpn.NOTIFICATION_ALWAYS_VISIBLE";
+
+    public static final String EXTRA_DO_NOT_REPLACE_RUNNING_VPN = "de.blinkt.openvpn.DO_NOT_REPLACE_RUNNING_VPN";
+
+    public static final String EXTRA_START_REASON = "de.blinkt.openvpn.startReason";
+
     public static final String DISCONNECT_VPN = "de.blinkt.openvpn.DISCONNECT_VPN";
     public static final String NOTIFICATION_CHANNEL_BG_ID = "openvpn_bg";
     public static final String NOTIFICATION_CHANNEL_NEWSTATUS_ID = "openvpn_newstat";
@@ -86,6 +93,7 @@ public class OpenVPNService extends VpnService implements StateListener, Callbac
     private static final int PRIORITY_MAX = 2;
     private static boolean mNotificationAlwaysVisible = false;
 
+
     static class TunConfig {
         private final Vector<String> mDnslist = new Vector<>();
         private final NetworkSpace mRoutes = new NetworkSpace();
@@ -554,45 +562,46 @@ public class OpenVPNService extends VpnService implements StateListener, Callbac
 
     private VpnProfile fetchVPNProfile(Intent intent)
     {
+        VpnProfile vpnProfile = null;
         String startReason;
-        if (intent != null && intent.hasExtra(getPackageName() + ".profileUUID")) {
-            String profileUUID = intent.getStringExtra(getPackageName() + ".profileUUID");
-            int profileVersion = intent.getIntExtra(getPackageName() + ".profileVersion", 0);
-            startReason = intent.getStringExtra(getPackageName() + ".startReason");
+        if (intent != null && intent.hasExtra(EXTRA_PROFILEUUID)) {
+            String profileUUID = intent.getStringExtra(EXTRA_PROFILEUUID);
+            int profileVersion = intent.getIntExtra(EXTRA_PROFILE_VERSION, 0);
+            startReason = intent.getStringExtra(EXTRA_START_REASON);
             if (startReason == null)
                 startReason = "(unknown)";
             // Try for 10s to get current version of the profile
-            mProfile = ProfileManager.get(this, profileUUID, profileVersion, 100);
+            vpnProfile = ProfileManager.get(this, profileUUID, profileVersion, 100);
             if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N_MR1) {
-                updateShortCutUsage(mProfile);
+                updateShortCutUsage(vpnProfile);
             }
 
         } else {
             /* The intent is null when we are set as always-on or the service has been restarted. */
-            mProfile = ProfileManager.getLastConnectedProfile(this);
+            vpnProfile = ProfileManager.getLastConnectedProfile(this);
             startReason = "Using last connected profile (started with null intent, always-on or restart after crash)";
             VpnStatus.logInfo(R.string.service_restarted);
 
             /* Got no profile, just stop */
-            if (mProfile == null) {
+            if (vpnProfile == null) {
                 startReason = "could not get last connected profile, using default (started with null intent, always-on or restart after crash)";
 
                 Log.d("OpenVPN", "Got no last connected profile on null intent. Assuming always on.");
-                mProfile = ProfileManager.getAlwaysOnVPN(this);
+                vpnProfile = ProfileManager.getAlwaysOnVPN(this);
 
 
-                if (mProfile == null) {
+                if (vpnProfile == null) {
                     return null;
                 }
             }
             /* Do the asynchronous keychain certificate stuff */
-            mProfile.checkForRestart(this);
+            vpnProfile.checkForRestart(this);
         }
         String name = "(null)";
-        if (mProfile != null)
-            name = mProfile.getName();
+        if (vpnProfile != null)
+            name = vpnProfile.getName();
         VpnStatus.logDebug(String.format("Fetched VPN profile (%s) triggered by %s", name, startReason));
-        return mProfile;
+        return vpnProfile;
     }
 
     private boolean checkVPNPermission(VpnProfile startprofile) {
@@ -608,7 +617,7 @@ public class OpenVPNService extends VpnService implements StateListener, Callbac
 
         Intent launchVPNIntent = new Intent(this, LaunchVPN.class);
         launchVPNIntent.putExtra(LaunchVPN.EXTRA_KEY, startprofile.getUUIDString());
-        launchVPNIntent.putExtra(LaunchVPN.EXTRA_START_REASON, "OpenService lacks permission");
+        launchVPNIntent.putExtra(EXTRA_START_REASON, "OpenService lacks permission");
         launchVPNIntent.putExtra(de.blinkt.openvpn.LaunchVPN.EXTRA_HIDELOG, true);
         launchVPNIntent.addFlags(FLAG_ACTIVITY_NEW_TASK);
         launchVPNIntent.setAction(Intent.ACTION_MAIN);
@@ -633,8 +642,23 @@ public class OpenVPNService extends VpnService implements StateListener, Callbac
         if (!checkVPNPermission(vp))
             return;
 
-        ProfileManager.setConnectedVpnProfile(this, mProfile);
-        VpnStatus.setConnectedVPNProfile(mProfile.getUUIDString());
+        boolean noReplaceRequested =  (intent != null) && intent.getBooleanExtra(EXTRA_DO_NOT_REPLACE_RUNNING_VPN, false);
+
+
+        /* we get an empty start request or explicitly get told to not replace the VPN then ignore
+         * a start request. This avoids OnBootreciver, Always and user quickly clicking to have
+         * weird race conditions
+         */
+        if (mProfile != null && mProfile == vp && (intent == null || noReplaceRequested))
+        {
+            /* we do not want to replace the running VPN */
+            VpnStatus.logInfo("VPN already running. Ignoring request to start VPN");
+            return;
+        }
+
+        mProfile = vp;
+        ProfileManager.setConnectedVpnProfile(this, vp);
+        VpnStatus.setConnectedVPNProfile(vp.getUUIDString());
         keepVPNAlive.scheduleKeepVPNAliveJobService(this, vp);
 
         String nativeLibraryDirectory = getApplicationInfo().nativeLibraryDir;
diff --git a/main/src/main/java/de/blinkt/openvpn/core/VPNLaunchHelper.java b/main/src/main/java/de/blinkt/openvpn/core/VPNLaunchHelper.java
index bc04bc5e..0784361c 100644
--- a/main/src/main/java/de/blinkt/openvpn/core/VPNLaunchHelper.java
+++ b/main/src/main/java/de/blinkt/openvpn/core/VPNLaunchHelper.java
@@ -100,8 +100,8 @@ public class VPNLaunchHelper {
     }
 
 
-    public static void startOpenVpn(VpnProfile startprofile, Context context, String startReason) {
-        Intent startVPN = startprofile.getStartServiceIntent(context, startReason);
+    public static void startOpenVpn(VpnProfile startprofile, Context context, String startReason, boolean replace_running_vpn) {
+        Intent startVPN = startprofile.getStartServiceIntent(context, startReason, replace_running_vpn);
         if (startVPN != null) {
             if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O)
                 //noinspection NewApi
diff --git a/main/src/main/java/de/blinkt/openvpn/core/keepVPNAlive.java b/main/src/main/java/de/blinkt/openvpn/core/keepVPNAlive.java
index b4264aba..130dc68c 100644
--- a/main/src/main/java/de/blinkt/openvpn/core/keepVPNAlive.java
+++ b/main/src/main/java/de/blinkt/openvpn/core/keepVPNAlive.java
@@ -49,7 +49,7 @@ public class keepVPNAlive extends JobService implements VpnStatus.StateListener
                 unscheduleKeepVPNAliveJobService(this);
                 return false;
             }
-            VPNLaunchHelper.startOpenVpn(vp, getApplicationContext(), "VPN keep alive Job");
+            VPNLaunchHelper.startOpenVpn(vp, getApplicationContext(), "VPN keep alive Job", false);
         } else {
             VpnStatus.logDebug("Keepalive service called but VPN still connected.");
         }
diff --git a/main/src/ui/java/de/blinkt/openvpn/OpenVPNTileService.java b/main/src/ui/java/de/blinkt/openvpn/OpenVPNTileService.java
index 94c1f1db..09593047 100644
--- a/main/src/ui/java/de/blinkt/openvpn/OpenVPNTileService.java
+++ b/main/src/ui/java/de/blinkt/openvpn/OpenVPNTileService.java
@@ -85,7 +85,7 @@ public class OpenVPNTileService extends TileService implements VpnStatus.StateLi
     @SuppressLint("Override")
     @TargetApi(Build.VERSION_CODES.N)
     void launchVPN(VpnProfile profile, Context context) {
-          VPNLaunchHelper.startOpenVpn(profile, getBaseContext(), "QuickTile");
+          VPNLaunchHelper.startOpenVpn(profile, getBaseContext(), "QuickTile", true);
     }
 
     @TargetApi(Build.VERSION_CODES.N)
diff --git a/main/src/ui/java/de/blinkt/openvpn/activities/CreateShortcuts.java b/main/src/ui/java/de/blinkt/openvpn/activities/CreateShortcuts.java
index 82455895..3b9312e5 100644
--- a/main/src/ui/java/de/blinkt/openvpn/activities/CreateShortcuts.java
+++ b/main/src/ui/java/de/blinkt/openvpn/activities/CreateShortcuts.java
@@ -5,6 +5,8 @@
 
 package de.blinkt.openvpn.activities;
 
+import static de.blinkt.openvpn.core.OpenVPNService.EXTRA_START_REASON;
+
 import android.app.ListActivity;
 import android.content.Intent;
 import android.os.Bundle;
@@ -126,7 +128,7 @@ public class CreateShortcuts extends ListActivity implements OnItemClickListener
         Intent shortcutIntent = new Intent(Intent.ACTION_MAIN);
         shortcutIntent.setClass(this, LaunchVPN.class);
         shortcutIntent.putExtra(LaunchVPN.EXTRA_KEY, profile.getUUID().toString());
-        shortcutIntent.putExtra(LaunchVPN.EXTRA_START_REASON, "shortcut");
+        shortcutIntent.putExtra(EXTRA_START_REASON, "shortcut");
 
         // Then, set up the container intent (the response to the caller)
 
diff --git a/main/src/ui/java/de/blinkt/openvpn/fragments/LogFragment.java b/main/src/ui/java/de/blinkt/openvpn/fragments/LogFragment.java
index c5c48b0e..9ce65316 100644
--- a/main/src/ui/java/de/blinkt/openvpn/fragments/LogFragment.java
+++ b/main/src/ui/java/de/blinkt/openvpn/fragments/LogFragment.java
@@ -538,7 +538,7 @@ public class LogFragment extends ListFragment implements StateListener, SeekBar.
                     (dialog1, which) -> {
                         Intent intent = new Intent(getActivity(), LaunchVPN.class);
                         intent.putExtra(LaunchVPN.EXTRA_KEY, profile.getUUIDString());
-                        intent.putExtra(LaunchVPN.EXTRA_START_REASON, "restart from logwindow");
+                        intent.putExtra(OpenVPNService.EXTRA_START_REASON, "restart from logwindow");
                         intent.setAction(Intent.ACTION_MAIN);
                         startActivity(intent);
                     });
diff --git a/main/src/ui/java/de/blinkt/openvpn/fragments/VPNProfileList.java b/main/src/ui/java/de/blinkt/openvpn/fragments/VPNProfileList.java
index fa9438cb..5b4c736f 100644
--- a/main/src/ui/java/de/blinkt/openvpn/fragments/VPNProfileList.java
+++ b/main/src/ui/java/de/blinkt/openvpn/fragments/VPNProfileList.java
@@ -68,6 +68,7 @@ import de.blinkt.openvpn.core.VpnStatus;
 import static de.blinkt.openvpn.core.ConnectionStatus.LEVEL_WAITING_FOR_USER_INPUT;
 import static de.blinkt.openvpn.core.OpenVPNService.DISCONNECT_VPN;
 import static de.blinkt.openvpn.core.OpenVPNService.EXTRA_CHALLENGE_TXT;
+import static de.blinkt.openvpn.core.OpenVPNService.EXTRA_START_REASON;
 
 
 public class VPNProfileList extends ListFragment implements OnClickListener, VpnStatus.StateListener {
@@ -242,7 +243,7 @@ public class VPNProfileList extends ListFragment implements OnClickListener, Vpn
         shortcutIntent.setClass(requireContext(), LaunchVPN.class);
         shortcutIntent.putExtra(LaunchVPN.EXTRA_KEY, profile.getUUID().toString());
         shortcutIntent.setAction(Intent.ACTION_MAIN);
-        shortcutIntent.putExtra(LaunchVPN.EXTRA_START_REASON, "shortcut");
+        shortcutIntent.putExtra(EXTRA_START_REASON, "shortcut");
         shortcutIntent.putExtra("EXTRA_HIDELOG", true);
 
         PersistableBundle versionExtras = new PersistableBundle();
@@ -563,7 +564,7 @@ public class VPNProfileList extends ListFragment implements OnClickListener, Vpn
 
         Intent intent = new Intent(getActivity(), LaunchVPN.class);
         intent.putExtra(LaunchVPN.EXTRA_KEY, profile.getUUID().toString());
-        intent.putExtra(LaunchVPN.EXTRA_START_REASON, "main profile list");
+        intent.putExtra(EXTRA_START_REASON, "main profile list");
         intent.setAction(Intent.ACTION_MAIN);
         startActivity(intent);
     }