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
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:8a7453435b633282a9445ee01a902f090adc138c)
Merged-In: I232e9b74eece745560ed2e762071b48984b3f176
Change-Id: I232e9b74eece745560ed2e762071b48984b3f176

Author: Matías Hernández

core/java/android/app/Notification.java

@@ -2565,8 +2565,11 @@ private void readFromParcelImpl(Parcel parcel)
         if (mAllowlistToken == null) {
             mAllowlistToken = processAllowlistToken;
         }
-        // Propagate this token to all pending intents that are unmarshalled from the parcel.
-        parcel.setClassCookie(PendingIntent.class, mAllowlistToken);
+        // 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();
@@ -3027,9 +3030,24 @@ public void writeToParcel(Parcel parcel, int flags) {
             });
         }
         try {
-            // IMPORTANT: Add marshaling code in writeToParcelImpl as we
-            // want to intercept all pending events written to the parcel.
-            writeToParcelImpl(parcel, flags);
+            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);
@@ -3044,7 +3062,10 @@ public void writeToParcel(Parcel parcel, int flags) {
     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) {
@@ -3400,18 +3421,23 @@ public void setLatestEventInfo(Context context,
      * 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;
+    }
+
     /**
      * @hide
      */

core/java/android/os/Parcel.java

@@ -782,6 +782,28 @@ public final Object getClassCookie(Class clz) {
         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;

core/tests/coretests/src/android/os/ParcelTest.java

@@ -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 {
@@ -239,4 +244,48 @@ public void testCompareDataInRange_whenNegativeOffset_throws() {
         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);
+    }
 }

services/core/java/com/android/server/notification/NotificationManagerService.java

@@ -643,7 +643,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;
@@ -4398,7 +4398,7 @@ private StatusBarNotification sanitizeSbn(String pkg, int userId,
                     // 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(),
@@ -6560,6 +6560,15 @@ void enqueueNotificationInternal(final String pkg, final String opPkg, final int
                     + " 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,
@@ -7480,6 +7489,11 @@ protected class EnqueueNotificationRunnable implements Runnable {
         @Override
         public void run() {
             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(),