Merge changes from topic "lskf-chars-fix" into main

* changes:
  Properly validate credential in setLock(int, byte[], int, byte[])
  DPMS: allow getPasswordMinimumMetrics() to anyone who can set LSKF
  Allow constructing a PIN LockscreenCredential with mInvalidChars
This commit is contained in:
Eric Biggers
2023-07-06 21:05:46 +00:00
committed by Android (Google) Code Review
5 changed files with 47 additions and 17 deletions

View File

@@ -61,6 +61,7 @@ import com.android.internal.widget.IWeakEscrowTokenRemovedListener;
import com.android.internal.widget.LockPatternUtils;
import com.android.internal.widget.LockPatternView;
import com.android.internal.widget.LockscreenCredential;
import com.android.internal.widget.PasswordValidationError;
import com.android.internal.widget.VerifyCredentialResponse;
import java.nio.charset.Charset;
@@ -885,12 +886,8 @@ public class KeyguardManager {
}
Objects.requireNonNull(password, "Password cannot be null.");
complexity = PasswordMetrics.sanitizeComplexityLevel(complexity);
// TODO: b/131755827 add devicePolicyManager support for Auto
DevicePolicyManager devicePolicyManager =
(DevicePolicyManager) mContext.getSystemService(Context.DEVICE_POLICY_SERVICE);
PasswordMetrics adminMetrics =
devicePolicyManager.getPasswordMinimumMetrics(mContext.getUserId());
mLockPatternUtils.getRequestedPasswordMetrics(mContext.getUserId());
try (LockscreenCredential credential = createLockscreenCredential(lockType, password)) {
return PasswordMetrics.validateCredential(adminMetrics, complexity,
credential).size() == 0;
@@ -913,11 +910,8 @@ public class KeyguardManager {
return -1;
}
complexity = PasswordMetrics.sanitizeComplexityLevel(complexity);
// TODO: b/131755827 add devicePolicyManager support for Auto
DevicePolicyManager devicePolicyManager =
(DevicePolicyManager) mContext.getSystemService(Context.DEVICE_POLICY_SERVICE);
PasswordMetrics adminMetrics =
devicePolicyManager.getPasswordMinimumMetrics(mContext.getUserId());
mLockPatternUtils.getRequestedPasswordMetrics(mContext.getUserId());
PasswordMetrics minMetrics =
PasswordMetrics.applyComplexity(adminMetrics, isPin, complexity);
return minMetrics.length;
@@ -1139,6 +1133,14 @@ public class KeyguardManager {
currentLockType, currentPassword);
LockscreenCredential newCredential = createLockscreenCredential(
newLockType, newPassword);
PasswordMetrics adminMetrics =
mLockPatternUtils.getRequestedPasswordMetrics(mContext.getUserId());
List<PasswordValidationError> errors = PasswordMetrics.validateCredential(adminMetrics,
DevicePolicyManager.PASSWORD_COMPLEXITY_NONE, newCredential);
if (!errors.isEmpty()) {
Log.e(TAG, "New credential is not valid: " + errors.get(0));
return false;
}
return mLockPatternUtils.setLockCredential(newCredential, currentCredential, userId);
}

View File

@@ -64,9 +64,9 @@ public class LockscreenCredential implements Parcelable, AutoCloseable {
// is represented as a byte array of length 0.
private byte[] mCredential;
// This indicates that the credential is a password that used characters outside ASCII 32–127.
// This indicates that the credential used characters outside ASCII 32–127.
//
// Such passwords were never intended to be allowed. However, Android 10–14 had a bug where
// Such credentials were never intended to be allowed. However, Android 10–14 had a bug where
// conversion from the chars the user entered to the credential bytes used a simple truncation.
// Thus, any 'char' whose remainder mod 256 was in the range 32–127 was accepted and was
// equivalent to some ASCII character. For example, ™, which is U+2122, was truncated to ASCII
@@ -102,7 +102,6 @@ public class LockscreenCredential implements Parcelable, AutoCloseable {
// LockscreenCredential object to be constructed so that the validation logic can run,
// even though the validation logic will ultimately reject the credential as too short.
}
Preconditions.checkArgument(!hasInvalidChars || type == CREDENTIAL_TYPE_PASSWORD);
mType = type;
mCredential = credential;
mHasInvalidChars = hasInvalidChars;
@@ -262,6 +261,9 @@ public class LockscreenCredential implements Parcelable, AutoCloseable {
* short
*/
public void validateBasicRequirements() {
if (mHasInvalidChars) {
throw new IllegalArgumentException("credential contains invalid characters");
}
switch (getType()) {
case CREDENTIAL_TYPE_PATTERN:
if (size() < LockPatternUtils.MIN_LOCK_PATTERN_SIZE) {
@@ -276,9 +278,6 @@ public class LockscreenCredential implements Parcelable, AutoCloseable {
}
break;
case CREDENTIAL_TYPE_PASSWORD:
if (mHasInvalidChars) {
throw new IllegalArgumentException("password contains invalid characters");
}
if (size() < LockPatternUtils.MIN_LOCK_PASSWORD_SIZE) {
throw new IllegalArgumentException("password must be at least "
+ LockPatternUtils.MIN_LOCK_PASSWORD_SIZE + " characters long.");

View File

@@ -173,6 +173,22 @@ public class KeyguardManagerTest {
assertFalse(mKeyguardManager.isDeviceSecure());
}
@Test
public void setLock_validatesCredential() {
// setLock() should validate the credential before setting it. Test one example, which is
// that PINs must contain only ASCII digits 0-9, i.e. bytes 48-57. Using bytes 0-9 is
// incorrect and should *not* be accepted.
byte[] invalidPin = new byte[] { 1, 2, 3, 4 };
byte[] validPin = "1234".getBytes();
assertFalse(mKeyguardManager.setLock(KeyguardManager.PIN, invalidPin, -1, null));
assertFalse(mKeyguardManager.isDeviceSecure());
assertTrue(mKeyguardManager.setLock(KeyguardManager.PIN, validPin, -1, null));
assertTrue(mKeyguardManager.isDeviceSecure());
assertTrue(mKeyguardManager.setLock(-1, null, KeyguardManager.PIN, validPin));
}
@Test
public void checkLock_correctCredentials() {
// Set to `true` to behave as if SET_INITIAL_LOCK permission had been granted.

View File

@@ -157,6 +157,16 @@ public class LockscreenCredentialTest {
}
}
@Test
public void testPinWithInvalidChars() {
LockscreenCredential pin = LockscreenCredential.createPin("\n\n\n\n");
assertTrue(pin.hasInvalidChars());
try {
pin.validateBasicRequirements();
fail("should not be able to set PIN with invalid chars");
} catch (IllegalArgumentException expected) { }
}
@Test
public void testSanitize() {
LockscreenCredential password = LockscreenCredential.createPassword("password");

View File

@@ -5132,8 +5132,11 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub {
boolean deviceWideOnly) {
final CallerIdentity caller = getCallerIdentity();
Preconditions.checkCallAuthorization(hasFullCrossUsersPermission(caller, userHandle)
&& (isSystemUid(caller) || hasCallingOrSelfPermission(
permission.SET_INITIAL_LOCK)));
&& (isSystemUid(caller)
// Accept any permission that ILockSettings#setLockCredential() accepts.
|| hasCallingOrSelfPermission(permission.SET_INITIAL_LOCK)
|| hasCallingOrSelfPermission(permission.SET_AND_VERIFY_LOCKSCREEN_CREDENTIALS)
|| hasCallingOrSelfPermission(permission.ACCESS_KEYGUARD_SECURE_STORAGE)));
return getPasswordMinimumMetricsUnchecked(userHandle, deviceWideOnly);
}