Let <path-permission> block unprotected providers.

When accessing a ContentProvider that is unprotected by top-level
permissions, and caller doesn't hold a matching <path-permission>
permission, revoke the default top-level access.

This enables an otherwise unprotected provider to enforce permissions
on specific paths.

Bug: 6131916
Change-Id: Icab89f765ccd90b1acea6988b05f00877fe2c11e

2 files changed, 162 insertions, 110 deletions

diff --git a/core/java/android/content/ContentProvider.java b/core/java/android/content/ContentProvider.java
index 12e3ccf..ec67d8c 100644
--- a/core/java/android/content/ContentProvider.java
+++ b/core/java/android/content/ContentProvider.java
@@ -16,6 +16,8 @@
 
 package android.content;
 
+import static android.content.pm.PackageManager.PERMISSION_GRANTED;
+
 import android.content.pm.PackageManager;
 import android.content.pm.PathPermission;
 import android.content.pm.ProviderInfo;
@@ -30,6 +32,7 @@ import android.os.Bundle;
 import android.os.ParcelFileDescriptor;
 import android.os.Process;
 import android.os.RemoteException;
+import android.os.UserId;
 import android.util.Log;
 
 import java.io.File;
@@ -267,90 +270,116 @@ public abstract class ContentProvider implements ComponentCallbacks2 {
             return CancellationSignal.createTransport();
         }
 
-        private void enforceReadPermission(Uri uri) {
-            final int uid = Binder.getCallingUid();
-            if (uid == mMyUid) {
-                return;
-            }
-            
+        private boolean hasReadPermission(Uri uri) {
             final Context context = getContext();
-            final String rperm = getReadPermission();
             final int pid = Binder.getCallingPid();
-            if (mExported && (rperm == null
-                    || context.checkPermission(rperm, pid, uid)
-                    == PackageManager.PERMISSION_GRANTED)) {
-                return;
-            }
-            
-            PathPermission[] pps = getPathPermissions();
-            if (pps != null) {
-                final String path = uri.getPath();
-                int i = pps.length;
-                while (i > 0) {
-                    i--;
-                    final PathPermission pp = pps[i];
-                    final String pprperm = pp.getReadPermission();
-                    if (pprperm != null && pp.match(path)) {
-                        if (context.checkPermission(pprperm, pid, uid)
-                                == PackageManager.PERMISSION_GRANTED) {
-                            return;
+            final int uid = Binder.getCallingUid();
+
+            if (uid == mMyUid) {
+                return true;
+
+            } else if (mExported) {
+                final String componentPerm = getReadPermission();
+                if (componentPerm != null
+                        && (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED)) {
+                    return true;
+                }
+
+                // track if unprotected read is allowed; any denied
+                // <path-permission> below removes this ability
+                boolean allowDefaultRead = (componentPerm == null);
+
+                final PathPermission[] pps = getPathPermissions();
+                if (pps != null) {
+                    final String path = uri.getPath();
+                    for (PathPermission pp : pps) {
+                        final String pathPerm = pp.getReadPermission();
+                        if (pathPerm != null && pp.match(path)) {
+                            if (context.checkPermission(pathPerm, pid, uid) == PERMISSION_GRANTED) {
+                                return true;
+                            } else {
+                                // any denied <path-permission> means we lose
+                                // default <provider> access.
+                                allowDefaultRead = false;
+                            }
                         }
                     }
                 }
+
+                // if we passed <path-permission> checks above, and no default
+                // <provider> permission, then allow access.
+                if (allowDefaultRead) return true;
             }
-            
-            if (context.checkUriPermission(uri, pid, uid,
-                    Intent.FLAG_GRANT_READ_URI_PERMISSION)
-                    == PackageManager.PERMISSION_GRANTED) {
+
+            // last chance, check against any uri grants
+            if (context.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_READ_URI_PERMISSION)
+                    == PERMISSION_GRANTED) {
+                return true;
+            }
+
+            return false;
+        }
+
+        private void enforceReadPermission(Uri uri) {
+            if (hasReadPermission(uri)) {
                 return;
             }
-            
+
             String msg = "Permission Denial: reading "
                     + ContentProvider.this.getClass().getName()
                     + " uri " + uri + " from pid=" + Binder.getCallingPid()
                     + ", uid=" + Binder.getCallingUid()
-                    + " requires " + rperm;
+                    + " requires " + getReadPermission();
             throw new SecurityException(msg);
         }
 
         private boolean hasWritePermission(Uri uri) {
-            final int uid = Binder.getCallingUid();
-            if (uid == mMyUid) {
-                return true;
-            }
-            
             final Context context = getContext();
-            final String wperm = getWritePermission();
             final int pid = Binder.getCallingPid();
-            if (mExported && (wperm == null
-                    || context.checkPermission(wperm, pid, uid)
-                    == PackageManager.PERMISSION_GRANTED)) {
+            final int uid = Binder.getCallingUid();
+
+            if (uid == mMyUid) {
                 return true;
-            }
-            
-            PathPermission[] pps = getPathPermissions();
-            if (pps != null) {
-                final String path = uri.getPath();
-                int i = pps.length;
-                while (i > 0) {
-                    i--;
-                    final PathPermission pp = pps[i];
-                    final String ppwperm = pp.getWritePermission();
-                    if (ppwperm != null && pp.match(path)) {
-                        if (context.checkPermission(ppwperm, pid, uid)
-                                == PackageManager.PERMISSION_GRANTED) {
-                            return true;
+
+            } else if (mExported) {
+                final String componentPerm = getWritePermission();
+                if (componentPerm != null
+                        && (context.checkPermission(componentPerm, pid, uid) == PERMISSION_GRANTED)) {
+                    return true;
+                }
+
+                // track if unprotected write is allowed; any denied
+                // <path-permission> below removes this ability
+                boolean allowDefaultWrite = (componentPerm == null);
+
+                final PathPermission[] pps = getPathPermissions();
+                if (pps != null) {
+                    final String path = uri.getPath();
+                    for (PathPermission pp : pps) {
+                        final String pathPerm = pp.getWritePermission();
+                        if (pathPerm != null && pp.match(path)) {
+                            if (context.checkPermission(pathPerm, pid, uid) == PERMISSION_GRANTED) {
+                                return true;
+                            } else {
+                                // any denied <path-permission> means we lose
+                                // default <provider> access.
+                                allowDefaultWrite = false;
+                            }
                         }
                     }
                 }
+
+                // if we passed <path-permission> checks above, and no default
+                // <provider> permission, then allow access.
+                if (allowDefaultWrite) return true;
             }
-            
-            if (context.checkUriPermission(uri, pid, uid,
-                    Intent.FLAG_GRANT_WRITE_URI_PERMISSION)
-                    == PackageManager.PERMISSION_GRANTED) {
+
+            // last chance, check against any uri grants
+            if (context.checkUriPermission(uri, pid, uid, Intent.FLAG_GRANT_WRITE_URI_PERMISSION)
+                    == PERMISSION_GRANTED) {
                 return true;
             }
-            
+
             return false;
         }
         
diff --git a/services/java/com/android/server/am/ActivityManagerService.java b/services/java/com/android/server/am/ActivityManagerService.java
index fd968e0..86f4bc0 100644
--- a/services/java/com/android/server/am/ActivityManagerService.java
+++ b/services/java/com/android/server/am/ActivityManagerService.java
@@ -16,6 +16,8 @@
 
 package com.android.server.am;
 
+import static android.content.pm.PackageManager.PERMISSION_GRANTED;
+
 import com.android.internal.R;
 import com.android.internal.os.BatteryStatsImpl;
 import com.android.internal.os.ProcessStats;
@@ -55,6 +57,7 @@ import android.content.BroadcastReceiver;
 import android.content.ClipData;
 import android.content.ComponentCallbacks2;
 import android.content.ComponentName;
+import android.content.ContentProvider;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.content.DialogInterface;
@@ -4543,76 +4546,91 @@ public final class ActivityManagerService extends ActivityManagerNative
         throw new SecurityException(msg);
     }
 
-    private final boolean checkHoldingPermissionsLocked(IPackageManager pm,
-            ProviderInfo pi, Uri uri, int uid, int modeFlags) {
-        boolean readPerm = (modeFlags&Intent.FLAG_GRANT_READ_URI_PERMISSION) == 0;
-        boolean writePerm = (modeFlags&Intent.FLAG_GRANT_WRITE_URI_PERMISSION) == 0;
+    /**
+     * Determine if UID is holding permissions required to access {@link Uri} in
+     * the given {@link ProviderInfo}. Final permission checking is always done
+     * in {@link ContentProvider}.
+     */
+    private final boolean checkHoldingPermissionsLocked(
+            IPackageManager pm, ProviderInfo pi, Uri uri, int uid, int modeFlags) {
         if (DEBUG_URI_PERMISSION) Slog.v(TAG,
                 "checkHoldingPermissionsLocked: uri=" + uri + " uid=" + uid);
-        try {
-            // Is the component private from the target uid?
-            final boolean prv = !pi.exported && pi.applicationInfo.uid != uid;
 
-            // Acceptable if the there is no read permission needed from the
-            // target or the target is holding the read permission.
-            if (!readPerm) {
-                if ((!prv && pi.readPermission == null) ||
-                        (pm.checkUidPermission(pi.readPermission, uid)
-                                == PackageManager.PERMISSION_GRANTED)) {
-                    readPerm = true;
-                }
-            }
+        if (pi.applicationInfo.uid == uid) {
+            return true;
+        } else if (!pi.exported) {
+            return false;
+        }
 
-            // Acceptable if the there is no write permission needed from the
-            // target or the target is holding the read permission.
-            if (!writePerm) {
-                if (!prv && (pi.writePermission == null) ||
-                        (pm.checkUidPermission(pi.writePermission, uid)
-                                == PackageManager.PERMISSION_GRANTED)) {
-                    writePerm = true;
-                }
+        boolean readMet = (modeFlags & Intent.FLAG_GRANT_READ_URI_PERMISSION) == 0;
+        boolean writeMet = (modeFlags & Intent.FLAG_GRANT_WRITE_URI_PERMISSION) == 0;
+        try {
+            // check if target holds top-level <provider> permissions
+            if (!readMet && pi.readPermission != null
+                    && (pm.checkUidPermission(pi.readPermission, uid) == PERMISSION_GRANTED)) {
+                readMet = true;
+            }
+            if (!writeMet && pi.writePermission != null
+                    && (pm.checkUidPermission(pi.writePermission, uid) == PERMISSION_GRANTED)) {
+                writeMet = true;
             }
 
-            // Acceptable if there is a path permission matching the URI that
-            // the target holds the permission on.
-            PathPermission[] pps = pi.pathPermissions;
-            if (pps != null && (!readPerm || !writePerm)) {
+            // track if unprotected read/write is allowed; any denied
+            // <path-permission> below removes this ability
+            boolean allowDefaultRead = pi.readPermission == null;
+            boolean allowDefaultWrite = pi.writePermission == null;
+
+            // check if target holds any <path-permission> that match uri
+            final PathPermission[] pps = pi.pathPermissions;
+            if (pps != null) {
                 final String path = uri.getPath();
                 int i = pps.length;
-                while (i > 0 && (!readPerm || !writePerm)) {
+                while (i > 0 && (!readMet || !writeMet)) {
                     i--;
                     PathPermission pp = pps[i];
-                    if (!readPerm) {
-                        final String pprperm = pp.getReadPermission();
-                        if (DEBUG_URI_PERMISSION) Slog.v(TAG, "Checking read perm for "
-                                + pprperm + " for " + pp.getPath()
-                                + ": match=" + pp.match(path)
-                                + " check=" + pm.checkUidPermission(pprperm, uid));
-                        if (pprperm != null && pp.match(path) &&
-                                (pm.checkUidPermission(pprperm, uid)
-                                        == PackageManager.PERMISSION_GRANTED)) {
-                            readPerm = true;
+                    if (pp.match(path)) {
+                        if (!readMet) {
+                            final String pprperm = pp.getReadPermission();
+                            if (DEBUG_URI_PERMISSION) Slog.v(TAG, "Checking read perm for "
+                                    + pprperm + " for " + pp.getPath()
+                                    + ": match=" + pp.match(path)
+                                    + " check=" + pm.checkUidPermission(pprperm, uid));
+                            if (pprperm != null) {
+                                if (pm.checkUidPermission(pprperm, uid) == PERMISSION_GRANTED) {
+                                    readMet = true;
+                                } else {
+                                    allowDefaultRead = false;
+                                }
+                            }
                         }
-                    }
-                    if (!writePerm) {
-                        final String ppwperm = pp.getWritePermission();
-                        if (DEBUG_URI_PERMISSION) Slog.v(TAG, "Checking write perm "
-                                + ppwperm + " for " + pp.getPath()
-                                + ": match=" + pp.match(path)
-                                + " check=" + pm.checkUidPermission(ppwperm, uid));
-                        if (ppwperm != null && pp.match(path) &&
-                                (pm.checkUidPermission(ppwperm, uid)
-                                        == PackageManager.PERMISSION_GRANTED)) {
-                            writePerm = true;
+                        if (!writeMet) {
+                            final String ppwperm = pp.getWritePermission();
+                            if (DEBUG_URI_PERMISSION) Slog.v(TAG, "Checking write perm "
+                                    + ppwperm + " for " + pp.getPath()
+                                    + ": match=" + pp.match(path)
+                                    + " check=" + pm.checkUidPermission(ppwperm, uid));
+                            if (ppwperm != null) {
+                                if (pm.checkUidPermission(ppwperm, uid) == PERMISSION_GRANTED) {
+                                    writeMet = true;
+                                } else {
+                                    allowDefaultWrite = false;
+                                }
+                            }
                         }
                     }
                 }
             }
+
+            // grant unprotected <provider> read/write, if not blocked by
+            // <path-permission> above
+            if (allowDefaultRead) readMet = true;
+            if (allowDefaultWrite) writeMet = true;
+
         } catch (RemoteException e) {
             return false;
         }
 
-        return readPerm && writePerm;
+        return readMet && writeMet;
     }
 
     private final boolean checkUriPermissionLocked(Uri uri, int uid,
@@ -5819,6 +5837,11 @@ public final class ActivityManagerService extends ActivityManagerNative
         return providers;
     }
 
+    /**
+     * Check if {@link ProcessRecord} has a possible chance at accessing the
+     * given {@link ProviderInfo}. Final permission checking is always done
+     * in {@link ContentProvider}.
+     */
     private final String checkContentProviderPermissionLocked(
             ProviderInfo cpi, ProcessRecord r) {
         final int callingPid = (r != null) ? r.pid : Binder.getCallingPid();