From 5cbbac84c3b644cef8f13e4ca4cf4f7712233860 Mon Sep 17 00:00:00 2001 From: Yan Zhu Date: Thu, 3 Dec 2020 21:44:16 -0800 Subject: [PATCH] Improve headless system user's DO setup flow 1. Change manageUser to be setup on current user 2. add pre condition check for headless system user mode: disallows setup when current user is system user 0 and only allows device owner to be set on user 0 3. Changes method name and adds param names To verify: 1. Install TestDPC app: https://github.com/googlesamples/android-testdpc 2. make sure device is unprovisioned adb shell dumpsys device_policy 2'. deprovion if needed adb shell settings put global device_provisioned 0 && adb shell rm /data/system/device_owner_2.xml /data/system/device_policies.xml 3. make sure deivce_amdmin feature is available adb shell pm list features | grep "admin" 3'. install feature if needed adb push frameworks/native/data/etc/android.software.device_admin.xml /vendor/etc/permissions/ 4. run adb shell dpm set-device-owner --user 0 com.afwsamples.testdpc/com.afwsamples.testdpc.DeviceAdminReceiver 5. verify with adb shell dumpsys device_policy 6. try to set on user 10 will fail adb shell dpm set-device-owner --user 10 com.afwsamples.testdpc/com.afwsamples.testdpc.DeviceAdminReceiver 6. switch to user 0, repeat 4. It shall fail. Bug: 170333009 Bug: 174610623 Test: manual with steps above Change-Id: Iaa46032fa6487d792f7bde5de5ff6e55b9c36432 --- .../app/admin/DevicePolicyManager.java | 4 +- .../DevicePolicyManagerService.java | 95 ++++++++++++------- 2 files changed, 64 insertions(+), 35 deletions(-) diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index 94084b851ae03..af275bb3b0738 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -2056,7 +2056,9 @@ public class DevicePolicyManager { /** * Result code for {@link #checkProvisioningPreCondition}. * - *

Returned for {@link #ACTION_PROVISION_MANAGED_USER} if the user is a system user. + *

Returned for {@link #ACTION_PROVISION_MANAGED_USER} if the user is a system user and + * for {@link #ACTION_PROVISION_MANAGED_DEVICE} on devices running headless system user mode + * and the user is a system user. * * @hide */ diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index d35d806741e65..6537cd4fd982b 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -6095,9 +6095,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { private void forceWipeUser(int userId, String wipeReasonForUser, boolean wipeSilently) { boolean success = false; try { - IActivityManager am = mInjector.getIActivityManager(); - if (am.getCurrentUser().id == userId) { - am.switchUser(UserHandle.USER_SYSTEM); + if (getCurrentForegroundUser() == userId) { + mInjector.getIActivityManager().switchUser(UserHandle.USER_SYSTEM); } success = mUserManagerInternal.removeUserEvenWhenDisallowed(userId); @@ -7571,9 +7570,15 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { Slog.i(LOG_TAG, "Device owner set: " + admin + " on user " + userId); if (mInjector.userManagerIsHeadlessSystemUserMode()) { - Slog.i(LOG_TAG, "manageUser: " + admin + " on user " + userId); - - manageUser(admin, admin, caller.getUserId(), null); + int currentForegroundUser = getCurrentForegroundUser(); + Slog.i(LOG_TAG, "setDeviceOwner(): setting " + admin + + " as profile owner on user " + currentForegroundUser); + // Sets profile owner on current foreground user since + // the human user will complete the DO setup workflow from there. + mInjector.binderWithCleanCallingIdentity(() -> + manageUserUnchecked(/* deviceOwner= */ admin, /* profileOwner= */ admin, + /* managedUser= */ currentForegroundUser, + /* adminExtras= */ null)); } return true; } @@ -8558,16 +8563,19 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { * permission. */ private void enforceCanSetDeviceOwnerLocked(CallerIdentity caller, - @Nullable ComponentName owner, @UserIdInt int userId) { + @Nullable ComponentName owner, @UserIdInt int deviceOwnerUserId) { if (!isAdb(caller)) { Preconditions.checkCallAuthorization( hasCallingOrSelfPermission(permission.MANAGE_PROFILE_AND_DEVICE_OWNERS)); } - final int code = checkDeviceOwnerProvisioningPreConditionLocked(owner, userId, - isAdb(caller), hasIncompatibleAccountsOrNonAdbNoLock(caller, userId, owner)); + final int code = checkDeviceOwnerProvisioningPreConditionLocked(owner, + /* deviceOwnerUserId= */ deviceOwnerUserId, /* callingUserId*/ caller.getUserId(), + isAdb(caller), + hasIncompatibleAccountsOrNonAdbNoLock(caller, deviceOwnerUserId, owner)); if (code != CODE_OK) { - throw new IllegalStateException(computeProvisioningErrorString(code, userId)); + throw new IllegalStateException( + computeProvisioningErrorString(code, deviceOwnerUserId)); } } @@ -8695,6 +8703,15 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return UserHandle.isSameApp(caller.getUid(), Process.SHELL_UID); } + private @UserIdInt int getCurrentForegroundUser() { + try { + return mInjector.getIActivityManager().getCurrentUser().id; + } catch (RemoteException e) { + Slog.wtf(LOG_TAG, "cannot get current user"); + } + return UserHandle.USER_NULL; + } + protected int getProfileParentId(int userHandle) { return mInjector.binderWithCleanCallingIdentity(() -> { UserInfo parentUser = mUserManager.getProfileParent(userHandle); @@ -9636,7 +9653,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { final long id = mInjector.binderClearCallingIdentity(); try { - manageUser(admin, profileOwner, userHandle, adminExtras); + manageUserUnchecked(admin, profileOwner, userHandle, adminExtras); if ((flags & DevicePolicyManager.SKIP_SETUP_WIZARD) != 0) { Settings.Secure.putIntForUser(mContext.getContentResolver(), @@ -9657,43 +9674,34 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } } - private void manageUser(ComponentName admin, ComponentName profileOwner, + private void manageUserUnchecked(ComponentName admin, ComponentName profileOwner, @UserIdInt int userId, PersistableBundle adminExtras) { - // Check for permission - final CallerIdentity caller = getCallerIdentity(); - Preconditions.checkCallAuthorization(canManageUsers(caller)); - Preconditions.checkCallAuthorization( - hasCallingOrSelfPermission(permission.MANAGE_PROFILE_AND_DEVICE_OWNERS)); - mInjector.binderWithCleanCallingIdentity(() -> - manageUserNoCheck(admin, profileOwner, userId, adminExtras)); - } - - private void manageUserNoCheck(ComponentName admin, ComponentName profileOwner, - int user, PersistableBundle adminExtras) { - final String adminPkg = admin.getPackageName(); try { // Install the profile owner if not present. - if (!mIPackageManager.isPackageAvailable(adminPkg, user)) { - mIPackageManager.installExistingPackageAsUser(adminPkg, user, + if (!mIPackageManager.isPackageAvailable(adminPkg, userId)) { + mIPackageManager.installExistingPackageAsUser(adminPkg, userId, PackageManager.INSTALL_ALL_WHITELIST_RESTRICTED_PERMISSIONS, - PackageManager.INSTALL_REASON_POLICY, null); + PackageManager.INSTALL_REASON_POLICY, + /* allowlistedRestrictedPermissions= */ null); } } catch (RemoteException e) { // Does not happen, same process + Slog.wtf(LOG_TAG, String.format("Failed to install admin package %s for user %d", + adminPkg, userId), e); } // Set admin. - setActiveAdmin(profileOwner, true, user); + setActiveAdmin(profileOwner, /* refreshing= */ true, userId); final String ownerName = getProfileOwnerName(Process.myUserHandle().getIdentifier()); - setProfileOwner(profileOwner, ownerName, user); + setProfileOwner(profileOwner, ownerName, userId); synchronized (getLockObject()) { - DevicePolicyData policyData = getUserData(user); + DevicePolicyData policyData = getUserData(userId); policyData.mInitBundle = adminExtras; policyData.mAdminBroadcastPending = true; - saveSettingsLocked(user); + saveSettingsLocked(userId); } } @@ -12322,7 +12330,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { * except for adb command if no accounts or additional users are present on the device. */ private int checkDeviceOwnerProvisioningPreConditionLocked(@Nullable ComponentName owner, - @UserIdInt int deviceOwnerUserId, boolean isAdb, + @UserIdInt int deviceOwnerUserId, @UserIdInt int callingUserId, boolean isAdb, boolean hasIncompatibleAccountsOrNonAdb) { if (mOwners.hasDeviceOwner()) { return CODE_HAS_DEVICE_OWNER; @@ -12338,6 +12346,15 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (mIsWatch && hasPaired(UserHandle.USER_SYSTEM)) { return CODE_HAS_PAIRED; } + + if (mInjector.userManagerIsHeadlessSystemUserMode()) { + if (deviceOwnerUserId != UserHandle.USER_SYSTEM) { + Slog.e(LOG_TAG, "In headless system user mode, " + + "device owner can only be set on headless system user."); + return CODE_NOT_SYSTEM_USER; + } + } + // TODO (b/137101239): clean up split system user codes if (isAdb) { // If shell command runs after user setup completed check device status. Otherwise, OK. @@ -12349,6 +12366,15 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { && mUserManager.getUserCount() > 1) { return CODE_NONSYSTEM_USER_EXISTS; } + + int currentForegroundUser = getCurrentForegroundUser(); + if (callingUserId != currentForegroundUser + && mInjector.userManagerIsHeadlessSystemUserMode() + && currentForegroundUser == UserHandle.USER_SYSTEM) { + Slog.wtf(LOG_TAG, "In headless system user mode, " + + "current user cannot be system user when setting device owner"); + return CODE_SYSTEM_USER; + } if (hasIncompatibleAccountsOrNonAdb) { return CODE_ACCOUNTS_NOT_EMPTY; } @@ -12380,7 +12406,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { callingUserId, deviceOwnerUserId)); // hasIncompatibleAccountsOrNonAdb doesn't matter since the caller is not adb. return checkDeviceOwnerProvisioningPreConditionLocked(/* owner unknown */ null, - deviceOwnerUserId, /* isAdb= */ false, + deviceOwnerUserId, callingUserId, /* isAdb= */ false, /* hasIncompatibleAccountsOrNonAdb=*/ true); } } @@ -14919,8 +14945,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } private boolean isLockTaskFeatureEnabled(int lockTaskFeature) throws RemoteException { + //TODO(b/175285301): Explicitly get the user's identity to check. int lockTaskFeatures = - getUserData(mInjector.getIActivityManager().getCurrentUser().id).mLockTaskFeatures; + getUserData(getCurrentForegroundUser()).mLockTaskFeatures; return (lockTaskFeatures & lockTaskFeature) == lockTaskFeature; }