From 0e4385e2148bdc6692fde303bc9bb8535e89151c Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Mon, 26 Jun 2023 22:38:26 +0000
Subject: [PATCH] Properly validate credential in setLock(int, byte[], int,
 byte[])

In KeyguardManager, one of the two overloads of setLock() properly
validates the new credential using the logic in PasswordMetrics, while
the other doesn't.  This appears to be an oversight, and it was allowing
a test to set a PIN containing invalid characters.  Fix this by always
calling PasswordMetrics#validateCredential() on the new credential.

Bug: 219511761
Bug: 232900169
Bug: 243881358
Test: atest android.app.KeyguardManagerTest
Test: atest android.app.cts.KeyguardManagerTest
Test: atest CtsSensorPrivacyTestCases # since it calls setLock()
Change-Id: I46d8bf920526a00d6e6d2145d06c8e39f8047ea8
---
 core/java/android/app/KeyguardManager.java    | 20 ++++++++++---------
 .../src/android/app/KeyguardManagerTest.java  | 16 +++++++++++++++
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/core/java/android/app/KeyguardManager.java b/core/java/android/app/KeyguardManager.java
index f467200fab85f..e447dc511ced1 100644
--- a/core/java/android/app/KeyguardManager.java
+++ b/core/java/android/app/KeyguardManager.java
@@ -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);
     }
 
diff --git a/core/tests/coretests/src/android/app/KeyguardManagerTest.java b/core/tests/coretests/src/android/app/KeyguardManagerTest.java
index 958906c36d05d..ed8b2887ac790 100644
--- a/core/tests/coretests/src/android/app/KeyguardManagerTest.java
+++ b/core/tests/coretests/src/android/app/KeyguardManagerTest.java
@@ -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.