Merge "Fix clearing password with DPM.resetPasswordWithToken()" into qt-dev

3 files changed, 73 insertions, 42 deletions

diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java
index da836c271457..1b705bb7c4be 100644
--- a/services/core/java/com/android/server/locksettings/LockSettingsService.java
+++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java
@@ -189,6 +189,7 @@ public class LockSettingsService extends ILockSettings.Stub {
     private final LockPatternUtils mLockPatternUtils;
     private final NotificationManager mNotificationManager;
     private final UserManager mUserManager;
+    private final IStorageManager mStorageManager;
     private final IActivityManager mActivityManager;
     private final SyntheticPasswordManager mSpManager;
 
@@ -460,6 +461,7 @@ public class LockSettingsService extends ILockSettings.Stub {
         mStorage = injector.getStorage();
         mNotificationManager = injector.getNotificationManager();
         mUserManager = injector.getUserManager();
+        mStorageManager = injector.getStorageManager();
         mStrongAuthTracker = injector.getStrongAuthTracker();
         mStrongAuthTracker.register(mStrongAuth);
 
@@ -1186,6 +1188,14 @@ public class LockSettingsService extends ILockSettings.Stub {
         }
     }
 
+    /**
+     * Unlock the user (both storage and user state) and its associated managed profiles
+     * synchronously.
+     *
+     * <em>Be very careful about the risk of deadlock here: ActivityManager.unlockUser()
+     * can end up calling into other system services to process user unlock request (via
+     * {@link com.android.server.SystemServiceManager#unlockUser} </em>
+     */
     private void unlockUser(int userId, byte[] token, byte[] secret) {
         // TODO: make this method fully async so we can update UI with progress strings
         final CountDownLatch latch = new CountDownLatch(1);
@@ -1639,13 +1649,27 @@ public class LockSettingsService extends ILockSettings.Stub {
         }
     }
 
+    private boolean isUserKeyUnlocked(int userId) {
+        try {
+            return mStorageManager.isUserKeyUnlocked(userId);
+        } catch (RemoteException e) {
+            Log.e(TAG, "failed to check user key locked state", e);
+            return false;
+        }
+    }
+
+    /** Unlock disk encryption */
+    private void unlockUserKey(int userId, byte[] token, byte[] secret) throws RemoteException {
+        final UserInfo userInfo = mUserManager.getUserInfo(userId);
+        mStorageManager.unlockUserKey(userId, userInfo.serialNumber, token, secret);
+    }
+
     private void addUserKeyAuth(int userId, byte[] token, byte[] secret)
             throws RemoteException {
         final UserInfo userInfo = mUserManager.getUserInfo(userId);
-        final IStorageManager storageManager = mInjector.getStorageManager();
         final long callingId = Binder.clearCallingIdentity();
         try {
-            storageManager.addUserKeyAuth(userId, userInfo.serialNumber, token, secret);
+            mStorageManager.addUserKeyAuth(userId, userInfo.serialNumber, token, secret);
         } finally {
             Binder.restoreCallingIdentity(callingId);
         }
@@ -1654,10 +1678,9 @@ public class LockSettingsService extends ILockSettings.Stub {
     private void fixateNewestUserKeyAuth(int userId)
             throws RemoteException {
         if (DEBUG) Slog.d(TAG, "fixateNewestUserKeyAuth: user=" + userId);
-        final IStorageManager storageManager = mInjector.getStorageManager();
         final long callingId = Binder.clearCallingIdentity();
         try {
-            storageManager.fixateNewestUserKeyAuth(userId);
+            mStorageManager.fixateNewestUserKeyAuth(userId);
         } finally {
             Binder.restoreCallingIdentity(callingId);
         }
@@ -2571,7 +2594,7 @@ public class LockSettingsService extends ILockSettings.Stub {
                 credential, credentialType, auth, requestedQuality, userId);
         final Map<Integer, byte[]> profilePasswords;
         if (credential != null) {
-            // // not needed by synchronizeUnifiedWorkChallengeForProfiles()
+            // not needed by synchronizeUnifiedWorkChallengeForProfiles()
             profilePasswords = null;
 
             if (mSpManager.hasSidForUser(userId)) {
@@ -2599,9 +2622,12 @@ public class LockSettingsService extends ILockSettings.Stub {
             mSpManager.clearSidForUser(userId);
             getGateKeeperService().clearSecureUserId(userId);
             // Clear key from vold so ActivityManager can just unlock the user with empty secret
-            // during boot.
+            // during boot. Vold storage needs to be unlocked before manipulation of the keys can
+            // succeed.
+            unlockUserKey(userId, null, auth.deriveDiskEncryptionKey());
             clearUserKeyProtection(userId);
             fixateNewestUserKeyAuth(userId);
+            unlockKeystore(auth.deriveKeyStorePassword(), userId);
             setKeystorePassword(null, userId);
         }
         setLong(SYNTHETIC_PASSWORD_HANDLE_KEY, newHandle, userId);
@@ -2809,45 +2835,50 @@ public class LockSettingsService extends ILockSettings.Stub {
             if (!mSpManager.hasEscrowData(userId)) {
                 throw new SecurityException("Escrow token is disabled on the current user");
             }
-            result = setLockCredentialWithTokenInternal(credential, type, tokenHandle, token,
+            result = setLockCredentialWithTokenInternalLocked(credential, type, tokenHandle, token,
                     requestedQuality, userId);
         }
         if (result) {
             synchronized (mSeparateChallengeLock) {
                 setSeparateProfileChallengeEnabledLocked(userId, true, null);
             }
+            if (credential == null) {
+                // If clearing credential, unlock the user manually in order to progress user start
+                // Call unlockUser() on a handler thread so no lock is held (either by LSS or by
+                // the caller like DPMS), otherwise it can lead to deadlock.
+                mHandler.post(() -> unlockUser(userId, null, null));
+            }
             notifyPasswordChanged(userId);
             notifySeparateProfileChallengeChanged(userId);
         }
         return result;
     }
 
-    private boolean setLockCredentialWithTokenInternal(byte[] credential, int type,
-            long tokenHandle, byte[] token, int requestedQuality, int userId) throws RemoteException {
+    @GuardedBy("mSpManager")
+    private boolean setLockCredentialWithTokenInternalLocked(byte[] credential, int type,
+            long tokenHandle, byte[] token, int requestedQuality, int userId)
+                    throws RemoteException {
         final AuthenticationResult result;
-        synchronized (mSpManager) {
-            result = mSpManager.unwrapTokenBasedSyntheticPassword(
-                    getGateKeeperService(), tokenHandle, token, userId);
-            if (result.authToken == null) {
-                Slog.w(TAG, "Invalid escrow token supplied");
-                return false;
-            }
-            if (result.gkResponse.getResponseCode() != VerifyCredentialResponse.RESPONSE_OK) {
-                // Most likely, an untrusted credential reset happened in the past which
-                // changed the synthetic password
-                Slog.e(TAG, "Obsolete token: synthetic password derived but it fails GK "
-                        + "verification.");
-                return false;
-            }
-            // Update PASSWORD_TYPE_KEY since it's needed by notifyActivePasswordMetricsAvailable()
-            // called by setLockCredentialWithAuthTokenLocked().
-            // TODO: refactor usage of PASSWORD_TYPE_KEY b/65239740
-            setLong(LockPatternUtils.PASSWORD_TYPE_KEY, requestedQuality, userId);
-            long oldHandle = getSyntheticPasswordHandleLocked(userId);
-            setLockCredentialWithAuthTokenLocked(credential, type, result.authToken,
-                    requestedQuality, userId);
-            mSpManager.destroyPasswordBasedSyntheticPassword(oldHandle, userId);
+        result = mSpManager.unwrapTokenBasedSyntheticPassword(
+                getGateKeeperService(), tokenHandle, token, userId);
+        if (result.authToken == null) {
+            Slog.w(TAG, "Invalid escrow token supplied");
+            return false;
         }
+        if (result.gkResponse.getResponseCode() != VerifyCredentialResponse.RESPONSE_OK) {
+            // Most likely, an untrusted credential reset happened in the past which
+            // changed the synthetic password
+            Slog.e(TAG, "Obsolete token: synthetic password derived but it fails GK "
+                    + "verification.");
+            return false;
+        }
+        // TODO: refactor usage of PASSWORD_TYPE_KEY b/65239740
+        setLong(LockPatternUtils.PASSWORD_TYPE_KEY, requestedQuality, userId);
+        long oldHandle = getSyntheticPasswordHandleLocked(userId);
+        setLockCredentialWithAuthTokenLocked(credential, type, result.authToken,
+                requestedQuality, userId);
+        mSpManager.destroyPasswordBasedSyntheticPassword(oldHandle, userId);
+
         onAuthTokenKnownForUser(userId, result.authToken);
         return true;
     }
diff --git a/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java b/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java
index ee22264112ab..a5d59e3c8884 100644
--- a/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java
+++ b/services/core/java/com/android/server/locksettings/LockSettingsShellCommand.java
@@ -260,6 +260,10 @@ class LockSettingsShellCommand extends ShellCommand {
                 return false;
             }
         } else {
+            if (!mOld.isEmpty()) {
+                getOutPrintWriter().println("Old password provided but user has no password");
+                return false;
+            }
             return true;
         }
     }
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/FakeStorageManager.java b/services/tests/servicestests/src/com/android/server/locksettings/FakeStorageManager.java
index dbdb41bc71b1..1ae1fa65ddf2 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/FakeStorageManager.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/FakeStorageManager.java
@@ -65,17 +65,13 @@ public class FakeStorageManager {
         listener.onStarted(userId, null);
         listener.onFinished(userId, null);
         ArrayList<Pair<byte[], byte[]>> auths = getUserAuth(userId);
-        if (secret != null) {
-            if (auths.size() > 1) {
-                throw new AssertionFailedError("More than one secret exists");
-            }
-            Pair<byte[], byte[]> auth = auths.get(0);
-            if ((!mIgnoreBadUnlock) && auth.second != null && !Arrays.equals(secret, auth.second)) {
-                throw new AssertionFailedError("Invalid secret to unlock user");
-            }
-        } else {
-            if (auths != null && auths.size() > 0) {
-                throw new AssertionFailedError("Cannot unlock encrypted user with empty token");
+        if (auths.size() > 1) {
+            throw new AssertionFailedError("More than one secret exists");
+        }
+        Pair<byte[], byte[]> auth = auths.get(0);
+        if (!Arrays.equals(secret, auth.second)) {
+            if (!mIgnoreBadUnlock) {
+                throw new AssertionFailedError("Invalid secret to unlock user " + userId);
             }
         }
     }