diff options
5 files changed, 451 insertions, 13 deletions
diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java index ced35549769a..b1261ae719e6 100644 --- a/core/java/android/app/Notification.java +++ b/core/java/android/app/Notification.java @@ -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. - 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(); @@ -3061,9 +3064,24 @@ public class Notification implements Parcelable PendingIntent.addOnMarshaledListener(addedListener); } 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); @@ -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,18 +3492,23 @@ 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; + } + /** * @hide */ diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index e784c2669575..453aba34dbcf 100644 --- a/core/java/android/os/Parcel.java +++ b/core/java/android/os/Parcel.java @@ -816,6 +816,28 @@ public final class Parcel { } /** @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; } diff --git a/core/tests/coretests/src/android/os/ParcelTest.java b/core/tests/coretests/src/android/os/ParcelTest.java index e2fe87b4cfe3..9143f74ea054 100644 --- a/core/tests/coretests/src/android/os/ParcelTest.java +++ b/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 { @@ -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); + } } diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 0e48c3f83bbe..bb1de5eada8b 100644 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -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(), diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index 4deaea99f117..096478dfc000 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -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; @@ -12454,6 +12456,331 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { } @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"); PendingIntent actionIntent1 = createPendingIntent("action1"); |