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

Commit 966c7f2d authored by Matías Hernández's avatar Matías Hernández
Browse files

Fix allowlist token issues

1) Don't accept enqueued notifications with an unexpected token.
2) Ensure allowlist token matches for all parceled and unparceled notifications (by only using the "root notification" one).
3) Simplify cookie usage in allowlist token serialization.
4) Ensure group summary (and any notifications added directly by NMS) have the correct token.

Bug: 328254922
Bug: 305695605
Test: atest NotificationManagerServiceTest ParcelTest CloseSystemDialogsTest + manually
Change-Id: I232e9b74eece745560ed2e762071b48984b3f176
Merged-In: I232e9b74eece745560ed2e762071b48984b3f176
parent 77ea996b
Loading
Loading
Loading
Loading
+37 −11
Original line number Diff line number Diff line
@@ -2598,8 +2598,11 @@ public class Notification implements Parcelable
        if (mAllowlistToken == null) {
            mAllowlistToken = processAllowlistToken;
        }
        // Propagate this token to all pending intents that are unmarshalled from the parcel.
        // Propagate this token to all pending intents that are unmarshalled from the parcel,
        // or keep the one we're already propagating, if that's the case.
        if (!parcel.hasClassCookie(PendingIntent.class)) {
            parcel.setClassCookie(PendingIntent.class, mAllowlistToken);
        }
        when = parcel.readLong();
        creationTime = parcel.readLong();
@@ -3060,10 +3063,25 @@ public class Notification implements Parcelable
            };
            PendingIntent.addOnMarshaledListener(addedListener);
        }
        try {
            boolean mustClearCookie = false;
            if (!parcel.hasClassCookie(Notification.class)) {
                // This is the "root" notification, and not an "inner" notification (including
                // publicVersion or anything else that might be embedded in extras). So we want
                // to use its token for every inner notification (might be null).
                parcel.setClassCookie(Notification.class, mAllowlistToken);
                mustClearCookie = true;
            }
            try {
                // IMPORTANT: Add marshaling code in writeToParcelImpl as we
                // want to intercept all pending events written to the parcel.
                writeToParcelImpl(parcel, flags);
            } finally {
                if (mustClearCookie) {
                    parcel.removeClassCookie(Notification.class, mAllowlistToken);
                }
            }
            synchronized (this) {
                // Must be written last!
                parcel.writeArraySet(allPendingIntents);
@@ -3078,7 +3096,10 @@ public class Notification implements Parcelable
    private void writeToParcelImpl(Parcel parcel, int flags) {
        parcel.writeInt(1);
        parcel.writeStrongBinder(mAllowlistToken);
        // Always use the same token as the root notification (might be null).
        IBinder rootNotificationToken = (IBinder) parcel.getClassCookie(Notification.class);
        parcel.writeStrongBinder(rootNotificationToken);
        parcel.writeLong(when);
        parcel.writeLong(creationTime);
        if (mSmallIcon == null && icon != 0) {
@@ -3471,16 +3492,21 @@ public class Notification implements Parcelable
     * Sets the token used for background operations for the pending intents associated with this
     * notification.
     *
     * This token is automatically set during deserialization for you, you usually won't need to
     * call this unless you want to change the existing token, if any.
     * Note: Should <em>only</em> be invoked by NotificationManagerService, since this is normally
     * populated by unparceling (and also used there). Any other usage is suspect.
     *
     * @hide
     */
    public void clearAllowlistToken() {
        mAllowlistToken = null;
    public void overrideAllowlistToken(IBinder token) {
        mAllowlistToken = token;
        if (publicVersion != null) {
            publicVersion.clearAllowlistToken();
            publicVersion.overrideAllowlistToken(token);
        }
    }
    /** @hide */
    public IBinder getAllowlistToken() {
        return mAllowlistToken;
    }
    /**
+22 −0
Original line number Diff line number Diff line
@@ -815,6 +815,28 @@ public final class Parcel {
        return mClassCookies != null ? mClassCookies.get(clz) : null;
    }

    /** @hide */
    public void removeClassCookie(Class clz, Object expectedCookie) {
        if (mClassCookies != null) {
            Object removedCookie = mClassCookies.remove(clz);
            if (removedCookie != expectedCookie) {
                Log.wtf(TAG, "Expected to remove " + expectedCookie + " (with key=" + clz
                        + ") but instead removed " + removedCookie);
            }
        } else {
            Log.wtf(TAG, "Expected to remove " + expectedCookie + " (with key=" + clz
                    + ") but no cookies were present");
        }
    }

    /**
     * Whether {@link #setClassCookie} has been called with the specified {@code clz}.
     * @hide
     */
    public boolean hasClassCookie(Class clz) {
        return mClassCookies != null && mClassCookies.containsKey(clz);
    }

    /** @hide */
    public final void adoptClassCookies(Parcel from) {
        mClassCookies = from.mClassCookies;
+49 −0
Original line number Diff line number Diff line
@@ -16,18 +16,23 @@

package android.os;

import static com.google.common.truth.Truth.assertThat;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;

import android.platform.test.annotations.Presubmit;
import android.util.Log;

import androidx.test.runner.AndroidJUnit4;

import org.junit.Test;
import org.junit.runner.RunWith;

import java.util.ArrayList;

@Presubmit
@RunWith(AndroidJUnit4.class)
public class ParcelTest {
@@ -246,4 +251,48 @@ public class ParcelTest {
        assertThrows(IllegalArgumentException.class, () -> Parcel.compareData(pA, -1, pB, iB, 0));
        assertThrows(IllegalArgumentException.class, () -> Parcel.compareData(pA, 0, pB, -1, 0));
    }

    @Test
    public void testClassCookies() {
        Parcel p = Parcel.obtain();
        assertThat(p.hasClassCookie(ParcelTest.class)).isFalse();

        p.setClassCookie(ParcelTest.class, "string_cookie");
        assertThat(p.hasClassCookie(ParcelTest.class)).isTrue();
        assertThat(p.getClassCookie(ParcelTest.class)).isEqualTo("string_cookie");

        p.removeClassCookie(ParcelTest.class, "string_cookie");
        assertThat(p.hasClassCookie(ParcelTest.class)).isFalse();
        assertThat(p.getClassCookie(ParcelTest.class)).isEqualTo(null);

        p.setClassCookie(ParcelTest.class, "to_be_discarded_cookie");
        p.recycle();
        assertThat(p.getClassCookie(ParcelTest.class)).isNull();
    }

    @Test
    public void testClassCookies_removeUnexpected() {
        Parcel p = Parcel.obtain();

        assertLogsWtf(() -> p.removeClassCookie(ParcelTest.class, "not_present"));

        p.setClassCookie(ParcelTest.class, "value");

        assertLogsWtf(() -> p.removeClassCookie(ParcelTest.class, "different"));
        assertThat(p.getClassCookie(ParcelTest.class)).isNull(); // still removed

        p.recycle();
    }

    private static void assertLogsWtf(Runnable test) {
        ArrayList<Log.TerribleFailure> wtfs = new ArrayList<>();
        Log.TerribleFailureHandler oldHandler = Log.setWtfHandler(
                (tag, what, system) -> wtfs.add(what));
        try {
            test.run();
        } finally {
            Log.setWtfHandler(oldHandler);
        }
        assertThat(wtfs).hasSize(1);
    }
}
+16 −2
Original line number Diff line number Diff line
@@ -665,7 +665,7 @@ public class NotificationManagerService extends SystemService {
    private static final int MY_UID = Process.myUid();
    private static final int MY_PID = Process.myPid();
    private static final IBinder ALLOWLIST_TOKEN = new Binder();
    static final IBinder ALLOWLIST_TOKEN = new Binder();
    protected RankingHandler mRankingHandler;
    private long mLastOverRateLogTime;
    private float mMaxPackageEnqueueRate = DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE;
@@ -4477,7 +4477,7 @@ public class NotificationManagerService extends SystemService {
                    // Remove background token before returning notification to untrusted app, this
                    // ensures the app isn't able to perform background operations that are
                    // associated with notification interactions.
                    notification.clearAllowlistToken();
                    notification.overrideAllowlistToken(null);
                    return new StatusBarNotification(
                            sbn.getPackageName(),
                            sbn.getOpPkg(),
@@ -6736,6 +6736,15 @@ public class NotificationManagerService extends SystemService {
                    + " trying to post for invalid pkg " + pkg + " in user " + incomingUserId);
        }
        IBinder allowlistToken = notification.getAllowlistToken();
        if (allowlistToken != null && allowlistToken != ALLOWLIST_TOKEN) {
            throw new SecurityException(
                    "Unexpected allowlist token received from " + callingUid);
        }
        // allowlistToken is populated by unparceling, so it can be null if the notification was
        // posted from inside system_server. Ensure it's the expected value.
        notification.overrideAllowlistToken(ALLOWLIST_TOKEN);
        checkRestrictedCategories(notification);
        // Notifications passed to setForegroundService() have FLAG_FOREGROUND_SERVICE,
@@ -7800,6 +7809,11 @@ public class NotificationManagerService extends SystemService {
         */
        private boolean enqueueNotification() {
            synchronized (mNotificationLock) {
                // allowlistToken is populated by unparceling, so it will be absent if the
                // EnqueueNotificationRunnable is created directly by NMS (as we do for group
                // summaries) instead of via notify(). Fix that.
                r.getNotification().overrideAllowlistToken(ALLOWLIST_TOKEN);
                final long snoozeAt =
                        mSnoozeHelper.getSnoozeTimeForUnpostedNotification(
                                r.getUser().getIdentifier(),
+327 −0
Original line number Diff line number Diff line
@@ -309,6 +309,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
    private final int mUid = Binder.getCallingUid();
    private final @UserIdInt int mUserId = UserHandle.getUserId(mUid);
    private final UserHandle mUser = UserHandle.of(mUserId);
    private final String mPkg = mContext.getPackageName();
    private TestableNotificationManagerService mService;
    private INotificationManager mBinderService;
@@ -12453,6 +12455,331 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
        assertThat(mService.mNotificationList.get(0).getNotification().when).isEqualTo(111); // old
    }
    @Test
    public void enqueueNotification_acceptsCorrectToken() throws RemoteException {
        Notification sent = new Notification.Builder(mContext, TEST_CHANNEL_ID)
                .setContentIntent(createPendingIntent("content"))
                .build();
        Notification received = parcelAndUnparcel(sent, Notification.CREATOR);
        assertThat(received.getAllowlistToken()).isEqualTo(
                NotificationManagerService.ALLOWLIST_TOKEN);
        mBinderService.enqueueNotificationWithTag(mPkg, mPkg, "tag", 1,
                parcelAndUnparcel(received, Notification.CREATOR), mUserId);
        waitForIdle();
        assertThat(mService.mNotificationList).hasSize(1);
        assertThat(mService.mNotificationList.get(0).getNotification().getAllowlistToken())
                .isEqualTo(NotificationManagerService.ALLOWLIST_TOKEN);
    }
    @Test
    public void enqueueNotification_acceptsNullToken_andPopulatesIt() throws RemoteException {
        Notification receivedWithoutParceling = new Notification.Builder(mContext, TEST_CHANNEL_ID)
                .setContentIntent(createPendingIntent("content"))
                .build();
        assertThat(receivedWithoutParceling.getAllowlistToken()).isNull();
        mBinderService.enqueueNotificationWithTag(mPkg, mPkg, "tag", 1,
                parcelAndUnparcel(receivedWithoutParceling, Notification.CREATOR), mUserId);
        waitForIdle();
        assertThat(mService.mNotificationList).hasSize(1);
        assertThat(mService.mNotificationList.get(0).getNotification().getAllowlistToken())
                .isEqualTo(NotificationManagerService.ALLOWLIST_TOKEN);
    }
    @Test
    public void enqueueNotification_directlyThroughRunnable_populatesAllowlistToken() {
        Notification receivedWithoutParceling = new Notification.Builder(mContext, TEST_CHANNEL_ID)
                .setContentIntent(createPendingIntent("content"))
                .build();
        NotificationRecord record = new NotificationRecord(
                mContext,
                new StatusBarNotification(mPkg, mPkg, 1, "tag", mUid, 44, receivedWithoutParceling,
                        mUser, "groupKey", 0),
                mTestNotificationChannel);
        assertThat(record.getNotification().getAllowlistToken()).isNull();
        mWorkerHandler.post(
                mService.new EnqueueNotificationRunnable(mUserId, record, false,
                mPostNotificationTrackerFactory.newTracker(null)));
        waitForIdle();
        assertThat(mService.mNotificationList).hasSize(1);
        assertThat(mService.mNotificationList.get(0).getNotification().getAllowlistToken())
                .isEqualTo(NotificationManagerService.ALLOWLIST_TOKEN);
    }
    @Test
    public void enqueueNotification_rejectsOtherToken() throws RemoteException {
        Notification sent = new Notification.Builder(mContext, TEST_CHANNEL_ID)
                .setContentIntent(createPendingIntent("content"))
                .build();
        sent.overrideAllowlistToken(new Binder());
        Notification received = parcelAndUnparcel(sent, Notification.CREATOR);
        assertThat(received.getAllowlistToken()).isEqualTo(sent.getAllowlistToken());
        assertThrows(SecurityException.class, () ->
                mBinderService.enqueueNotificationWithTag(mPkg, mPkg, "tag", 1,
                        parcelAndUnparcel(received, Notification.CREATOR), mUserId));
        waitForIdle();
        assertThat(mService.mNotificationList).isEmpty();
    }
    @Test
    public void enqueueNotification_customParcelingWithFakeInnerToken_hasCorrectTokenInIntents()
            throws RemoteException {
        Notification sentFromApp = new Notification.Builder(mContext, TEST_CHANNEL_ID)
                .setContentIntent(createPendingIntent("content"))
                .setPublicVersion(new Notification.Builder(mContext, TEST_CHANNEL_ID)
                        .setContentIntent(createPendingIntent("public"))
                        .build())
                .build();
        sentFromApp.publicVersion.overrideAllowlistToken(new Binder());
        // Instead of using the normal parceling, assume the caller parcels it by hand, including a
        // null token in the outer notification (as would be expected, and as is verified by
        // enqueue) but trying to sneak in a different one in the inner notification, hoping it gets
        // propagated to the PendingIntents.
        Parcel parcelSentFromApp = Parcel.obtain();
        writeNotificationToParcelCustom(parcelSentFromApp, sentFromApp, new ArraySet<>(
                Lists.newArrayList(sentFromApp.contentIntent,
                        sentFromApp.publicVersion.contentIntent)));
        // Use the unparceling as received in enqueueNotificationWithTag()
        parcelSentFromApp.setDataPosition(0);
        Notification receivedByNms = new Notification(parcelSentFromApp);
        // Verify that all the pendingIntents have the correct token.
        assertThat(receivedByNms.contentIntent.getWhitelistToken()).isEqualTo(
                NotificationManagerService.ALLOWLIST_TOKEN);
        assertThat(receivedByNms.publicVersion.contentIntent.getWhitelistToken()).isEqualTo(
                NotificationManagerService.ALLOWLIST_TOKEN);
    }
    /**
     * Replicates the behavior of {@link Notification#writeToParcel} but excluding the
     * "always use the same allowlist token as the root notification" parts.
     */
    private static void writeNotificationToParcelCustom(Parcel parcel, Notification notif,
            ArraySet<PendingIntent> allPendingIntents) {
        int flags = 0;
        parcel.writeInt(1); // version?
        parcel.writeStrongBinder(notif.getAllowlistToken());
        parcel.writeLong(notif.when);
        parcel.writeLong(1234L); // notif.creationTime is private
        if (notif.getSmallIcon() != null) {
            parcel.writeInt(1);
            notif.getSmallIcon().writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        parcel.writeInt(notif.number);
        if (notif.contentIntent != null) {
            parcel.writeInt(1);
            notif.contentIntent.writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        if (notif.deleteIntent != null) {
            parcel.writeInt(1);
            notif.deleteIntent.writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        if (notif.tickerText != null) {
            parcel.writeInt(1);
            TextUtils.writeToParcel(notif.tickerText, parcel, flags);
        } else {
            parcel.writeInt(0);
        }
        if (notif.tickerView != null) {
            parcel.writeInt(1);
            notif.tickerView.writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        if (notif.contentView != null) {
            parcel.writeInt(1);
            notif.contentView.writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        if (notif.getLargeIcon() != null) {
            parcel.writeInt(1);
            notif.getLargeIcon().writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        parcel.writeInt(notif.defaults);
        parcel.writeInt(notif.flags);
        if (notif.sound != null) {
            parcel.writeInt(1);
            notif.sound.writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        parcel.writeInt(notif.audioStreamType);
        if (notif.audioAttributes != null) {
            parcel.writeInt(1);
            notif.audioAttributes.writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        parcel.writeLongArray(notif.vibrate);
        parcel.writeInt(notif.ledARGB);
        parcel.writeInt(notif.ledOnMS);
        parcel.writeInt(notif.ledOffMS);
        parcel.writeInt(notif.iconLevel);
        if (notif.fullScreenIntent != null) {
            parcel.writeInt(1);
            notif.fullScreenIntent.writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        parcel.writeInt(notif.priority);
        parcel.writeString8(notif.category);
        parcel.writeString8(notif.getGroup());
        parcel.writeString8(notif.getSortKey());
        parcel.writeBundle(notif.extras); // null ok
        parcel.writeTypedArray(notif.actions, 0); // null ok
        if (notif.bigContentView != null) {
            parcel.writeInt(1);
            notif.bigContentView.writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        if (notif.headsUpContentView != null) {
            parcel.writeInt(1);
            notif.headsUpContentView.writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        parcel.writeInt(notif.visibility);
        if (notif.publicVersion != null) {
            parcel.writeInt(1);
            writeNotificationToParcelCustom(parcel, notif.publicVersion, new ArraySet<>());
        } else {
            parcel.writeInt(0);
        }
        parcel.writeInt(notif.color);
        if (notif.getChannelId() != null) {
            parcel.writeInt(1);
            parcel.writeString8(notif.getChannelId());
        } else {
            parcel.writeInt(0);
        }
        parcel.writeLong(notif.getTimeoutAfter());
        if (notif.getShortcutId() != null) {
            parcel.writeInt(1);
            parcel.writeString8(notif.getShortcutId());
        } else {
            parcel.writeInt(0);
        }
        if (notif.getLocusId() != null) {
            parcel.writeInt(1);
            notif.getLocusId().writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        parcel.writeInt(notif.getBadgeIconType());
        if (notif.getSettingsText() != null) {
            parcel.writeInt(1);
            TextUtils.writeToParcel(notif.getSettingsText(), parcel, flags);
        } else {
            parcel.writeInt(0);
        }
        parcel.writeInt(notif.getGroupAlertBehavior());
        if (notif.getBubbleMetadata() != null) {
            parcel.writeInt(1);
            notif.getBubbleMetadata().writeToParcel(parcel, 0);
        } else {
            parcel.writeInt(0);
        }
        parcel.writeBoolean(notif.getAllowSystemGeneratedContextualActions());
        parcel.writeInt(Notification.FOREGROUND_SERVICE_DEFAULT); // no getter for mFgsDeferBehavior
        // mUsesStandardHeader is not written because it should be recomputed in listeners
        parcel.writeArraySet(allPendingIntents);
    }
    @Test
    @SuppressWarnings("unchecked")
    public void getActiveNotifications_doesNotLeakAllowlistToken() throws RemoteException {
        Notification sentFromApp = new Notification.Builder(mContext, TEST_CHANNEL_ID)
                .setContentIntent(createPendingIntent("content"))
                .setPublicVersion(new Notification.Builder(mContext, TEST_CHANNEL_ID)
                        .setContentIntent(createPendingIntent("public"))
                        .build())
                .extend(new Notification.WearableExtender()
                        .addPage(new Notification.Builder(mContext, TEST_CHANNEL_ID)
                                .setContentIntent(createPendingIntent("wearPage"))
                                .build()))
                .build();
        // Binder transition: app -> NMS
        Notification receivedByNms = parcelAndUnparcel(sentFromApp, Notification.CREATOR);
        assertThat(receivedByNms.getAllowlistToken()).isEqualTo(
                NotificationManagerService.ALLOWLIST_TOKEN);
        mBinderService.enqueueNotificationWithTag(mPkg, mPkg, "tag", 1,
                parcelAndUnparcel(receivedByNms, Notification.CREATOR), mUserId);
        waitForIdle();
        assertThat(mService.mNotificationList).hasSize(1);
        Notification posted = mService.mNotificationList.get(0).getNotification();
        assertThat(posted.getAllowlistToken()).isEqualTo(
                NotificationManagerService.ALLOWLIST_TOKEN);
        assertThat(posted.contentIntent.getWhitelistToken()).isEqualTo(
                NotificationManagerService.ALLOWLIST_TOKEN);
        ParceledListSlice<StatusBarNotification> listSentFromNms =
                mBinderService.getAppActiveNotifications(mPkg, mUserId);
        // Binder transition: NMS -> app. App doesn't have the allowlist token so clear it
        // (having a different one would produce the same effect; the relevant thing is to not let
        // out ALLOWLIST_TOKEN).
        // Note: for other tests, this is restored by constructing TestableNMS in setup().
        Notification.processAllowlistToken = null;
        ParceledListSlice<StatusBarNotification> listReceivedByApp = parcelAndUnparcel(
                listSentFromNms, ParceledListSlice.CREATOR);
        Notification gottenBackByApp = listReceivedByApp.getList().get(0).getNotification();
        assertThat(gottenBackByApp.getAllowlistToken()).isNull();
        assertThat(gottenBackByApp.contentIntent.getWhitelistToken()).isNull();
        assertThat(gottenBackByApp.publicVersion.getAllowlistToken()).isNull();
        assertThat(gottenBackByApp.publicVersion.contentIntent.getWhitelistToken()).isNull();
        assertThat(new Notification.WearableExtender(gottenBackByApp).getPages()
                .get(0).getAllowlistToken()).isNull();
        assertThat(new Notification.WearableExtender(gottenBackByApp).getPages()
                .get(0).contentIntent.getWhitelistToken()).isNull();
    }
    @Test
    public void enqueueNotification_allowlistsPendingIntents() throws RemoteException {
        PendingIntent contentIntent = createPendingIntent("content");