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

Commit 6a97cc3b authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Grant notification Uri permissions as sending app.

For security reasons, the system UID can't make URI permission as
itself; it always needs to do so on behalf of a specific app.  To
handle this, we grant notification Uri permissions as the UID that
sent a given notification.

To give meaningful debug messages to developers, check to see if the
caller has permissions to grant Uri access when they're enqueuing
a notification.  If they're targeting P, throw any security issues
back at the caller; if older SDK, log and ignore that Uri.

Since multiple notifications can grant access to the same content,
we need unique UriPermissionOwner per active notification.  For
example, consider these two notifications:

1. sound=content://sound, image=content://image1
2. sound=content://sound, image=content://image2

When #1 is cancelled, we still need to keep the content://sound
grant active until #2 is also cancelled.  Using unique owners
means that ActivityManagerService tracks reference counting on
our behalf.

Optimizations to avoid allocations in hot code paths.

Test: atest frameworks/base/services/tests/uiservicestests/src/com/android/server/notification
Bug: 9069730
Change-Id: I69601793538adcbf06c4986a2fb1ea2dd9d876eb
parent 167032ab
Loading
Loading
Loading
Loading
+41 −33
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ import android.app.KeyguardManager;
import android.content.ClipData;
import android.content.ClipDescription;
import android.content.ContentProvider;
import android.content.ContentResolver;
import android.content.Context;
import android.content.IClipboard;
import android.content.IOnPrimaryClipChangedListener;
@@ -490,17 +491,18 @@ public class ClipboardService extends SystemService {
        }
    }

    private final void checkUriOwnerLocked(Uri uri, int uid) {
        if (!"content".equals(uri.getScheme())) {
            return;
        }
        long ident = Binder.clearCallingIdentity();
    private final void checkUriOwnerLocked(Uri uri, int sourceUid) {
        if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return;

        final long ident = Binder.clearCallingIdentity();
        try {
            // This will throw SecurityException for us.
            mAm.checkGrantUriPermission(uid, null, ContentProvider.getUriWithoutUserId(uri),
            // This will throw SecurityException if caller can't grant
            mAm.checkGrantUriPermission(sourceUid, null,
                    ContentProvider.getUriWithoutUserId(uri),
                    Intent.FLAG_GRANT_READ_URI_PERMISSION,
                    ContentProvider.getUserIdFromUri(uri, UserHandle.getUserId(uid)));
        } catch (RemoteException e) {
                    ContentProvider.getUserIdFromUri(uri, UserHandle.getUserId(sourceUid)));
        } catch (RemoteException ignored) {
            // Ignored because we're in same process
        } finally {
            Binder.restoreCallingIdentity(ident);
        }
@@ -523,27 +525,32 @@ public class ClipboardService extends SystemService {
        }
    }

    private final void grantUriLocked(Uri uri, int primaryClipUid, String pkg, int userId) {
        long ident = Binder.clearCallingIdentity();
    private final void grantUriLocked(Uri uri, int sourceUid, String targetPkg,
            int targetUserId) {
        if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return;

        final long ident = Binder.clearCallingIdentity();
        try {
            int sourceUserId = ContentProvider.getUserIdFromUri(uri, userId);
            uri = ContentProvider.getUriWithoutUserId(uri);
            mAm.grantUriPermissionFromOwner(mPermissionOwner, primaryClipUid, pkg,
                    uri, Intent.FLAG_GRANT_READ_URI_PERMISSION, sourceUserId, userId);
        } catch (RemoteException e) {
            mAm.grantUriPermissionFromOwner(mPermissionOwner, sourceUid, targetPkg,
                    ContentProvider.getUriWithoutUserId(uri),
                    Intent.FLAG_GRANT_READ_URI_PERMISSION,
                    ContentProvider.getUserIdFromUri(uri, UserHandle.getUserId(sourceUid)),
                    targetUserId);
        } catch (RemoteException ignored) {
            // Ignored because we're in same process
        } finally {
            Binder.restoreCallingIdentity(ident);
        }
    }

    private final void grantItemLocked(ClipData.Item item, int primaryClipUid, String pkg,
            int userId) {
    private final void grantItemLocked(ClipData.Item item, int sourceUid, String targetPkg,
            int targetUserId) {
        if (item.getUri() != null) {
            grantUriLocked(item.getUri(), primaryClipUid, pkg, userId);
            grantUriLocked(item.getUri(), sourceUid, targetPkg, targetUserId);
        }
        Intent intent = item.getIntent();
        if (intent != null && intent.getData() != null) {
            grantUriLocked(intent.getData(), primaryClipUid, pkg, userId);
            grantUriLocked(intent.getData(), sourceUid, targetPkg, targetUserId);
        }
    }

@@ -576,28 +583,29 @@ public class ClipboardService extends SystemService {
        }
    }

    private final void revokeUriLocked(Uri uri) {
        int userId = ContentProvider.getUserIdFromUri(uri,
                UserHandle.getUserId(Binder.getCallingUid()));
        long ident = Binder.clearCallingIdentity();
    private final void revokeUriLocked(Uri uri, int sourceUid) {
        if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return;

        final long ident = Binder.clearCallingIdentity();
        try {
            uri = ContentProvider.getUriWithoutUserId(uri);
            mAm.revokeUriPermissionFromOwner(mPermissionOwner, uri,
                    Intent.FLAG_GRANT_READ_URI_PERMISSION | Intent.FLAG_GRANT_WRITE_URI_PERMISSION,
                    userId);
        } catch (RemoteException e) {
            mAm.revokeUriPermissionFromOwner(mPermissionOwner,
                    ContentProvider.getUriWithoutUserId(uri),
                    Intent.FLAG_GRANT_READ_URI_PERMISSION,
                    ContentProvider.getUserIdFromUri(uri, UserHandle.getUserId(sourceUid)));
        } catch (RemoteException ignored) {
            // Ignored because we're in same process
        } finally {
            Binder.restoreCallingIdentity(ident);
        }
    }

    private final void revokeItemLocked(ClipData.Item item) {
    private final void revokeItemLocked(ClipData.Item item, int sourceUid) {
        if (item.getUri() != null) {
            revokeUriLocked(item.getUri());
            revokeUriLocked(item.getUri(), sourceUid);
        }
        Intent intent = item.getIntent();
        if (intent != null && intent.getData() != null) {
            revokeUriLocked(intent.getData());
            revokeUriLocked(intent.getData(), sourceUid);
        }
    }

@@ -607,7 +615,7 @@ public class ClipboardService extends SystemService {
        }
        final int N = clipboard.primaryClip.getItemCount();
        for (int i=0; i<N; i++) {
            revokeItemLocked(clipboard.primaryClip.getItemAt(i));
            revokeItemLocked(clipboard.primaryClip.getItemAt(i), clipboard.primaryClipUid);
        }
    }

+120 −63
Original line number Diff line number Diff line
@@ -37,7 +37,6 @@ import static android.content.pm.PackageManager.FEATURE_TELEVISION;
import static android.content.pm.PackageManager.PERMISSION_GRANTED;
import static android.os.IServiceManager.DUMP_FLAG_PRIORITY_CRITICAL;
import static android.os.IServiceManager.DUMP_FLAG_PRIORITY_NORMAL;
import static android.os.UserHandle.USER_ALL;
import static android.os.UserHandle.USER_NULL;
import static android.os.UserHandle.USER_SYSTEM;
import static android.service.notification.NotificationListenerService
@@ -228,7 +227,6 @@ import java.nio.charset.StandardCharsets;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map.Entry;
@@ -315,7 +313,6 @@ public class NotificationManagerService extends SystemService {
    private ICompanionDeviceManager mCompanionManager;
    private AccessibilityManager mAccessibilityManager;
    private IDeviceIdleController mDeviceIdleController;
    private IBinder mPermissionOwner;

    final IBinder mForegroundToken = new Binder();
    private WorkerHandler mHandler;
@@ -1379,12 +1376,6 @@ public class NotificationManagerService extends SystemService {
                ServiceManager.getService(Context.DEVICE_IDLE_CONTROLLER));
        mDpm = dpm;

        try {
            mPermissionOwner = mAm.newUriPermissionOwner("notification");
        } catch (RemoteException e) {
            Slog.w(TAG, "AM dead", e);
        }

        mHandler = new WorkerHandler(looper);
        mRankingThread.start();
        String[] extractorNames;
@@ -3967,7 +3958,7 @@ public class NotificationManagerService extends SystemService {
            sbn.getNotification().flags =
                    (r.mOriginalFlags & ~Notification.FLAG_FOREGROUND_SERVICE);
            mRankingHelper.sort(mNotificationList);
            mListeners.notifyPostedLocked(r, sbn /* oldSbn */);
            mListeners.notifyPostedLocked(r, r);
        }
    };

@@ -4433,8 +4424,6 @@ public class NotificationManagerService extends SystemService {
                        // Make sure we don't lose the foreground service state.
                        notification.flags |=
                                old.getNotification().flags & Notification.FLAG_FOREGROUND_SERVICE;
                        // revoke uri permissions for changed uris
                        revokeUriPermissions(r, old);
                        r.isUpdate = true;
                        r.setInterruptive(isVisuallyInterruptive(old, r));
                    }
@@ -4453,7 +4442,7 @@ public class NotificationManagerService extends SystemService {

                    if (notification.getSmallIcon() != null) {
                        StatusBarNotification oldSbn = (old != null) ? old.sbn : null;
                        mListeners.notifyPostedLocked(r, oldSbn);
                        mListeners.notifyPostedLocked(r, old);
                        if (oldSbn == null || !Objects.equals(oldSbn.getGroup(), n.getGroup())) {
                            mHandler.post(new Runnable() {
                                @Override
@@ -5306,9 +5295,6 @@ public class NotificationManagerService extends SystemService {
            r.recordDismissalSurface(NotificationStats.DISMISSAL_OTHER);
        }

        // Revoke permissions
        revokeUriPermissions(null, r);

        // tell the app
        if (sendDelete) {
            if (r.getNotification().deleteIntent != null) {
@@ -5406,25 +5392,111 @@ public class NotificationManagerService extends SystemService {
                r.getLifespanMs(now), r.getFreshnessMs(now), r.getExposureMs(now), listenerName);
    }

    void revokeUriPermissions(NotificationRecord newRecord, NotificationRecord oldRecord) {
        Set<Uri> oldUris = oldRecord.getNotificationUris();
        Set<Uri> newUris = newRecord == null ? new HashSet<>() : newRecord.getNotificationUris();
        oldUris.removeAll(newUris);
    @VisibleForTesting
    void updateUriPermissions(@Nullable NotificationRecord newRecord,
            @Nullable NotificationRecord oldRecord, String targetPkg, int targetUserId) {
        final String key = (newRecord != null) ? newRecord.getKey() : oldRecord.getKey();
        if (DBG) Slog.d(TAG, key + ": updating permissions");

        final ArraySet<Uri> newUris = (newRecord != null) ? newRecord.getGrantableUris() : null;
        final ArraySet<Uri> oldUris = (oldRecord != null) ? oldRecord.getGrantableUris() : null;

        long ident = Binder.clearCallingIdentity();
        // Shortcut when no Uris involved
        if (newUris == null && oldUris == null) {
            return;
        }

        // Inherit any existing owner
        IBinder permissionOwner = null;
        if (newRecord != null && permissionOwner == null) {
            permissionOwner = newRecord.permissionOwner;
        }
        if (oldRecord != null && permissionOwner == null) {
            permissionOwner = oldRecord.permissionOwner;
        }

        // If we have Uris to grant, but no owner yet, go create one
        if (newUris != null && permissionOwner == null) {
            try {
            for (Uri uri : oldUris) {
                if (uri != null) {
                    int notiUserId = oldRecord.getUserId();
                    int sourceUserId = notiUserId == USER_ALL ? USER_SYSTEM
                            : ContentProvider.getUserIdFromUri(uri, notiUserId);
                    uri = ContentProvider.getUriWithoutUserId(uri);
                    mAm.revokeUriPermissionFromOwner(mPermissionOwner,
                            uri, Intent.FLAG_GRANT_READ_URI_PERMISSION, sourceUserId);
                if (DBG) Slog.d(TAG, key + ": creating owner");
                permissionOwner = mAm.newUriPermissionOwner("NOTIF:" + key);
            } catch (RemoteException ignored) {
                // Ignored because we're in same process
            }
        }
        } catch (RemoteException e) {
            Log.e(TAG, "Count not revoke uri permissions", e);

        // If we have no Uris to grant, but an existing owner, go destroy it
        if (newUris == null && permissionOwner != null) {
            final long ident = Binder.clearCallingIdentity();
            try {
                if (DBG) Slog.d(TAG, key + ": destroying owner");
                mAm.revokeUriPermissionFromOwner(permissionOwner, null, ~0,
                        UserHandle.getUserId(oldRecord.getUid()));
                permissionOwner = null;
            } catch (RemoteException ignored) {
                // Ignored because we're in same process
            } finally {
                Binder.restoreCallingIdentity(ident);
            }
        }

        // Grant access to new Uris
        if (newUris != null && permissionOwner != null) {
            for (int i = 0; i < newUris.size(); i++) {
                final Uri uri = newUris.valueAt(i);
                if (oldUris == null || !oldUris.contains(uri)) {
                    if (DBG) Slog.d(TAG, key + ": granting " + uri);
                    grantUriPermission(permissionOwner, uri, newRecord.getUid(), targetPkg,
                            targetUserId);
                }
            }
        }

        // Revoke access to old Uris
        if (oldUris != null && permissionOwner != null) {
            for (int i = 0; i < oldUris.size(); i++) {
                final Uri uri = oldUris.valueAt(i);
                if (newUris == null || !newUris.contains(uri)) {
                    if (DBG) Slog.d(TAG, key + ": revoking " + uri);
                    revokeUriPermission(permissionOwner, uri, oldRecord.getUid());
                }
            }
        }

        if (newRecord != null) {
            newRecord.permissionOwner = permissionOwner;
        }
    }

    private void grantUriPermission(IBinder owner, Uri uri, int sourceUid, String targetPkg,
            int targetUserId) {
        if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return;

        final long ident = Binder.clearCallingIdentity();
        try {
            mAm.grantUriPermissionFromOwner(owner, sourceUid, targetPkg,
                    ContentProvider.getUriWithoutUserId(uri),
                    Intent.FLAG_GRANT_READ_URI_PERMISSION,
                    ContentProvider.getUserIdFromUri(uri, UserHandle.getUserId(sourceUid)),
                    targetUserId);
        } catch (RemoteException ignored) {
            // Ignored because we're in same process
        } finally {
            Binder.restoreCallingIdentity(ident);
        }
    }

    private void revokeUriPermission(IBinder owner, Uri uri, int sourceUid) {
        if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return;

        final long ident = Binder.clearCallingIdentity();
        try {
            mAm.revokeUriPermissionFromOwner(owner,
                    ContentProvider.getUriWithoutUserId(uri),
                    Intent.FLAG_GRANT_READ_URI_PERMISSION,
                    ContentProvider.getUserIdFromUri(uri, UserHandle.getUserId(sourceUid)));
        } catch (RemoteException ignored) {
            // Ignored because we're in same process
        } finally {
            Binder.restoreCallingIdentity(ident);
        }
@@ -6317,8 +6389,8 @@ public class NotificationManagerService extends SystemService {
         * but isn't anymore.
         */
        @GuardedBy("mNotificationLock")
        public void notifyPostedLocked(NotificationRecord r, StatusBarNotification oldSbn) {
            notifyPostedLocked(r, oldSbn, true);
        public void notifyPostedLocked(NotificationRecord r, NotificationRecord old) {
            notifyPostedLocked(r, old, true);
        }

        /**
@@ -6326,14 +6398,13 @@ public class NotificationManagerService extends SystemService {
         *                           targetting <= O_MR1
         */
        @GuardedBy("mNotificationLock")
        private void notifyPostedLocked(NotificationRecord r, StatusBarNotification oldSbn,
        private void notifyPostedLocked(NotificationRecord r, NotificationRecord old,
                boolean notifyAllListeners) {
            // Lazily initialized snapshots of the notification.
            StatusBarNotification sbn = r.sbn;
            StatusBarNotification oldSbn = (old != null) ? old.sbn : null;
            TrimCache trimCache = new TrimCache(sbn);

            Set<Uri> uris = r.getNotificationUris();

            for (final ManagedServiceInfo info : getServices()) {
                boolean sbnVisible = isVisibleToListener(sbn, info);
                boolean oldSbnVisible = oldSbn != null ? isVisibleToListener(oldSbn, info) : false;
@@ -6371,8 +6442,10 @@ public class NotificationManagerService extends SystemService {
                    continue;
                }

                grantUriPermissions(uris, sbn.getUserId(), info.component.getPackageName(),
                        info.userid);
                // Grant access before listener is notified
                final int targetUserId = (info.userid == UserHandle.USER_ALL)
                        ? UserHandle.USER_SYSTEM : info.userid;
                updateUriPermissions(r, old, info.component.getPackageName(), targetUserId);

                final StatusBarNotification sbnToPost = trimCache.ForListener(info);
                mHandler.post(new Runnable() {
@@ -6384,28 +6457,6 @@ public class NotificationManagerService extends SystemService {
            }
        }

        private void grantUriPermissions(Set<Uri> uris, int notiUserId, String listenerPkg,
                int listenerUserId) {
            long ident = Binder.clearCallingIdentity();
            try {
                for (Uri uri : uris) {
                    if (uri != null) {
                        int sourceUserId = notiUserId == USER_ALL ? USER_SYSTEM
                                : ContentProvider.getUserIdFromUri(uri, notiUserId);
                        uri = ContentProvider.getUriWithoutUserId(uri);
                        mAm.grantUriPermissionFromOwner(mPermissionOwner, Process.myUid(),
                                listenerPkg,
                                uri, Intent.FLAG_GRANT_READ_URI_PERMISSION, sourceUserId,
                                listenerUserId == USER_ALL ? USER_SYSTEM : listenerUserId);
                    }
                }
            } catch (RemoteException e) {
                Log.e(TAG, "Count not grant uri permission to " + listenerPkg, e);
            } finally {
                Binder.restoreCallingIdentity(ident);
            }
        }

        /**
         * asynchronously notify all listeners about a removed notification
         */
@@ -6413,6 +6464,7 @@ public class NotificationManagerService extends SystemService {
        public void notifyRemovedLocked(NotificationRecord r, int reason,
                NotificationStats notificationStats) {
            final StatusBarNotification sbn = r.sbn;

            // make a copy in case changes are made to the underlying Notification object
            // NOTE: this copy is lightweight: it doesn't include heavyweight parts of the
            // notification
@@ -6447,6 +6499,11 @@ public class NotificationManagerService extends SystemService {
                    }
                });
            }

            // Revoke access after all listeners have been updated
            mHandler.post(() -> {
                updateUriPermissions(null, r, null, UserHandle.USER_SYSTEM);
            });
        }

        /**
@@ -6548,7 +6605,7 @@ public class NotificationManagerService extends SystemService {
            int numChangedNotifications = changedNotifications.size();
            for (int i = 0; i < numChangedNotifications; i++) {
                NotificationRecord rec = changedNotifications.get(i);
                mListeners.notifyPostedLocked(rec, rec.sbn, false);
                mListeners.notifyPostedLocked(rec, rec, false);
            }
        }

+109 −40

File changed.

Preview size limit exceeded, changes collapsed.

+1 −0
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@
    <uses-permission android:name="android.permission.ACCESS_VOICE_INTERACTION_SERVICE" />
    <uses-permission android:name="android.permission.DEVICE_POWER" />
    <uses-permission android:name="android.permission.ACCESS_CONTENT_PROVIDERS_EXTERNALLY" />
    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />

    <application android:debuggable="true">
        <uses-library android:name="android.test.runner" />
+29 −2
Original line number Diff line number Diff line
@@ -13,15 +13,25 @@
 */
package com.android.server;

import android.content.Context;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.when;

import android.content.pm.PackageManagerInternal;
import android.os.Build;
import android.support.test.InstrumentationRegistry;
import android.testing.TestableContext;

import org.junit.Before;
import org.junit.Rule;

import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

public class UiServiceTestCase {
    @Mock protected PackageManagerInternal mPmi;

    protected static final String PKG_N_MR1 = "com.example.n_mr1";
    protected static final String PKG_O = "com.example.o";

    @Rule
    public final TestableContext mContext =
            new TestableContext(InstrumentationRegistry.getContext(), null);
@@ -32,7 +42,24 @@ public class UiServiceTestCase {

    @Before
    public void setup() {
        MockitoAnnotations.initMocks(this);

        // Share classloader to allow package access.
        System.setProperty("dexmaker.share_classloader", "true");

        // Assume some default packages
        LocalServices.removeServiceForTest(PackageManagerInternal.class);
        LocalServices.addService(PackageManagerInternal.class, mPmi);
        when(mPmi.getPackageTargetSdkVersion(anyString()))
                .thenAnswer((iom) -> {
                    switch ((String) iom.getArgument(0)) {
                        case PKG_N_MR1:
                            return Build.VERSION_CODES.N_MR1;
                        case PKG_O:
                            return Build.VERSION_CODES.O;
                        default:
                            return Build.VERSION_CODES.CUR_DEVELOPMENT;
                    }
                });
    }
}
Loading