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

Commit ef34c77b authored by Matías Hernández's avatar Matías Hernández Committed by Android (Google) Code Review
Browse files

Merge "Fix allowlist token issues" into udc-dev

parents cfd0d1c5 697c31d7
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();
@@ -3058,10 +3061,25 @@ public class Notification implements Parcelable
                }
            });
        }
        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);
@@ -3076,7 +3094,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) {
@@ -3469,16 +3490,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
@@ -667,7 +667,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;
@@ -4472,7 +4472,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(),
@@ -6713,6 +6713,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,
@@ -7776,6 +7785,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(),
+346 −3

File changed.

Preview size limit exceeded, changes collapsed.