reduce su calls, add logging, save if su exists and was allowed to preferences

4 files changed, 133 insertions, 76 deletions

diff --git a/app/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java b/app/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
index 724fd0fd..766dc925 100644
--- a/app/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
+++ b/app/src/main/java/de/blinkt/openvpn/core/OpenVPNService.java
@@ -518,7 +518,7 @@ public class OpenVPNService extends VpnService implements StateListener, Callbac
         super.onCreate();
         notificationManager = new VpnNotificationManager(this, this);
         notificationManager.createOpenVpnNotificationChannel();
-        firewallHelper = new FirewallHelper();
+        firewallHelper = new FirewallHelper(this);
     }
 
     @Override
diff --git a/app/src/main/java/se/leap/bitmaskclient/Constants.java b/app/src/main/java/se/leap/bitmaskclient/Constants.java
index d3c09f08..0cbf82e1 100644
--- a/app/src/main/java/se/leap/bitmaskclient/Constants.java
+++ b/app/src/main/java/se/leap/bitmaskclient/Constants.java
@@ -15,6 +15,7 @@ public interface Constants {
     String LAST_USED_PROFILE = "last_used_profile";
     String EXCLUDED_APPS = "excluded_apps";
     String USE_PLUGGABLE_TRANSPORTS = "usePluggableTransports";
+    String SU_PERMISSION = "su_permission";
 
 
      //////////////////////////////////////////////
diff --git a/app/src/main/java/se/leap/bitmaskclient/utils/FirewallHelper.java b/app/src/main/java/se/leap/bitmaskclient/utils/FirewallHelper.java
index 43a5296f..8b80f1f0 100644
--- a/app/src/main/java/se/leap/bitmaskclient/utils/FirewallHelper.java
+++ b/app/src/main/java/se/leap/bitmaskclient/utils/FirewallHelper.java
@@ -16,17 +16,20 @@ package se.leap.bitmaskclient.utils;
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
+import android.content.Context;
 import android.os.AsyncTask;
-import android.text.TextUtils;
 import android.util.Log;
 
 import java.lang.ref.WeakReference;
 
+import de.blinkt.openvpn.core.VpnStatus;
+
 import static se.leap.bitmaskclient.utils.Cmd.runBlockingCmd;
 
 interface FirewallCallback {
     void onFirewallStarted(boolean success);
     void onFirewallStopped(boolean success);
+    void onSuRequested(boolean success);
 }
 
 
@@ -34,64 +37,92 @@ public class FirewallHelper implements FirewallCallback {
     private static String BITMASK_CHAIN = "bitmask_fw";
     private static final String TAG = FirewallHelper.class.getSimpleName();
 
+    private Context context;
+
+    public FirewallHelper(Context context) {
+        this.context = context;
+    }
+
 
     @Override
     public void onFirewallStarted(boolean success) {
-        Log.d(TAG, "Firewall started " + success);
+        if (success) {
+            VpnStatus.logInfo("[FIREWALL] custom rules established");
+        } else {
+            VpnStatus.logError("[FIREWALL] could not establish custom rules.");
+        }
     }
 
     @Override
     public void onFirewallStopped(boolean success) {
-        Log.d(TAG, "Firewall stopped " + success);
+        if (success) {
+            VpnStatus.logInfo("[FIREWALL] custom rules deleted");
+        } else {
+            VpnStatus.logError("[FIREWALL] could not delete custom rules");
+        }
     }
 
+    @Override
+    public void onSuRequested(boolean success) {
+        PreferenceHelper.setSuPermission(context, success);
+        if (!success) {
+            VpnStatus.logError("[FIREWALL] Bitmask needs root permission to execute custom firewall rules.");
+        }
+    }
 
-    static class StartFirewallTask extends  AsyncTask<Void, Boolean, Boolean> {
+
+    private static class StartFirewallTask extends  AsyncTask<Void, Boolean, Boolean> {
 
        WeakReference<FirewallCallback> callbackWeakReference;
 
-        public StartFirewallTask(FirewallCallback callback) {
+        StartFirewallTask(FirewallCallback callback) {
             callbackWeakReference = new WeakReference<>(callback);
         }
 
         @Override
         protected Boolean doInBackground(Void... voids) {
-            if (requestSU()) {
-                Log.d(TAG, "su acquired");
-                StringBuilder log = new StringBuilder();
-                String[] bitmaskChain = new String[]{
-                        "su",
-                        "ip6tables --list " + BITMASK_CHAIN };
+            StringBuilder log = new StringBuilder();
+            String[] bitmaskChain = new String[]{
+                    "su",
+                    "id",
+                    "ip6tables --list " + BITMASK_CHAIN };
+
+
+            try {
+                boolean hasBitmaskChain = runBlockingCmd(bitmaskChain, log) == 0;
+                boolean allowSu = log.toString().contains("uid=0");
                 try {
-                    boolean hasBitmaskChain = runBlockingCmd(bitmaskChain, log) == 0;
+                    callbackWeakReference.get().onSuRequested(allowSu);
+                    Thread.sleep(1000);
+                } catch (Exception e) {
+                    //ignore
+                }
+
+                boolean success;
+                log = new StringBuilder();
+                if (!hasBitmaskChain) {
+                    String[] createChainAndRules = new String[]{
+                            "su",
+                            "ip6tables --new-chain " + BITMASK_CHAIN,
+                            "ip6tables --insert OUTPUT --jump " + BITMASK_CHAIN,
+                            "ip6tables --append " + BITMASK_CHAIN + " -p tcp --jump REJECT",
+                            "ip6tables --append " + BITMASK_CHAIN + " -p udp --jump REJECT"
+                    };
+                    success = runBlockingCmd(createChainAndRules, log) == 0;
+                    Log.d(TAG, "added " + BITMASK_CHAIN + " to ip6tables: " + success);
                     Log.d(TAG, log.toString());
-                    if (!hasBitmaskChain) {
-                        String[] createChain = new String[]{
-                                "su",
-                                "ip6tables --new-chain " + BITMASK_CHAIN,
-                                "ip6tables --insert OUTPUT --jump " + BITMASK_CHAIN };
-                        log = new StringBuilder();
-                        int success = runBlockingCmd(createChain, log);
-                        Log.d(TAG, "added " + BITMASK_CHAIN + " to ip6tables: " + success);
-                        Log.d(TAG, log.toString());
-                        if (success != 0) {
-                            return false;
-                        }
-                    }
-
-                    log = new StringBuilder();
+                    return success;
+                } else {
                     String[] addRules = new String[] {
                             "su",
                             "ip6tables --append " + BITMASK_CHAIN + " -p tcp --jump REJECT",
                             "ip6tables --append " + BITMASK_CHAIN + " -p udp --jump REJECT" };
-                    boolean successResult = runBlockingCmd(addRules, log) == 0;
-                    Log.d(TAG, log.toString());
-                    return successResult;
-                } catch (Exception e) {
-                    e.printStackTrace();
-                    Log.e(TAG, log.toString());
+                    return runBlockingCmd(addRules, log) == 0;
                 }
-            };
+            } catch (Exception e) {
+                e.printStackTrace();
+                Log.e(TAG, log.toString());
+            }
             return false;
         }
 
@@ -105,29 +136,49 @@ public class FirewallHelper implements FirewallCallback {
         }
     }
 
-    static class ShutdownFirewallTask extends AsyncTask<Void, Boolean, Boolean> {
+    private static class ShutdownFirewallTask extends AsyncTask<Void, Boolean, Boolean> {
+
+        WeakReference<FirewallCallback> callbackWeakReference;
+
+        ShutdownFirewallTask(FirewallCallback callback) {
+            callbackWeakReference = new WeakReference<>(callback);
+        }
 
         @Override
         protected Boolean doInBackground(Void... voids) {
+            boolean success;
+            StringBuilder log = new StringBuilder();
+            String[] deleteChain = new String[]{
+                    "su",
+                    "id",
+                    "ip6tables --delete OUTPUT --jump " + BITMASK_CHAIN,
+                    "ip6tables --flush " + BITMASK_CHAIN,
+                    "ip6tables --delete-chain " + BITMASK_CHAIN
+            };
+            try {
+                success = runBlockingCmd(deleteChain, log) == 0;
+            } catch (Exception e) {
+                e.printStackTrace();
+                Log.e(TAG, log.toString());
+                return false;
+            }
 
-            if (requestSU()) {
-                StringBuilder log = new StringBuilder();
-                String[] deleteChain = new String[]{
-                        "su",
-                        "ip6tables --delete OUTPUT --jump " + BITMASK_CHAIN,
-                        "ip6tables --flush " + BITMASK_CHAIN,
-                        "ip6tables --delete-chain " + BITMASK_CHAIN
-                };
-                try {
-                    runBlockingCmd(deleteChain, log);
-                } catch (Exception e) {
-                    e.printStackTrace();
-                    Log.e(TAG, log.toString());
-                }
-
+            try {
+                boolean allowSu = log.toString().contains("uid=0");
+                callbackWeakReference.get().onSuRequested(allowSu);
+            } catch (Exception e) {
+                //ignore
             }
+            return success;
+        }
 
-            return null;
+        @Override
+        protected void onPostExecute(Boolean result) {
+            super.onPostExecute(result);
+            FirewallCallback callback = callbackWeakReference.get();
+            if (callback != null) {
+                callback.onFirewallStopped(result);
+            }
         }
     }
 
@@ -138,30 +189,8 @@ public class FirewallHelper implements FirewallCallback {
     }
 
     public void shutdownFirewall() {
-        ShutdownFirewallTask task = new ShutdownFirewallTask();
+        ShutdownFirewallTask task = new ShutdownFirewallTask(this);
         task.execute();
     }
 
-    public static boolean hasSU() {
-        StringBuilder log = new StringBuilder();
-
-        try {
-            String suCommand = "su -v";
-            runBlockingCmd(new String[]{suCommand}, log);
-        } catch (Exception e) {
-            return false;
-        }
-
-        return !TextUtils.isEmpty(log) && !log.toString().contains("su: not found");
-    }
-
-    public static boolean requestSU() {
-        try {
-            String suCommand = "su";
-            return  runBlockingCmd(new String[]{suCommand}, null) == 0;
-        } catch (Exception e) {
-            return false;
-        }
-    }
-
 }
diff --git a/app/src/main/java/se/leap/bitmaskclient/utils/PreferenceHelper.java b/app/src/main/java/se/leap/bitmaskclient/utils/PreferenceHelper.java
index 9eac7187..bf97b5c5 100644
--- a/app/src/main/java/se/leap/bitmaskclient/utils/PreferenceHelper.java
+++ b/app/src/main/java/se/leap/bitmaskclient/utils/PreferenceHelper.java
@@ -31,6 +31,7 @@ import static se.leap.bitmaskclient.Constants.PROVIDER_EIP_DEFINITION;
 import static se.leap.bitmaskclient.Constants.PROVIDER_PRIVATE_KEY;
 import static se.leap.bitmaskclient.Constants.PROVIDER_VPN_CERTIFICATE;
 import static se.leap.bitmaskclient.Constants.SHARED_PREFERENCES;
+import static se.leap.bitmaskclient.Constants.SU_PERMISSION;
 import static se.leap.bitmaskclient.Constants.USE_PLUGGABLE_TRANSPORTS;
 import static se.leap.bitmaskclient.Constants.EXCLUDED_APPS;
 
@@ -214,6 +215,14 @@ public class PreferenceHelper {
                 apply();
     }
 
+    public static boolean hasSuPermission(Context context) {
+        return getBoolean(context, SU_PERMISSION, false);
+    }
+
+    public static void setSuPermission(Context context, boolean allowed) {
+        putBoolean(context, SU_PERMISSION, allowed);
+    }
+
     public static boolean getUsePluggableTransports(Context context) {
         if (context == null) {
             return false;
@@ -296,9 +305,27 @@ public class PreferenceHelper {
         return preferences.getString(key, defValue);
     }
 
-    public static void putString(Context context, String key, String value){
+    public static void putString(Context context, String key, String value) {
         SharedPreferences preferences = context.getSharedPreferences(SHARED_PREFERENCES, MODE_PRIVATE);
         preferences.edit().putString(key, value).apply();
     }
 
+    public static Boolean getBoolean(Context context, String key, Boolean defValue) {
+        if (context == null) {
+            return false;
+        }
+
+        SharedPreferences preferences = context.getSharedPreferences(SHARED_PREFERENCES, MODE_PRIVATE);
+        return preferences.getBoolean(key, defValue);
+    }
+
+    public static void putBoolean(Context context, String key, Boolean value) {
+        if (context == null) {
+            return;
+        }
+
+        SharedPreferences preferences = context.getSharedPreferences(SHARED_PREFERENCES, MODE_PRIVATE);
+        preferences.edit().putBoolean(key, value).apply();
+    }
+
 }