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

Commit 9674c1f1 authored by Bernardo Rufino's avatar Bernardo Rufino
Browse files

Log on unauthorized Intent.ACTION_CLOSE_SYSTEM_DIALOGS 

Putting initial code to detect unauthorized use of Intent.ACSD. For now
we only allow permission holders and apps instrumented via permission
holders (eg. CTS tests via shell).

The plan is to drop the broadcast when unauthorized for targetSdk < S
(to fix the security issue) and throw a SecurityException when apps
target SDK level S+. Because of this I'm using two separate @ChangeIds
for now.

Since there are still legit use-cases where apps use Intent.ACSD, I'm
checking this in disabled mode for now, only logging. Once I implement
the exemptions for these cases I'll turn on the feature. Once that
happens I can remove one of the @ChangeIds.

It's good to get this code now rather than all at once after all the
exemptions so we can see how things fit (plus have some tests).

I'll also protect IAM.closeSystemDialogs() in a future CL.

Bug: 159105552
Test: atest CtsAppTestCases:android.app.cts.CloseSystemDialogsTest
Change-Id: I055f42a34fecebcded55a5eb6ccb3cefa2031812
parent fef77320

core/api/test-current.txt

+2 −0
Original line number Diff line number Diff line
@@ -94,6 +94,8 @@ package android.app { 
    method public static void resumeAppSwitches() throws android.os.RemoteException;

    method @RequiresPermission(android.Manifest.permission.CHANGE_CONFIGURATION) public void scheduleApplicationInfoChanged(java.util.List<java.lang.String>, int);

    method @RequiresPermission(android.Manifest.permission.CHANGE_CONFIGURATION) public boolean updateMccMncConfiguration(@NonNull String, @NonNull String);

    field public static final long DROP_CLOSE_SYSTEM_DIALOGS = 174664120L; // 0xa6929b8L

    field public static final long LOCK_DOWN_CLOSE_SYSTEM_DIALOGS = 174664365L; // 0xa692aadL

    field public static final int PROCESS_CAPABILITY_ALL = 7; // 0x7

    field public static final int PROCESS_CAPABILITY_ALL_EXPLICIT = 1; // 0x1

    field public static final int PROCESS_CAPABILITY_ALL_IMPLICIT = 6; // 0x6

core/java/android/app/ActivityManager.java

+35 −3
Original line number Diff line number Diff line
@@ -31,6 +31,8 @@ import android.annotation.RequiresPermission; 
import android.annotation.SystemApi;

import android.annotation.SystemService;

import android.annotation.TestApi;

import android.compat.annotation.ChangeId;

import android.compat.annotation.Disabled;

import android.compat.annotation.UnsupportedAppUsage;

import android.content.ComponentName;

import android.content.Context;
@@ -91,9 +93,6 @@ import com.android.internal.util.MemInfoReader; 
import com.android.internal.util.Preconditions;

import com.android.server.LocalServices;



import org.xmlpull.v1.XmlPullParser;

import org.xmlpull.v1.XmlSerializer;



import java.io.FileDescriptor;

import java.io.FileOutputStream;

import java.io.IOException;
@@ -870,6 +869,39 @@ public class ActivityManager { 
    private static final boolean DEVELOPMENT_FORCE_LOW_RAM =

            SystemProperties.getBoolean("debug.force_low_ram", false);



    /**

     * Intent {@link Intent#ACTION_CLOSE_SYSTEM_DIALOGS} is too powerful to be unrestricted. We

     * restrict its usage for a few legitimate use-cases only, regardless of targetSdk. For the

     * other use-cases we drop the intent with a log message.

     *

     * Note that this is the lighter version of {@link ActivityManager

     * #LOCK_DOWN_CLOSE_SYSTEM_DIALOGS} which is not gated on targetSdk in order to eliminate the

     * abuse vector.

     *

     * @hide

     */

    @TestApi

    @ChangeId

    @Disabled

    public static final long DROP_CLOSE_SYSTEM_DIALOGS = 174664120L;



    /**

     * Intent {@link Intent#ACTION_CLOSE_SYSTEM_DIALOGS} is too powerful to be unrestricted. So,

     * apps targeting {@link Build.VERSION_CODES#S} or higher will crash if they try to send such

     * intent and don't have permission {@code android.permission.BROADCAST_CLOSE_SYSTEM_DIALOGS}.

     *

     * Note that this is the more restrict version of {@link ActivityManager

     * #DROP_CLOSE_SYSTEM_DIALOGS} that expects the app to stop sending aforementioned intent once

     * it bumps its targetSdk to {@link Build.VERSION_CODES#S} or higher.

     *

     * @hide

     */

    @TestApi

    @ChangeId

    @Disabled

    // @EnabledSince(targetSdkVersion = VERSION_CODES.S)

    public static final long LOCK_DOWN_CLOSE_SYSTEM_DIALOGS = 174664365L;



    /** @hide */

    public int getFrontActivityScreenCompatMode() {

        try {

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

+58 −0
Original line number Diff line number Diff line
@@ -174,6 +174,7 @@ 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;
@@ -13702,6 +13703,37 @@ public class ActivityManagerService extends IActivityManager.Stub 
                    forceStopPackageLocked(packageName, -1, false, true, true,

                            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.");

                            // 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;

            }















            if (Intent.ACTION_PACKAGE_ADDED.equals(action) ||










@@ -13994,6 +14026,32 @@ 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);













            }













        }













        // 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.













        if (callerApp != null) {













            ActiveInstrumentation instrumentation = callerApp.getActiveInstrumentation();













            if (instrumentation != null && checkPermission(













                    permission.BROADCAST_CLOSE_SYSTEM_DIALOGS, -1, instrumentation.mSourceUid)













                    == PERMISSION_GRANTED) {













                return true;













            }













        }













        return false;













    }



























    /**















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















     */