diff options
author | 2020-04-02 16:37:32 +0100 | |
---|---|---|
committer | 2020-04-08 14:43:26 +0100 | |
commit | de633f32ea98997c5b425297eed360de7d410be1 (patch) | |
tree | 1a8cb67c6e457b48a87a764f2e52d2cc6f02e555 | |
parent | c6eea102c789ab321ad0e8327a0bf044e7de4aef (diff) |
Improve work profile unification flow
Expose internal API to check if the user's password
will be sufficient after profile unification. Also
expose some other helper methods and refactor
DevicePolicyManagerService to unify a few similar
methods that gather admins from user and its profiles.
Bug: 148630506
Fix: 149682344
Test: atest com.android.server.locksettings
Test: atest FrameworksServicesTests:DevicePolicyManagerTest
Change-Id: Ic647c14d5bab7e7337185bc40b1368e42c65f738
8 files changed, 167 insertions, 32 deletions
diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index 10309a9b4a03..dc816ad324c2 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -3664,6 +3664,28 @@ public class DevicePolicyManager { } /** + * Returns whether the given user's credential will be sufficient for all password policy + * requirement, once the user's profile has switched to unified challenge. + * + * <p>This is different from {@link #isActivePasswordSufficient()} since once the profile + * switches to unified challenge, policies set explicitly on the profile will start to affect + * the parent user. + * @param userHandle the user whose password requirement will be checked + * @param profileUser the profile user whose lockscreen challenge will be unified. + * @hide + */ + public boolean isPasswordSufficientAfterProfileUnification(int userHandle, int profileUser) { + if (mService != null) { + try { + return mService.isPasswordSufficientAfterProfileUnification(userHandle, + profileUser); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + return false; + } + /** * Retrieve the number of times the user has failed at entering a password since that last * successful password entry. * <p> diff --git a/core/java/android/app/admin/IDevicePolicyManager.aidl b/core/java/android/app/admin/IDevicePolicyManager.aidl index fc1eb0a7b9c1..036c3d4f1294 100644 --- a/core/java/android/app/admin/IDevicePolicyManager.aidl +++ b/core/java/android/app/admin/IDevicePolicyManager.aidl @@ -85,6 +85,7 @@ interface IDevicePolicyManager { boolean isActivePasswordSufficient(int userHandle, boolean parent); boolean isProfileActivePasswordSufficientForParent(int userHandle); + boolean isPasswordSufficientAfterProfileUnification(int userHandle, int profileUser); int getPasswordComplexity(boolean parent); boolean isUsingUnifiedPassword(in ComponentName admin); int getCurrentFailedPasswordAttempts(int userHandle, boolean parent); diff --git a/core/java/android/app/admin/PasswordMetrics.java b/core/java/android/app/admin/PasswordMetrics.java index 86ebb47400c7..39e1f0dc2d2c 100644 --- a/core/java/android/app/admin/PasswordMetrics.java +++ b/core/java/android/app/admin/PasswordMetrics.java @@ -350,7 +350,7 @@ public final class PasswordMetrics implements Parcelable { * * TODO: move to PasswordPolicy */ - private void maxWith(PasswordMetrics other) { + public void maxWith(PasswordMetrics other) { credType = Math.max(credType, other.credType); if (credType != CREDENTIAL_TYPE_PASSWORD) { return; diff --git a/packages/SettingsLib/RestrictedLockUtils/src/com/android/settingslib/RestrictedLockUtils.java b/packages/SettingsLib/RestrictedLockUtils/src/com/android/settingslib/RestrictedLockUtils.java index fa2ec55bd81a..a77e34b4af1e 100644 --- a/packages/SettingsLib/RestrictedLockUtils/src/com/android/settingslib/RestrictedLockUtils.java +++ b/packages/SettingsLib/RestrictedLockUtils/src/com/android/settingslib/RestrictedLockUtils.java @@ -147,6 +147,28 @@ public class RestrictedLockUtils { public EnforcedAdmin() { } + /** + * Combines two {@link EnforcedAdmin} into one: if one of them is null, then just return + * the other. If both of them are the same, then return that. Otherwise return the symbolic + * {@link #MULTIPLE_ENFORCED_ADMIN} + */ + public static EnforcedAdmin combine(EnforcedAdmin admin1, EnforcedAdmin admin2) { + if (admin1 == null) { + return admin2; + } + if (admin2 == null) { + return admin1; + } + if (admin1.equals(admin2)) { + return admin1; + } + if (!admin1.enforcedRestriction.equals(admin2.enforcedRestriction)) { + throw new IllegalArgumentException( + "Admins with different restriction cannot be combined"); + } + return MULTIPLE_ENFORCED_ADMIN; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 9297a43b04aa..7972f247b46d 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -366,10 +366,15 @@ public class LockSettingsService extends ILockSettings.Stub { if (mStorage.hasChildProfileLock(managedUserId)) { return; } - // Do not tie it to parent when parent does not have a screen lock + // If parent does not have a screen lock, simply clear credential from the managed profile, + // to maintain the invariant that unified profile should always have the same secure state + // as its parent. final int parentId = mUserManager.getProfileParent(managedUserId).id; - if (!isUserSecure(parentId)) { - if (DEBUG) Slog.v(TAG, "Parent does not have a screen lock"); + if (!isUserSecure(parentId) && !managedUserPassword.isNone()) { + if (DEBUG) Slog.v(TAG, "Parent does not have a screen lock but profile has one"); + + setLockCredentialInternal(LockscreenCredential.createNone(), managedUserPassword, + managedUserId, /* isLockTiedToParent= */ true); return; } // Do not tie when the parent has no SID (but does have a screen lock). @@ -3161,6 +3166,21 @@ public class LockSettingsService extends ILockSettings.Stub { return new SimpleDateFormat("yyyy-MM-dd HH:mm:ss").format(new Date(timestamp)); } + private static String credentialTypeToString(int credentialType) { + switch (credentialType) { + case CREDENTIAL_TYPE_NONE: + return "None"; + case CREDENTIAL_TYPE_PATTERN: + return "Pattern"; + case CREDENTIAL_TYPE_PIN: + return "Pin"; + case CREDENTIAL_TYPE_PASSWORD: + return "Password"; + default: + return "Unknown " + credentialType; + } + } + @Override protected void dump(FileDescriptor fd, PrintWriter printWriter, String[] args) { if (!DumpUtils.checkDumpPermission(mContext, TAG, printWriter)) return; @@ -3192,7 +3212,8 @@ public class LockSettingsService extends ILockSettings.Stub { // It's OK to dump the password type since anyone with physical access can just // observe it from the keyguard directly. pw.println("Quality: " + getKeyguardStoredQuality(userId)); - pw.println("CredentialType: " + getCredentialTypeInternal(userId)); + pw.println("CredentialType: " + credentialTypeToString( + getCredentialTypeInternal(userId))); pw.println("SeparateChallenge: " + getSeparateProfileChallengeEnabledInternal(userId)); pw.println(String.format("Metrics: %s", getUserPasswordMetrics(userId) != null ? "known" : "unknown")); diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 1544ff127121..e3d40d5e5cfd 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -322,6 +322,7 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; +import java.util.function.Predicate; /** * Implementation of the device policy APIs. @@ -4689,33 +4690,13 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (!parent && isSeparateProfileChallengeEnabled(userHandle)) { // If this user has a separate challenge, only return its restrictions. return getUserDataUnchecked(userHandle).mAdminList; - } else { - // Return all admins for this user and the profiles that are visible from this - // user that do not use a separate work challenge. - ArrayList<ActiveAdmin> admins = new ArrayList<ActiveAdmin>(); - for (UserInfo userInfo : mUserManager.getProfiles(userHandle)) { - DevicePolicyData policy = getUserData(userInfo.id); - if (!userInfo.isManagedProfile()) { - admins.addAll(policy.mAdminList); - } else { - // For managed profiles, we always include the policies set on the parent - // profile. Additionally, we include the ones set on the managed profile - // if no separate challenge is in place. - boolean hasSeparateChallenge = isSeparateProfileChallengeEnabled(userInfo.id); - final int N = policy.mAdminList.size(); - for (int i = 0; i < N; i++) { - ActiveAdmin admin = policy.mAdminList.get(i); - if (admin.hasParentActiveAdmin()) { - admins.add(admin.getParentActiveAdmin()); - } - if (!hasSeparateChallenge) { - admins.add(admin); - } - } - } - } - return admins; } + // Either parent == true, or isSeparateProfileChallengeEnabled == false + // If parent is true, query the parent user of userHandle by definition, + // If isSeparateProfileChallengeEnabled is false, userHandle points to a managed profile + // with unified challenge so also need to query the parent user who owns the credential. + return getActiveAdminsForUserAndItsManagedProfilesLocked(getProfileParentId(userHandle), + (user) -> !mLockPatternUtils.isSeparateProfileChallengeEnabled(user.id)); } /** @@ -4733,6 +4714,19 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (isManagedProfile(userHandle)) { return getUserDataUnchecked(userHandle).mAdminList; } + return getActiveAdminsForUserAndItsManagedProfilesLocked(userHandle, + /* shouldIncludeProfileAdmins */ (user) -> false); + } + + /** + * Returns the list of admins on the given user, as well as parent admins for each managed + * profile associated with the given user. Optionally also include the admin of each managed + * profile. + * <p> Should not be called on a profile user. + */ + @GuardedBy("getLockObject()") + private List<ActiveAdmin> getActiveAdminsForUserAndItsManagedProfilesLocked(int userHandle, + Predicate<UserInfo> shouldIncludeProfileAdmins) { ArrayList<ActiveAdmin> admins = new ArrayList<>(); mInjector.binderWithCleanCallingIdentity(() -> { for (UserInfo userInfo : mUserManager.getProfiles(userHandle)) { @@ -4740,12 +4734,14 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { if (userInfo.id == userHandle) { admins.addAll(policy.mAdminList); } else if (userInfo.isManagedProfile()) { - // For managed profiles, policies set on the parent profile will be included for (int i = 0; i < policy.mAdminList.size(); i++) { ActiveAdmin admin = policy.mAdminList.get(i); if (admin.hasParentActiveAdmin()) { admins.add(admin.getParentActiveAdmin()); } + if (shouldIncludeProfileAdmins.test(userInfo)) { + admins.add(admin); + } } } else { Slog.w(LOG_TAG, "Unknown user type: " + userInfo); @@ -5322,6 +5318,32 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } } + @Override + public boolean isPasswordSufficientAfterProfileUnification(int userHandle, int profileUser) { + if (!mHasFeature) { + return true; + } + enforceFullCrossUsersPermission(userHandle); + enforceNotManagedProfile(userHandle, "check password sufficiency"); + enforceUserUnlocked(userHandle); + + synchronized (getLockObject()) { + PasswordMetrics metrics = mLockSettingsInternal.getUserPasswordMetrics(userHandle); + + // Combine password policies across the user and its profiles. Profile admins are + // included if the profile is to be unified or currently has unified challenge + List<ActiveAdmin> admins = getActiveAdminsForUserAndItsManagedProfilesLocked(userHandle, + /* shouldIncludeProfileAdmins */ (user) -> user.id == profileUser + || !mLockPatternUtils.isSeparateProfileChallengeEnabled(user.id)); + ArrayList<PasswordMetrics> adminMetrics = new ArrayList<>(admins.size()); + for (ActiveAdmin admin : admins) { + adminMetrics.add(admin.mPasswordPolicy.getMinMetrics()); + } + return PasswordMetrics.validatePasswordMetrics(PasswordMetrics.merge(adminMetrics), + PASSWORD_COMPLEXITY_NONE, false, metrics).isEmpty(); + } + } + private boolean isActivePasswordSufficientForUserLocked( boolean passwordValidAtLastCheckpoint, @Nullable PasswordMetrics metrics, int userHandle, boolean parent) { 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 baf551e756e8..c8bd038e983d 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -4797,6 +4797,33 @@ public class DevicePolicyManagerTest extends DpmTestBase { assertFalse(dpm.isActivePasswordSufficient()); } + public void testIsPasswordSufficientAfterProfileUnification() throws Exception { + final int managedProfileUserId = DpmMockContext.CALLER_USER_HANDLE; + final int managedProfileAdminUid = + UserHandle.getUid(managedProfileUserId, DpmMockContext.SYSTEM_UID); + mContext.binder.callingUid = managedProfileAdminUid; + + addManagedProfile(admin1, managedProfileAdminUid, admin1); + doReturn(true).when(getServices().lockPatternUtils) + .isSeparateProfileChallengeEnabled(managedProfileUserId); + + dpm.setPasswordQuality(admin1, DevicePolicyManager.PASSWORD_QUALITY_ALPHABETIC); + parentDpm.setPasswordQuality(admin1, DevicePolicyManager.PASSWORD_QUALITY_NUMERIC); + + when(getServices().lockSettingsInternal.getUserPasswordMetrics(UserHandle.USER_SYSTEM)) + .thenReturn(computeForPassword("1234".getBytes())); + + // Numeric password is compliant with current requirement (QUALITY_NUMERIC set explicitly + // on the parent admin) + assertTrue(dpm.isPasswordSufficientAfterProfileUnification(UserHandle.USER_SYSTEM, + UserHandle.USER_NULL)); + // Numeric password is not compliant if profile is to be unified: the profile has a + // QUALITY_ALPHABETIC policy on itself which will be enforced on the password after + // unification. + assertFalse(dpm.isPasswordSufficientAfterProfileUnification(UserHandle.USER_SYSTEM, + managedProfileUserId)); + } + private void setActivePasswordState(PasswordMetrics passwordMetrics) throws Exception { final int userHandle = UserHandle.getUserId(mContext.binder.callingUid); diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java index 07d7830c9b0f..12b144f2b778 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java @@ -209,6 +209,26 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { } @Test + public void testManagedProfileChallengeUnification_parentUserNoPassword() throws Exception { + // Start with a profile with unified challenge, parent user has not password + mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, false, null); + assertEquals(0, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); + assertEquals(CREDENTIAL_TYPE_NONE, mService.getCredentialType(MANAGED_PROFILE_USER_ID)); + + // Set a separate challenge on the profile + assertTrue(mService.setLockCredential( + newPassword("12345678"), nonePassword(), MANAGED_PROFILE_USER_ID)); + assertNotEquals(0, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); + assertEquals(CREDENTIAL_TYPE_PASSWORD, mService.getCredentialType(MANAGED_PROFILE_USER_ID)); + + // Now unify again, profile should become passwordless again + mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, false, + newPassword("12345678")); + assertEquals(0, mGateKeeperService.getSecureUserId(MANAGED_PROFILE_USER_ID)); + assertEquals(CREDENTIAL_TYPE_NONE, mService.getCredentialType(MANAGED_PROFILE_USER_ID)); + } + + @Test public void testSetLockCredential_forPrimaryUser_sendsCredentials() throws Exception { assertTrue(mService.setLockCredential( newPassword("password"), |