DPMS setDeviceOwner access control
* Check whether the caller is system or has
permission to set the device owner before
checking whether the package exists on
device.
Manual testing
* Call DPMS setDeviceOwner with an installed
package but not as the system caller
* Call DPMS setDeviceOwner with an uninstalled
package and not as the system caller
* Both calls should give the same error message
and logs.
Bug: 184525740
Test: manual testing
atest com.android.server.devicepolicy.DevicePolicyManagerTest
atest com.android.cts.devicepolicy.DeviceOwnerTest
Change-Id: I5654ef420ff44af01d475595609bb85fec132ce1
diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
index 00a1786d..7b66377 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -8094,20 +8094,16 @@
+ " as device owner for user " + userId);
return false;
}
- if (admin == null
- || !isPackageInstalledForUser(admin.getPackageName(), userId)) {
- throw new IllegalArgumentException("Invalid component " + admin
- + " for device owner");
- }
+ Preconditions.checkArgument(admin != null);
final CallerIdentity caller = getCallerIdentity();
synchronized (getLockObject()) {
enforceCanSetDeviceOwnerLocked(caller, admin, userId);
+ Preconditions.checkArgument(isPackageInstalledForUser(admin.getPackageName(), userId),
+ "Invalid component " + admin + " for device owner");
final ActiveAdmin activeAdmin = getActiveAdminUncheckedLocked(admin, userId);
- if (activeAdmin == null
- || getUserData(userId).mRemovingAdmins.contains(admin)) {
- throw new IllegalArgumentException("Not active admin: " + admin);
- }
+ Preconditions.checkArgument(activeAdmin != null && !getUserData(
+ userId).mRemovingAdmins.contains(admin), "Not active admin: " + admin);
// Shutting down backup manager service permanently.
toggleBackupServiceActive(UserHandle.USER_SYSTEM, /* makeActive= */ false);
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 6113e4c..c2c550d 100644
--- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java
@@ -1304,6 +1304,16 @@
@Test
public void testSetDeviceOwner_failures() throws Exception {
// TODO Test more failure cases. Basically test all chacks in enforceCanSetDeviceOwner().
+ // Package doesn't exist and caller is not system
+ assertExpectException(SecurityException.class,
+ /* messageRegex= */ "Calling identity is not authorized",
+ () -> dpm.setDeviceOwner(admin1, "owner-name", UserHandle.USER_SYSTEM));
+
+ // Package exists, but caller is not system
+ setUpPackageManagerForAdmin(admin1, DpmMockContext.CALLER_SYSTEM_USER_UID);
+ assertExpectException(SecurityException.class,
+ /* messageRegex= */ "Calling identity is not authorized",
+ () -> dpm.setDeviceOwner(admin1, "owner-name", UserHandle.USER_SYSTEM));
}
@Test
