From 91edfc163b98bb81760429aeb11ddb204f961dc4 Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Thu, 5 May 2022 19:24:10 +0100 Subject: [PATCH] Allow profile owners to protect packages. Changed how protected packages are stored in package manager: * previously they were stored in a map from DO package into a list of protected packages. This isn't expressive enough to allow POs: the same PO package on different users may protect different packages. * now they are stored in a map from userId into a list of protected packages. In case when the DO sets the policy the userId will be UserHandle.USER_ALL, otherwise it will be the calling user. Bug: 218639412 Test: atest UserControlDisabledPackagesTest Change-Id: I2d1d33acad035610c8db365f69b7a15faa03a2d5 Merged-In: I2d1d33acad035610c8db365f69b7a15faa03a2d5 --- .../app/admin/DevicePolicyManager.java | 15 +++--- .../content/pm/PackageManagerInternal.java | 6 +-- .../server/pm/PackageManagerInternalBase.java | 7 ++- .../android/server/pm/ProtectedPackages.java | 43 +++++++++-------- .../DevicePolicyManagerService.java | 46 ++++++++++--------- .../devicepolicy/DevicePolicyManagerTest.java | 15 +++--- 6 files changed, 71 insertions(+), 61 deletions(-) diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index 0a2b421215450..63cdfe60d40b6 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -14569,12 +14569,13 @@ public class DevicePolicyManager { } /** - * Called by Device owner to disable user control over apps. User will not be able to clear - * app data or force-stop packages. + * Called by a device owner or a profile owner to disable user control over apps. User will not + * be able to clear app data or force-stop packages. When called by a device owner, applies to + * all users on the device. * * @param admin which {@link DeviceAdminReceiver} this request is associated with * @param packages The package names for the apps. - * @throws SecurityException if {@code admin} is not a device owner. + * @throws SecurityException if {@code admin} is not a device owner or a profile owner. */ public void setUserControlDisabledPackages(@NonNull ComponentName admin, @NonNull List packages) { @@ -14589,12 +14590,14 @@ public class DevicePolicyManager { } /** - * Returns the list of packages over which user control is disabled by the device owner. + * Returns the list of packages over which user control is disabled by a device or profile + * owner. * * @param admin which {@link DeviceAdminReceiver} this request is associated with - * @throws SecurityException if {@code admin} is not a device owner. + * @throws SecurityException if {@code admin} is not a device or profile owner. */ - public @NonNull List getUserControlDisabledPackages(@NonNull ComponentName admin) { + @NonNull + public List getUserControlDisabledPackages(@NonNull ComponentName admin) { throwIfParentInstance("getUserControlDisabledPackages"); if (mService != null) { try { diff --git a/services/core/java/android/content/pm/PackageManagerInternal.java b/services/core/java/android/content/pm/PackageManagerInternal.java index 06f698efde2b3..dc8c852902e6d 100644 --- a/services/core/java/android/content/pm/PackageManagerInternal.java +++ b/services/core/java/android/content/pm/PackageManagerInternal.java @@ -371,10 +371,10 @@ public abstract class PackageManagerInternal { int deviceOwnerUserId, String deviceOwner, SparseArray profileOwners); /** - * Called by Owners to set the package names protected by the device owner. + * Marks packages as protected for a given user or all users in case of USER_ALL. */ - public abstract void setDeviceOwnerProtectedPackages( - String deviceOwnerPackageName, List packageNames); + public abstract void setOwnerProtectedPackages( + @UserIdInt int userId, @NonNull List packageNames); /** * Returns {@code true} if a given package can't be wiped. Otherwise, returns {@code false}. diff --git a/services/core/java/com/android/server/pm/PackageManagerInternalBase.java b/services/core/java/com/android/server/pm/PackageManagerInternalBase.java index ec6443db9f0f5..5a811c89c5b93 100644 --- a/services/core/java/com/android/server/pm/PackageManagerInternalBase.java +++ b/services/core/java/com/android/server/pm/PackageManagerInternalBase.java @@ -352,10 +352,9 @@ abstract class PackageManagerInternalBase extends PackageManagerInternal { @Override @Deprecated - public final void setDeviceOwnerProtectedPackages( - String deviceOwnerPackageName, List packageNames) { - getProtectedPackages().setDeviceOwnerProtectedPackages( - deviceOwnerPackageName, packageNames); + public final void setOwnerProtectedPackages( + @UserIdInt int userId, @NonNull List packageNames) { + getProtectedPackages().setOwnerProtectedPackages(userId, packageNames); } @Override diff --git a/services/core/java/com/android/server/pm/ProtectedPackages.java b/services/core/java/com/android/server/pm/ProtectedPackages.java index bf46129730231..e9239889973a0 100644 --- a/services/core/java/com/android/server/pm/ProtectedPackages.java +++ b/services/core/java/com/android/server/pm/ProtectedPackages.java @@ -16,11 +16,11 @@ package com.android.server.pm; +import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.UserIdInt; import android.content.Context; import android.os.UserHandle; -import android.util.ArrayMap; import android.util.ArraySet; import android.util.SparseArray; @@ -56,7 +56,7 @@ public class ProtectedPackages { @Nullable @GuardedBy("this") - private final ArrayMap> mDeviceOwnerProtectedPackages = new ArrayMap<>(); + private final SparseArray> mOwnerProtectedPackages = new SparseArray<>(); private final Context mContext; @@ -79,13 +79,13 @@ public class ProtectedPackages { : profileOwnerPackages.clone(); } - /** Sets the protected packages for the device owner. */ - public synchronized void setDeviceOwnerProtectedPackages( - String deviceOwnerPackageName, List packageNames) { + /** Sets packages protected by a device or profile owner. */ + public synchronized void setOwnerProtectedPackages( + @UserIdInt int userId, @NonNull List packageNames) { if (packageNames.isEmpty()) { - mDeviceOwnerProtectedPackages.remove(deviceOwnerPackageName); + mOwnerProtectedPackages.remove(userId); } else { - mDeviceOwnerProtectedPackages.put(deviceOwnerPackageName, new ArraySet<>(packageNames)); + mOwnerProtectedPackages.put(userId, new ArraySet<>(packageNames)); } } @@ -123,19 +123,24 @@ public class ProtectedPackages { *

A protected package means that, apart from the package owner, no system or privileged apps * can modify its data or package state. */ - private synchronized boolean isProtectedPackage(String packageName) { + private synchronized boolean isProtectedPackage(@UserIdInt int userId, String packageName) { return packageName != null && (packageName.equals(mDeviceProvisioningPackage) - || isDeviceOwnerProtectedPackage(packageName)); + || isOwnerProtectedPackage(userId, packageName)); } - /** Returns {@code true} if the given package is a protected package set by any device owner. */ - private synchronized boolean isDeviceOwnerProtectedPackage(String packageName) { - for (Set protectedPackages : mDeviceOwnerProtectedPackages.values()) { - if (protectedPackages.contains(packageName)) { - return true; - } - } - return false; + /** + * Returns {@code true} if the given package is a protected package set by any device or + * profile owner. + */ + private synchronized boolean isOwnerProtectedPackage( + @UserIdInt int userId, String packageName) { + return isPackageProtectedForUser(UserHandle.USER_ALL, packageName) + || isPackageProtectedForUser(userId, packageName); + } + + private synchronized boolean isPackageProtectedForUser(int userId, String packageName) { + int userIdx = mOwnerProtectedPackages.indexOfKey(userId); + return userIdx >= 0 && mOwnerProtectedPackages.valueAt(userIdx).contains(packageName); } /** @@ -146,7 +151,7 @@ public class ProtectedPackages { */ public boolean isPackageStateProtected(@UserIdInt int userId, String packageName) { return hasDeviceOwnerOrProfileOwner(userId, packageName) - || isProtectedPackage(packageName); + || isProtectedPackage(userId, packageName); } /** @@ -155,6 +160,6 @@ public class ProtectedPackages { */ public boolean isPackageDataProtected(@UserIdInt int userId, String packageName) { return hasDeviceOwnerOrProfileOwner(userId, packageName) - || isProtectedPackage(packageName); + || isProtectedPackage(userId, packageName); } } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index ceac1023dfb02..1ce24173d3a2a 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -3116,6 +3116,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { deleteTransferOwnershipBundleLocked(metadata.userId); } updateSystemUpdateFreezePeriodsRecord(/* saveIfChanged */ true); + pushUserControlDisabledPackagesLocked(metadata.userId); } private void maybeLogStart() { @@ -3178,18 +3179,22 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { startOwnerService(userId, "start-user"); } - // TODO(b/218639412): Once PM stores these on a per-user basis, push even empty lists to handle - // DO/PO removal correctly. void pushUserControlDisabledPackagesLocked(int userId) { - if (userId != mOwners.getDeviceOwnerUserId()) { - return; + final int targetUserId; + final ActiveAdmin owner; + if (getDeviceOwnerUserIdUncheckedLocked() == userId) { + owner = getDeviceOwnerAdminLocked(); + targetUserId = UserHandle.USER_ALL; + } else { + owner = getProfileOwnerAdminLocked(userId); + targetUserId = userId; } - ActiveAdmin deviceOwner = getDeviceOwnerAdminLocked(); - if (deviceOwner == null || deviceOwner.protectedPackages == null) { - return; - } - mInjector.getPackageManagerInternal().setDeviceOwnerProtectedPackages( - deviceOwner.info.getPackageName(), deviceOwner.protectedPackages); + + List protectedPackages = (owner == null || owner.protectedPackages == null) + ? Collections.emptyList() : owner.protectedPackages; + mInjector.binderWithCleanCallingIdentity(() -> + mInjector.getPackageManagerInternal().setOwnerProtectedPackages( + targetUserId, protectedPackages)); } @Override @@ -16967,23 +16972,20 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Objects.requireNonNull(who, "ComponentName is null"); Objects.requireNonNull(packages, "packages is null"); final CallerIdentity caller = getCallerIdentity(who); - Preconditions.checkCallAuthorization( - isDefaultDeviceOwner(caller) || isFinancedDeviceOwner(caller)); + Preconditions.checkCallAuthorization(isDefaultDeviceOwner(caller) || isProfileOwner(caller) + || isFinancedDeviceOwner(caller)); checkCanExecuteOrThrowUnsafe( DevicePolicyManager.OPERATION_SET_USER_CONTROL_DISABLED_PACKAGES); synchronized (getLockObject()) { - ActiveAdmin deviceOwner = getDeviceOwnerAdminLocked(); - if (!Objects.equals(deviceOwner.protectedPackages, packages)) { - deviceOwner.protectedPackages = packages.isEmpty() ? null : packages; + ActiveAdmin owner = getDeviceOrProfileOwnerAdminLocked(caller.getUserId()); + if (!Objects.equals(owner.protectedPackages, packages)) { + owner.protectedPackages = packages.isEmpty() ? null : packages; saveSettingsLocked(caller.getUserId()); + pushUserControlDisabledPackagesLocked(caller.getUserId()); } } - mInjector.binderWithCleanCallingIdentity( - () -> mInjector.getPackageManagerInternal().setDeviceOwnerProtectedPackages( - who.getPackageName(), packages)); - DevicePolicyEventLogger .createEvent(DevicePolicyEnums.SET_USER_CONTROL_DISABLED_PACKAGES) .setAdmin(who) @@ -16996,11 +16998,11 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Objects.requireNonNull(who, "ComponentName is null"); final CallerIdentity caller = getCallerIdentity(who); - Preconditions.checkCallAuthorization( - isDefaultDeviceOwner(caller) || isFinancedDeviceOwner(caller)); + Preconditions.checkCallAuthorization(isDefaultDeviceOwner(caller) || isProfileOwner(caller) + || isFinancedDeviceOwner(caller)); synchronized (getLockObject()) { - ActiveAdmin deviceOwner = getDeviceOwnerAdminLocked(); + ActiveAdmin deviceOwner = getDeviceOrProfileOwnerAdminLocked(caller.getUserId()); return deviceOwner.protectedPackages != null ? deviceOwner.protectedPackages : Collections.emptyList(); } diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index 0fc201eae3d41..ec6b67450a1a9 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -6996,22 +6996,23 @@ public class DevicePolicyManagerTest extends DpmTestBase { dpm.setUserControlDisabledPackages(admin1, testPackages); verify(getServices().packageManagerInternal) - .setDeviceOwnerProtectedPackages(admin1.getPackageName(), testPackages); + .setOwnerProtectedPackages(UserHandle.USER_ALL, testPackages); assertThat(dpm.getUserControlDisabledPackages(admin1)).isEqualTo(testPackages); } @Test - public void testSetUserControlDisabledPackages_failingAsPO() { + public void testSetUserControlDisabledPackages_asPO() { final List testPackages = new ArrayList<>(); testPackages.add("package_1"); testPackages.add("package_2"); mServiceContext.permissions.add(permission.MANAGE_DEVICE_ADMINS); setAsProfileOwner(admin1); - assertExpectException(SecurityException.class, /* messageRegex= */ null, - () -> dpm.setUserControlDisabledPackages(admin1, testPackages)); - assertExpectException(SecurityException.class, /* messageRegex= */ null, - () -> dpm.getUserControlDisabledPackages(admin1)); + dpm.setUserControlDisabledPackages(admin1, testPackages); + + verify(getServices().packageManagerInternal) + .setOwnerProtectedPackages(CALLER_USER_HANDLE, testPackages); + assertThat(dpm.getUserControlDisabledPackages(admin1)).isEqualTo(testPackages); } private void configureProfileOwnerOfOrgOwnedDevice(ComponentName who, int userId) { @@ -7845,7 +7846,7 @@ public class DevicePolicyManagerTest extends DpmTestBase { dpm.setUserControlDisabledPackages(admin1, packages); verify(getServices().packageManagerInternal) - .setDeviceOwnerProtectedPackages(eq(admin1.getPackageName()), eq(packages)); + .setOwnerProtectedPackages(eq(UserHandle.USER_ALL), eq(packages)); } @Test