From 73c35de08c588da6ef824a348a5073ed7dda3007 Mon Sep 17 00:00:00 2001 From: Kholoud Mohamed Date: Tue, 9 May 2023 21:17:55 +0000 Subject: [PATCH] Remove policies for admins on role/package removal Bug: 256852787 Bug: 273494642 Test: N/A Change-Id: I2a6ccecd10b8bdaf80679f6d52eb85e9d16ca21f --- .../devicepolicy/DevicePolicyEngine.java | 40 ++++++++++++++++++- .../DevicePolicyManagerService.java | 8 ++-- .../server/devicepolicy/EnforcingAdmin.java | 25 ++++-------- .../server/devicepolicy/PolicyDefinition.java | 10 ++--- 4 files changed, 54 insertions(+), 29 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyEngine.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyEngine.java index 641a03b931c41..1392d0232262a 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyEngine.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyEngine.java @@ -81,6 +81,10 @@ import java.util.Set; final class DevicePolicyEngine { static final String TAG = "DevicePolicyEngine"; + // TODO(b/281701062): reference role name from role manager once its exposed. + static final String DEVICE_LOCK_CONTROLLER_ROLE = + "android.app.role.SYSTEM_FINANCED_DEVICE_CONTROLLER"; + private static final String CELLULAR_2G_USER_RESTRICTION_ID = DevicePolicyIdentifiers.getIdentifierForUserRestriction( UserManager.DISALLOW_CELLULAR_2G); @@ -1010,11 +1014,21 @@ final class DevicePolicyEngine { /** * Handles internal state related to packages getting updated. */ - void handlePackageChanged(@Nullable String updatedPackage, int userId) { + void handlePackageChanged(@Nullable String updatedPackage, int userId, boolean packageRemoved) { if (updatedPackage == null) { return; } - updateDeviceAdminServiceOnPackageChanged(updatedPackage, userId); + if (packageRemoved) { + Set admins = getEnforcingAdminsOnUser(userId); + for (EnforcingAdmin admin : admins) { + if (admin.getPackageName().equals(updatedPackage)) { + // remove policies for the uninstalled package + removePoliciesForAdmin(admin); + } + } + } else { + updateDeviceAdminServiceOnPackageChanged(updatedPackage, userId); + } } /** @@ -1032,6 +1046,28 @@ final class DevicePolicyEngine { enforcePoliciesOnInheritableProfilesIfApplicable(user); } + /** + * Handles internal state related to roles getting updated. + */ + void handleRoleChanged(@NonNull String roleName, int userId) { + // TODO(b/256852787): handle all roles changing. + if (!DEVICE_LOCK_CONTROLLER_ROLE.equals(roleName)) { + // We only support device lock controller role for now. + return; + } + String roleAuthority = EnforcingAdmin.getRoleAuthorityOf(roleName); + Set admins = getEnforcingAdminsOnUser(userId); + for (EnforcingAdmin admin : admins) { + if (admin.hasAuthority(roleAuthority)) { + admin.reloadRoleAuthorities(); + // remove admin policies if role was lost + if (!admin.hasAuthority(roleAuthority)) { + removePoliciesForAdmin(admin); + } + } + } + } + private void enforcePoliciesOnInheritableProfilesIfApplicable(UserInfo user) { if (!user.isProfile()) { return; diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 85be10d424bac..662fd3daeed9b 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -259,7 +259,6 @@ import android.Manifest.permission; import android.accessibilityservice.AccessibilityServiceInfo; import android.accounts.Account; import android.accounts.AccountManager; -import android.accounts.AccountManagerFuture; import android.accounts.AuthenticatorException; import android.accounts.OperationCanceledException; import android.annotation.IntDef; @@ -1450,8 +1449,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { && (owner.getPackageName().equals(packageName))) { startOwnerService(userHandle, "package-broadcast"); } - if (isPermissionCheckFlagEnabled()) { - mDevicePolicyEngine.handlePackageChanged(packageName, userHandle); + if (isPolicyEngineForFinanceFlagEnabled() || isPermissionCheckFlagEnabled()) { + mDevicePolicyEngine.handlePackageChanged(packageName, userHandle, removedAdmin); } // Persist updates if the removed package was an admin or delegate. if (removedAdmin || removedDelegate) { @@ -12747,7 +12746,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { final int userId = user.id; - if (isPermissionCheckFlagEnabled()) { + if (isPolicyEngineForFinanceFlagEnabled() || isPermissionCheckFlagEnabled()) { mDevicePolicyEngine.handleUserCreated(user); } @@ -22685,6 +22684,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { @Override public void onRoleHoldersChanged(@NonNull String roleName, @NonNull UserHandle user) { + mDevicePolicyEngine.handleRoleChanged(roleName, user.getIdentifier()); if (RoleManager.ROLE_DEVICE_POLICY_MANAGEMENT.equals(roleName)) { handleDevicePolicyManagementRoleChange(user); return; diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/EnforcingAdmin.java b/services/devicepolicy/java/com/android/server/devicepolicy/EnforcingAdmin.java index 7ed148bd76f19..3ed2d34be6dfc 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/EnforcingAdmin.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/EnforcingAdmin.java @@ -93,22 +93,6 @@ final class EnforcingAdmin { activeAdmin); } - - static EnforcingAdmin createEnterpriseEnforcingAdmin( - @NonNull String packageName, int userId) { - Objects.requireNonNull(packageName); - return new EnforcingAdmin( - packageName, /* componentName= */ null, Set.of(DPC_AUTHORITY), userId, - /* activeAdmin= */ null); - } - - static EnforcingAdmin createDeviceAdminEnforcingAdmin(ComponentName componentName, int userId) { - Objects.requireNonNull(componentName); - return new EnforcingAdmin( - componentName.getPackageName(), componentName, Set.of(DEVICE_ADMIN_AUTHORITY), - userId, /* activeAdmin=*/ null); - } - static EnforcingAdmin createDeviceAdminEnforcingAdmin(ComponentName componentName, int userId, ActiveAdmin activeAdmin) { Objects.requireNonNull(componentName); @@ -190,12 +174,18 @@ final class EnforcingAdmin { } private Set getAuthorities() { - if (mAuthorities == null) { + if (mAuthorities == null && mIsRoleAuthority) { mAuthorities = getRoleAuthoritiesOrDefault(mPackageName, mUserId); } return mAuthorities; } + void reloadRoleAuthorities() { + if (mIsRoleAuthority) { + mAuthorities = getRoleAuthoritiesOrDefault(mPackageName, mUserId); + } + } + boolean hasAuthority(String authority) { return getAuthorities().contains(authority); } @@ -304,6 +294,7 @@ final class EnforcingAdmin { int userId = parser.getAttributeInt(/* namespace= */ null, ATTR_USER_ID); if (isRoleAuthority) { + // TODO(b/281697976): load active admin return new EnforcingAdmin(packageName, userId, null); } else { String className = parser.getAttributeValue(/* namespace= */ null, ATTR_CLASS_NAME); diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyDefinition.java b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyDefinition.java index bac39e021d2f4..7e5bb0bb40ba0 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyDefinition.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyDefinition.java @@ -16,6 +16,8 @@ package com.android.server.devicepolicy; +import static com.android.server.devicepolicy.DevicePolicyEngine.DEVICE_LOCK_CONTROLLER_ROLE; + import android.annotation.NonNull; import android.annotation.Nullable; import android.app.admin.AccountTypePolicyKey; @@ -131,9 +133,7 @@ final class PolicyDefinition { static PolicyDefinition LOCK_TASK = new PolicyDefinition<>( new NoArgsPolicyKey(DevicePolicyIdentifiers.LOCK_TASK_POLICY), new TopPriority<>(List.of( - // TODO(b/258166155): add correct device lock role name - EnforcingAdmin.getRoleAuthorityOf( - "android.app.role.SYSTEM_FINANCED_DEVICE_CONTROLLER"), + EnforcingAdmin.getRoleAuthorityOf(DEVICE_LOCK_CONTROLLER_ROLE), EnforcingAdmin.DPC_AUTHORITY)), POLICY_FLAG_LOCAL_ONLY_POLICY, (LockTaskPolicy value, Context context, Integer userId, PolicyKey policyKey) -> @@ -157,9 +157,7 @@ final class PolicyDefinition { new IntentFilterPolicyKey( DevicePolicyIdentifiers.PERSISTENT_PREFERRED_ACTIVITY_POLICY), new TopPriority<>(List.of( - // TODO(b/258166155): add correct device lock role name - EnforcingAdmin.getRoleAuthorityOf( - "android.app.role.SYSTEM_FINANCED_DEVICE_CONTROLLER"), + EnforcingAdmin.getRoleAuthorityOf(DEVICE_LOCK_CONTROLLER_ROLE), EnforcingAdmin.DPC_AUTHORITY)), POLICY_FLAG_LOCAL_ONLY_POLICY, PolicyEnforcerCallbacks::addPersistentPreferredActivity,