From c6f4ebf799a25028e5f0deff4d370a6f0b30db8e Mon Sep 17 00:00:00 2001
From: Rubin Xu <rubinxu@google.com>
Date: Thu, 25 Jun 2020 17:54:20 +0100
Subject: [PATCH] Force garbage collection after credential verification

This is to sanitize memory containing sensitive user
lockscreen credentials. Most of LockSettingsService
already sanitizes credentials when needed, but there
is one copy of the credential unmarshalled from the
binder transaction and passed into LSS as argument
which is not easily sanitiziable manually except
by forcing a garbage collection.

Bug: 144537463
Test: atest com.android.server.locksettings
Test: go through Settings password change flow,
      then take a heapdump of system_server and
      verifies no password shard exists in the dump.
Change-Id: I3b0a2dab5766c40bc3ba9b38311c039337c408d3
---
 .../locksettings/LockSettingsService.java     | 76 +++++++++++++------
 1 file changed, 54 insertions(+), 22 deletions(-)

diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java
index 1a8de97714512..90370ddd21dd3 100644
--- a/services/core/java/com/android/server/locksettings/LockSettingsService.java
+++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java
@@ -1616,6 +1616,7 @@ public class LockSettingsService extends ILockSettings.Stub {
         synchronized (mSeparateChallengeLock) {
             if (!setLockCredentialInternal(credential, savedCredential,
                     userId, /* isLockTiedToParent= */ false)) {
+                scheduleGc();
                 return false;
             }
             setSeparateProfileChallengeEnabledLocked(userId, true, /* unused */ null);
@@ -1626,6 +1627,7 @@ public class LockSettingsService extends ILockSettings.Stub {
             setDeviceUnlockedForUser(userId);
         }
         notifySeparateProfileChallengeChanged(userId);
+        scheduleGc();
         return true;
     }
 
@@ -1965,7 +1967,11 @@ public class LockSettingsService extends ILockSettings.Stub {
     public VerifyCredentialResponse checkCredential(LockscreenCredential credential, int userId,
             ICheckCredentialProgressCallback progressCallback) {
         checkPasswordReadPermission(userId);
-        return doVerifyCredential(credential, CHALLENGE_NONE, 0, userId, progressCallback);
+        try {
+            return doVerifyCredential(credential, CHALLENGE_NONE, 0, userId, progressCallback);
+        } finally {
+            scheduleGc();
+        }
     }
 
     @Override
@@ -1978,8 +1984,12 @@ public class LockSettingsService extends ILockSettings.Stub {
             challengeType = CHALLENGE_NONE;
 
         }
-        return doVerifyCredential(credential, challengeType, challenge, userId,
-                null /* progressCallback */);
+        try {
+            return doVerifyCredential(credential, challengeType, challenge, userId,
+                    null /* progressCallback */);
+        } finally {
+            scheduleGc();
+        }
     }
 
     private VerifyCredentialResponse doVerifyCredential(LockscreenCredential credential,
@@ -2070,6 +2080,8 @@ public class LockSettingsService extends ILockSettings.Stub {
                 | BadPaddingException | CertificateException | IOException e) {
             Slog.e(TAG, "Failed to decrypt child profile key", e);
             throw new IllegalStateException("Unable to get tied profile token");
+        } finally {
+            scheduleGc();
         }
     }
 
@@ -2983,27 +2995,31 @@ public class LockSettingsService extends ILockSettings.Stub {
     @Override
     public byte[] getHashFactor(LockscreenCredential currentCredential, int userId) {
         checkPasswordReadPermission(userId);
-        if (isManagedProfileWithUnifiedLock(userId)) {
-            try {
-                currentCredential = getDecryptedPasswordForTiedProfile(userId);
-            } catch (Exception e) {
-                Slog.e(TAG, "Failed to get work profile credential", e);
-                return null;
+        try {
+            if (isManagedProfileWithUnifiedLock(userId)) {
+                try {
+                    currentCredential = getDecryptedPasswordForTiedProfile(userId);
+                } catch (Exception e) {
+                    Slog.e(TAG, "Failed to get work profile credential", e);
+                    return null;
+                }
             }
-        }
-        synchronized (mSpManager) {
-            if (!isSyntheticPasswordBasedCredentialLocked(userId)) {
-                Slog.w(TAG, "Synthetic password not enabled");
-                return null;
+            synchronized (mSpManager) {
+                if (!isSyntheticPasswordBasedCredentialLocked(userId)) {
+                    Slog.w(TAG, "Synthetic password not enabled");
+                    return null;
+                }
+                long handle = getSyntheticPasswordHandleLocked(userId);
+                AuthenticationResult auth = mSpManager.unwrapPasswordBasedSyntheticPassword(
+                        getGateKeeperService(), handle, currentCredential, userId, null);
+                if (auth.authToken == null) {
+                    Slog.w(TAG, "Current credential is incorrect");
+                    return null;
+                }
+                return auth.authToken.derivePasswordHashFactor();
             }
-            long handle = getSyntheticPasswordHandleLocked(userId);
-            AuthenticationResult auth = mSpManager.unwrapPasswordBasedSyntheticPassword(
-                    getGateKeeperService(), handle, currentCredential, userId, null);
-            if (auth.authToken == null) {
-                Slog.w(TAG, "Current credential is incorrect");
-                return null;
-            }
-            return auth.authToken.derivePasswordHashFactor();
+        } finally {
+            scheduleGc();
         }
     }
 
@@ -3287,6 +3303,22 @@ public class LockSettingsService extends ILockSettings.Stub {
         }
     }
 
+    /**
+     * Schedules garbage collection to sanitize lockscreen credential remnants in memory.
+     *
+     * One source of leftover lockscreen credentials is the unmarshalled binder method arguments.
+     * Since this method will be called within the binder implementation method, a small delay is
+     * added before the GC operation to allow the enclosing binder proxy code to complete and
+     * release references to the argument.
+     */
+    private void scheduleGc() {
+        mHandler.postDelayed(() -> {
+            System.gc();
+            System.runFinalization();
+            System.gc();
+        }, 2000);
+    }
+
     private class DeviceProvisionedObserver extends ContentObserver {
         private final Uri mDeviceProvisionedUri = Settings.Global.getUriFor(
                 Settings.Global.DEVICE_PROVISIONED);