Detailed ContentProvider permissions checks.

The new MediaProvider design has an internal dynamic security model
based on the value stored in OWNER_PACKAGE_NAME, so the OS always
needs to consult the provider when resolving Uri permission grants.

Blocking calls from the system process like this are typically
discouraged, but this is the best we can do with the limited time
left, and there is existing precident with getType().

For now, use "forceUriPermissions" as a proxy for determining when
we need to consult the provider directly.

Bug: 115619667
Test: atest --test-mapping packages/providers/MediaProvider
Test: atest android.appsecurity.cts.ExternalStorageHostTest
Change-Id: I1d54feeec93fbb4cf5ff55240ef4eae3a35ed068

12 files changed, 224 insertions, 1 deletions

diff --git a/core/java/android/app/ActivityManagerInternal.java b/core/java/android/app/ActivityManagerInternal.java
index f8e6ae50d765..69e71185f228 100644
--- a/core/java/android/app/ActivityManagerInternal.java
+++ b/core/java/android/app/ActivityManagerInternal.java
@@ -26,6 +26,7 @@ import android.content.pm.ActivityInfo;
 import android.content.pm.ActivityPresentationInfo;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.UserInfo;
+import android.net.Uri;
 import android.os.Bundle;
 import android.os.IBinder;
 import android.os.TransactionTooLargeException;
@@ -52,6 +53,12 @@ public abstract class ActivityManagerInternal {
      */
     public abstract String checkContentProviderAccess(String authority, int userId);
 
+    /**
+     * Verify that calling UID has access to the given provider.
+     */
+    public abstract int checkContentProviderUriPermission(Uri uri, int userId,
+            int callingUid, int modeFlags);
+
     // Called by the power manager.
     public abstract void onWakefulnessChanged(int wakefulness);
 
diff --git a/core/java/android/content/ContentInterface.java b/core/java/android/content/ContentInterface.java
index d41d8d9cc1e2..197de9711296 100644
--- a/core/java/android/content/ContentInterface.java
+++ b/core/java/android/content/ContentInterface.java
@@ -56,6 +56,9 @@ public interface ContentInterface {
     public boolean refresh(@NonNull Uri uri, @Nullable Bundle args,
             @Nullable CancellationSignal cancellationSignal) throws RemoteException;
 
+    public int checkUriPermission(@NonNull Uri uri, int uid, @Intent.AccessUriMode int modeFlags)
+            throws RemoteException;
+
     public @Nullable Uri insert(@NonNull Uri uri, @Nullable ContentValues initialValues)
             throws RemoteException;
 
diff --git a/core/java/android/content/ContentProvider.java b/core/java/android/content/ContentProvider.java
index 7cdd2683905f..3c799915b871 100644
--- a/core/java/android/content/ContentProvider.java
+++ b/core/java/android/content/ContentProvider.java
@@ -28,6 +28,7 @@ import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.UnsupportedAppUsage;
 import android.app.AppOpsManager;
+import android.content.pm.PackageManager;
 import android.content.pm.PathPermission;
 import android.content.pm.ProviderInfo;
 import android.content.res.AssetFileDescriptor;
@@ -582,6 +583,22 @@ public abstract class ContentProvider implements ContentInterface, ComponentCall
             }
         }
 
+        @Override
+        public int checkUriPermission(String callingPkg, Uri uri, int uid, int modeFlags) {
+            uri = validateIncomingUri(uri);
+            uri = maybeGetUriWithoutUserId(uri);
+            Trace.traceBegin(TRACE_TAG_DATABASE, "checkUriPermission");
+            final String original = setCallingPackage(callingPkg);
+            try {
+                return mInterface.checkUriPermission(uri, uid, modeFlags);
+            } catch (RemoteException e) {
+                throw e.rethrowAsRuntimeException();
+            } finally {
+                setCallingPackage(original);
+                Trace.traceEnd(TRACE_TAG_DATABASE);
+            }
+        }
+
         private void enforceFilePermission(String callingPkg, Uri uri, String mode,
                 IBinder callerToken) throws FileNotFoundException, SecurityException {
             if (mode != null && mode.indexOf('w') != -1) {
@@ -1416,6 +1433,12 @@ public abstract class ContentProvider implements ContentInterface, ComponentCall
         return false;
     }
 
+    /** {@hide} */
+    @Override
+    public int checkUriPermission(@NonNull Uri uri, int uid, @Intent.AccessUriMode int modeFlags) {
+        return PackageManager.PERMISSION_DENIED;
+    }
+
     /**
      * @hide
      * Implementation when a caller has performed an insert on the content
diff --git a/core/java/android/content/ContentProviderClient.java b/core/java/android/content/ContentProviderClient.java
index 93bf5188705c..8a4330ec0ede 100644
--- a/core/java/android/content/ContentProviderClient.java
+++ b/core/java/android/content/ContentProviderClient.java
@@ -307,6 +307,25 @@ public class ContentProviderClient implements ContentInterface, AutoCloseable {
         }
     }
 
+    /** {@hide} */
+    @Override
+    public int checkUriPermission(@NonNull Uri uri, int uid, @Intent.AccessUriMode int modeFlags)
+            throws RemoteException {
+        Preconditions.checkNotNull(uri, "uri");
+
+        beforeRemote();
+        try {
+            return mContentProvider.checkUriPermission(mPackageName, uri, uid, modeFlags);
+        } catch (DeadObjectException e) {
+            if (!mStable) {
+                mContentResolver.unstableProviderDied(mContentProvider);
+            }
+            throw e;
+        } finally {
+            afterRemote();
+        }
+    }
+
     /** See {@link ContentProvider#insert ContentProvider.insert} */
     @Override
     public @Nullable Uri insert(@NonNull Uri url, @Nullable ContentValues initialValues)
diff --git a/core/java/android/content/ContentProviderNative.java b/core/java/android/content/ContentProviderNative.java
index 994833866f40..cd735d4b10a3 100644
--- a/core/java/android/content/ContentProviderNative.java
+++ b/core/java/android/content/ContentProviderNative.java
@@ -363,6 +363,19 @@ abstract public class ContentProviderNative extends Binder implements IContentPr
                     reply.writeInt(out ? 0 : -1);
                     return true;
                 }
+
+                case CHECK_URI_PERMISSION_TRANSACTION: {
+                    data.enforceInterface(IContentProvider.descriptor);
+                    String callingPkg = data.readString();
+                    Uri uri = Uri.CREATOR.createFromParcel(data);
+                    int uid = data.readInt();
+                    int modeFlags = data.readInt();
+
+                    int out = checkUriPermission(callingPkg, uri, uid, modeFlags);
+                    reply.writeNoException();
+                    reply.writeInt(out);
+                    return true;
+                }
             }
         } catch (Exception e) {
             DatabaseUtils.writeExceptionToParcel(reply, e);
@@ -800,6 +813,29 @@ final class ContentProviderProxy implements IContentProvider
         }
     }
 
+    @Override
+    public int checkUriPermission(String callingPkg, Uri url, int uid, int modeFlags)
+            throws RemoteException {
+        Parcel data = Parcel.obtain();
+        Parcel reply = Parcel.obtain();
+        try {
+            data.writeInterfaceToken(IContentProvider.descriptor);
+
+            data.writeString(callingPkg);
+            url.writeToParcel(data, 0);
+            data.writeInt(uid);
+            data.writeInt(modeFlags);
+
+            mRemote.transact(IContentProvider.CHECK_URI_PERMISSION_TRANSACTION, data, reply, 0);
+
+            DatabaseUtils.readExceptionFromParcel(reply);
+            return reply.readInt();
+        } finally {
+            data.recycle();
+            reply.recycle();
+        }
+    }
+
     @UnsupportedAppUsage
     private IBinder mRemote;
 }
diff --git a/core/java/android/content/ContentResolver.java b/core/java/android/content/ContentResolver.java
index 0a1bc85202ff..9c863591f16f 100644
--- a/core/java/android/content/ContentResolver.java
+++ b/core/java/android/content/ContentResolver.java
@@ -31,6 +31,7 @@ import android.app.ActivityManager;
 import android.app.ActivityThread;
 import android.app.AppGlobals;
 import android.app.UriGrantsManager;
+import android.content.pm.PackageManager;
 import android.content.pm.PackageManager.NameNotFoundException;
 import android.content.res.AssetFileDescriptor;
 import android.content.res.Resources;
@@ -1146,6 +1147,24 @@ public abstract class ContentResolver implements ContentInterface {
         }
     }
 
+    /** {@hide} */
+    @Override
+    public int checkUriPermission(@NonNull Uri uri, int uid, @Intent.AccessUriMode int modeFlags) {
+        Preconditions.checkNotNull(uri, "uri");
+
+        try {
+            if (mWrapped != null) return mWrapped.checkUriPermission(uri, uid, modeFlags);
+        } catch (RemoteException e) {
+            return PackageManager.PERMISSION_DENIED;
+        }
+
+        try (ContentProviderClient client = acquireUnstableContentProviderClient(uri)) {
+            return client.checkUriPermission(uri, uid, modeFlags);
+        } catch (RemoteException e) {
+            return PackageManager.PERMISSION_DENIED;
+        }
+    }
+
     /**
      * Open a stream on to the content associated with a content URI.  If there
      * is no data associated with the URI, FileNotFoundException is thrown.
diff --git a/core/java/android/content/IContentProvider.java b/core/java/android/content/IContentProvider.java
index 0427c2f52415..fade0ab6bb5c 100644
--- a/core/java/android/content/IContentProvider.java
+++ b/core/java/android/content/IContentProvider.java
@@ -16,6 +16,7 @@
 
 package android.content;
 
+import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.annotation.UnsupportedAppUsage;
 import android.content.res.AssetFileDescriptor;
@@ -82,6 +83,9 @@ public interface IContentProvider extends IInterface {
     public Bundle call(String callingPkg, String authority, String method,
             @Nullable String arg, @Nullable Bundle extras) throws RemoteException;
 
+    public int checkUriPermission(String callingPkg, Uri uri, int uid, int modeFlags)
+            throws RemoteException;
+
     public ICancellationSignal createCancellationSignal() throws RemoteException;
 
     public Uri canonicalize(String callingPkg, Uri uri) throws RemoteException;
@@ -116,4 +120,5 @@ public interface IContentProvider extends IInterface {
     static final int CANONICALIZE_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION + 24;
     static final int UNCANONICALIZE_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION + 25;
     static final int REFRESH_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION + 26;
+    static final int CHECK_URI_PERMISSION_TRANSACTION = IBinder.FIRST_CALL_TRANSACTION + 27;
 }
diff --git a/core/java/android/content/LoggingContentInterface.java b/core/java/android/content/LoggingContentInterface.java
index 83c0c9112387..1df1c4faf2fe 100644
--- a/core/java/android/content/LoggingContentInterface.java
+++ b/core/java/android/content/LoggingContentInterface.java
@@ -165,6 +165,19 @@ public class LoggingContentInterface implements ContentInterface {
     }
 
     @Override
+    public int checkUriPermission(@NonNull Uri uri, int uid, @Intent.AccessUriMode int modeFlags)
+            throws RemoteException {
+        try (Logger l = new Logger("checkUriPermission", uri, uid, modeFlags)) {
+            try {
+                return l.setResult(delegate.checkUriPermission(uri, uid, modeFlags));
+            } catch (Exception res) {
+                l.setResult(res);
+                throw res;
+            }
+        }
+    }
+
+    @Override
     public @Nullable Uri insert(@NonNull Uri uri, @Nullable ContentValues initialValues)
             throws RemoteException {
         try (Logger l = new Logger("insert", uri, initialValues)) {
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index adb0909f46b8..54fc73993323 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -392,7 +392,9 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -7706,6 +7708,34 @@ public class ActivityManagerService extends IActivityManager.Stub
         return null;
     }
 
+    int checkContentProviderUriPermission(Uri uri, int userId, int callingUid, int modeFlags) {
+        final String name = uri.getAuthority();
+        final long ident = Binder.clearCallingIdentity();
+        ContentProviderHolder holder = null;
+        try {
+            holder = getContentProviderExternalUnchecked(name, null, callingUid,
+                    "*checkContentProviderUriPermission*", userId);
+            if (holder != null) {
+                return holder.provider.checkUriPermission(null, uri, callingUid, modeFlags);
+            }
+        } catch (RemoteException e) {
+            Log.w(TAG, "Content provider dead retrieving " + uri, e);
+            return PackageManager.PERMISSION_DENIED;
+        } catch (Exception e) {
+            Log.w(TAG, "Exception while determining type of " + uri, e);
+            return PackageManager.PERMISSION_DENIED;
+        } finally {
+            try {
+                if (holder != null) {
+                    removeContentProviderExternalUnchecked(name, null, userId);
+                }
+            } finally {
+                Binder.restoreCallingIdentity(ident);
+            }
+        }
+        return PackageManager.PERMISSION_DENIED;
+    }
+
     private boolean canClearIdentity(int callingPid, int callingUid, int userId) {
         if (UserHandle.getUserId(callingUid) == userId) {
             return true;
@@ -17826,6 +17856,35 @@ public class ActivityManagerService extends IActivityManager.Stub
         }
 
         @Override
+        public int checkContentProviderUriPermission(Uri uri, int userId,
+                int callingUid, int modeFlags) {
+            // We can find ourselves needing to check Uri permissions while
+            // already holding the WM lock, which means reaching back here for
+            // the AM lock would cause an inversion. The WM team has requested
+            // that we use the strategy below instead of shifting where Uri
+            // grants are calculated.
+
+            // Since we could also arrive here while holding the AM lock, we
+            // can't always delegate the call through the handler, and we need
+            // to delicately dance between the deadlocks.
+            if (Thread.currentThread().holdsLock(ActivityManagerService.this)) {
+                return ActivityManagerService.this.checkContentProviderUriPermission(uri,
+                        userId, callingUid, modeFlags);
+            } else {
+                final CompletableFuture<Integer> res = new CompletableFuture<>();
+                mHandler.post(() -> {
+                    res.complete(ActivityManagerService.this.checkContentProviderUriPermission(uri,
+                            userId, callingUid, modeFlags));
+                });
+                try {
+                    return res.get();
+                } catch (InterruptedException | ExecutionException e) {
+                    throw new RuntimeException(e);
+                }
+            }
+        }
+
+        @Override
         public void onWakefulnessChanged(int wakefulness) {
             ActivityManagerService.this.onWakefulnessChanged(wakefulness);
         }
diff --git a/services/core/java/com/android/server/uri/UriGrantsManagerService.java b/services/core/java/com/android/server/uri/UriGrantsManagerService.java
index 55f062bca2d1..b2f115374305 100644
--- a/services/core/java/com/android/server/uri/UriGrantsManagerService.java
+++ b/services/core/java/com/android/server/uri/UriGrantsManagerService.java
@@ -949,7 +949,25 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub {
             return false;
         }
 
-        return readMet && writeMet;
+        // If this provider says that grants are always required, we need to
+        // consult it directly to determine if the UID has permission
+        final boolean forceMet;
+        if (pi.forceUriPermissions) {
+            final int providerUserId = UserHandle.getUserId(pi.applicationInfo.uid);
+            final int clientUserId = UserHandle.getUserId(uid);
+            if (providerUserId == clientUserId) {
+                forceMet = (mAmInternal.checkContentProviderUriPermission(grantUri.uri,
+                        providerUserId, uid, modeFlags) == PackageManager.PERMISSION_GRANTED);
+            } else {
+                // The provider can't track cross-user permissions, so we have
+                // to assume they're always denied
+                forceMet = false;
+            }
+        } else {
+            forceMet = true;
+        }
+
+        return readMet && writeMet && forceMet;
     }
 
     private void removeUriPermissionIfNeeded(UriPermission perm) {
diff --git a/test-mock/src/android/test/mock/MockContentProvider.java b/test-mock/src/android/test/mock/MockContentProvider.java
index e9a5ff70a7cc..4d8c7d930bde 100644
--- a/test-mock/src/android/test/mock/MockContentProvider.java
+++ b/test-mock/src/android/test/mock/MockContentProvider.java
@@ -16,6 +16,7 @@
 
 package android.test.mock;
 
+import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.content.ContentProvider;
 import android.content.ContentProviderOperation;
@@ -23,6 +24,7 @@ import android.content.ContentProviderResult;
 import android.content.ContentValues;
 import android.content.Context;
 import android.content.IContentProvider;
+import android.content.Intent;
 import android.content.OperationApplicationException;
 import android.content.pm.PathPermission;
 import android.content.pm.ProviderInfo;
@@ -154,6 +156,11 @@ public class MockContentProvider extends ContentProvider {
                 ICancellationSignal cancellationSignal) throws RemoteException {
             return MockContentProvider.this.refresh(url, args);
         }
+
+        @Override
+        public int checkUriPermission(String callingPkg, Uri uri, int uid, int modeFlags) {
+            return MockContentProvider.this.checkUriPermission(uri, uid, modeFlags);
+        }
     }
     private final InversionIContentProvider mIContentProvider = new InversionIContentProvider();
 
@@ -266,6 +273,12 @@ public class MockContentProvider extends ContentProvider {
         throw new UnsupportedOperationException("unimplemented mock method call");
     }
 
+    /** {@hide} */
+    @Override
+    public int checkUriPermission(@NonNull Uri uri, int uid, @Intent.AccessUriMode int modeFlags) {
+        throw new UnsupportedOperationException("unimplemented mock method call");
+    }
+
     /**
      * Returns IContentProvider which calls back same methods in this class.
      * By overriding this class, we avoid the mechanism hidden behind ContentProvider
diff --git a/test-mock/src/android/test/mock/MockIContentProvider.java b/test-mock/src/android/test/mock/MockIContentProvider.java
index fc2a4644b994..b072d7440de4 100644
--- a/test-mock/src/android/test/mock/MockIContentProvider.java
+++ b/test-mock/src/android/test/mock/MockIContentProvider.java
@@ -16,12 +16,14 @@
 
 package android.test.mock;
 
+import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.content.ContentProviderOperation;
 import android.content.ContentProviderResult;
 import android.content.ContentValues;
 import android.content.EntityIterator;
 import android.content.IContentProvider;
+import android.content.Intent;
 import android.content.res.AssetFileDescriptor;
 import android.database.Cursor;
 import android.net.Uri;
@@ -144,4 +146,10 @@ public class MockIContentProvider implements IContentProvider {
             ICancellationSignal cancellationSignal) throws RemoteException {
         throw new UnsupportedOperationException("unimplemented mock method");
     }
+
+    /** {@hide} */
+    @Override
+    public int checkUriPermission(String callingPkg, Uri uri, int uid, int modeFlags) {
+        throw new UnsupportedOperationException("unimplemented mock method call");
+    }
 }