Fix allowlist token issues (4426de38) · Commits · e / os / android_frameworks_base

core/java/android/app/Notification.java

+56 −11

Original line number	Original line	Diff line number	Diff line
	@@ -2654,8 +2654,16 @@ public class Notification implements Parcelable
	if (mAllowlistToken == null) {		if (mAllowlistToken == null) {
	mAllowlistToken = processAllowlistToken;		mAllowlistToken = processAllowlistToken;
	}		}
			if (Flags.secureAllowlistToken()) {
			// 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);
			}
			} else {
	// 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.
	parcel.setClassCookie(PendingIntent.class, mAllowlistToken);		parcel.setClassCookie(PendingIntent.class, mAllowlistToken);
			}

	when = parcel.readLong();		when = parcel.readLong();
	creationTime = parcel.readLong();		creationTime = parcel.readLong();
	@@ -3189,9 +3197,29 @@ public class Notification implements Parcelable
	PendingIntent.addOnMarshaledListener(addedListener);		PendingIntent.addOnMarshaledListener(addedListener);
	}		}
	try {		try {
			if (Flags.secureAllowlistToken()) {
			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).
			parcel.setClassCookie(Notification.class, this);
			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, this);
			}
			}
			} else {
	// IMPORTANT: Add marshaling code in writeToParcelImpl as we		// IMPORTANT: Add marshaling code in writeToParcelImpl as we
	// want to intercept all pending events written to the parcel.		// want to intercept all pending events written to the parcel.
	writeToParcelImpl(parcel, flags);		writeToParcelImpl(parcel, flags);
			}

	synchronized (this) {		synchronized (this) {
	// Must be written last!		// Must be written last!
	parcel.writeArraySet(allPendingIntents);		parcel.writeArraySet(allPendingIntents);
	@@ -3206,7 +3234,19 @@ public class Notification implements Parcelable
	private void writeToParcelImpl(Parcel parcel, int flags) {		private void writeToParcelImpl(Parcel parcel, int flags) {
	parcel.writeInt(1);		parcel.writeInt(1);

			if (Flags.secureAllowlistToken()) {
			Notification rootNotification = (Notification) parcel.getClassCookie(
			Notification.class);
			if (rootNotification != null && rootNotification != this) {
			// Always use the same token as the root notification
			parcel.writeStrongBinder(rootNotification.mAllowlistToken);
			} else {
			parcel.writeStrongBinder(mAllowlistToken);
			}
			} else {
	parcel.writeStrongBinder(mAllowlistToken);		parcel.writeStrongBinder(mAllowlistToken);
			}

	parcel.writeLong(when);		parcel.writeLong(when);
	parcel.writeLong(creationTime);		parcel.writeLong(creationTime);
	if (mSmallIcon == null && icon != 0) {		if (mSmallIcon == null && icon != 0) {
	@@ -3599,18 +3639,23 @@ public class Notification implements Parcelable
	* Sets the token used for background operations for the pending intents associated with this		* Sets the token used for background operations for the pending intents associated with this
	* notification.		* notification.
	*		*
	* This token is automatically set during deserialization for you, you usually won't need to		* Note: Should <em>only</em> be invoked by NotificationManagerService, since this is normally
	* call this unless you want to change the existing token, if any.		* populated by unparceling (and also used there). Any other usage is suspect.
	*		*
	* @hide		* @hide
	*/		*/
	public void clearAllowlistToken() {		public void overrideAllowlistToken(IBinder token) {
	mAllowlistToken = null;		mAllowlistToken = token;
	if (publicVersion != null) {		if (publicVersion != null) {
	publicVersion.clearAllowlistToken();		publicVersion.overrideAllowlistToken(token);
	}		}
	}		}

			/** @hide */
			public IBinder getAllowlistToken() {
			return mAllowlistToken;
			}

	/**		/**
	* @hide		* @hide
	*/		*/

core/java/android/app/notification.aconfig

+10 −0

Original line number	Original line	Diff line number	Diff line
	@@ -74,6 +74,16 @@ flag {
	}		}
	}		}

			flag {
			name: "secure_allowlist_token"
			namespace: "systemui"
			description: "Prevents allowlist_token from leaking out and foreign tokens from being accepted"
			bug: "328254922"
			metadata {
			purpose: PURPOSE_BUGFIX
			}
			}

	flag {		flag {
	name: "update_ranking_time"		name: "update_ranking_time"
	namespace: "systemui"		namespace: "systemui"

core/java/android/os/Parcel.java

+22 −0

Original line number	Original line	Diff line number	Diff line
	@@ -862,6 +862,28 @@ public final class Parcel {
	return mClassCookies != null ? mClassCookies.get(clz) : null;		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 */		/** @hide */
	public final void adoptClassCookies(Parcel from) {		public final void adoptClassCookies(Parcel from) {
	mClassCookies = from.mClassCookies;		mClassCookies = from.mClassCookies;

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

+49 −1

Original line number	Original line	Diff line number	Diff line
	@@ -16,6 +16,8 @@

	package android.os;		package android.os;

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

	import static org.junit.Assert.assertEquals;		import static org.junit.Assert.assertEquals;
	import static org.junit.Assert.assertFalse;		import static org.junit.Assert.assertFalse;
	import static org.junit.Assert.assertThrows;		import static org.junit.Assert.assertThrows;
	@@ -24,6 +26,7 @@ import static org.junit.Assert.assertTrue;
	import android.platform.test.annotations.IgnoreUnderRavenwood;		import android.platform.test.annotations.IgnoreUnderRavenwood;
	import android.platform.test.annotations.Presubmit;		import android.platform.test.annotations.Presubmit;
	import android.platform.test.ravenwood.RavenwoodRule;		import android.platform.test.ravenwood.RavenwoodRule;
			import android.util.Log;

	import androidx.test.runner.AndroidJUnit4;		import androidx.test.runner.AndroidJUnit4;

	@@ -31,6 +34,8 @@ import org.junit.Rule;
	import org.junit.Test;		import org.junit.Test;
	import org.junit.runner.RunWith;		import org.junit.runner.RunWith;

			import java.util.ArrayList;

	@Presubmit		@Presubmit
	@RunWith(AndroidJUnit4.class)		@RunWith(AndroidJUnit4.class)
	public class ParcelTest {		public class ParcelTest {
	@@ -348,6 +353,50 @@ public class ParcelTest {
	Binder.setIsDirectlyHandlingTransactionOverride(false);		Binder.setIsDirectlyHandlingTransactionOverride(false);
	}		}

			@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);
			}

	@Test		@Test
	@IgnoreUnderRavenwood(blockedBy = Parcel.class)		@IgnoreUnderRavenwood(blockedBy = Parcel.class)
	public void testHasBinders_AfterWritingBinderToParcel() {		public void testHasBinders_AfterWritingBinderToParcel() {
	@@ -360,7 +409,6 @@ public class ParcelTest {
	assertTrue(pA.hasBinders());		assertTrue(pA.hasBinders());
	}		}


	@Test		@Test
	@IgnoreUnderRavenwood(blockedBy = Parcel.class)		@IgnoreUnderRavenwood(blockedBy = Parcel.class)
	public void testHasBindersInRange_AfterWritingBinderToParcel() {		public void testHasBindersInRange_AfterWritingBinderToParcel() {

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

+13 −2

Original line number	Original line	Diff line number	Diff line
	@@ -701,7 +701,7 @@ public class NotificationManagerService extends SystemService {

	private static final int MY_UID = Process.myUid();		private static final int MY_UID = Process.myUid();
	private static final int MY_PID = Process.myPid();		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;		protected RankingHandler mRankingHandler;
	private long mLastOverRateLogTime;		private long mLastOverRateLogTime;
	private float mMaxPackageEnqueueRate = DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE;		private float mMaxPackageEnqueueRate = DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE;
	@@ -4643,7 +4643,7 @@ public class NotificationManagerService extends SystemService {
	// Remove background token before returning notification to untrusted app, this		// Remove background token before returning notification to untrusted app, this
	// ensures the app isn't able to perform background operations that are		// ensures the app isn't able to perform background operations that are
	// associated with notification interactions.		// associated with notification interactions.
	notification.clearAllowlistToken();		notification.overrideAllowlistToken(null);
	return new StatusBarNotification(		return new StatusBarNotification(
	sbn.getPackageName(),		sbn.getPackageName(),
	sbn.getOpPkg(),		sbn.getOpPkg(),
	@@ -7240,6 +7240,17 @@ public class NotificationManagerService extends SystemService {
	+ " trying to post for invalid pkg " + pkg + " in user " + incomingUserId);		+ " trying to post for invalid pkg " + pkg + " in user " + incomingUserId);
	}		}

			if (android.app.Flags.secureAllowlistToken()) {
			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);		checkRestrictedCategories(notification);

	// Notifications passed to setForegroundService() have FLAG_FOREGROUND_SERVICE,		// Notifications passed to setForegroundService() have FLAG_FOREGROUND_SERVICE,