diff options
author | Alex Kershaw <alexkershaw@google.com> | 2020-02-07 14:02:48 +0000 |
---|---|---|
committer | Alex Kershaw <alexkershaw@google.com> | 2020-02-11 14:06:33 +0000 |
commit | 0124a0984455d07a5dc5141fe0a97ec8d0532eff (patch) | |
tree | 73a817b97321157e1eaa568407783b5b41dde569 | |
parent | 1e77561653feb79994c5b7bf5738d345303efb3b (diff) |
Give new permission to set cross-profile app-op to ManagedProvisioning
This is required for the provisioning cross-profile consent screen which
is used to take some apps off INTERACT_ACROSS_USERS.
Hidden API CrossProfileApps#setInteractAcrossProfilesAppOp is changed
from requiring the broad app-op permissions to requiring
CONFIGURE_INTERACT_ACROSS_PROFILES. It then clears identity before
calling into AppOpsManager. For convenience, we also allow apps (such as
Settings) with the broader app-op permissions to continue to call this
method; in that case, we simply don't clear the identity and let
AppOpsManager check the permissions (so we allow AppOpsManager to set
the requirements if you don't have the new
CONFIGURE_INTERACT_ACROSS_PROFILES).
The CL also adds 'withCleanCallingIdentity' support to
CrossProfileAppsServiceImpl and moves over existing calls.
Bug: 136249261
Bug: 140728653
Test: atest --verbose com.android.managedprovisioning.provisioning.crossprofile.CrossProfileConsentActivityRoboTest
Change-Id: Ibd304563dd1ef5f16784e3502be5ef1ec4675b63
6 files changed, 175 insertions, 61 deletions
diff --git a/core/java/android/content/pm/CrossProfileApps.java b/core/java/android/content/pm/CrossProfileApps.java index edc20d9f65ad..25f0ff3cd411 100644 --- a/core/java/android/content/pm/CrossProfileApps.java +++ b/core/java/android/content/pm/CrossProfileApps.java @@ -316,14 +316,21 @@ public class CrossProfileApps { * * <p>If other changes could have affected the app's ability to interact across profiles, as * defined by the return value of {@link #canInteractAcrossProfiles()}, such as changes to the - * admin or OEM consent whitelists, then {@link - * #resetInteractAcrossProfilesAppOpsIfInvalid(List)} should be used. + * admin or OEM consent whitelists, then {@link #resetInteractAcrossProfilesAppOps(Collection, + * Set)} should be used. + * + * <p>If the caller does not have the {@link android.Manifest.permission + * #CONFIGURE_INTERACT_ACROSS_PROFILES} permission, then they must have the permissions that + * would have been required to call {@link android.app.AppOpsManager#setMode(int, int, String, + * int)}, which includes {@link android.Manifest.permission#MANAGE_APP_OPS_MODES}. + * + * <p>Also requires either {@link android.Manifest.permission#INTERACT_ACROSS_USERS} or {@link + * android.Manifest.permission#INTERACT_ACROSS_USERS_FULL}. * * @hide */ @RequiresPermission( - allOf={android.Manifest.permission.MANAGE_APP_OPS_MODES, - android.Manifest.permission.UPDATE_APP_OPS_STATS, + allOf={android.Manifest.permission.CONFIGURE_INTERACT_ACROSS_PROFILES, android.Manifest.permission.INTERACT_ACROSS_USERS}) public void setInteractAcrossProfilesAppOp(@NonNull String packageName, @Mode int newMode) { try { @@ -360,11 +367,18 @@ public class CrossProfileApps { * have changed as a result of non-user actions, such as changes to admin or OEM consent * whitelists. * + * <p>If the caller does not have the {@link android.Manifest.permission + * #CONFIGURE_INTERACT_ACROSS_PROFILES} permission, then they must have the permissions that + * would have been required to call {@link android.app.AppOpsManager#setMode(int, int, String, + * int)}, which includes {@link android.Manifest.permission#MANAGE_APP_OPS_MODES}. + * + * <p>Also requires either {@link android.Manifest.permission#INTERACT_ACROSS_USERS} or {@link + * android.Manifest.permission#INTERACT_ACROSS_USERS_FULL}. + * * @hide */ @RequiresPermission( - allOf={android.Manifest.permission.MANAGE_APP_OPS_MODES, - android.Manifest.permission.UPDATE_APP_OPS_STATS, + allOf={android.Manifest.permission.CONFIGURE_INTERACT_ACROSS_PROFILES, android.Manifest.permission.INTERACT_ACROSS_USERS}) public void resetInteractAcrossProfilesAppOps( @NonNull Collection<String> previousCrossProfilePackages, diff --git a/core/res/AndroidManifest.xml b/core/res/AndroidManifest.xml index ff696715e94d..437aae7f46c5 100644 --- a/core/res/AndroidManifest.xml +++ b/core/res/AndroidManifest.xml @@ -2381,6 +2381,12 @@ <permission android:name="android.permission.INTERACT_ACROSS_PROFILES" android:protectionLevel="signature|appop|documenter|wellbeing" /> + <!-- Allows configuring apps to have the INTERACT_ACROSS_PROFILES permission so that they can + interact across profiles in the same profile group. + @hide --> + <permission android:name="android.permission.CONFIGURE_INTERACT_ACROSS_PROFILES" + android:protectionLevel="signature" /> + <!-- @SystemApi @hide Allows an application to call APIs that allow it to query and manage users on the device. This permission is not available to third party applications. --> diff --git a/data/etc/privapp-permissions-platform.xml b/data/etc/privapp-permissions-platform.xml index f83fb3f312e9..76def66efb4e 100644 --- a/data/etc/privapp-permissions-platform.xml +++ b/data/etc/privapp-permissions-platform.xml @@ -70,6 +70,7 @@ applications that come with the platform <privapp-permissions package="com.android.managedprovisioning"> <permission name="android.permission.CHANGE_COMPONENT_ENABLED_STATE"/> <permission name="android.permission.CHANGE_CONFIGURATION"/> + <permission name="android.permission.CONFIGURE_INTERACT_ACROSS_PROFILES"/> <permission name="android.permission.CRYPT_KEEPER"/> <permission name="android.permission.DELETE_PACKAGES"/> <permission name="android.permission.INSTALL_PACKAGES"/> diff --git a/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java b/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java index 74d2efeceb63..ad037bc09245 100644 --- a/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java +++ b/services/core/java/com/android/server/pm/CrossProfileAppsServiceImpl.java @@ -56,6 +56,8 @@ import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.app.IAppOpsService; import com.android.internal.util.ArrayUtils; +import com.android.internal.util.FunctionalUtils.ThrowingRunnable; +import com.android.internal.util.FunctionalUtils.ThrowingSupplier; import com.android.server.LocalServices; import com.android.server.appop.AppOpsService; import com.android.server.wm.ActivityTaskManagerInternal; @@ -276,19 +278,14 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { } private boolean isCrossProfilePackageWhitelisted(String packageName) { - final long ident = mInjector.clearCallingIdentity(); - try { - return mInjector.getDevicePolicyManagerInternal() - .getAllCrossProfilePackages().contains(packageName); - } finally { - mInjector.restoreCallingIdentity(ident); - } + return mInjector.withCleanCallingIdentity(() -> + mInjector.getDevicePolicyManagerInternal() + .getAllCrossProfilePackages().contains(packageName)); } private List<UserHandle> getTargetUserProfilesUnchecked( String packageName, @UserIdInt int userId) { - final long ident = mInjector.clearCallingIdentity(); - try { + return mInjector.withCleanCallingIdentity(() -> { final int[] enabledProfileIds = mInjector.getUserManager().getEnabledProfileIds(userId); @@ -303,15 +300,12 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { targetProfiles.add(UserHandle.of(profileId)); } return targetProfiles; - } finally { - mInjector.restoreCallingIdentity(ident); - } + }); } private boolean isPackageEnabled(String packageName, @UserIdInt int userId) { final int callingUid = mInjector.getCallingUid(); - final long ident = mInjector.clearCallingIdentity(); - try { + return mInjector.withCleanCallingIdentity(() -> { final PackageInfo info = mInjector.getPackageManagerInternal() .getPackageInfo( packageName, @@ -319,15 +313,12 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { callingUid, userId); return info != null && info.applicationInfo.enabled; - } finally { - mInjector.restoreCallingIdentity(ident); - } + }); } private void verifyActivityCanHandleIntent( Intent launchIntent, int callingUid, @UserIdInt int userId) { - final long ident = mInjector.clearCallingIdentity(); - try { + mInjector.withCleanCallingIdentity(() -> { final List<ResolveInfo> activities = mInjector.getPackageManagerInternal().queryIntentActivities( launchIntent, @@ -340,9 +331,7 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { return; } throw new SecurityException("Activity cannot handle intent"); - } finally { - mInjector.restoreCallingIdentity(ident); - } + }); } /** @@ -351,8 +340,7 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { */ private void verifyActivityCanHandleIntentAndExported( Intent launchIntent, ComponentName component, int callingUid, @UserIdInt int userId) { - final long ident = mInjector.clearCallingIdentity(); - try { + mInjector.withCleanCallingIdentity(() -> { final List<ResolveInfo> apps = mInjector.getPackageManagerInternal().queryIntentActivities( launchIntent, @@ -371,9 +359,7 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { } throw new SecurityException("Attempt to launch activity without " + " category Intent.CATEGORY_LAUNCHER or activity is not exported" + component); - } finally { - mInjector.restoreCallingIdentity(ident); - } + }); } @Override @@ -385,7 +371,13 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { "INTERACT_ACROSS_USERS or INTERACT_ACROSS_USERS_FULL is required to set the" + " app-op for interacting across profiles."); } - final int callingUserId = mInjector.getCallingUserId(); + if (!isPermissionGranted(Manifest.permission.MANAGE_APP_OPS_MODES, callingUid) + && !isPermissionGranted( + Manifest.permission.CONFIGURE_INTERACT_ACROSS_PROFILES, callingUid)) { + throw new SecurityException( + "MANAGE_APP_OPS_MODES or CONFIGURE_INTERACT_ACROSS_PROFILES is required to set" + + " the app-op for interacting across profiles."); + } if (newMode == AppOpsManager.MODE_ALLOWED && !canConfigureInteractAcrossProfiles(packageName)) { // The user should not be prompted for apps that cannot request to interact across @@ -395,7 +387,8 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { return; } final int[] profileIds = - mInjector.getUserManager().getProfileIds(callingUserId, /* enabledOnly= */ false); + mInjector.getUserManager() + .getProfileIds(mInjector.getCallingUserId(), /* enabledOnly= */ false); for (int profileId : profileIds) { if (!isPackageInstalled(packageName, profileId)) { continue; @@ -406,8 +399,7 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { private boolean isPackageInstalled(String packageName, @UserIdInt int userId) { final int callingUid = mInjector.getCallingUid(); - final long identity = mInjector.clearCallingIdentity(); - try { + return mInjector.withCleanCallingIdentity(() -> { final PackageInfo info = mInjector.getPackageManagerInternal() .getPackageInfo( @@ -416,9 +408,7 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { callingUid, userId); return info != null; - } finally { - mInjector.restoreCallingIdentity(identity); - } + }); } private void setInteractAcrossProfilesAppOpForUser( @@ -440,19 +430,31 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { + packageName + " on user ID " + userId); return; } - mInjector.getAppOpsManager() - .setMode(OP_INTERACT_ACROSS_PROFILES, - uid, - packageName, - newMode); + final int callingUid = mInjector.getCallingUid(); + if (isPermissionGranted( + Manifest.permission.CONFIGURE_INTERACT_ACROSS_PROFILES, callingUid)) { + // Clear calling identity since the CONFIGURE_INTERACT_ACROSS_PROFILES permission allows + // this particular app-op to be modified without the broader app-op permissions. + mInjector.withCleanCallingIdentity(() -> + mInjector.getAppOpsManager() + .setMode(OP_INTERACT_ACROSS_PROFILES, uid, packageName, newMode)); + } else { + mInjector.getAppOpsManager() + .setMode(OP_INTERACT_ACROSS_PROFILES, uid, packageName, newMode); + } sendCanInteractAcrossProfilesChangedBroadcast(packageName, uid, UserHandle.of(userId)); } + /** + * Returns whether the given app-op mode is equivalent to the currently-set app-op of the given + * package name and UID. Clears identity to avoid permission checks, so ensure the caller does + * any necessary permission checks. + */ private boolean currentModeEquals(@Mode int otherMode, String packageName, int uid) { final String op = AppOpsManager.permissionToOp(Manifest.permission.INTERACT_ACROSS_PROFILES); - return otherMode == - mInjector.getAppOpsManager().unsafeCheckOpNoThrow(op, uid, packageName); + return mInjector.withCleanCallingIdentity(() -> otherMode + == mInjector.getAppOpsManager().unsafeCheckOpNoThrow(op, uid, packageName)); } private void sendCanInteractAcrossProfilesChangedBroadcast( @@ -493,8 +495,7 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { } private boolean hasOtherProfileWithPackageInstalled(String packageName, @UserIdInt int userId) { - final long ident = mInjector.clearCallingIdentity(); - try { + return mInjector.withCleanCallingIdentity(() -> { final int[] profileIds = mInjector.getUserManager().getProfileIds(userId, /* enabledOnly= */ false); for (int profileId : profileIds) { @@ -502,10 +503,8 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { return true; } } - } finally { - mInjector.restoreCallingIdentity(ident); - } - return false; + return false; + }); } @Override @@ -525,12 +524,8 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { } private boolean isSameProfileGroup(@UserIdInt int callerUserId, @UserIdInt int userId) { - final long ident = mInjector.clearCallingIdentity(); - try { - return mInjector.getUserManager().isSameProfileGroup(callerUserId, userId); - } finally { - mInjector.restoreCallingIdentity(ident); - } + return mInjector.withCleanCallingIdentity(() -> + mInjector.getUserManager().isSameProfileGroup(callerUserId, userId)); } /** @@ -560,42 +555,62 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { mContext = context; } + @Override public int getCallingUid() { return Binder.getCallingUid(); } + @Override public int getCallingPid() { return Binder.getCallingPid(); } + @Override public int getCallingUserId() { return UserHandle.getCallingUserId(); } + @Override public UserHandle getCallingUserHandle() { return Binder.getCallingUserHandle(); } + @Override public long clearCallingIdentity() { return Binder.clearCallingIdentity(); } + @Override public void restoreCallingIdentity(long token) { Binder.restoreCallingIdentity(token); } + @Override + public void withCleanCallingIdentity(ThrowingRunnable action) { + Binder.withCleanCallingIdentity(action); + } + + @Override + public final <T> T withCleanCallingIdentity(ThrowingSupplier<T> action) { + return Binder.withCleanCallingIdentity(action); + } + + @Override public UserManager getUserManager() { return mContext.getSystemService(UserManager.class); } + @Override public PackageManagerInternal getPackageManagerInternal() { return LocalServices.getService(PackageManagerInternal.class); } + @Override public PackageManager getPackageManager() { return mContext.getPackageManager(); } + @Override public AppOpsManager getAppOpsManager() { return mContext.getSystemService(AppOpsManager.class); } @@ -646,6 +661,10 @@ public class CrossProfileAppsServiceImpl extends ICrossProfileApps.Stub { void restoreCallingIdentity(long token); + void withCleanCallingIdentity(ThrowingRunnable action); + + <T> T withCleanCallingIdentity(ThrowingSupplier<T> action); + UserManager getUserManager(); PackageManagerInternal getPackageManagerInternal(); diff --git a/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java b/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java index 1a7b1d3f6039..6190802d033d 100644 --- a/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java +++ b/services/robotests/src/com/android/server/pm/CrossProfileAppsServiceImplRoboTest.java @@ -55,6 +55,8 @@ import android.platform.test.annotations.Presubmit; import androidx.test.core.app.ApplicationProvider; +import com.android.internal.util.FunctionalUtils.ThrowingRunnable; +import com.android.internal.util.FunctionalUtils.ThrowingSupplier; import com.android.server.LocalServices; import com.android.server.testing.shadows.ShadowApplicationPackageManager; import com.android.server.testing.shadows.ShadowUserManager; @@ -190,6 +192,8 @@ public class CrossProfileAppsServiceImplRoboTest { public void grantPermissions() { grantPermissions( Manifest.permission.MANAGE_APP_OPS_MODES, + Manifest.permission.UPDATE_APP_OPS_STATS, + Manifest.permission.CONFIGURE_INTERACT_ACROSS_PROFILES, Manifest.permission.INTERACT_ACROSS_USERS, Manifest.permission.INTERACT_ACROSS_USERS_FULL); } @@ -213,9 +217,26 @@ public class CrossProfileAppsServiceImplRoboTest { } @Test + public void setInteractAcrossProfilesAppOp_noPermissions_throwsSecurityException() { + denyPermissions( + Manifest.permission.MANAGE_APP_OPS_MODES, + Manifest.permission.UPDATE_APP_OPS_STATS, + Manifest.permission.CONFIGURE_INTERACT_ACROSS_PROFILES, + Manifest.permission.INTERACT_ACROSS_USERS, + Manifest.permission.INTERACT_ACROSS_USERS_FULL); + try { + mCrossProfileAppsServiceImpl.setInteractAcrossProfilesAppOp( + CROSS_PROFILE_APP_PACKAGE_NAME, MODE_ALLOWED); + fail(); + } catch (SecurityException expected) {} + } + + @Test public void setInteractAcrossProfilesAppOp_missingInteractAcrossUsersAndFull_throwsSecurityException() { - denyPermissions(Manifest.permission.INTERACT_ACROSS_USERS); - denyPermissions(Manifest.permission.INTERACT_ACROSS_USERS_FULL); + denyPermissions( + Manifest.permission.INTERACT_ACROSS_USERS, + Manifest.permission.INTERACT_ACROSS_USERS_FULL); + grantPermissions(Manifest.permission.CONFIGURE_INTERACT_ACROSS_PROFILES); try { mCrossProfileAppsServiceImpl.setInteractAcrossProfilesAppOp( CROSS_PROFILE_APP_PACKAGE_NAME, MODE_ALLOWED); @@ -231,8 +252,38 @@ public class CrossProfileAppsServiceImplRoboTest { } @Test + public void setInteractAcrossProfilesAppOp_configureInteractAcrossProfilesPermissionWithoutAppOpsPermissions_setsAppOp() { + denyPermissions( + Manifest.permission.MANAGE_APP_OPS_MODES, + Manifest.permission.UPDATE_APP_OPS_STATS); + grantPermissions( + Manifest.permission.CONFIGURE_INTERACT_ACROSS_PROFILES, + Manifest.permission.INTERACT_ACROSS_USERS); + + mCrossProfileAppsServiceImpl.setInteractAcrossProfilesAppOp( + CROSS_PROFILE_APP_PACKAGE_NAME, MODE_ALLOWED); + + assertThat(getCrossProfileAppOp()).isEqualTo(MODE_ALLOWED); + } + + @Test + public void setInteractAcrossProfilesAppOp_appOpsPermissionsWithoutConfigureInteractAcrossProfilesPermission_setsAppOp() { + denyPermissions(Manifest.permission.CONFIGURE_INTERACT_ACROSS_PROFILES); + grantPermissions( + Manifest.permission.MANAGE_APP_OPS_MODES, + Manifest.permission.UPDATE_APP_OPS_STATS, + Manifest.permission.INTERACT_ACROSS_USERS); + + mCrossProfileAppsServiceImpl.setInteractAcrossProfilesAppOp( + CROSS_PROFILE_APP_PACKAGE_NAME, MODE_ALLOWED); + + assertThat(getCrossProfileAppOp()).isEqualTo(MODE_ALLOWED); + } + + @Test public void setInteractAcrossProfilesAppOp_setsAppOpWithUsersAndWithoutFull() { denyPermissions(Manifest.permission.INTERACT_ACROSS_USERS_FULL); + grantPermissions(Manifest.permission.INTERACT_ACROSS_USERS); mCrossProfileAppsServiceImpl.setInteractAcrossProfilesAppOp( CROSS_PROFILE_APP_PACKAGE_NAME, MODE_ALLOWED); assertThat(getCrossProfileAppOp()).isEqualTo(MODE_ALLOWED); @@ -241,6 +292,7 @@ public class CrossProfileAppsServiceImplRoboTest { @Test public void setInteractAcrossProfilesAppOp_setsAppOpWithFullAndWithoutUsers() { denyPermissions(Manifest.permission.INTERACT_ACROSS_USERS); + grantPermissions(Manifest.permission.INTERACT_ACROSS_USERS_FULL); mCrossProfileAppsServiceImpl.setInteractAcrossProfilesAppOp( CROSS_PROFILE_APP_PACKAGE_NAME, MODE_ALLOWED); assertThat(getCrossProfileAppOp()).isEqualTo(MODE_ALLOWED); @@ -479,6 +531,16 @@ public class CrossProfileAppsServiceImplRoboTest { public void restoreCallingIdentity(long token) {} @Override + public void withCleanCallingIdentity(ThrowingRunnable action) { + action.run(); + } + + @Override + public <T> T withCleanCallingIdentity(ThrowingSupplier<T> action) { + return action.get(); + } + + @Override public UserManager getUserManager() { return mUserManager; } diff --git a/services/tests/servicestests/src/com/android/server/pm/CrossProfileAppsServiceImplTest.java b/services/tests/servicestests/src/com/android/server/pm/CrossProfileAppsServiceImplTest.java index 91cc9f35da1e..ef87e5574275 100644 --- a/services/tests/servicestests/src/com/android/server/pm/CrossProfileAppsServiceImplTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/CrossProfileAppsServiceImplTest.java @@ -35,6 +35,8 @@ import android.os.UserManager; import android.platform.test.annotations.Presubmit; import android.util.SparseArray; +import com.android.internal.util.FunctionalUtils.ThrowingRunnable; +import com.android.internal.util.FunctionalUtils.ThrowingSupplier; import com.android.server.wm.ActivityTaskManagerInternal; import org.junit.Before; @@ -574,6 +576,16 @@ public class CrossProfileAppsServiceImplTest { } @Override + public void withCleanCallingIdentity(ThrowingRunnable action) { + action.run(); + } + + @Override + public <T> T withCleanCallingIdentity(ThrowingSupplier<T> action) { + return action.get(); + } + + @Override public UserManager getUserManager() { return mUserManager; } |