From 6e6d9f18f342f7cd3c6e0c378a2d65393405b588 Mon Sep 17 00:00:00 2001 From: Pavel Grafov Date: Fri, 3 Dec 2021 15:05:41 +0000 Subject: [PATCH] Check caller owns admin for per-admin getters. Currently some functions allow the caller to check policy enforced by a particular admin by passing a non-null "who" argument. This circumvents package visibility rules. There is no legitimate reason for an 3rd party app to query policies for arbitrary admins. With this change whenever "who" is not null, the code will ensure that the admin referenced by "who" is owned by the caller. Exception to the above are methods that are also called by Setting to query policy for a particular admin for policy transparency. For those methods callers with system UID are allowed to query per-admin policy: * getMaximumFailedPasswordsForWipe * getMaximumTimeToLock * getPasswordQuality There is no legitimate reason for an 3rd party app to query policies for arbitrary admins. Code search for getPasswordHistoryLength and getPasswordMinimum* methods doesn't return any priviledged usage of these methods inside Android. getPasswordQuality is used by Settings, hence the system uid is exempt. Bug: 204995407 Test: atest android.devicepolicy.cts.NoAdminLeakingTest Test: atest android.devicepolicy.cts.ResetPasswordWithTokenTest Test: atest com.android.cts.devicepolicy.MixedDeviceOwnerTest#testPasswordRequirementsApi Change-Id: I443ed1f6dcd5e5b161c99dd09a4b2aef9f8ef0a7 Merged-In: I443ed1f6dcd5e5b161c99dd09a4b2aef9f8ef0a7 --- .../DevicePolicyManagerService.java | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index d0ca298b1219c..81e4b166b7393 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -4001,6 +4001,10 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { final CallerIdentity caller = getCallerIdentity(); Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)); + // System caller can query policy for a particular admin. + Preconditions.checkCallAuthorization( + who == null || isCallingFromPackage(who.getPackageName(), caller.getUid()) + || isSystemUid(caller)); synchronized (getLockObject()) { int mode = PASSWORD_QUALITY_UNSPECIFIED; @@ -4216,7 +4220,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } Preconditions.checkArgumentNonnegative(userHandle, "Invalid userId"); - final CallerIdentity caller = getCallerIdentity(); + final CallerIdentity caller = getCallerIdentity(who); Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)); synchronized (getLockObject()) { @@ -4366,7 +4370,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } Preconditions.checkArgumentNonnegative(userHandle, "Invalid userId"); - final CallerIdentity caller = getCallerIdentity(); + final CallerIdentity caller = getCallerIdentity(who); Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)); synchronized (getLockObject()) { @@ -4579,7 +4583,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } Preconditions.checkArgumentNonnegative(userHandle, "Invalid userId"); - final CallerIdentity caller = getCallerIdentity(); + final CallerIdentity caller = getCallerIdentity(who); Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)); synchronized (getLockObject()) { @@ -4997,6 +5001,10 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { final CallerIdentity caller = getCallerIdentity(); Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)); + // System caller can query policy for a particular admin. + Preconditions.checkCallAuthorization( + who == null || isCallingFromPackage(who.getPackageName(), caller.getUid()) + || isSystemUid(caller)); synchronized (getLockObject()) { ActiveAdmin admin = (who != null) @@ -5308,6 +5316,10 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { final CallerIdentity caller = getCallerIdentity(); Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)); + // System caller can query policy for a particular admin. + Preconditions.checkCallAuthorization( + who == null || isCallingFromPackage(who.getPackageName(), caller.getUid()) + || isSystemUid(caller)); synchronized (getLockObject()) { if (who != null) { @@ -5385,7 +5397,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } Preconditions.checkArgumentNonnegative(userId, "Invalid userId"); - final CallerIdentity caller = getCallerIdentity(); + final CallerIdentity caller = getCallerIdentity(who); Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userId)); if (!mLockPatternUtils.hasSecureLockScreen()) { @@ -7728,6 +7740,10 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (!mHasFeature) { return false; } + + final CallerIdentity caller = getCallerIdentity(who); + Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)); + if (parent) { Preconditions.checkCallAuthorization( isProfileOwnerOfOrganizationOwnedDevice(getCallerIdentity().getUserId())); @@ -9948,7 +9964,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Objects.requireNonNull(agent, "agent null"); Preconditions.checkArgumentNonnegative(userHandle, "Invalid userId"); - final CallerIdentity caller = getCallerIdentity(); + final CallerIdentity caller = getCallerIdentity(admin); Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)); synchronized (getLockObject()) {