DPM: Fix isActivePasswordSufficient for non-FBE devices
For non-FBE devices (devices will full disk encryption or no encryption
at all) the password metrics are not kept in plaintext anymore, so it
is not possible to evaluate the sufficiency of the password when new
password quality requirements are set. Instead, the sufficiency value is
stored in a checkpoint that gets loaded after boot and used before the
user has entered any credentials.
However this checkpoint value was not evaluated correctly (there was a
circular dependency between setting it and reading it) and was not
stored on the right DevicePolicyData object.
Fix the checkpoint value to be the correct one and stored on the right
object, by doing the following:
* Removing the short-circuit introduced in ag/2866930.
* Change the default checkpoint value to true, to prevent re-introducing
b/63887564, and because a newly-created profile, before any password
requirements are set, has a sufficient password.
* Get rid of the circular dependency between setting and reading
mPasswordValidAtLastCheckpoint, by extracting from
isActivePasswordSufficientForUserLocked a function that does not look at
the checkpoint value.
* Store the checkpoint value in the DevicePolicyData object of the
credential owner, as this is where isActivePasswordSufficient reads it.
More details can be found in https://docs.google.com/document/d/15CRewlsy1lyonjAWg3VmhLC5jAyVKazUMKlcPnnId58/edit#
Bug: 71697938,73347414
Test: cts-tradefed run commandAndExit cts-dev -s 127.0.0.1:56619 -a x86_64 -m CtsDevicePolicyManagerTestCases -t com.android.cts.devicepolicy.DeviceAdminHostSideTestApi23#testResetPassword_nycRestrictions -l DEBUG on a gce instance.
Change-Id: Ide5b2e53cf100b087822844ca51b3bc69e7ddf82
diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
index ab8a6c4..0194635 100644
--- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
+++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java
@@ -563,7 +563,7 @@
@NonNull PasswordMetrics mActivePasswordMetrics = new PasswordMetrics();
int mFailedPasswordAttempts = 0;
boolean mPasswordStateHasBeenSetSinceBoot = false;
- boolean mPasswordValidAtLastCheckpoint = false;
+ boolean mPasswordValidAtLastCheckpoint = true;
int mUserHandle;
int mPasswordOwner = -1;
@@ -3883,23 +3883,28 @@
final PasswordMetrics metrics = ap.minimumPasswordMetrics;
if (metrics.quality != quality) {
metrics.quality = quality;
- updatePasswordValidityCheckpointLocked(userId);
- saveSettingsLocked(userId);
+ updatePasswordValidityCheckpointLocked(userId, parent);
}
maybeLogPasswordComplexitySet(who, userId, parent, metrics);
}
}
/**
- * Updates flag in memory that tells us whether the user's password currently satisfies the
- * requirements set by all of the user's active admins. This should be called before
- * {@link #saveSettingsLocked} whenever the password or the admin policies have changed.
+ * Updates a flag that tells us whether the user's password currently satisfies the
+ * requirements set by all of the user's active admins. The flag is updated both in memory
+ * and persisted to disk by calling {@link #saveSettingsLocked}, for the value of the flag
+ * be the correct one upon boot.
+ * This should be called whenever the password or the admin policies have changed.
*/
@GuardedBy("DevicePolicyManagerService.this")
- private void updatePasswordValidityCheckpointLocked(int userHandle) {
- DevicePolicyData policy = getUserData(userHandle);
- policy.mPasswordValidAtLastCheckpoint = isActivePasswordSufficientForUserLocked(
- policy, policy.mUserHandle, false);
+ private void updatePasswordValidityCheckpointLocked(int userHandle, boolean parent) {
+ final int credentialOwner = getCredentialOwner(userHandle, parent);
+ DevicePolicyData policy = getUserData(credentialOwner);
+ policy.mPasswordValidAtLastCheckpoint =
+ isPasswordSufficientForUserWithoutCheckpointLocked(
+ policy.mActivePasswordMetrics, userHandle, parent);
+
+ saveSettingsLocked(credentialOwner);
}
@Override
@@ -3986,8 +3991,7 @@
final PasswordMetrics metrics = ap.minimumPasswordMetrics;
if (metrics.length != length) {
metrics.length = length;
- updatePasswordValidityCheckpointLocked(userId);
- saveSettingsLocked(userId);
+ updatePasswordValidityCheckpointLocked(userId, parent);
}
maybeLogPasswordComplexitySet(who, userId, parent, metrics);
}
@@ -4011,8 +4015,7 @@
who, DeviceAdminInfo.USES_POLICY_LIMIT_PASSWORD, parent);
if (ap.passwordHistoryLength != length) {
ap.passwordHistoryLength = length;
- updatePasswordValidityCheckpointLocked(userId);
- saveSettingsLocked(userId);
+ updatePasswordValidityCheckpointLocked(userId, parent);
}
}
if (SecurityLog.isLoggingEnabled()) {
@@ -4213,8 +4216,7 @@
final PasswordMetrics metrics = ap.minimumPasswordMetrics;
if (metrics.upperCase != length) {
metrics.upperCase = length;
- updatePasswordValidityCheckpointLocked(userId);
- saveSettingsLocked(userId);
+ updatePasswordValidityCheckpointLocked(userId, parent);
}
maybeLogPasswordComplexitySet(who, userId, parent, metrics);
}
@@ -4236,8 +4238,7 @@
final PasswordMetrics metrics = ap.minimumPasswordMetrics;
if (metrics.lowerCase != length) {
metrics.lowerCase = length;
- updatePasswordValidityCheckpointLocked(userId);
- saveSettingsLocked(userId);
+ updatePasswordValidityCheckpointLocked(userId, parent);
}
maybeLogPasswordComplexitySet(who, userId, parent, metrics);
}
@@ -4262,8 +4263,7 @@
final PasswordMetrics metrics = ap.minimumPasswordMetrics;
if (metrics.letters != length) {
metrics.letters = length;
- updatePasswordValidityCheckpointLocked(userId);
- saveSettingsLocked(userId);
+ updatePasswordValidityCheckpointLocked(userId, parent);
}
maybeLogPasswordComplexitySet(who, userId, parent, metrics);
}
@@ -4288,8 +4288,7 @@
final PasswordMetrics metrics = ap.minimumPasswordMetrics;
if (metrics.numeric != length) {
metrics.numeric = length;
- updatePasswordValidityCheckpointLocked(userId);
- saveSettingsLocked(userId);
+ updatePasswordValidityCheckpointLocked(userId, parent);
}
maybeLogPasswordComplexitySet(who, userId, parent, metrics);
}
@@ -4314,8 +4313,7 @@
final PasswordMetrics metrics = ap.minimumPasswordMetrics;
if (metrics.symbols != length) {
ap.minimumPasswordMetrics.symbols = length;
- updatePasswordValidityCheckpointLocked(userId);
- saveSettingsLocked(userId);
+ updatePasswordValidityCheckpointLocked(userId, parent);
}
maybeLogPasswordComplexitySet(who, userId, parent, metrics);
}
@@ -4340,8 +4338,7 @@
final PasswordMetrics metrics = ap.minimumPasswordMetrics;
if (metrics.nonLetter != length) {
ap.minimumPasswordMetrics.nonLetter = length;
- updatePasswordValidityCheckpointLocked(userId);
- saveSettingsLocked(userId);
+ updatePasswordValidityCheckpointLocked(userId, parent);
}
maybeLogPasswordComplexitySet(who, userId, parent, metrics);
}
@@ -4562,16 +4559,6 @@
private boolean isActivePasswordSufficientForUserLocked(
DevicePolicyData policy, int userHandle, boolean parent) {
- final int requiredPasswordQuality = getPasswordQuality(null, userHandle, parent);
- if (requiredPasswordQuality == PASSWORD_QUALITY_UNSPECIFIED) {
- // A special case is when there is no required password quality, then we just return
- // true since any password would be sufficient. This is for the scenario when a work
- // profile is first created so there is no information about the current password but
- // it should be considered sufficient as there is no password requirement either.
- // This is useful since it short-circuits the password checkpoint for FDE device below.
- return true;
- }
-
if (!mInjector.storageManagerIsFileBasedEncryptionEnabled()
&& !policy.mPasswordStateHasBeenSetSinceBoot) {
// Before user enters their password for the first time after a reboot, return the
@@ -4582,28 +4569,41 @@
return policy.mPasswordValidAtLastCheckpoint;
}
- if (policy.mActivePasswordMetrics.quality < requiredPasswordQuality) {
+ return isPasswordSufficientForUserWithoutCheckpointLocked(
+ policy.mActivePasswordMetrics, userHandle, parent);
+ }
+
+ /**
+ * Returns {@code true} if the password represented by the {@code passwordMetrics} argument
+ * sufficiently fulfills the password requirements for the user corresponding to
+ * {@code userHandle} (or its parent, if {@code parent} is set to {@code true}).
+ */
+ private boolean isPasswordSufficientForUserWithoutCheckpointLocked(
+ PasswordMetrics passwordMetrics, int userHandle, boolean parent) {
+ final int requiredPasswordQuality = getPasswordQuality(null, userHandle, parent);
+
+ if (passwordMetrics.quality < requiredPasswordQuality) {
return false;
}
if (requiredPasswordQuality >= DevicePolicyManager.PASSWORD_QUALITY_NUMERIC
- && policy.mActivePasswordMetrics.length < getPasswordMinimumLength(
+ && passwordMetrics.length < getPasswordMinimumLength(
null, userHandle, parent)) {
return false;
}
if (requiredPasswordQuality != DevicePolicyManager.PASSWORD_QUALITY_COMPLEX) {
return true;
}
- return policy.mActivePasswordMetrics.upperCase >= getPasswordMinimumUpperCase(
+ return passwordMetrics.upperCase >= getPasswordMinimumUpperCase(
null, userHandle, parent)
- && policy.mActivePasswordMetrics.lowerCase >= getPasswordMinimumLowerCase(
+ && passwordMetrics.lowerCase >= getPasswordMinimumLowerCase(
null, userHandle, parent)
- && policy.mActivePasswordMetrics.letters >= getPasswordMinimumLetters(
+ && passwordMetrics.letters >= getPasswordMinimumLetters(
null, userHandle, parent)
- && policy.mActivePasswordMetrics.numeric >= getPasswordMinimumNumeric(
+ && passwordMetrics.numeric >= getPasswordMinimumNumeric(
null, userHandle, parent)
- && policy.mActivePasswordMetrics.symbols >= getPasswordMinimumSymbols(
+ && passwordMetrics.symbols >= getPasswordMinimumSymbols(
null, userHandle, parent)
- && policy.mActivePasswordMetrics.nonLetter >= getPasswordMinimumNonLetter(
+ && passwordMetrics.nonLetter >= getPasswordMinimumNonLetter(
null, userHandle, parent);
}
@@ -6144,8 +6144,7 @@
try {
synchronized (this) {
policy.mFailedPasswordAttempts = 0;
- updatePasswordValidityCheckpointLocked(userId);
- saveSettingsLocked(userId);
+ updatePasswordValidityCheckpointLocked(userId, /* parent */ false);
updatePasswordExpirationsLocked(userId);
setExpirationAlarmCheckLocked(mContext, userId, /* parent */ false);