From 8d4c407201519f39dcaf38a942808141d0ff9fbc Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Wed, 10 Apr 2019 23:35:31 -0400 Subject: [PATCH] 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 --- .../topjohnwu/magisk/SuRequestActivity.java | 125 ++++++++++-------- .../magisk/components/GeneralReceiver.java | 31 ++--- native/jni/su/connect.cpp | 77 +++++------ 3 files changed, 114 insertions(+), 119 deletions(-) diff --git a/app/src/main/java/com/topjohnwu/magisk/SuRequestActivity.java b/app/src/main/java/com/topjohnwu/magisk/SuRequestActivity.java index afaade948..e5b482e47 100644 --- a/app/src/main/java/com/topjohnwu/magisk/SuRequestActivity.java +++ b/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) { - SuConnector connector; - try { - connector = new SuConnector(socketName) { - @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(); + + String action = intent.getAction(); + + if (TextUtils.equals(action, REQUEST)) { + if (!handleRequest()) finish(); - return; - } - handler = new ActionHandler() { - @Override - void handleAction() { - connector.response(); - done(); - } + return; + } - @Override - void handleAction(int action) { - int pos = timeout.getSelectedItemPosition(); - timeoutPrefs.edit().putInt(policy.packageName, pos).apply(); - handleAction(action, Config.Value.TIMEOUT_LIST[pos]); - } + if (TextUtils.equals(action, LOG)) + SuLogger.handleLogs(intent); + else if (TextUtils.equals(action, NOTIFY)) + SuLogger.handleNotify(intent); + 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) { - policy.policy = action; - if (time >= 0) { - policy.until = (time == 0) ? 0 - : (System.currentTimeMillis() / 1000 + time * 60); - app.mDB.updatePolicy(policy); - } - handleAction(); + protected void onResponse() throws IOException { + out.writeInt(policy.policy); } }; - } else { - finish(); - return; + Bundle bundle = connector.readSocketInput(); + int uid = Integer.parseInt(bundle.getString("uid")); + 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)) { - finish(); - return; - } + if (TextUtils.equals(policy.packageName, BuildConfig.APPLICATION_ID)) + return false; // 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") diff --git a/app/src/main/java/com/topjohnwu/magisk/components/GeneralReceiver.java b/app/src/main/java/com/topjohnwu/magisk/components/GeneralReceiver.java index e31b90ce8..40c103f35 100644 --- a/app/src/main/java/com/topjohnwu/magisk/components/GeneralReceiver.java +++ b/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) - action = "boot_complete"; + if (action == null) { + // 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: diff --git a/native/jni/su/connect.cpp b/native/jni/su/connect.cpp index 07d6c13c7..fca409354 100644 --- a/native/jni/su/connect.cpp +++ b/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 #include #include @@ -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", \ -"-f", "0x10000020" +"start", "-n", nullptr, "--user", nullptr, "-f", "0x18000020", "-a" + +// 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", - ctx->info->cfg[SU_MULTIUSER_MODE] == MULTIUSER_MODE_OWNER_MANAGED ? - ctx->info->uid % 100000 : ctx->info->uid); + get_uid(fromUid, ctx->info); 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, - "--user", user, - "--es", "action", "log", + START_ACTIVITY, "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", - ctx->info->cfg[SU_MULTIUSER_MODE] == MULTIUSER_MODE_OWNER_MANAGED ? - ctx->info->uid % 100000 : ctx->info->uid); + get_uid(fromUid, ctx->info); char policy[2]; sprintf(policy, "%d", ctx->info->access.policy); const char *cmd[] = { - BROADCAST_REBOOT, - "--user", user, - "--es", "action", "notify", + START_ACTIVITY, "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, - "--user", user, - "--es", "action", "request", + START_ACTIVITY, "request", "--es", "socket", socket, nullptr };