From e3b11528b699df68873991177ef1f1482c5a1abc Mon Sep 17 00:00:00 2001 From: Alex Johnston Date: Wed, 28 Oct 2020 13:04:11 +0000 Subject: [PATCH] Replace isAdb and enforceShell in DPMS * Replace isAdb with isAdb(caller) * Remove enforceShell and use isAdb(caller) instead Bug: 167960209 Test: atest com.android.server.devicepolicy.DevicePolicyManagerTest atest com.android.cts.devicepolicy.MixedDeviceOwnerTest atest com.android.cts.devicepolicy.ProfileOwnerTest atest com.android.cts.devicepolicy.ManagedProfileTest Change-Id: Id739a6a41bb647b4e485e1f7a109362874cfb2f0 --- .../DevicePolicyManagerService.java | 79 +++++++++---------- 1 file changed, 37 insertions(+), 42 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index a253f778744a2..cc97ae0a08d7f 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -3230,7 +3230,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return; } Objects.requireNonNull(adminReceiver, "ComponentName is null"); - enforceShell("forceRemoveActiveAdmin"); + Preconditions.checkCallAuthorization(isAdb(getCallerIdentity()), + "Non-shell user attempted to call forceRemoveActiveAdmin"); mInjector.binderWithCleanCallingIdentity(() -> { synchronized (getLockObject()) { if (!isAdminTestOnlyLocked(adminReceiver, userHandle)) { @@ -3309,13 +3310,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return (admin != null) && admin.testOnlyAdmin; } - private void enforceShell(String method) { - final int callingUid = mInjector.binderGetCallingUid(); - if (callingUid != Process.SHELL_UID && callingUid != Process.ROOT_UID) { - throw new SecurityException("Non-shell user attempted to call " + method); - } - } - @Override public void removeActiveAdmin(ComponentName adminReceiver, int userHandle) { if (!mHasFeature) { @@ -7248,10 +7242,10 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { throw new IllegalArgumentException("Invalid component " + admin + " for device owner"); } - final boolean hasIncompatibleAccountsOrNonAdb = - hasIncompatibleAccountsOrNonAdbNoLock(userId, admin); + + final CallerIdentity caller = getCallerIdentity(); synchronized (getLockObject()) { - enforceCanSetDeviceOwnerLocked(admin, userId, hasIncompatibleAccountsOrNonAdb); + enforceCanSetDeviceOwnerLocked(caller, admin, userId); final ActiveAdmin activeAdmin = getActiveAdminUncheckedLocked(admin, userId); if (activeAdmin == null || getUserData(userId).mRemovingAdmins.contains(admin)) { @@ -7260,7 +7254,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { // Shutting down backup manager service permanently. toggleBackupServiceActive(UserHandle.USER_SYSTEM, /* makeActive= */ false); - if (isAdb()) { + if (isAdb(caller)) { // Log device owner provisioning was started using adb. MetricsLogger.action(mContext, PROVISIONING_ENTRY_POINT_ADB, LOG_TAG_DEVICE_OWNER); DevicePolicyEventLogger @@ -7616,10 +7610,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { + " not installed for userId:" + userHandle); } - final boolean hasIncompatibleAccountsOrNonAdb = - hasIncompatibleAccountsOrNonAdbNoLock(userHandle, who); + final CallerIdentity caller = getCallerIdentity(); synchronized (getLockObject()) { - enforceCanSetProfileOwnerLocked(who, userHandle, hasIncompatibleAccountsOrNonAdb); + enforceCanSetProfileOwnerLocked(caller, who, userHandle); final ActiveAdmin admin = getActiveAdminUncheckedLocked(who, userHandle); if (admin == null || getUserData(userHandle).mRemovingAdmins.contains(who)) { @@ -7637,7 +7630,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return false; } - if (isAdb()) { + if (isAdb(caller)) { // Log profile owner provisioning was started using adb. MetricsLogger.action(mContext, PROVISIONING_ENTRY_POINT_ADB, LOG_TAG_PROFILE_OWNER); DevicePolicyEventLogger @@ -7830,6 +7823,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return; } + final CallerIdentity caller = getCallerIdentity(); if (userHandle != mOwners.getDeviceOwnerUserId() && !mOwners.hasProfileOwner(userHandle) && getManagedUserId(userHandle) == -1) { // No managed device, user or profile, so setting provisioning state makes no sense. @@ -7841,7 +7835,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { boolean transitionCheckNeeded = true; // Calling identity/permission checks. - if (isAdb()) { + if (isAdb(caller)) { // ADB shell can only move directly from un-managed to finalized as part of directly // setting profile-owner or device-owner. if (getUserProvisioningState(userHandle) != @@ -8204,8 +8198,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { * - SYSTEM_UID * - adb unless hasIncompatibleAccountsOrNonAdb is true. */ - private void enforceCanSetProfileOwnerLocked(@Nullable ComponentName owner, int userHandle, - boolean hasIncompatibleAccountsOrNonAdb) { + private void enforceCanSetProfileOwnerLocked(CallerIdentity caller, + @Nullable ComponentName owner, int userHandle) { UserInfo info = getUserInfo(userHandle); if (info == null) { // User doesn't exist. @@ -8223,9 +8217,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { throw new IllegalStateException("Trying to set the profile owner, but the user " + "already has a device owner."); } - if (isAdb()) { + if (isAdb(caller)) { if ((mIsWatch || hasUserSetupCompleted(userHandle)) - && hasIncompatibleAccountsOrNonAdb) { + && hasIncompatibleAccountsOrNonAdbNoLock(caller, userHandle, owner)) { throw new IllegalStateException("Not allowed to set the profile owner because " + "there are already some accounts on the profile"); } @@ -8264,16 +8258,15 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { * The Device owner can only be set by adb or an app with the MANAGE_PROFILE_AND_DEVICE_OWNERS * permission. */ - private void enforceCanSetDeviceOwnerLocked(@Nullable ComponentName owner, - @UserIdInt int userId, - boolean hasIncompatibleAccountsOrNonAdb) { - if (!isAdb()) { + private void enforceCanSetDeviceOwnerLocked(CallerIdentity caller, + @Nullable ComponentName owner, @UserIdInt int userId) { + if (!isAdb(caller)) { Preconditions.checkCallAuthorization( hasCallingOrSelfPermission(permission.MANAGE_PROFILE_AND_DEVICE_OWNERS)); } - final int code = checkDeviceOwnerProvisioningPreConditionLocked( - owner, userId, isAdb(), hasIncompatibleAccountsOrNonAdb); + final int code = checkDeviceOwnerProvisioningPreConditionLocked(owner, userId, + isAdb(caller), hasIncompatibleAccountsOrNonAdbNoLock(caller, userId, owner)); if (code != CODE_OK) { throw new IllegalStateException(computeProvisioningErrorString(code, userId)); } @@ -11676,7 +11669,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { @Override public void clearSystemUpdatePolicyFreezePeriodRecord() { - enforceShell("clearSystemUpdatePolicyFreezePeriodRecord"); + Preconditions.checkCallAuthorization(isAdb(getCallerIdentity()), + "Non-shell user attempted to call clearSystemUpdatePolicyFreezePeriodRecord"); synchronized (getLockObject()) { // Print out current record to help diagnosed CTS failures Slog.i(LOG_TAG, "Clear freeze period record: " @@ -12582,23 +12576,23 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { @Override public void markProfileOwnerOnOrganizationOwnedDevice(ComponentName who, int userId) { + if (!mHasFeature) { + return; + } // As the caller is the system, it must specify the component name of the profile owner // as a safety check. Objects.requireNonNull(who); - if (!mHasFeature) { - return; - } - + final CallerIdentity caller = getCallerIdentity(); // Only adb or system apps with the right permission can mark a profile owner on // organization-owned device. - if (!(isAdb() || hasCallingPermission(permission.MARK_DEVICE_ORGANIZATION_OWNED))) { + if (!(isAdb(caller) || hasCallingPermission(permission.MARK_DEVICE_ORGANIZATION_OWNED))) { throw new SecurityException( "Only the system can mark a profile owner of organization-owned device."); } - if (isAdb()) { - if (hasIncompatibleAccountsOrNonAdbNoLock(userId, who)) { + if (isAdb(caller)) { + if (hasIncompatibleAccountsOrNonAdbNoLock(caller, userId, who)) { throw new SecurityException( "Can only be called from ADB if the device has no accounts."); } @@ -12919,7 +12913,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { @Override public long forceSecurityLogs() { - enforceShell("forceSecurityLogs"); + Preconditions.checkCallAuthorization(isAdb(getCallerIdentity()), + "Non-shell user attempted to call forceSecurityLogs"); if (!mInjector.securityLogGetLoggingEnabledProperty()) { throw new IllegalStateException("logging is not available"); } @@ -13287,9 +13282,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { * * DO NOT CALL IT WITH THE DPMS LOCK HELD. */ - private boolean hasIncompatibleAccountsOrNonAdbNoLock( + private boolean hasIncompatibleAccountsOrNonAdbNoLock(CallerIdentity caller, int userId, @Nullable ComponentName owner) { - if (!isAdb()) { + if (!isAdb(caller)) { return true; } wtfIfInLock(); @@ -13344,9 +13339,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } } - private boolean isAdb() { - final int callingUid = mInjector.binderGetCallingUid(); - return callingUid == Process.SHELL_UID || callingUid == Process.ROOT_UID; + private boolean isAdb(CallerIdentity caller) { + return isShellUid(caller) || isRootUid(caller); } @Override @@ -13408,7 +13402,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { @Override public long forceNetworkLogs() { - enforceShell("forceNetworkLogs"); + Preconditions.checkCallAuthorization(isAdb(getCallerIdentity()), + "Non-shell user attempted to call forceNetworkLogs"); synchronized (getLockObject()) { if (!isNetworkLoggingEnabledInternalLocked()) { throw new IllegalStateException("logging is not available");