Directly communicate with Activity

Since Android Q does not allow launching activities from the background
(Services/BroadcastReceivers) and our native process is root, directly
launch activities and use it for communication between native and app.

The target activity is not exported, so non-root apps cannot send an
intent to fool Magisk Manager. This is as safe as the previous
implementation, which uses protected system broadcasts.

This also workaround broadcast limitations in many ROMs (especially
in Chinese ROMs) which blocks the su request dialog if the app is
frozen/force stopped by the system.

Close #1326

app/src/main/java/com/topjohnwu/magisk/SuRequestActivity.java

@@ -24,6 +24,7 @@ import com.topjohnwu.magisk.components.BaseActivity;
 import com.topjohnwu.magisk.container.Policy;
 import com.topjohnwu.magisk.utils.FingerprintHelper;
 import com.topjohnwu.magisk.utils.SuConnector;
+import com.topjohnwu.magisk.utils.SuLogger;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -33,6 +34,7 @@ import butterknife.BindView;
 import java9.lang.Iterables;
 
 public class SuRequestActivity extends BaseActivity {
+
     @BindView(R.id.su_popup) LinearLayout suPopup;
     @BindView(R.id.timeout) Spinner timeout;
     @BindView(R.id.app_icon) ImageView appIcon;
@@ -47,6 +49,10 @@ public class SuRequestActivity extends BaseActivity {
     private Policy policy;
     private SharedPreferences timeoutPrefs;
 
+    public static final String REQUEST = "request";
+    public static final String LOG = "log";
+    public static final String NOTIFY = "notify";
+
     @Override
     public int getDarkTheme() {
         return R.style.SuRequest_Dark;
@@ -63,84 +69,97 @@ public class SuRequestActivity extends BaseActivity {
         lockOrientation();
         supportRequestWindowFeature(Window.FEATURE_NO_TITLE);
 
-        app.mDB.clearOutdated();
         timeoutPrefs = App.deContext.getSharedPreferences("su_timeout", 0);
-
-        // Get policy
         Intent intent = getIntent();
-        String socketName = intent.getStringExtra("socket");
+
-        if (socketName != null) {
+        String action = intent.getAction();
-            SuConnector connector;
+
-            try {
+        if (TextUtils.equals(action, REQUEST)) {
-                connector = new SuConnector(socketName) {
+            if (!handleRequest())
-                    @Override
-                    protected void onResponse() throws IOException {
-                        out.writeInt(policy.policy);
-                    }
-                };
-                Bundle bundle = connector.readSocketInput();
-                int uid = Integer.parseInt(bundle.getString("uid"));
-                policy = app.mDB.getPolicy(uid);
-                if (policy == null) {
-                    policy = new Policy(uid, getPackageManager());
-                }
-            } catch (IOException | PackageManager.NameNotFoundException e) {
-                e.printStackTrace();
                 finish();
-                return;
+            return;
-            }
+        }
-            handler = new ActionHandler() {
-                @Override
-                void handleAction() {
-                    connector.response();
-                    done();
-                }
 
-                @Override
+        if (TextUtils.equals(action, LOG))
-                void handleAction(int action) {
+            SuLogger.handleLogs(intent);
-                    int pos = timeout.getSelectedItemPosition();
+        else if (TextUtils.equals(action, NOTIFY))
-                    timeoutPrefs.edit().putInt(policy.packageName, pos).apply();
+            SuLogger.handleNotify(intent);
-                    handleAction(action, Config.Value.TIMEOUT_LIST[pos]);
-                }
 
+        finish();
+    }
+
+    private boolean handleRequest() {
+        String socketName = getIntent().getStringExtra("socket");
+
+        if (socketName == null)
+            return false;
+
+        SuConnector connector;
+        try {
+            connector = new SuConnector(socketName) {
                 @Override
-                void handleAction(int action, int time) {
+                protected void onResponse() throws IOException {
-                    policy.policy = action;
+                    out.writeInt(policy.policy);
-                    if (time >= 0) {
-                        policy.until = (time == 0) ? 0
-                                : (System.currentTimeMillis() / 1000 + time * 60);
-                        app.mDB.updatePolicy(policy);
-                    }
-                    handleAction();
                 }
             };
-        } else {
+            Bundle bundle = connector.readSocketInput();
-            finish();
+            int uid = Integer.parseInt(bundle.getString("uid"));
-            return;
+            app.mDB.clearOutdated();
+            policy = app.mDB.getPolicy(uid);
+            if (policy == null) {
+                policy = new Policy(uid, getPackageManager());
+            }
+        } catch (IOException | PackageManager.NameNotFoundException e) {
+            e.printStackTrace();
+            return false;
         }
+        handler = new ActionHandler() {
+            @Override
+            void handleAction() {
+                connector.response();
+                done();
+            }
+
+            @Override
+            void handleAction(int action) {
+                int pos = timeout.getSelectedItemPosition();
+                timeoutPrefs.edit().putInt(policy.packageName, pos).apply();
+                handleAction(action, Config.Value.TIMEOUT_LIST[pos]);
+            }
+
+            @Override
+            void handleAction(int action, int time) {
+                policy.policy = action;
+                if (time >= 0) {
+                    policy.until = (time == 0) ? 0
+                            : (System.currentTimeMillis() / 1000 + time * 60);
+                    app.mDB.updatePolicy(policy);
+                }
+                handleAction();
+            }
+        };
 
         // Never allow com.topjohnwu.magisk (could be malware)
-        if (TextUtils.equals(policy.packageName, BuildConfig.APPLICATION_ID)) {
+        if (TextUtils.equals(policy.packageName, BuildConfig.APPLICATION_ID))
-            finish();
+            return false;
-            return;
-        }
 
         // If not interactive, response directly
         if (policy.policy != Policy.INTERACTIVE) {
             handler.handleAction();
-            return;
+            return true;
         }
 
         switch ((int) Config.get(Config.Key.SU_AUTO_RESPONSE)) {
             case Config.Value.SU_AUTO_DENY:
                 handler.handleAction(Policy.DENY, 0);
-                return;
+                return true;
             case Config.Value.SU_AUTO_ALLOW:
                 handler.handleAction(Policy.ALLOW, 0);
-                return;
+                return true;
         }
 
         showUI();
+        return true;
     }
 
     @SuppressLint("ClickableViewAccessibility")

app/src/main/java/com/topjohnwu/magisk/components/GeneralReceiver.java

@@ -33,36 +33,29 @@ public class GeneralReceiver extends BroadcastReceiver {
             case Intent.ACTION_REBOOT:
             case Intent.ACTION_BOOT_COMPLETED:
                 action = intent.getStringExtra("action");
-                if (action == null)
+                if (action == null) {
-                    action = "boot_complete";
+                    // Actual boot completed event
+                    Shell.su("mm_patch_dtbo").submit(result -> {
+                        if (result.isSuccess())
+                            Notifications.dtboPatched();
+                    });
+                    break;
+                }
                 switch (action) {
-                    case "request":
+                    case SuRequestActivity.REQUEST:
                         Intent i = new Intent(app, ClassMap.get(SuRequestActivity.class))
+                                .setAction(action)
                                 .putExtra("socket", intent.getStringExtra("socket"))
                                 .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
                                 .addFlags(Intent.FLAG_ACTIVITY_MULTIPLE_TASK);
                         app.startActivity(i);
                         break;
-                    case "log":
+                    case SuRequestActivity.LOG:
                         SuLogger.handleLogs(intent);
                         break;
-                    case "notify":
+                    case SuRequestActivity.NOTIFY:
                         SuLogger.handleNotify(intent);
                         break;
-                    case "boot_complete":
-                    default:
-                        /* Devices with DTBO might want to patch dtbo.img.
-                         * However, that is not possible if Magisk is installed by
-                         * patching boot image with Magisk Manager and flashed via
-                         * fastboot, since at that time we do not have root.
-                         * Check for dtbo status every boot time, and prompt user
-                         * to reboot if dtbo wasn't patched and patched by Magisk Manager.
-                         * */
-                        Shell.su("mm_patch_dtbo").submit(result -> {
-                            if (result.isSuccess())
-                                Notifications.dtboPatched();
-                        });
-                        break;
                 }
                 break;
             case Intent.ACTION_PACKAGE_REPLACED:

native/jni/su/connect.cpp

@@ -1,10 +1,3 @@
-/*
-** Copyright 2018, John Wu (@topjohnwu)
-** Copyright 2010, Adam Shanks (@ChainsDD)
-** Copyright 2008, Zinx Verituse (@zinxv)
-**
-*/
-
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
@@ -18,10 +11,11 @@
 
 #include "su.h"
 
-#define BROADCAST_REBOOT \
+#define START_ACTIVITY \
 "/system/bin/app_process", "/system/bin", "com.android.commands.am.Am", \
-"broadcast", "-n", nullptr, "-a", "android.intent.action.REBOOT", \
+"start", "-n", nullptr, "--user", nullptr, "-f", "0x18000020", "-a"
-"-f", "0x10000020"
+
+// 0x18000020 = FLAG_ACTIVITY_NEW_TASK|FLAG_ACTIVITY_MULTIPLE_TASK|FLAG_INCLUDE_STOPPED_PACKAGES
 
 static inline const char *get_command(const struct su_request *to) {
 	if (to->command[0])
@@ -31,10 +25,30 @@ static inline const char *get_command(const struct su_request *to) {
 	return DEFAULT_SHELL;
 }
 
+static inline void get_user(char *user, struct su_info *info) {
+	sprintf(user, "%d",
+			info->cfg[SU_MULTIUSER_MODE] == MULTIUSER_MODE_USER
+			? info->uid / 100000
+			: 0);
+}
+
+static inline void get_uid(char *uid, struct su_info *info) {
+	sprintf(uid, "%d",
+			info->cfg[SU_MULTIUSER_MODE] == MULTIUSER_MODE_OWNER_MANAGED
+			? info->uid % 100000
+			: info->uid);
+}
+
 static void silent_run(const char **args, struct su_info *info) {
 	char component[128];
-	sprintf(component, "%s/a.h", info->str[SU_MANAGER].data());
+	sprintf(component, "%s/a.m", info->str[SU_MANAGER].data());
+	char user[8];
+	get_user(user, info);
+
+	/* Fill in dynamic arguments */
 	args[5] = component;
+	args[7] = user;
+
 	exec_t exec {
 		.pre_exec = []() -> void {
 			int null = xopen("/dev/null", O_WRONLY | O_CLOEXEC);
@@ -48,26 +62,9 @@ static void silent_run(const char **args, struct su_info *info) {
 	exec_command(exec);
 }
 
-static void setup_user(char *user, struct su_info *info) {
-	switch (info->cfg[SU_MULTIUSER_MODE]) {
-		case MULTIUSER_MODE_OWNER_ONLY:
-		case MULTIUSER_MODE_OWNER_MANAGED:
-			sprintf(user, "%d", 0);
-			break;
-		case MULTIUSER_MODE_USER:
-			sprintf(user, "%d", info->uid / 100000);
-			break;
-	}
-}
-
 void app_log(struct su_context *ctx) {
-	char user[8];
-	setup_user(user, ctx->info);
-
 	char fromUid[8];
-	sprintf(fromUid, "%d",
+	get_uid(fromUid, ctx->info);
-			ctx->info->cfg[SU_MULTIUSER_MODE] == MULTIUSER_MODE_OWNER_MANAGED ?
-			ctx->info->uid % 100000 : ctx->info->uid);
 
 	char toUid[8];
 	sprintf(toUid, "%d", ctx->req.uid);
@@ -79,9 +76,7 @@ void app_log(struct su_context *ctx) {
 	sprintf(policy, "%d", ctx->info->access.policy);
 
 	const char *cmd[] = {
-		BROADCAST_REBOOT,
+		START_ACTIVITY, "log",
-		"--user", user,
-		"--es", "action", "log",
 		"--ei", "from.uid", fromUid,
 		"--ei", "to.uid", toUid,
 		"--ei", "pid", pid,
@@ -94,21 +89,14 @@ void app_log(struct su_context *ctx) {
 }
 
 void app_notify(struct su_context *ctx) {
-	char user[8];
-	setup_user(user, ctx->info);
-
 	char fromUid[8];
-	sprintf(fromUid, "%d",
+	get_uid(fromUid, ctx->info);
-			ctx->info->cfg[SU_MULTIUSER_MODE] == MULTIUSER_MODE_OWNER_MANAGED ?
-			ctx->info->uid % 100000 : ctx->info->uid);
 
 	char policy[2];
 	sprintf(policy, "%d", ctx->info->access.policy);
 
 	const char *cmd[] = {
-		BROADCAST_REBOOT,
+		START_ACTIVITY, "notify",
-		"--user", user,
-		"--es", "action", "notify",
 		"--ei", "from.uid", fromUid,
 		"--ei", "policy", policy,
 		nullptr
@@ -117,13 +105,8 @@ void app_notify(struct su_context *ctx) {
 }
 
 void app_connect(const char *socket, struct su_info *info) {
-	char user[8];
-	setup_user(user, info);
-
 	const char *cmd[] = {
-		BROADCAST_REBOOT,
+		START_ACTIVITY, "request",
-		"--user", user,
-		"--es", "action", "request",
 		"--es", "socket", socket,
 		nullptr
 	};