Initial splitting of calculation and grants.

In upcoming changes, we'll need to shift the calculation of needed
permission grants to occur before we acquire any AM/WM locks; we'll
continue to use that calculated list when actually granting.

This change also reduces the surface area of how callers in the
system server interact with Uri permissions to reduce the risk of
accidental misuse.

This is a no-op refactoring.

Bug: 115619667
Test: atest FrameworksServicesTests:com.android.server.uri
Test: atest CtsAppSecurityHostTestCases:android.appsecurity.cts.AppSecurityTests#testPermissionDiffCert
Change-Id: Ied529156205903f9b02b4265963fdf59f7dd7f92

8 files changed, 69 insertions, 87 deletions

diff --git a/services/core/java/com/android/server/MmsServiceBroker.java b/services/core/java/com/android/server/MmsServiceBroker.java
index 043f657b7f2c..1804b7f66fa2 100644
--- a/services/core/java/com/android/server/MmsServiceBroker.java
+++ b/services/core/java/com/android/server/MmsServiceBroker.java
@@ -45,6 +45,7 @@ import android.telephony.TelephonyManager;
 import android.util.Slog;
 
 import com.android.internal.telephony.IMms;
+import com.android.server.uri.NeededUriGrants;
 import com.android.server.uri.UriGrantsManagerInternal;
 
 import java.util.List;
@@ -512,9 +513,11 @@ public class MmsServiceBroker extends SystemService {
 
             long token = Binder.clearCallingIdentity();
             try {
-                LocalServices.getService(UriGrantsManagerInternal.class)
-                        .grantUriPermissionFromIntent(callingUid, PHONE_PACKAGE_NAME,
-                                grantIntent, UserHandle.USER_SYSTEM);
+                final UriGrantsManagerInternal ugm = LocalServices
+                        .getService(UriGrantsManagerInternal.class);
+                final NeededUriGrants needed = ugm.checkGrantUriPermissionFromIntent(
+                        grantIntent, callingUid, PHONE_PACKAGE_NAME, UserHandle.USER_SYSTEM);
+                ugm.grantUriPermissionUncheckedFromIntent(needed, null);
 
                 // Grant permission for the carrier app.
                 Intent intent = new Intent(action);
@@ -524,9 +527,10 @@ public class MmsServiceBroker extends SystemService {
                         .getCarrierPackageNamesForIntentAndPhone(
                                 intent, getPhoneIdFromSubId(subId));
                 if (carrierPackages != null && carrierPackages.size() == 1) {
-                    LocalServices.getService(UriGrantsManagerInternal.class)
-                            .grantUriPermissionFromIntent(callingUid, carrierPackages.get(0),
-                                    grantIntent, UserHandle.USER_SYSTEM);
+                    final NeededUriGrants carrierNeeded = ugm.checkGrantUriPermissionFromIntent(
+                            grantIntent, callingUid, carrierPackages.get(0),
+                            UserHandle.USER_SYSTEM);
+                    ugm.grantUriPermissionUncheckedFromIntent(carrierNeeded, null);
                 }
             } finally {
                 Binder.restoreCallingIdentity(token);
diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java
index ec12aebc37f6..e133b19c6f31 100644
--- a/services/core/java/com/android/server/am/ActiveServices.java
+++ b/services/core/java/com/android/server/am/ActiveServices.java
@@ -590,7 +590,7 @@ public final class ActiveServices {
         }
 
         NeededUriGrants neededGrants = mAm.mUgmInternal.checkGrantUriPermissionFromIntent(
-                callingUid, r.packageName, service, service.getFlags(), null, r.userId);
+                service, callingUid, r.packageName, r.userId);
 
         // If permissions need a review before any of the app components can run,
         // we do not start the service and launch a review activity if the calling app
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index 7ef527cb7d84..45099800dde3 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -365,6 +365,7 @@ import com.android.server.job.JobSchedulerInternal;
 import com.android.server.pm.Installer;
 import com.android.server.pm.permission.PermissionManagerServiceInternal;
 import com.android.server.uri.GrantUri;
+import com.android.server.uri.NeededUriGrants;
 import com.android.server.uri.UriGrantsManagerInternal;
 import com.android.server.utils.PriorityDump;
 import com.android.server.utils.TimingsTraceAndSlog;
@@ -6511,17 +6512,19 @@ public class ActivityManagerService extends IActivityManager.Stub
             if (targetPkg == null) {
                 throw new IllegalArgumentException("null target");
             }
-            if (grantUri == null) {
-                throw new IllegalArgumentException("null uri");
-            }
 
             Preconditions.checkFlagsArgument(modeFlags, Intent.FLAG_GRANT_READ_URI_PERMISSION
                     | Intent.FLAG_GRANT_WRITE_URI_PERMISSION
                     | Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION
                     | Intent.FLAG_GRANT_PREFIX_URI_PERMISSION);
 
-            mUgmInternal.grantUriPermission(r.uid, targetPkg, grantUri, modeFlags, null,
-                    UserHandle.getUserId(r.uid));
+            final Intent intent = new Intent();
+            intent.setData(uri);
+            intent.setFlags(modeFlags);
+
+            final NeededUriGrants needed = mUgmInternal.checkGrantUriPermissionFromIntent(intent,
+                    r.uid, targetPkg, UserHandle.getUserId(r.uid));
+            mUgmInternal.grantUriPermissionUncheckedFromIntent(needed, null);
         }
     }
 
diff --git a/services/core/java/com/android/server/uri/UriGrantsManagerInternal.java b/services/core/java/com/android/server/uri/UriGrantsManagerInternal.java
index 8afb87f3020d..cdb61995c336 100644
--- a/services/core/java/com/android/server/uri/UriGrantsManagerInternal.java
+++ b/services/core/java/com/android/server/uri/UriGrantsManagerInternal.java
@@ -31,28 +31,35 @@ import java.io.PrintWriter;
 public interface UriGrantsManagerInternal {
     void onSystemReady();
     void removeUriPermissionIfNeeded(UriPermission perm);
-    void grantUriPermission(int callingUid, String targetPkg, GrantUri grantUri,
-            final int modeFlags, UriPermissionOwner owner, int targetUserId);
+
     void revokeUriPermission(String targetPackage, int callingUid,
             GrantUri grantUri, final int modeFlags);
+
     boolean checkUriPermission(GrantUri grantUri, int uid, final int modeFlags);
-    int checkGrantUriPermission(int callingUid, String targetPkg, GrantUri grantUri,
-            final int modeFlags, int lastTargetUid);
     int checkGrantUriPermission(
             int callingUid, String targetPkg, Uri uri, int modeFlags, int userId);
-    NeededUriGrants checkGrantUriPermissionFromIntent(int callingUid,
-            String targetPkg, Intent intent, int mode, NeededUriGrants needed, int targetUserId);
+
     /**
-     * Grant Uri permissions from one app to another. This method only extends
-     * permission grants if {@code callingUid} has permission to them.
+     * Calculate the set of permission grants that would be needed to extend
+     * access for the given {@link Intent} to the given target package.
+     *
+     * @throws SecurityException if the caller doesn't have permission to the
+     *             {@link Intent} data, or if the underlying provider doesn't
+     *             allow permissions to be granted.
+     */
+    NeededUriGrants checkGrantUriPermissionFromIntent(Intent intent, int callingUid,
+            String targetPkg, int targetUserId);
+
+    /**
+     * Extend a previously calculated set of permissions grants to the given
+     * owner. All security checks will have already been performed as part of
+     * calculating {@link NeededUriGrants}.
      */
-    void grantUriPermissionFromIntent(int callingUid,
-            String targetPkg, Intent intent, int targetUserId);
-    void grantUriPermissionFromIntent(int callingUid,
-            String targetPkg, Intent intent, UriPermissionOwner owner, int targetUserId);
     void grantUriPermissionUncheckedFromIntent(
             NeededUriGrants needed, UriPermissionOwner owner);
+
     IBinder newUriPermissionOwner(String name);
+
     /**
      * Remove any {@link UriPermission} granted <em>from</em> or <em>to</em> the
      * given package.
diff --git a/services/core/java/com/android/server/uri/UriGrantsManagerService.java b/services/core/java/com/android/server/uri/UriGrantsManagerService.java
index 72cdf4afb007..9476e9260c73 100644
--- a/services/core/java/com/android/server/uri/UriGrantsManagerService.java
+++ b/services/core/java/com/android/server/uri/UriGrantsManagerService.java
@@ -635,17 +635,6 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub {
         return needed;
     }
 
-    void grantUriPermissionFromIntent(int callingUid,
-            String targetPkg, Intent intent, UriPermissionOwner owner, int targetUserId) {
-        NeededUriGrants needed = checkGrantUriPermissionFromIntent(callingUid, targetPkg,
-                intent, intent != null ? intent.getFlags() : 0, null, targetUserId);
-        if (needed == null) {
-            return;
-        }
-
-        grantUriPermissionUncheckedFromIntent(needed, owner);
-    }
-
     void readGrantedUriPermissions() {
         if (DEBUG) Slog.v(TAG, "readGrantedUriPermissions()");
 
@@ -1326,15 +1315,6 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub {
         }
 
         @Override
-        public void grantUriPermission(int callingUid, String targetPkg, GrantUri grantUri,
-                int modeFlags, UriPermissionOwner owner, int targetUserId) {
-            synchronized (mLock) {
-                UriGrantsManagerService.this.grantUriPermission(
-                        callingUid, targetPkg, grantUri, modeFlags, owner, targetUserId);
-            }
-        }
-
-        @Override
         public void revokeUriPermission(String targetPackage, int callingUid, GrantUri grantUri,
                 int modeFlags) {
             synchronized (mLock) {
@@ -1351,15 +1331,6 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub {
         }
 
         @Override
-        public int checkGrantUriPermission(int callingUid, String targetPkg, GrantUri uri,
-                int modeFlags, int userId) {
-            synchronized (mLock) {
-                return UriGrantsManagerService.this.checkGrantUriPermission(
-                        callingUid, targetPkg, uri, modeFlags, userId);
-            }
-        }
-
-        @Override
         public int checkGrantUriPermission(int callingUid, String targetPkg, Uri uri, int modeFlags,
                 int userId) {
             enforceNotIsolatedCaller("checkGrantUriPermission");
@@ -1370,29 +1341,12 @@ public class UriGrantsManagerService extends IUriGrantsManager.Stub {
         }
 
         @Override
-        public NeededUriGrants checkGrantUriPermissionFromIntent(int callingUid, String targetPkg,
-                Intent intent, int mode, NeededUriGrants needed, int targetUserId) {
+        public NeededUriGrants checkGrantUriPermissionFromIntent(Intent intent, int callingUid,
+                String targetPkg, int targetUserId) {
             synchronized (mLock) {
+                final int mode = (intent != null) ? intent.getFlags() : 0;
                 return UriGrantsManagerService.this.checkGrantUriPermissionFromIntent(
-                        callingUid, targetPkg, intent, mode, needed, targetUserId);
-            }
-        }
-
-        @Override
-        public void grantUriPermissionFromIntent(int callingUid, String targetPkg, Intent intent,
-                int targetUserId) {
-            synchronized (mLock) {
-                UriGrantsManagerService.this.grantUriPermissionFromIntent(
-                        callingUid, targetPkg, intent, null, targetUserId);
-            }
-        }
-
-        @Override
-        public void grantUriPermissionFromIntent(int callingUid, String targetPkg, Intent intent,
-                UriPermissionOwner owner, int targetUserId) {
-            synchronized (mLock) {
-                UriGrantsManagerService.this.grantUriPermissionFromIntent(
-                        callingUid, targetPkg, intent, owner, targetUserId);
+                        callingUid, targetPkg, intent, mode, null, targetUserId);
             }
         }
 
diff --git a/services/core/java/com/android/server/wm/ActivityRecord.java b/services/core/java/com/android/server/wm/ActivityRecord.java
index 5a6e0a1ff9c3..c3760a05afc1 100644
--- a/services/core/java/com/android/server/wm/ActivityRecord.java
+++ b/services/core/java/com/android/server/wm/ActivityRecord.java
@@ -306,6 +306,7 @@ import com.android.server.am.PendingIntentRecord;
 import com.android.server.display.color.ColorDisplayService;
 import com.android.server.policy.WindowManagerPolicy;
 import com.android.server.protolog.common.ProtoLog;
+import com.android.server.uri.NeededUriGrants;
 import com.android.server.uri.UriPermissionOwner;
 import com.android.server.wm.ActivityMetricsLogger.TransitionInfoSnapshot;
 import com.android.server.wm.ActivityStack.ActivityState;
@@ -2449,9 +2450,11 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
                 }
             }
             if (info.applicationInfo.uid > 0) {
-                mAtmService.mUgmInternal.grantUriPermissionFromIntent(info.applicationInfo.uid,
-                        resultTo.packageName, resultData,
-                        resultTo.getUriPermissionsLocked(), resultTo.mUserId);
+                final NeededUriGrants needed = mAtmService.mUgmInternal
+                        .checkGrantUriPermissionFromIntent(resultData, info.applicationInfo.uid,
+                                resultTo.packageName, resultTo.mUserId);
+                mAtmService.mUgmInternal.grantUriPermissionUncheckedFromIntent(needed,
+                        resultTo.getUriPermissionsLocked());
             }
             resultTo.addResultLocked(this, resultWho, requestCode, resultCode, resultData);
             resultTo = null;
@@ -3663,8 +3666,10 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
     void sendResult(int callingUid, String resultWho, int requestCode, int resultCode,
             Intent data) {
         if (callingUid > 0) {
-            mAtmService.mUgmInternal.grantUriPermissionFromIntent(callingUid, packageName,
-                    data, getUriPermissionsLocked(), mUserId);
+            final NeededUriGrants needed = mAtmService.mUgmInternal
+                    .checkGrantUriPermissionFromIntent(data, callingUid, packageName, mUserId);
+            mAtmService.mUgmInternal.grantUriPermissionUncheckedFromIntent(needed,
+                    getUriPermissionsLocked());
         }
 
         if (DEBUG_RESULTS) {
@@ -3705,8 +3710,10 @@ final class ActivityRecord extends WindowToken implements WindowManagerService.A
      */
     final void deliverNewIntentLocked(int callingUid, Intent intent, String referrer) {
         // The activity now gets access to the data associated with this Intent.
-        mAtmService.mUgmInternal.grantUriPermissionFromIntent(callingUid, packageName,
-                intent, getUriPermissionsLocked(), mUserId);
+        final NeededUriGrants needed = mAtmService.mUgmInternal.checkGrantUriPermissionFromIntent(
+                intent, callingUid, packageName, mUserId);
+        mAtmService.mUgmInternal.grantUriPermissionUncheckedFromIntent(needed,
+                getUriPermissionsLocked());
         final ReferrerIntent rintent = new ReferrerIntent(intent, referrer);
         boolean unsent = true;
         final boolean isTopActivityWhileSleeping = isTopRunningActivity() && isSleeping();
diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java
index bcdd6e39e85c..5b9b126df29b 100644
--- a/services/core/java/com/android/server/wm/ActivityStarter.java
+++ b/services/core/java/com/android/server/wm/ActivityStarter.java
@@ -118,6 +118,7 @@ import com.android.internal.app.HeavyWeightSwitcherActivity;
 import com.android.internal.app.IVoiceInteractor;
 import com.android.server.am.PendingIntentRecord;
 import com.android.server.pm.InstantAppResolver;
+import com.android.server.uri.NeededUriGrants;
 import com.android.server.wm.ActivityMetricsLogger.LaunchingState;
 import com.android.server.wm.ActivityStackSupervisor.PendingActivityLaunch;
 import com.android.server.wm.LaunchParamsController.LaunchParams;
@@ -1625,8 +1626,10 @@ class ActivityStarter {
             }
         }
 
-        mService.mUgmInternal.grantUriPermissionFromIntent(mCallingUid, mStartActivity.packageName,
-                mIntent, mStartActivity.getUriPermissionsLocked(), mStartActivity.mUserId);
+        final NeededUriGrants needed = mService.mUgmInternal.checkGrantUriPermissionFromIntent(
+                mIntent, mCallingUid, mStartActivity.packageName, mStartActivity.mUserId);
+        mService.mUgmInternal.grantUriPermissionUncheckedFromIntent(needed,
+                mStartActivity.getUriPermissionsLocked());
         if (mStartActivity.resultTo != null && mStartActivity.resultTo.info != null) {
             // we need to resolve resultTo to a uid as grantImplicitAccess deals explicitly in UIDs
             final PackageManagerInternal pmInternal =
diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
index b7a9ba56c013..06a7a9e4db4a 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -297,6 +297,7 @@ import com.android.server.pm.RestrictionsSet;
 import com.android.server.pm.UserRestrictionsUtils;
 import com.android.server.pm.parsing.pkg.AndroidPackage;
 import com.android.server.storage.DeviceStorageMonitorInternal;
+import com.android.server.uri.NeededUriGrants;
 import com.android.server.uri.UriGrantsManagerInternal;
 import com.android.server.wm.ActivityTaskManagerInternal;
 
@@ -8365,10 +8366,13 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager {
                 intent.putExtra(DeviceAdminReceiver.EXTRA_BUGREPORT_HASH, bugreportHash);
                 intent.setFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
 
-                LocalServices.getService(UriGrantsManagerInternal.class)
-                        .grantUriPermissionFromIntent(Process.SHELL_UID,
-                                mOwners.getDeviceOwnerComponent().getPackageName(),
-                                intent, mOwners.getDeviceOwnerUserId());
+                final UriGrantsManagerInternal ugm = LocalServices
+                        .getService(UriGrantsManagerInternal.class);
+                final NeededUriGrants needed = ugm.checkGrantUriPermissionFromIntent(intent,
+                        Process.SHELL_UID, mOwners.getDeviceOwnerComponent().getPackageName(),
+                        mOwners.getDeviceOwnerUserId());
+                ugm.grantUriPermissionUncheckedFromIntent(needed, null);
+
                 mContext.sendBroadcastAsUser(intent, UserHandle.of(mOwners.getDeviceOwnerUserId()));
             }
         } catch (FileNotFoundException e) {