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

Commit 515e88cb authored by Bernardo Rufino's avatar Bernardo Rufino
Browse files

Allow Intent.ACSD for notification action trampolines for targetSdk < S

This is the major use-case for Intent.ACTION_CLOSE_SYSTEM_DIALOGS
internally. That is, since the notification shade is not automatically
closed when the user taps a notification action whose pending intent
(PI) is not an activity PI, the app sends Intent.ACSD before starting
the activity in the trampoline. This is a legit use-case with a lot of
usages internally (and even externally if we use Stack Overflow as a
proxy), so we will allow this case for apps targeting SDK level < S.

For targetSdk S+, trampolines are blocked, so there is no valid use for
sending Intent.ACSD.

In the implementation we piggy-back on the trampoline callback, but in
this case since we're *allowing* instead of *blocking*, we allow the app
to close system dialogs when any of the tokens that allow activity
starts is the notification token, meaning that if we consider that the
app is processing a notification broadcast receiver or service, we allow
it to send Intent.ACSD assuming trampolines are not blocked (ie. that
the app targets SDK level < S).

Bug: 159105552
Test: atest CtsAppTestCases:android.app.cts.CloseSystemDialogsTest
Change-Id: Id29991ad8c7f2c6c3dca0d2d689b903de8b0ec81
parent 7b14bd63
Loading
Loading
Loading
Loading
+12 −5
Original line number Diff line number Diff line
@@ -14052,18 +14052,25 @@ public class ActivityManagerService extends IActivityManager.Stub
                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.
        if (callerApp != null) {
            // 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;
    }
+20 −8
Original line number Diff line number Diff line
@@ -105,6 +105,7 @@ import static com.android.internal.util.FrameworkStatsLog.DND_MODE_RULE;
import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_CHANNEL_GROUP_PREFERENCES;
import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_CHANNEL_PREFERENCES;
import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_PREFERENCES;
import static com.android.internal.util.Preconditions.checkArgument;
import static com.android.server.am.PendingIntentRecord.FLAG_ACTIVITY_SENDER;
import static com.android.server.am.PendingIntentRecord.FLAG_BROADCAST_SENDER;
import static com.android.server.am.PendingIntentRecord.FLAG_SERVICE_SENDER;
@@ -291,7 +292,6 @@ import libcore.io.IoUtils;

import org.json.JSONException;
import org.json.JSONObject;
import org.xmlpull.v1.XmlPullParser;
import org.xmlpull.v1.XmlPullParserException;

import java.io.ByteArrayInputStream;
@@ -308,7 +308,7 @@ import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
@@ -10416,12 +10416,15 @@ public class NotificationManagerService extends SystemService {
        private final Set<String> mPackagesShown = new ArraySet<>();

        @Override
        public IBinder getToken() {
            return ALLOWLIST_TOKEN;
        public boolean isActivityStartAllowed(Collection<IBinder> tokens, int uid,
                String packageName) {
            checkArgument(!tokens.isEmpty());
            for (IBinder token : tokens) {
                if (token != ALLOWLIST_TOKEN) {
                    // We only block or warn if the start is exclusively due to notification
                    return true;
                }
            }

        @Override
        public boolean isActivityStartAllowed(int uid, String packageName) {
            String toastMessage = "Indirect activity start from " + packageName;
            String logcatMessage =
                    "Indirect notification activity start (trampoline) from " + packageName;
@@ -10439,6 +10442,15 @@ public class NotificationManagerService extends SystemService {
            }
        }

        @Override
        public boolean canCloseSystemDialogs(Collection<IBinder> tokens, int uid) {
            // If the start is allowed via notification, we allow the app to close system dialogs
            // only if their targetSdk < S, otherwise they have no valid reason to do this since
            // trampolines are blocked.
            return tokens.contains(ALLOWLIST_TOKEN)
                    && !CompatChanges.isChangeEnabled(NOTIFICATION_TRAMPOLINE_BLOCK, uid);
        }

        private void toast(String message) {
            mUiHandler.post(() ->
                    Toast.makeText(getUiContext(), message + "\nSee go/s-trampolines.",
+15 −11
Original line number Diff line number Diff line
@@ -18,25 +18,29 @@ package com.android.server.wm;

import android.os.IBinder;

import java.util.Collection;

/**
 * Callback to be called when a background activity start is allowed exclusively because of the
 * token provided in {@link #getToken()}.
 * Callback to decide activity starts and related operations based on originating tokens.
 */
public interface BackgroundActivityStartCallback {
    /**
     * The token for which this callback is responsible for deciding whether the app can start
     * background activities or not.
     * Returns true if the background activity start originating from {@code tokens} should be
     * allowed or not.
     *
     * Note that if the start was allowed due to a mechanism other than tokens (eg. permission),
     * this won't be called.
     *
     * Ideally this should just return a final variable, don't do anything costly here (don't hold
     * any locks).
     * This will be called holding the WM lock, don't do anything costly here.
     */
    IBinder getToken();
    boolean isActivityStartAllowed(Collection<IBinder> tokens, int uid, String packageName);

    /**
     * Returns true if the background activity start due to originating token in {@link #getToken()}
     * should be allowed or not.
     * Returns whether {@code uid} can send {@link android.content.Intent
     * #ACTION_CLOSE_SYSTEM_DIALOGS}, presumably to start activities, based on the originating
     * tokens {@code tokens} currently associated with potential activity starts.
     *
     * This will be called holding the WM lock, don't do anything costly here.
     * This will be called holding the AM and WM lock, don't do anything costly here.
     */
    boolean isActivityStartAllowed(int uid, String packageName);
    boolean canCloseSystemDialogs(Collection<IBinder> tokens, int uid);
}
+17 −12
Original line number Diff line number Diff line
@@ -585,9 +585,8 @@ public class WindowProcessController extends ConfigurationContainer<Configuratio
    }

    /**
     * If there are no tokens, we don't allow *by token*. If there are tokens, we need to check if
     * the callback handles all the tokens, if so we ask the callback if the activity should be
     * started, otherwise we allow.
     * If there are no tokens, we don't allow *by token*. If there are tokens, we ask the callback
     * if the start is allowed for these tokens, otherwise if there is no callback we allow.
     */
    private boolean isBackgroundStartAllowedByToken() {
        if (mBackgroundActivityStartTokens.isEmpty()) {
@@ -597,16 +596,22 @@ public class WindowProcessController extends ConfigurationContainer<Configuratio
            // We have tokens but no callback to decide => allow
            return true;
        }
        IBinder callbackToken = mBackgroundActivityStartCallback.getToken();
        for (IBinder token : mBackgroundActivityStartTokens.values()) {
            if (token != callbackToken) {
                // The callback doesn't handle all the tokens => allow
                return true;
        // The callback will decide
        return mBackgroundActivityStartCallback.isActivityStartAllowed(
                mBackgroundActivityStartTokens.values(), mInfo.uid, mInfo.packageName);
    }

    /**
     * Returns whether this process is allowed to close system dialogs via a background activity
     * start token that allows the close system dialogs operation (eg. notification).
     */
    public boolean canCloseSystemDialogsByToken() {
        synchronized (mAtm.mGlobalLock) {
            return !mBackgroundActivityStartTokens.isEmpty()
                    && mBackgroundActivityStartCallback != null
                    && mBackgroundActivityStartCallback.canCloseSystemDialogs(
                            mBackgroundActivityStartTokens.values(), mInfo.uid);
        }
        // The callback handles all the tokens => callback decides
        return mBackgroundActivityStartCallback.isActivityStartAllowed(mInfo.uid,
                mInfo.packageName);
    }

    private boolean isBoundByForegroundUid() {