Merge "Fix allowlist token issues" into udc-qpr-dev
diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java
index ced3554..b1261ae 100644
--- a/core/java/android/app/Notification.java
+++ b/core/java/android/app/Notification.java
@@ -2598,8 +2598,11 @@
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 @@
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 @@
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 @@
* 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 e784c26..453aba3 100644
--- a/core/java/android/os/Parcel.java
+++ b/core/java/android/os/Parcel.java
@@ -816,6 +816,28 @@
}
/** @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 e2fe87b4..9143f74 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 @@
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 0e48c3f..bb1de5e 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 @@
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 @@
// 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 @@
+ " 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 @@
*/
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 4deaea9..096478d 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 @@
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 @@
}
@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");