OSDN Git Service

Merge "Grant notification Uri permissions as sending app." into pi-dev
authorJeff Sharkey <jsharkey@google.com>
Wed, 18 Apr 2018 18:18:48 +0000 (18:18 +0000)
committerAndroid (Google) Code Review <android-gerrit@google.com>
Wed, 18 Apr 2018 18:18:48 +0000 (18:18 +0000)
services/core/java/com/android/server/clipboard/ClipboardService.java
services/core/java/com/android/server/notification/NotificationManagerService.java
services/core/java/com/android/server/notification/NotificationRecord.java
services/tests/uiservicestests/AndroidManifest.xml
services/tests/uiservicestests/src/com/android/server/UiServiceTestCase.java
services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java
services/tests/uiservicestests/src/com/android/server/notification/NotificationRecordTest.java
services/tests/uiservicestests/src/com/android/server/slice/SliceManagerServiceTest.java

index 3f39f45..f74ac47 100644 (file)
@@ -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);
         }
     }
 
index aee4d28..53e741e 100644 (file)
@@ -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
@@ -5314,9 +5303,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) {
@@ -5420,25 +5406,111 @@ public class NotificationManagerService extends SystemService {
                 rank, count, 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();
-        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);
+        // 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 {
+                if (DBG) Slog.d(TAG, key + ": creating owner");
+                permissionOwner = mAm.newUriPermissionOwner("NOTIF:" + key);
+            } catch (RemoteException ignored) {
+                // Ignored because we're in same process
+            }
+        }
+
+        // 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);
                 }
             }
-        } catch (RemoteException e) {
-            Log.e(TAG, "Count not revoke uri permissions", e);
+        }
+
+        // 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);
         }
@@ -6343,8 +6415,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);
         }
 
         /**
@@ -6352,14 +6424,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;
@@ -6397,10 +6468,12 @@ 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);
+                final StatusBarNotification sbnToPost = trimCache.ForListener(info);
                 mHandler.post(new Runnable() {
                     @Override
                     public void run() {
@@ -6410,28 +6483,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
          */
@@ -6439,6 +6490,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
@@ -6473,6 +6525,11 @@ public class NotificationManagerService extends SystemService {
                     }
                 });
             }
+
+            // Revoke access after all listeners have been updated
+            mHandler.post(() -> {
+                updateUriPermissions(null, r, null, UserHandle.USER_SYSTEM);
+            });
         }
 
         /**
@@ -6574,7 +6631,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);
             }
         }
 
index 57af2ce..72a1a71 100644 (file)
  */
 package com.android.server.notification;
 
+import static android.app.Notification.EXTRA_AUDIO_CONTENTS_URI;
+import static android.app.Notification.EXTRA_BACKGROUND_IMAGE_URI;
+import static android.app.Notification.EXTRA_HISTORIC_MESSAGES;
+import static android.app.Notification.EXTRA_MESSAGES;
 import static android.app.NotificationChannel.USER_LOCKED_IMPORTANCE;
-import static android.app.NotificationManager.IMPORTANCE_MIN;
-import static android.app.NotificationManager.IMPORTANCE_UNSPECIFIED;
 import static android.app.NotificationManager.IMPORTANCE_DEFAULT;
 import static android.app.NotificationManager.IMPORTANCE_HIGH;
 import static android.app.NotificationManager.IMPORTANCE_LOW;
+import static android.app.NotificationManager.IMPORTANCE_MIN;
+import static android.app.NotificationManager.IMPORTANCE_UNSPECIFIED;
 import static android.service.notification.NotificationListenerService.Ranking
         .USER_SENTIMENT_NEUTRAL;
 import static android.service.notification.NotificationListenerService.Ranking
         .USER_SENTIMENT_POSITIVE;
 
+import android.annotation.Nullable;
+import android.app.ActivityManager;
+import android.app.ActivityManagerInternal;
+import android.app.IActivityManager;
 import android.app.Notification;
+import android.app.Notification.MessagingStyle;
 import android.app.NotificationChannel;
+import android.content.ContentProvider;
+import android.content.ContentResolver;
 import android.content.Context;
-import android.content.pm.ApplicationInfo;
+import android.content.Intent;
 import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
+import android.content.pm.PackageManagerInternal;
 import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.drawable.Icon;
@@ -39,9 +51,12 @@ import android.media.AudioAttributes;
 import android.media.AudioSystem;
 import android.metrics.LogMaker;
 import android.net.Uri;
+import android.os.Binder;
 import android.os.Build;
 import android.os.Bundle;
+import android.os.IBinder;
 import android.os.Parcelable;
+import android.os.RemoteException;
 import android.os.UserHandle;
 import android.provider.Settings;
 import android.service.notification.Adjustment;
@@ -53,7 +68,6 @@ import android.service.notification.StatusBarNotification;
 import android.text.TextUtils;
 import android.util.ArraySet;
 import android.util.Log;
-import android.util.Slog;
 import android.util.TimeUtils;
 import android.util.proto.ProtoOutputStream;
 import android.widget.RemoteViews;
@@ -61,7 +75,9 @@ import android.widget.RemoteViews;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.logging.MetricsLogger;
 import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
+import com.android.internal.util.ArrayUtils;
 import com.android.server.EventLogTags;
+import com.android.server.LocalServices;
 
 import java.io.PrintWriter;
 import java.lang.reflect.Array;
@@ -69,7 +85,6 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Objects;
-import java.util.Set;
 
 /**
  * Holds data about notifications that should not be shared with the
@@ -88,11 +103,13 @@ public final class NotificationRecord {
     static final boolean DBG = Log.isLoggable(TAG, Log.DEBUG);
     private static final int MAX_LOGTAG_LENGTH = 35;
     final StatusBarNotification sbn;
+    final int mTargetSdkVersion;
     final int mOriginalFlags;
     private final Context mContext;
 
     NotificationUsageStats.SingleNotificationStats stats;
     boolean isCanceled;
+    IBinder permissionOwner;
 
     // These members are used by NotificationSignalExtractors
     // to communicate with the ranking module.
@@ -156,10 +173,13 @@ public final class NotificationRecord {
      * conjunction with user sentiment calculation.
      */
     private boolean mIsAppImportanceLocked;
+    private ArraySet<Uri> mGrantableUris;
 
     public NotificationRecord(Context context, StatusBarNotification sbn,
             NotificationChannel channel) {
         this.sbn = sbn;
+        mTargetSdkVersion = LocalServices.getService(PackageManagerInternal.class)
+                .getPackageTargetSdkVersion(sbn.getPackageName());
         mOriginalFlags = sbn.getNotification().flags;
         mRankingTimeMs = calculateRankingTimeMs(0L);
         mCreationTimeMs = sbn.getPostTime();
@@ -176,20 +196,14 @@ public final class NotificationRecord {
         mAdjustments = new ArrayList<>();
         mStats = new NotificationStats();
         calculateUserSentiment();
+        calculateGrantableUris();
     }
 
     private boolean isPreChannelsNotification() {
-        try {
-            if (NotificationChannel.DEFAULT_CHANNEL_ID.equals(getChannel().getId())) {
-                  final ApplicationInfo applicationInfo =
-                        mContext.getPackageManager().getApplicationInfoAsUser(sbn.getPackageName(),
-                                0, UserHandle.getUserId(sbn.getUid()));
-                if (applicationInfo.targetSdkVersion < Build.VERSION_CODES.O) {
-                    return true;
-                }
+        if (NotificationChannel.DEFAULT_CHANNEL_ID.equals(getChannel().getId())) {
+            if (mTargetSdkVersion < Build.VERSION_CODES.O) {
+                return true;
             }
-        } catch (NameNotFoundException e) {
-            Slog.e(TAG, "Can't find package", e);
         }
         return false;
     }
@@ -377,6 +391,7 @@ public final class NotificationRecord {
     public String getKey() { return sbn.getKey(); }
     /** @deprecated Use {@link #getUser()} instead. */
     public int getUserId() { return sbn.getUserId(); }
+    public int getUid() { return sbn.getUid(); }
 
     void dump(ProtoOutputStream proto, long fieldId, boolean redact, int state) {
         final long token = proto.start(fieldId);
@@ -999,40 +1014,94 @@ public final class NotificationRecord {
         mHasSeenSmartReplies = hasSeenSmartReplies;
     }
 
-    public Set<Uri> getNotificationUris() {
-        Notification notification = getNotification();
-        Set<Uri> uris = new ArraySet<>();
+    /**
+     * @return all {@link Uri} that should have permission granted to whoever
+     *         will be rendering it. This list has already been vetted to only
+     *         include {@link Uri} that the enqueuing app can grant.
+     */
+    public @Nullable ArraySet<Uri> getGrantableUris() {
+        return mGrantableUris;
+    }
 
-        if (notification.sound != null) {
-            uris.add(notification.sound);
-        }
+    /**
+     * Collect all {@link Uri} that should have permission granted to whoever
+     * will be rendering it.
+     */
+    private void calculateGrantableUris() {
+        final Notification notification = getNotification();
+
+        noteGrantableUri(notification.sound);
         if (notification.getChannelId() != null) {
             NotificationChannel channel = getChannel();
-            if (channel != null && channel.getSound() != null) {
-                uris.add(channel.getSound());
+            if (channel != null) {
+                noteGrantableUri(channel.getSound());
             }
         }
-        if (notification.extras.containsKey(Notification.EXTRA_AUDIO_CONTENTS_URI)) {
-            uris.add(notification.extras.getParcelable(Notification.EXTRA_AUDIO_CONTENTS_URI));
-        }
-        if (notification.extras.containsKey(Notification.EXTRA_BACKGROUND_IMAGE_URI)) {
-            uris.add(notification.extras.getParcelable(Notification.EXTRA_BACKGROUND_IMAGE_URI));
+
+        final Bundle extras = notification.extras;
+        if (extras != null) {
+            noteGrantableUri(extras.getParcelable(EXTRA_AUDIO_CONTENTS_URI));
+            noteGrantableUri(extras.getParcelable(EXTRA_BACKGROUND_IMAGE_URI));
         }
-        if (Notification.MessagingStyle.class.equals(notification.getNotificationStyle())) {
-            Parcelable[] newMessages =
-                    notification.extras.getParcelableArray(Notification.EXTRA_MESSAGES);
-            List<Notification.MessagingStyle.Message> messages
-                    = Notification.MessagingStyle.Message.getMessagesFromBundleArray(newMessages);
-            Parcelable[] histMessages =
-                    notification.extras.getParcelableArray(Notification.EXTRA_HISTORIC_MESSAGES);
-            messages.addAll(
-                    Notification.MessagingStyle.Message.getMessagesFromBundleArray(histMessages));
-            for (Notification.MessagingStyle.Message message : messages) {
-                uris.add(message.getDataUri());
+
+        if (MessagingStyle.class.equals(notification.getNotificationStyle()) && extras != null) {
+            final Parcelable[] messages = extras.getParcelableArray(EXTRA_MESSAGES);
+            if (!ArrayUtils.isEmpty(messages)) {
+                for (MessagingStyle.Message message : MessagingStyle.Message
+                        .getMessagesFromBundleArray(messages)) {
+                    noteGrantableUri(message.getDataUri());
+                }
+            }
+
+            final Parcelable[] historic = extras.getParcelableArray(EXTRA_HISTORIC_MESSAGES);
+            if (!ArrayUtils.isEmpty(historic)) {
+                for (MessagingStyle.Message message : MessagingStyle.Message
+                        .getMessagesFromBundleArray(historic)) {
+                    noteGrantableUri(message.getDataUri());
+                }
             }
         }
+    }
 
-        return uris;
+    /**
+     * Note the presence of a {@link Uri} that should have permission granted to
+     * whoever will be rendering it.
+     * <p>
+     * If the enqueuing app has the ability to grant access, it will be added to
+     * {@link #mGrantableUris}. Otherwise, this will either log or throw
+     * {@link SecurityException} depending on target SDK of enqueuing app.
+     */
+    private void noteGrantableUri(Uri uri) {
+        if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return;
+
+        // We can't grant Uri permissions from system
+        final int sourceUid = sbn.getUid();
+        if (sourceUid == android.os.Process.SYSTEM_UID) return;
+
+        final IActivityManager am = ActivityManager.getService();
+        final long ident = Binder.clearCallingIdentity();
+        try {
+            // This will throw SecurityException if caller can't grant
+            am.checkGrantUriPermission(sourceUid, null,
+                    ContentProvider.getUriWithoutUserId(uri),
+                    Intent.FLAG_GRANT_READ_URI_PERMISSION,
+                    ContentProvider.getUserIdFromUri(uri, UserHandle.getUserId(sourceUid)));
+
+            if (mGrantableUris == null) {
+                mGrantableUris = new ArraySet<>();
+            }
+            mGrantableUris.add(uri);
+        } catch (RemoteException ignored) {
+            // Ignored because we're in same process
+        } catch (SecurityException e) {
+            if (mTargetSdkVersion >= Build.VERSION_CODES.P) {
+                throw e;
+            } else {
+                Log.w(TAG, "Ignoring " + uri + " from " + sourceUid + ": " + e.getMessage());
+            }
+        } finally {
+            Binder.restoreCallingIdentity(ident);
+        }
     }
 
     public LogMaker getLogMaker(long now) {
index 4c70466..aa3135f 100644 (file)
@@ -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" />
index f534b5c..345c1d7 100644 (file)
  */
 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;
+                    }
+                });
     }
 }
index 1a9b7db..0c2928a 100644 (file)
@@ -73,6 +73,7 @@ import android.app.admin.DevicePolicyManagerInternal;
 import android.app.usage.UsageStatsManagerInternal;
 import android.companion.ICompanionDeviceManager;
 import android.content.ComponentName;
+import android.content.ContentUris;
 import android.content.Context;
 import android.content.Intent;
 import android.content.pm.ApplicationInfo;
@@ -80,6 +81,7 @@ import android.content.pm.IPackageManager;
 import android.content.pm.PackageManager;
 import android.content.pm.ParceledListSlice;
 import android.graphics.Color;
+import android.media.AudioAttributes;
 import android.media.AudioManager;
 import android.net.Uri;
 import android.os.Binder;
@@ -88,6 +90,7 @@ import android.os.Bundle;
 import android.os.IBinder;
 import android.os.Process;
 import android.os.UserHandle;
+import android.provider.MediaStore;
 import android.provider.Settings.Secure;
 import android.service.notification.Adjustment;
 import android.service.notification.NotificationListenerService;
@@ -622,8 +625,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
                 mBinderService.getActiveNotifications(PKG);
         assertEquals(0, notifs.length);
         assertEquals(0, mService.getNotificationRecordCount());
-        verify(mAm, atLeastOnce()).revokeUriPermissionFromOwner(
-                any(), any(), anyInt(), anyInt());
     }
 
     @Test
@@ -642,7 +643,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         ArgumentCaptor<NotificationStats> captor = ArgumentCaptor.forClass(NotificationStats.class);
         verify(mListeners, times(1)).notifyRemovedLocked(any(), anyInt(), captor.capture());
         assertEquals(NotificationStats.DISMISSAL_OTHER, captor.getValue().getDismissalSurface());
-        verify(mAm, atLeastOnce()).revokeUriPermissionFromOwner(any(), any(), anyInt(), anyInt());
     }
 
     @Test
@@ -657,7 +657,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
                 mBinderService.getActiveNotifications(sbn.getPackageName());
         assertEquals(0, notifs.length);
         assertEquals(0, mService.getNotificationRecordCount());
-        verify(mAm, atLeastOnce()).revokeUriPermissionFromOwner(any(), any(), anyInt(), anyInt());
     }
 
     @Test
@@ -671,7 +670,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
                 mBinderService.getActiveNotifications(sbn.getPackageName());
         assertEquals(0, notifs.length);
         assertEquals(0, mService.getNotificationRecordCount());
-        verify(mAm, atLeastOnce()).revokeUriPermissionFromOwner(any(), any(), anyInt(), anyInt());
     }
 
     @Test
@@ -693,7 +691,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         ArgumentCaptor<NotificationStats> captor = ArgumentCaptor.forClass(NotificationStats.class);
         verify(mListeners, times(1)).notifyRemovedLocked(any(), anyInt(), captor.capture());
         assertEquals(NotificationStats.DISMISSAL_OTHER, captor.getValue().getDismissalSurface());
-        verify(mAm, atLeastOnce()).revokeUriPermissionFromOwner(any(), any(), anyInt(), anyInt());
     }
 
     @Test
@@ -712,7 +709,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         mBinderService.cancelAllNotifications(PKG, parent.sbn.getUserId());
         waitForIdle();
         assertEquals(0, mService.getNotificationRecordCount());
-        verify(mAm, atLeastOnce()).revokeUriPermissionFromOwner(any(), any(), anyInt(), anyInt());
     }
 
     @Test
@@ -726,7 +722,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
         waitForIdle();
 
         assertEquals(0, mService.getNotificationRecordCount());
-        verify(mAm, atLeastOnce()).revokeUriPermissionFromOwner(any(), any(), anyInt(), anyInt());
     }
 
     @Test
@@ -2245,7 +2240,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
 
     @Test
     public void testBumpFGImportance_noChannelChangePreOApp() throws Exception {
-        String preOPkg = "preO";
+        String preOPkg = PKG_N_MR1;
         int preOUid = 145;
         final ApplicationInfo legacy = new ApplicationInfo();
         legacy.targetSdkVersion = Build.VERSION_CODES.N_MR1;
@@ -2490,62 +2485,62 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
     }
 
     @Test
-    public void revokeUriPermissions_update() throws Exception {
+    public void updateUriPermissions_update() throws Exception {
         NotificationChannel c = new NotificationChannel(
                 TEST_CHANNEL_ID, TEST_CHANNEL_ID, NotificationManager.IMPORTANCE_DEFAULT);
         c.setSound(null, Notification.AUDIO_ATTRIBUTES_DEFAULT);
         Message message1 = new Message("", 0, "");
-        message1.setData("", Uri.fromParts("old", "", "old stuff"));
+        message1.setData("",
+                ContentUris.withAppendedId(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 1));
         Message message2 = new Message("", 1, "");
-        message2.setData("", Uri.fromParts("new", "", "new stuff"));
+        message2.setData("",
+                ContentUris.withAppendedId(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, 2));
 
-        Notification.Builder nb = new Notification.Builder(mContext, c.getId())
+        Notification.Builder nbA = new Notification.Builder(mContext, c.getId())
                 .setContentTitle("foo")
                 .setSmallIcon(android.R.drawable.sym_def_app_icon)
                 .setStyle(new Notification.MessagingStyle("")
                         .addMessage(message1)
                         .addMessage(message2));
-        StatusBarNotification oldSbn = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,
-                nb.build(), new UserHandle(mUid), null, 0);
-        NotificationRecord oldRecord =
-                new NotificationRecord(mContext, oldSbn, c);
-
-        Notification.Builder nb1 = new Notification.Builder(mContext, c.getId())
+        NotificationRecord recordA = new NotificationRecord(mContext, new StatusBarNotification(
+                PKG, PKG, 0, "tag", mUid, 0, nbA.build(), new UserHandle(mUid), null, 0), c);
+
+        // First post means we grant access to both
+        reset(mAm);
+        when(mAm.newUriPermissionOwner(any())).thenReturn(new Binder());
+        mService.updateUriPermissions(recordA, null, mContext.getPackageName(),
+                UserHandle.USER_SYSTEM);
+        verify(mAm, times(1)).grantUriPermissionFromOwner(any(), anyInt(), any(),
+                eq(message1.getDataUri()), anyInt(), anyInt(), anyInt());
+        verify(mAm, times(1)).grantUriPermissionFromOwner(any(), anyInt(), any(),
+                eq(message2.getDataUri()), anyInt(), anyInt(), anyInt());
+
+        Notification.Builder nbB = new Notification.Builder(mContext, c.getId())
                 .setContentTitle("foo")
                 .setSmallIcon(android.R.drawable.sym_def_app_icon)
                 .setStyle(new Notification.MessagingStyle("").addMessage(message2));
-        StatusBarNotification newSbn = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,
-                nb1.build(), new UserHandle(mUid), null, 0);
-        NotificationRecord newRecord =
-                new NotificationRecord(mContext, newSbn, c);
-
-        mService.revokeUriPermissions(newRecord, oldRecord);
+        NotificationRecord recordB = new NotificationRecord(mContext, new StatusBarNotification(PKG,
+                PKG, 0, "tag", mUid, 0, nbB.build(), new UserHandle(mUid), null, 0), c);
 
+        // Update means we drop access to first
+        reset(mAm);
+        mService.updateUriPermissions(recordB, recordA, mContext.getPackageName(),
+                UserHandle.USER_SYSTEM);
         verify(mAm, times(1)).revokeUriPermissionFromOwner(any(), eq(message1.getDataUri()),
                 anyInt(), anyInt());
-    }
-
-    @Test
-    public void revokeUriPermissions_cancel() throws Exception {
-        NotificationChannel c = new NotificationChannel(
-                TEST_CHANNEL_ID, TEST_CHANNEL_ID, NotificationManager.IMPORTANCE_DEFAULT);
-        c.setSound(null, Notification.AUDIO_ATTRIBUTES_DEFAULT);
-        Message message1 = new Message("", 0, "");
-        message1.setData("", Uri.fromParts("old", "", "old stuff"));
 
-        Notification.Builder nb = new Notification.Builder(mContext, c.getId())
-                .setContentTitle("foo")
-                .setSmallIcon(android.R.drawable.sym_def_app_icon)
-                .setStyle(new Notification.MessagingStyle("")
-                        .addMessage(message1));
-        StatusBarNotification oldSbn = new StatusBarNotification(PKG, PKG, 0, "tag", mUid, 0,
-                nb.build(), new UserHandle(mUid), null, 0);
-        NotificationRecord oldRecord =
-                new NotificationRecord(mContext, oldSbn, c);
-
-        mService.revokeUriPermissions(null, oldRecord);
-
-        verify(mAm, times(1)).revokeUriPermissionFromOwner(any(), eq(message1.getDataUri()),
+        // Update back means we grant access to first again
+        reset(mAm);
+        mService.updateUriPermissions(recordA, recordB, mContext.getPackageName(),
+                UserHandle.USER_SYSTEM);
+        verify(mAm, times(1)).grantUriPermissionFromOwner(any(), anyInt(), any(),
+                eq(message1.getDataUri()), anyInt(), anyInt(), anyInt());
+
+        // And update to empty means we drop everything
+        reset(mAm);
+        mService.updateUriPermissions(null, recordB, mContext.getPackageName(),
+                UserHandle.USER_SYSTEM);
+        verify(mAm, times(1)).revokeUriPermissionFromOwner(any(), eq(null),
                 anyInt(), anyInt());
     }
 
index 6303184..e3289ab 100644 (file)
@@ -29,8 +29,6 @@ import static junit.framework.Assert.assertNotNull;
 import static junit.framework.Assert.assertNull;
 import static junit.framework.Assert.assertTrue;
 
-import static org.mockito.Matchers.anyInt;
-import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.when;
 
 import android.app.ActivityManager;
@@ -45,7 +43,6 @@ import android.graphics.Color;
 import android.media.AudioAttributes;
 import android.metrics.LogMaker;
 import android.net.Uri;
-import android.os.Build;
 import android.os.Bundle;
 import android.os.UserHandle;
 import android.provider.Settings;
@@ -54,7 +51,6 @@ import android.service.notification.StatusBarNotification;
 import android.support.test.runner.AndroidJUnit4;
 import android.test.suitebuilder.annotation.SmallTest;
 
-
 import com.android.internal.logging.nano.MetricsProto.MetricsEvent;
 import com.android.server.UiServiceTestCase;
 
@@ -74,9 +70,9 @@ public class NotificationRecordTest extends UiServiceTestCase {
     private final Context mMockContext = Mockito.mock(Context.class);
     @Mock PackageManager mPm;
 
-    private final String pkg = "com.android.server.notification";
+    private final String pkg = PKG_N_MR1;
     private final int uid = 9583;
-    private final String pkg2 = "pkg2";
+    private final String pkg2 = PKG_O;
     private final int uid2 = 1111111;
     private final int id1 = 1;
     private final int id2 = 2;
@@ -119,13 +115,6 @@ public class NotificationRecordTest extends UiServiceTestCase {
 
         when(mMockContext.getResources()).thenReturn(getContext().getResources());
         when(mMockContext.getPackageManager()).thenReturn(mPm);
-
-        legacy.targetSdkVersion = Build.VERSION_CODES.N_MR1;
-        upgrade.targetSdkVersion = Build.VERSION_CODES.O;
-        try {
-            when(mPm.getApplicationInfoAsUser(eq(pkg), anyInt(), anyInt())).thenReturn(legacy);
-            when(mPm.getApplicationInfoAsUser(eq(pkg2), anyInt(), anyInt())).thenReturn(upgrade);
-        } catch (PackageManager.NameNotFoundException e) {}
     }
 
     private StatusBarNotification getNotification(boolean preO, boolean noisy, boolean defaultSound,
index 5e2a364..d49ba3e 100644 (file)
@@ -71,7 +71,6 @@ public class SliceManagerServiceTest extends UiServiceTestCase {
 
     @Before
     public void setup() {
-        LocalServices.addService(PackageManagerInternal.class, mock(PackageManagerInternal.class));
         LocalServices.addService(UsageStatsManagerInternal.class,
                 mock(UsageStatsManagerInternal.class));
         mContext.addMockSystemService(AppOpsManager.class, mock(AppOpsManager.class));
@@ -85,7 +84,6 @@ public class SliceManagerServiceTest extends UiServiceTestCase {
 
     @After
     public void teardown() {
-        LocalServices.removeServiceForTest(PackageManagerInternal.class);
         LocalServices.removeServiceForTest(UsageStatsManagerInternal.class);
     }