Moving enforcing restrictions to only one place
checkContentProviderPermission is the only place where
the enforcement is needed to restrict Content Provider
access
Bug: 265965249
Test: atest ContentProviderRestrictionsHostTest
Change-Id: Id3d7ab802e4e379515ee71547752276b24c697dc
diff --git a/services/core/java/com/android/server/am/ContentProviderHelper.java b/services/core/java/com/android/server/am/ContentProviderHelper.java
index 3357708..1aa63a5 100644
--- a/services/core/java/com/android/server/am/ContentProviderHelper.java
+++ b/services/core/java/com/android/server/am/ContentProviderHelper.java
@@ -250,7 +250,6 @@
if (r != null && cpr.canRunHere(r)) {
checkAssociationAndPermissionLocked(r, cpi, callingUid, userId, checkCrossUser,
cpr.name.flattenToShortString(), startTime);
- enforceContentProviderRestrictionsForSdkSandbox(cpi);
// This provider has been published or is in the process
// of being published... but it is also allowed to run
@@ -443,7 +442,6 @@
// info and allow the caller to instantiate it. Only do
// this if the provider is the same user as the caller's
// process, or can run as root (so can be in any process).
- enforceContentProviderRestrictionsForSdkSandbox(cpi);
return cpr.newHolder(null, true);
}
@@ -586,8 +584,6 @@
// Return a holder instance even if we are waiting for the publishing of the
// provider, client will check for the holder.provider to see if it needs to wait
// for it.
- //todo(b/265965249) Need to perform cleanup before calling enforce method here
- enforceContentProviderRestrictionsForSdkSandbox(cpi);
return cpr.newHolder(conn, false);
}
}
@@ -649,7 +645,6 @@
+ " caller=" + callerName + "/" + Binder.getCallingUid());
return null;
}
- enforceContentProviderRestrictionsForSdkSandbox(cpi);
return cpr.newHolder(conn, false);
}
@@ -1234,7 +1229,6 @@
appName = r.toString();
}
- enforceContentProviderRestrictionsForSdkSandbox(cpi);
return checkContentProviderPermission(cpi, callingPid, Binder.getCallingUid(),
userId, checkUser, appName);
}
@@ -1609,11 +1603,17 @@
/**
* Check if {@link ProcessRecord} has a possible chance at accessing the
- * given {@link ProviderInfo}. Final permission checking is always done
+ * given {@link ProviderInfo}. First permission checking is for enforcing
+ * ContentProvider Restrictions from SdkSandboxManager.
+ * Final permission checking is always done
* in {@link ContentProvider}.
*/
private String checkContentProviderPermission(ProviderInfo cpi, int callingPid, int callingUid,
int userId, boolean checkUser, String appName) {
+ if (!canAccessContentProviderFromSdkSandbox(cpi, callingUid)) {
+ return "ContentProvider access not allowed from sdk sandbox UID. "
+ + "ProviderInfo: " + cpi.toString();
+ }
boolean checkedGrants = false;
if (checkUser) {
// Looking for cross-user grants before enforcing the typical cross-users permissions
@@ -2003,11 +2003,10 @@
}
}
- // Binder.clearCallingIdentity() shouldn't be called before this method
- // as Binder should have its original callingUid for the check
- private void enforceContentProviderRestrictionsForSdkSandbox(ProviderInfo cpi) {
- if (!Process.isSdkSandboxUid(Binder.getCallingUid())) {
- return;
+ private boolean canAccessContentProviderFromSdkSandbox(ProviderInfo cpi,
+ int callingUid) {
+ if (!Process.isSdkSandboxUid(callingUid)) {
+ return true;
}
final SdkSandboxManagerLocal sdkSandboxManagerLocal =
LocalManagerRegistry.getManager(SdkSandboxManagerLocal.class);
@@ -2016,11 +2015,7 @@
+ "when checking whether SDK sandbox uid may "
+ "access the contentprovider.");
}
- if (!sdkSandboxManagerLocal
- .canAccessContentProviderFromSdkSandbox(cpi)) {
- throw new SecurityException(
- "SDK sandbox uid may not access contentprovider " + cpi.name);
- }
+ return sdkSandboxManagerLocal.canAccessContentProviderFromSdkSandbox(cpi);
}
/**
