From f04e1ae05c74978726476565b9f10b36139a2921 Mon Sep 17 00:00:00 2001 From: Rubin Xu Date: Mon, 21 Jun 2021 10:15:48 +0100 Subject: [PATCH] Enforce profile password quality policy on unified device lock Effectively revert I59354066dafb02fe1d464fd1ddabf3969a4d0f1b to allow password quality policy set on the profile DPM instance to be enforced on device lockscreen provided the device has no separate work challenge. Bug: 182561862 Test: atest FrameworksServicesTests:DevicePolicyManagerTest Test: atest ManagedProfilePasswordTest Test: atest MixedManagedProfileOwnerTest#testResetPasswordWithToken Test: Create work profile, set profile password quality policy, update device password via Settings and verify policy is applied. Change-Id: I160997822f57c1da9327fe1a5482f445df8e1094 --- .../server/devicepolicy/ActiveAdmin.java | 11 - .../DevicePolicyManagerService.java | 22 +- .../devicepolicy/DevicePolicyManagerTest.java | 199 ++++++++++++++++++ 3 files changed, 204 insertions(+), 28 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/ActiveAdmin.java b/services/devicepolicy/java/com/android/server/devicepolicy/ActiveAdmin.java index ce8f6df9ce3de..37a84f3698c13 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/ActiveAdmin.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/ActiveAdmin.java @@ -104,8 +104,6 @@ class ActiveAdmin { private static final String TAG_PASSWORD_HISTORY_LENGTH = "password-history-length"; private static final String TAG_MIN_PASSWORD_LENGTH = "min-password-length"; private static final String TAG_PASSWORD_QUALITY = "password-quality"; - private static final String TAG_PASSWORD_QUALITY_APPLIES_TO_PARENT = - "password-quality-applies-parent"; private static final String TAG_POLICIES = "policies"; private static final String TAG_CROSS_PROFILE_WIDGET_PROVIDERS = "cross-profile-widget-providers"; @@ -158,7 +156,6 @@ class ActiveAdmin { @NonNull PasswordPolicy mPasswordPolicy = new PasswordPolicy(); - boolean mPasswordPolicyAppliesToParent = true; @DevicePolicyManager.PasswordComplexity int mPasswordComplexity = PASSWORD_COMPLEXITY_NONE; @@ -363,9 +360,6 @@ class ActiveAdmin { writeAttributeValueToXml( out, TAG_MIN_PASSWORD_NONLETTER, mPasswordPolicy.nonLetter); } - - writeAttributeValueToXml(out, TAG_PASSWORD_QUALITY_APPLIES_TO_PARENT, - mPasswordPolicyAppliesToParent); } if (passwordHistoryLength != DEF_PASSWORD_HISTORY_LENGTH) { writeAttributeValueToXml( @@ -669,8 +663,6 @@ class ActiveAdmin { mPasswordPolicy.symbols = parser.getAttributeInt(null, ATTR_VALUE); } else if (TAG_MIN_PASSWORD_NONLETTER.equals(tag)) { mPasswordPolicy.nonLetter = parser.getAttributeInt(null, ATTR_VALUE); - } else if (TAG_PASSWORD_QUALITY_APPLIES_TO_PARENT.equals(tag)) { - mPasswordPolicyAppliesToParent = parser.getAttributeBoolean(null, ATTR_VALUE); } else if (TAG_MAX_TIME_TO_UNLOCK.equals(tag)) { maximumTimeToUnlock = parser.getAttributeLong(null, ATTR_VALUE); } else if (TAG_STRONG_AUTH_UNLOCK_TIMEOUT.equals(tag)) { @@ -1036,9 +1028,6 @@ class ActiveAdmin { pw.print("minimumPasswordNonLetter="); pw.println(mPasswordPolicy.nonLetter); - pw.print("passwordPolicyAppliesToParent="); - pw.println(mPasswordPolicyAppliesToParent); - pw.print("maximumTimeToUnlock="); pw.println(maximumTimeToUnlock); diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 9a35738744710..2035cbd3036ce 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -3770,10 +3770,9 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { isProfileOwner(caller) || isDeviceOwner(caller) || isSystemUid(caller) || isPasswordLimitingAdminTargetingP(caller)); - final boolean qualityMayApplyToParent = - canSetPasswordQualityOnParent(who.getPackageName(), caller); - if (!qualityMayApplyToParent) { - Preconditions.checkCallAuthorization(!parent, + if (parent) { + Preconditions.checkCallAuthorization( + canSetPasswordQualityOnParent(who.getPackageName(), caller), "Profile Owner may not apply password quality requirements device-wide"); } @@ -3799,7 +3798,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (passwordPolicy.quality != quality) { passwordPolicy.quality = quality; ap.mPasswordComplexity = PASSWORD_COMPLEXITY_NONE; - ap.mPasswordPolicyAppliesToParent = qualityMayApplyToParent; resetInactivePasswordRequirementsIfRPlus(userId, ap); updatePasswordValidityCheckpointLocked(userId, parent); updatePasswordQualityCacheForUserGroup(userId); @@ -4531,8 +4529,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } ArrayList adminMetrics = new ArrayList<>(); + final List admins; synchronized (getLockObject()) { - final List admins; if (deviceWideOnly) { admins = getActiveAdminsForUserAndItsManagedProfilesLocked(userId, /* shouldIncludeProfileAdmins */ (user) -> false); @@ -4540,16 +4538,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { admins = getActiveAdminsForLockscreenPoliciesLocked(userId); } for (ActiveAdmin admin : admins) { - final boolean isAdminOfUser = userId == admin.getUserHandle().getIdentifier(); - // Use the password metrics from the admin in one of three cases: - // (1) The admin is of the user we're getting the minimum metrics for. The admin - // always affects the user it's managing. This applies also to the parent - // ActiveAdmin instance: It'd have the same user handle. - // (2) The mPasswordPolicyAppliesToParent field is true: That indicates the - // call to setPasswordQuality was made by an admin that may affect the parent. - if (isAdminOfUser || admin.mPasswordPolicyAppliesToParent) { - adminMetrics.add(admin.mPasswordPolicy.getMinMetrics()); - } + adminMetrics.add(admin.mPasswordPolicy.getMinMetrics()); } } return PasswordMetrics.merge(adminMetrics); @@ -4764,7 +4753,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { admin.mPasswordComplexity = passwordComplexity; // Reset the password policy. admin.mPasswordPolicy = new PasswordPolicy(); - admin.mPasswordPolicyAppliesToParent = true; updatePasswordValidityCheckpointLocked(caller.getUserId(), calledOnParent); updatePasswordQualityCacheForUserGroup(caller.getUserId()); saveSettingsLocked(caller.getUserId()); 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 654d9fcf9210e..234270206ffad 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -5418,6 +5418,205 @@ public class DevicePolicyManagerTest extends DpmTestBase { } } + @Test + public void isActivePasswordSufficient_SeparateWorkChallenge_ProfileQualityRequirementMet() + throws Exception { + // Create work profile with empty separate challenge + final int managedProfileUserId = 15; + final int managedProfileAdminUid = UserHandle.getUid(managedProfileUserId, 19436); + addManagedProfileForPasswordTests(managedProfileUserId, managedProfileAdminUid, + /* separateChallenge */ true); + + // Set profile password quality requirement. No password added yet so + // profile.isActivePasswordSufficient should return false + mContext.binder.callingUid = managedProfileAdminUid; + dpm.setPasswordQuality(admin1, DevicePolicyManager.PASSWORD_QUALITY_ALPHABETIC); + assertThat(dpm.isActivePasswordSufficient()).isFalse(); + assertThat(parentDpm.isActivePasswordSufficient()).isTrue(); + + // Set a work challenge and verify profile.isActivePasswordSufficient is now true + when(getServices().lockSettingsInternal.getUserPasswordMetrics(managedProfileUserId)) + .thenReturn(computeForPasswordOrPin("abcdXYZ5".getBytes(), /* isPin */ false)); + assertThat(dpm.isActivePasswordSufficient()).isTrue(); + assertThat(parentDpm.isActivePasswordSufficient()).isTrue(); + } + + @Test + public void isActivePasswordSufficient_SeparateWorkChallenge_ProfileComplexityRequirementMet() + throws Exception { + // Create work profile with empty separate challenge + final int managedProfileUserId = 15; + final int managedProfileAdminUid = UserHandle.getUid(managedProfileUserId, 19436); + addManagedProfileForPasswordTests(managedProfileUserId, managedProfileAdminUid, + /* separateChallenge */ true); + + // Set profile password complexity requirement. No password added yet so + // profile.isActivePasswordSufficient should return false + mContext.binder.callingUid = managedProfileAdminUid; + dpm.setRequiredPasswordComplexity(PASSWORD_COMPLEXITY_MEDIUM); + assertThat(dpm.isActivePasswordSufficient()).isFalse(); + assertThat(parentDpm.isActivePasswordSufficient()).isTrue(); + + // Set a work challenge and verify profile.isActivePasswordSufficient is now true + when(getServices().lockSettingsInternal.getUserPasswordMetrics(managedProfileUserId)) + .thenReturn(computeForPasswordOrPin("5156".getBytes(), /* isPin */ true)); + assertThat(dpm.isActivePasswordSufficient()).isTrue(); + assertThat(parentDpm.isActivePasswordSufficient()).isTrue(); + } + + @Test + public void isActivePasswordSufficient_SeparateWorkChallenge_ParentQualityRequirementMet() + throws Exception { + // Create work profile with empty separate challenge + final int managedProfileUserId = 15; + final int managedProfileAdminUid = UserHandle.getUid(managedProfileUserId, 19436); + addManagedProfileForPasswordTests(managedProfileUserId, managedProfileAdminUid, + /* separateChallenge */ true); + + // Set parent password quality requirement. No password added yet so + // parent.isActivePasswordSufficient should return false + mContext.binder.callingUid = managedProfileAdminUid; + parentDpm.setPasswordQuality(admin1, DevicePolicyManager.PASSWORD_QUALITY_NUMERIC_COMPLEX); + assertThat(dpm.isActivePasswordSufficient()).isTrue(); + assertThat(parentDpm.isActivePasswordSufficient()).isFalse(); + + // Set a device lockscreen and verify parent.isActivePasswordSufficient is now true + when(getServices().lockSettingsInternal.getUserPasswordMetrics(UserHandle.USER_SYSTEM)) + .thenReturn(computeForPasswordOrPin("acbdXYZ5".getBytes(), /* isPin */ false)); + assertThat(dpm.isActivePasswordSufficient()).isTrue(); + assertThat(parentDpm.isActivePasswordSufficient()).isTrue(); + } + + @Test + public void isActivePasswordSufficient_SeparateWorkChallenge_ParentComplexityRequirementMet() + throws Exception { + // Create work profile with empty separate challenge + final int managedProfileUserId = 15; + final int managedProfileAdminUid = UserHandle.getUid(managedProfileUserId, 19436); + addManagedProfileForPasswordTests(managedProfileUserId, managedProfileAdminUid, + /* separateChallenge */ true); + + // Set parent password complexity requirement. No password added yet so + // parent.isActivePasswordSufficient should return false + mContext.binder.callingUid = managedProfileAdminUid; + parentDpm.setRequiredPasswordComplexity(PASSWORD_COMPLEXITY_LOW); + assertThat(dpm.isActivePasswordSufficient()).isTrue(); + assertThat(parentDpm.isActivePasswordSufficient()).isFalse(); + + // Set a device lockscreen and verify parent.isActivePasswordSufficient is now true + when(getServices().lockSettingsInternal.getUserPasswordMetrics(UserHandle.USER_SYSTEM)) + .thenReturn(computeForPasswordOrPin("1234".getBytes(), /* isPin */ true)); + assertThat(dpm.isActivePasswordSufficient()).isTrue(); + assertThat(parentDpm.isActivePasswordSufficient()).isTrue(); + } + + @Test + public void isActivePasswordSufficient_UnifiedWorkChallenge_ProfileQualityRequirementMet() + throws Exception { + // Create work profile with unified challenge + final int managedProfileUserId = 15; + final int managedProfileAdminUid = UserHandle.getUid(managedProfileUserId, 19436); + addManagedProfileForPasswordTests(managedProfileUserId, managedProfileAdminUid, + /* separateChallenge */ false); + + // Set profile password quality requirement. No password added yet so + // {profile, parent}.isActivePasswordSufficient should return false + mContext.binder.callingUid = managedProfileAdminUid; + dpm.setPasswordQuality(admin1, DevicePolicyManager.PASSWORD_QUALITY_ALPHABETIC); + assertThat(dpm.isActivePasswordSufficient()).isFalse(); + assertThat(parentDpm.isActivePasswordSufficient()).isFalse(); + + // Set a device lockscreen and verify {profile, parent}.isActivePasswordSufficient is true + when(getServices().lockSettingsInternal.getUserPasswordMetrics(UserHandle.USER_SYSTEM)) + .thenReturn(computeForPasswordOrPin("abcdXYZ5".getBytes(), /* isPin */ false)); + assertThat(dpm.isActivePasswordSufficient()).isTrue(); + assertThat(parentDpm.isActivePasswordSufficient()).isTrue(); + } + + @Test + public void isActivePasswordSufficient_UnifiedWorkChallenge_ProfileComplexityRequirementMet() + throws Exception { + // Create work profile with unified challenge + final int managedProfileUserId = 15; + final int managedProfileAdminUid = UserHandle.getUid(managedProfileUserId, 19436); + addManagedProfileForPasswordTests(managedProfileUserId, managedProfileAdminUid, + /* separateChallenge */ false); + + // Set profile password complexity requirement. No password added yet so + // {profile, parent}.isActivePasswordSufficient should return false + mContext.binder.callingUid = managedProfileAdminUid; + dpm.setRequiredPasswordComplexity(PASSWORD_COMPLEXITY_HIGH); + assertThat(dpm.isActivePasswordSufficient()).isFalse(); + assertThat(parentDpm.isActivePasswordSufficient()).isFalse(); + + // Set a device lockscreen and verify {profile, parent}.isActivePasswordSufficient is true + when(getServices().lockSettingsInternal.getUserPasswordMetrics(UserHandle.USER_SYSTEM)) + .thenReturn(computeForPasswordOrPin("51567548".getBytes(), /* isPin */ true)); + assertThat(dpm.isActivePasswordSufficient()).isTrue(); + assertThat(parentDpm.isActivePasswordSufficient()).isTrue(); + } + + @Test + public void isActivePasswordSufficient_UnifiedWorkChallenge_ParentQualityRequirementMet() + throws Exception { + // Create work profile with unified challenge + final int managedProfileUserId = 15; + final int managedProfileAdminUid = UserHandle.getUid(managedProfileUserId, 19436); + addManagedProfileForPasswordTests(managedProfileUserId, managedProfileAdminUid, + /* separateChallenge */ false); + + // Set parent password quality requirement. No password added yet so + // {profile, parent}.isActivePasswordSufficient should return false + mContext.binder.callingUid = managedProfileAdminUid; + parentDpm.setPasswordQuality(admin1, DevicePolicyManager.PASSWORD_QUALITY_ALPHABETIC); + assertThat(dpm.isActivePasswordSufficient()).isFalse(); + assertThat(parentDpm.isActivePasswordSufficient()).isFalse(); + + // Set a device lockscreen and verify {profile, parent}.isActivePasswordSufficient is true + when(getServices().lockSettingsInternal.getUserPasswordMetrics(UserHandle.USER_SYSTEM)) + .thenReturn(computeForPasswordOrPin("abcdXYZ5".getBytes(), /* isPin */ false)); + assertThat(dpm.isActivePasswordSufficient()).isTrue(); + assertThat(parentDpm.isActivePasswordSufficient()).isTrue(); + } + + @Test + public void isActivePasswordSufficient_UnifiedWorkChallenge_ParentComplexityRequirementMet() + throws Exception { + // Create work profile with unified challenge + final int managedProfileUserId = 15; + final int managedProfileAdminUid = UserHandle.getUid(managedProfileUserId, 19436); + addManagedProfileForPasswordTests(managedProfileUserId, managedProfileAdminUid, + /* separateChallenge */ false); + + // Set parent password complexity requirement. No password added yet so + // {profile, parent}.isActivePasswordSufficient should return false + mContext.binder.callingUid = managedProfileAdminUid; + parentDpm.setRequiredPasswordComplexity(PASSWORD_COMPLEXITY_MEDIUM); + assertThat(dpm.isActivePasswordSufficient()).isFalse(); + assertThat(parentDpm.isActivePasswordSufficient()).isFalse(); + + // Set a device lockscreen and verify {profile, parent}.isActivePasswordSufficient is true + when(getServices().lockSettingsInternal.getUserPasswordMetrics(UserHandle.USER_SYSTEM)) + .thenReturn(computeForPasswordOrPin("5156".getBytes(), /* isPin */ true)); + assertThat(dpm.isActivePasswordSufficient()).isTrue(); + assertThat(parentDpm.isActivePasswordSufficient()).isTrue(); + } + + private void addManagedProfileForPasswordTests(int userId, int adminUid, + boolean separateChallenge) throws Exception { + addManagedProfile(admin1, adminUid, admin1); + when(getServices().userManager.getProfileParent(userId)) + .thenReturn(new UserInfo(UserHandle.USER_SYSTEM, "user system", 0)); + doReturn(separateChallenge).when(getServices().lockPatternUtils) + .isSeparateProfileChallengeEnabled(userId); + when(getServices().userManager.getCredentialOwnerProfile(userId)) + .thenReturn(separateChallenge ? userId : UserHandle.USER_SYSTEM); + when(getServices().lockSettingsInternal.getUserPasswordMetrics(userId)) + .thenReturn(new PasswordMetrics(CREDENTIAL_TYPE_NONE)); + when(getServices().lockSettingsInternal.getUserPasswordMetrics(UserHandle.USER_SYSTEM)) + .thenReturn(new PasswordMetrics(CREDENTIAL_TYPE_NONE)); + } + @Test public void testPasswordQualityAppliesToParentPreS() throws Exception { final int managedProfileUserId = CALLER_USER_HANDLE;