Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4347ccb7 authored by Bernardo Rufino's avatar Bernardo Rufino
Browse files

Restrict {AM,WM}.closeSystemDialogs() 

As part of locking down Intent.ACTION_CLOSE_SYSTEM_DIALOGS, we need to
protect AM.closeSystemDialogs() too (which ends up calling the broadcast
under the system UID). We should also protect WM.closeSystemDialogs(),
which is slightly different in that it asks windows/views to close
themselves, for the same reasons that we protected the intent and for
consistency too. Check go/close-system-dialogs for details on the
general effort.

To achieve this we move the logic from AM to WM, since we can't call out
from WM to AM. Part of the logic was already in WM (the trampoline
case), so I mostly moved code from AMS to wm.ATMS. The part I had to
wire from AM was the sourceUid of the instrumentation, but that fit
nicely since we had to pipe similar information before for BAL too.

Bug: 159105552
Test: atest CtsAppTestCases:android.app.cts.CloseSystemDialogsTest
Change-Id: Ib720fdb2a0a044bf4d05471b32a9b99d3684ae2d
parent a5680aba

services/core/java/com/android/server/am/ActivityManagerService.java

+4 −62
Original line number Diff line number Diff line
@@ -174,7 +174,6 @@ import android.app.ProfilerInfo; 
import android.app.WaitResult;

import android.app.backup.BackupManager.OperationType;

import android.app.backup.IBackupManager;

import android.app.compat.CompatChanges;

import android.app.usage.UsageEvents;

import android.app.usage.UsageEvents.Event;

import android.app.usage.UsageStatsManager;
@@ -13720,34 +13719,10 @@ public class ActivityManagerService extends IActivityManager.Stub 
                            false, false, userId, "package unstartable");

                    break;

                case Intent.ACTION_CLOSE_SYSTEM_DIALOGS:

                    if (!canCloseSystemDialogs(callingPid, callingUid, callerApp)) {

                        // The app can't close system dialogs, throw only if it targets S+

                        if (CompatChanges.isChangeEnabled(

                                ActivityManager.LOCK_DOWN_CLOSE_SYSTEM_DIALOGS, callingUid)) {

                            throw new SecurityException(

                                    "Permission Denial: " + Intent.ACTION_CLOSE_SYSTEM_DIALOGS

                                            + " broadcast from " + callerPackage + " (pid="

                                            + callingPid + ", uid=" + callingUid + ")"

                                            + " requires "

                                            + permission.BROADCAST_CLOSE_SYSTEM_DIALOGS + ".");

                        } else if (CompatChanges.isChangeEnabled(

                                ActivityManager.DROP_CLOSE_SYSTEM_DIALOGS, callingUid)) {

                            Slog.w(TAG, "Permission Denial: " + intent.getAction()

                                    + " broadcast from " + callerPackage + " (pid=" + callingPid

                                    + ", uid=" + callingUid + ")"

                                    + " requires "

                                    + permission.BROADCAST_CLOSE_SYSTEM_DIALOGS

                                    + ", dropping broadcast.");

                    if (!mAtmInternal.checkCanCloseSystemDialogs(callingPid, callingUid,

                            callerPackage)) {

                        // Returning success seems to be the pattern here

                        return ActivityManager.BROADCAST_SUCCESS;

                        } else {

                            Slog.w(TAG, intent.getAction()

                                    + " broadcast from " + callerPackage + " (pid=" + callingPid

                                    + ", uid=" + callingUid + ")"

                                    + " will require  "

                                    + permission.BROADCAST_CLOSE_SYSTEM_DIALOGS

                                    + " in future builds.");

                        }

                    }

                    break;

            }
@@ -14042,39 +14017,6 @@ public class ActivityManagerService extends IActivityManager.Stub 
        return ActivityManager.BROADCAST_SUCCESS;

    }













    private boolean canCloseSystemDialogs(int pid, int uid, @Nullable ProcessRecord callerApp) {













        if (checkPermission(permission.BROADCAST_CLOSE_SYSTEM_DIALOGS, pid, uid)













                == PERMISSION_GRANTED) {













            return true;













        }













        if (callerApp == null) {













            synchronized (mPidsSelfLocked) {













                callerApp = mPidsSelfLocked.get(pid);













            }













        }

























        if (callerApp != null) {













            // Check if the instrumentation of the process has the permission. This covers the usual













            // test started from the shell (which has the permission) case. This is needed for apps













            // targeting SDK level < S but we are also allowing for targetSdk S+ as a convenience to













            // avoid breaking a bunch of existing tests and asking them to adopt shell permissions













            // to do this.













            ActiveInstrumentation instrumentation = callerApp.getActiveInstrumentation();













            if (instrumentation != null && checkPermission(













                    permission.BROADCAST_CLOSE_SYSTEM_DIALOGS, -1, instrumentation.mSourceUid)













                    == PERMISSION_GRANTED) {













                return true;













            }













            // This is the notification trampoline use-case for example, where apps use Intent.ACSD













            // to close the shade prior to starting an activity.













            WindowProcessController wmApp = callerApp.getWindowProcessController();













            if (wmApp.canCloseSystemDialogsByToken()) {













                return true;













            }













        }













        return false;













    }



























    /**















     * @return uid from the extra field {@link Intent#EXTRA_UID} if present, Otherwise -1















     */

























services/core/java/com/android/server/am/ProcessRecord.java



+3
−1




























Original line number
Diff line number
Diff line








@@ -1435,7 +1435,9 @@ class ProcessRecord implements WindowProcessListener {













    void setActiveInstrumentation(ActiveInstrumentation instr) {















        mInstr = instr;















        boolean isInstrumenting = instr != null;













        mWindowProcessController.setInstrumenting(isInstrumenting,













        mWindowProcessController.setInstrumenting(













                isInstrumenting,













                isInstrumenting ? instr.mSourceUid : -1,















                isInstrumenting && instr.mHasBackgroundActivityStartsPermission);















    }

















































services/core/java/com/android/server/wm/ActivityTaskManagerInternal.java



+8
−0




























Original line number
Diff line number
Diff line








@@ -284,6 +284,14 @@ public abstract class ActivityTaskManagerInternal {













     */















    public abstract void enforceCallerIsRecentsOrHasPermission(String permission, String func);





























    /**













     * Returns true if the app can close system dialogs. Otherwise it either throws a {@link













     * SecurityException} or returns false with a logcat message depending on whether the app













     * targets SDK level {@link android.os.Build.VERSION_CODES#S} or not.













     */













    public abstract boolean checkCanCloseSystemDialogs(int pid, int uid,













            @Nullable String packageName);





























    /**















     * Called after the voice interaction service has changed.















     */

































services/core/java/com/android/server/wm/ActivityTaskManagerService.java



+91
−1




























Original line number
Diff line number
Diff line








@@ -148,6 +148,7 @@ import android.app.WindowConfiguration;













import android.app.admin.DevicePolicyCache;















import android.app.assist.AssistContent;















import android.app.assist.AssistStructure;













import android.app.compat.CompatChanges;















import android.app.usage.UsageStatsManagerInternal;















import android.content.ActivityNotFoundException;















import android.content.ComponentName;










@@ -2922,6 +2923,86 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub {













        }















    }





























    /**













     * Returns true if the app can close system dialogs. Otherwise it either throws a {@link













     * SecurityException} or returns false with a logcat message depending on whether the app













     * targets SDK level {@link android.os.Build.VERSION_CODES#S} or not.













     */













    private boolean checkCanCloseSystemDialogs(int pid, int uid, @Nullable String packageName) {













        final WindowProcessController process;













        synchronized (mGlobalLock) {













            process = mProcessMap.getProcess(pid);













        }













        if (packageName == null && process != null) {













            // WindowProcessController.mInfo is final, so after the synchronized memory barrier













            // above, process.mInfo can't change. As for reading mInfo.packageName,













            // WindowProcessController doesn't own the ApplicationInfo object referenced by mInfo.













            // ProcessRecord for example also holds a reference to that object, so protecting access













            // to packageName with the WM lock would not be enough as we'd also need to synchronize













            // on the AM lock if we are worried about races, but we can't synchronize on AM lock













            // here. Hence, since this is only used for logging, we don't synchronize here.













            packageName = process.mInfo.packageName;













        }













        String caller = "(pid=" + pid + ", uid=" + uid + ")";













        if (packageName != null) {













            caller = packageName + " " + caller;













        }













        if (!canCloseSystemDialogs(pid, uid, process)) {













            // The app can't close system dialogs, throw only if it targets S+













            if (CompatChanges.isChangeEnabled(













                    ActivityManager.LOCK_DOWN_CLOSE_SYSTEM_DIALOGS, uid)) {













                throw new SecurityException(













                        "Permission Denial: " + Intent.ACTION_CLOSE_SYSTEM_DIALOGS













                                + " broadcast from " + caller + " requires "













                                + Manifest.permission.BROADCAST_CLOSE_SYSTEM_DIALOGS + ".");













            } else if (CompatChanges.isChangeEnabled(













                    ActivityManager.DROP_CLOSE_SYSTEM_DIALOGS, uid)) {













                Slog.e(TAG,













                        "Permission Denial: " + Intent.ACTION_CLOSE_SYSTEM_DIALOGS













                                + " broadcast from " + caller + " requires "













                                + Manifest.permission.BROADCAST_CLOSE_SYSTEM_DIALOGS













                                + ", dropping broadcast.");













                return false;













            } else {













                Slog.w(TAG, Intent.ACTION_CLOSE_SYSTEM_DIALOGS













                        + " broadcast from " + caller + " will require "













                        + Manifest.permission.BROADCAST_CLOSE_SYSTEM_DIALOGS













                        + " in future builds.");













                return true;













            }













        }













        return true;













    }



























    private boolean canCloseSystemDialogs(int pid, int uid,













            @Nullable WindowProcessController process) {













        if (checkPermission(Manifest.permission.BROADCAST_CLOSE_SYSTEM_DIALOGS, pid, uid)













                == PERMISSION_GRANTED) {













            return true;













        }













        if (process != null) {













            // Check if the instrumentation of the process has the permission. This covers the













            // usual test started from the shell (which has the permission) case. This is needed













            // for apps targeting SDK level < S but we are also allowing for targetSdk S+ as a













            // convenience to avoid breaking a bunch of existing tests and asking them to adopt













            // shell permissions to do this.













            // Note that these getters all read from volatile fields in WindowProcessController, so













            // no need to lock.













            int sourceUid = process.getInstrumentationSourceUid();













            if (process.isInstrumenting() && sourceUid != -1 && checkPermission(













                    Manifest.permission.BROADCAST_CLOSE_SYSTEM_DIALOGS, -1, sourceUid)













                    == PERMISSION_GRANTED) {













                return true;













            }













            // This is the notification trampoline use-case for example, where apps use Intent.ACSD













            // to close the shade prior to starting an activity.













            if (process.canCloseSystemDialogsByToken()) {













                return true;













            }













        }













        return false;













    }





























    static void enforceTaskPermission(String func) {















        if (checkCallingPermission(MANAGE_ACTIVITY_TASKS) == PackageManager.PERMISSION_GRANTED) {















            return;










@@ -5198,6 +5279,12 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub {













            ActivityTaskManagerService.this.enforceCallerIsRecentsOrHasPermission(permission, func);















        }





























        @Override













        public boolean checkCanCloseSystemDialogs(int pid, int uid, @Nullable String packageName) {













            return ActivityTaskManagerService.this.checkCanCloseSystemDialogs(pid, uid,













                    packageName);













        }





























        @Override















        public void notifyActiveVoiceInteractionServiceChanged(ComponentName component) {















            synchronized (mGlobalLock) {










@@ -5698,9 +5785,12 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub {













        @Override















        public void closeSystemDialogs(String reason) {















            enforceNotIsolatedCaller("closeSystemDialogs");





























            final int pid = Binder.getCallingPid();















            final int uid = Binder.getCallingUid();













            if (!checkCanCloseSystemDialogs(pid, uid, null)) {













                return;













            }





























            final long origId = Binder.clearCallingIdentity();















            try {















                synchronized (mGlobalLock) {

































services/core/java/com/android/server/wm/WindowManagerService.java



+5
−0




























Original line number
Diff line number
Diff line








@@ -3242,6 +3242,11 @@ public class WindowManagerService extends IWindowManager.Stub





























    @Override















    public void closeSystemDialogs(String reason) {













        int callingPid = Binder.getCallingPid();













        int callingUid = Binder.getCallingUid();













        if (!mAtmInternal.checkCanCloseSystemDialogs(callingPid, callingUid, null)) {













            return;













        }















        synchronized (mGlobalLock) {















            mRoot.closeSystemDialogs(reason);















        }