From fabb749d64c59176044eb24ef0abba6293657f1b Mon Sep 17 00:00:00 2001 From: Lee Shombert Date: Wed, 2 Feb 2022 07:59:04 -0800 Subject: [PATCH] Add a binder cache to ILockSettings#getCredentialType Bug: 214623625 Put a binder cache in front of ILockSettings#getCredentialType(). The cache is in LockPatternUtils and the invalidation logic is in LogSettingsService. The cache is invalidated whenever the LockSettingsStorage changes. Three unit tests disable caches because they do not have permission to write to the system properties. Test: atest com.android.server.locksettings Change-Id: Iea4c50ba5cc39de9196d9a9873ae501a946bf9fd --- .../internal/widget/LockPatternUtils.java | 49 ++++++++++++++++--- .../locksettings/LockSettingsService.java | 24 +++++++++ .../locksettings/LockSettingsStorage.java | 13 +++-- .../LockSettingsServiceTests.java | 7 +++ .../locksettings/LockscreenFrpTest.java | 6 +++ .../locksettings/SyntheticPasswordTests.java | 7 +++ 6 files changed, 97 insertions(+), 9 deletions(-) diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index f8ccde4cf4d99..895bc868c53ae 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -26,6 +26,7 @@ import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; +import android.app.PropertyInvalidatedCache; import android.app.admin.DevicePolicyManager; import android.app.admin.PasswordMetrics; import android.app.trust.IStrongAuthTracker; @@ -915,18 +916,54 @@ public class LockPatternUtils { return Long.toHexString(salt); } + /** + * Retrieve the credential type of a user. + */ + private final PropertyInvalidatedCache.QueryHandler mCredentialTypeQuery = + new PropertyInvalidatedCache.QueryHandler<>() { + @Override + public Integer apply(Integer userHandle) { + try { + return getLockSettings().getCredentialType(userHandle); + } catch (RemoteException re) { + Log.e(TAG, "failed to get credential type", re); + return CREDENTIAL_TYPE_NONE; + } + } + @Override + public boolean shouldBypassCache(Integer userHandle) { + return userHandle == USER_FRP; + } + }; + + /** + * The API that is cached. + */ + private final static String CREDENTIAL_TYPE_API = "getCredentialType"; + + /** + * Cache the credential type of a user. + */ + private final PropertyInvalidatedCache mCredentialTypeCache = + new PropertyInvalidatedCache<>(4, PropertyInvalidatedCache.MODULE_SYSTEM, + CREDENTIAL_TYPE_API, CREDENTIAL_TYPE_API, mCredentialTypeQuery); + + /** + * Invalidate the credential cache + * @hide + */ + public final static void invalidateCredentialTypeCache() { + PropertyInvalidatedCache.invalidateCache(PropertyInvalidatedCache.MODULE_SYSTEM, + CREDENTIAL_TYPE_API); + } + /** * Returns the credential type of the user, can be one of {@link #CREDENTIAL_TYPE_NONE}, * {@link #CREDENTIAL_TYPE_PATTERN}, {@link #CREDENTIAL_TYPE_PIN} and * {@link #CREDENTIAL_TYPE_PASSWORD} */ public @CredentialType int getCredentialTypeForUser(int userHandle) { - try { - return getLockSettings().getCredentialType(userHandle); - } catch (RemoteException re) { - Log.e(TAG, "failed to get credential type", re); - return CREDENTIAL_TYPE_NONE; - } + return mCredentialTypeCache.query(userHandle); } /** diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 752add8ca8253..81c12c182f771 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -148,6 +148,8 @@ import com.android.server.locksettings.SyntheticPasswordManager.AuthenticationTo import com.android.server.locksettings.SyntheticPasswordManager.TokenType; import com.android.server.locksettings.recoverablekeystore.RecoverableKeyStoreManager; import com.android.server.pm.UserManagerInternal; +import com.android.server.utils.Watchable; +import com.android.server.utils.Watcher; import com.android.server.wm.WindowManagerInternal; import libcore.util.HexEncoding; @@ -229,6 +231,7 @@ public class LockSettingsService extends ILockSettings.Stub { protected final Handler mHandler; @VisibleForTesting protected final LockSettingsStorage mStorage; + private final Watcher mStorageWatcher; private final LockSettingsStrongAuth mStrongAuth; private final SynchronizedStrongAuthTracker mStrongAuthTracker; private final BiometricDeferredQueue mBiometricDeferredQueue; @@ -569,6 +572,12 @@ public class LockSettingsService extends ILockSettings.Stub { } } + private class StorageWatcher extends Watcher { + public void onChange(Watchable what) { + LockSettingsService.this.onChange(); + } + } + public LockSettingsService(Context context) { this(new Injector(context)); } @@ -610,6 +619,16 @@ public class LockSettingsService extends ILockSettings.Stub { mStorage); LocalServices.addService(LockSettingsInternal.class, new LocalService()); + + mStorageWatcher = new StorageWatcher(); + mStorage.registerObserver(mStorageWatcher); + } + + /** + * Invalidate caches if the storage has changed. + */ + private void onChange() { + LockPatternUtils.invalidateCredentialTypeCache(); } /** @@ -1273,6 +1292,11 @@ public class LockSettingsService extends ILockSettings.Stub { DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED, userId); } + /** + * This API is cached; whenever the result would change, + * {@link com.android.internal.widget.LockPatternUtils#invalidateCredentialTypeCache} + * must be called. + */ @Override public int getCredentialType(int userId) { checkPasswordHavePermission(userId); diff --git a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java index f69a3bd9d7ce7..f9db5cf998bb4 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java @@ -47,6 +47,7 @@ import com.android.internal.widget.LockPatternUtils; import com.android.internal.widget.LockPatternUtils.CredentialType; import com.android.server.LocalServices; import com.android.server.PersistentDataBlockManagerInternal; +import com.android.server.utils.WatchableImpl; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -66,7 +67,7 @@ import java.util.Map; /** * Storage for the lock settings service. */ -class LockSettingsStorage { +class LockSettingsStorage extends WatchableImpl { private static final String TAG = "LockSettingsStorage"; private static final String TABLE = "locksettings"; @@ -173,7 +174,7 @@ class LockSettingsStorage { } finally { db.endTransaction(); } - + dispatchChange(this); } @VisibleForTesting @@ -221,7 +222,7 @@ class LockSettingsStorage { } finally { db.endTransaction(); } - + dispatchChange(this); } public void prefetchUser(int userId) { @@ -412,6 +413,7 @@ class LockSettingsStorage { } } mCache.putFile(name, hash); + dispatchChange(this); } } @@ -423,6 +425,7 @@ class LockSettingsStorage { file.delete(); mCache.putFile(name, null); } + dispatchChange(this); } } @@ -500,6 +503,7 @@ class LockSettingsStorage { Slog.w(TAG, "Failed to zeroize " + path, e); } finally { file.delete(); + dispatchChange(this); } mCache.putFile(path, null); } @@ -587,6 +591,7 @@ class LockSettingsStorage { } finally { db.endTransaction(); } + dispatchChange(this); } private void deleteFilesAndRemoveCache(String... names) { @@ -595,6 +600,7 @@ class LockSettingsStorage { if (file.exists()) { file.delete(); mCache.putFile(name, null); + dispatchChange(this); } } } @@ -675,6 +681,7 @@ class LockSettingsStorage { } persistentDataBlock.setFrpCredentialHandle(PersistentData.toBytes( persistentType, userId, qualityForUi, payload)); + dispatchChange(this); } public PersistentData readPersistentDataBlock() { diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java index 67d69292b476c..e7f4d3dccae76 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java @@ -34,6 +34,7 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.verify; +import android.app.PropertyInvalidatedCache; import android.os.RemoteException; import android.platform.test.annotations.Presubmit; import android.service.gatekeeper.GateKeeperResponse; @@ -47,6 +48,7 @@ import com.android.internal.widget.VerifyCredentialResponse; import com.android.server.locksettings.FakeGateKeeperService.VerifyHandle; import com.android.server.locksettings.LockSettingsStorage.CredentialHash; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -57,6 +59,11 @@ import org.junit.runner.RunWith; @Presubmit @RunWith(AndroidJUnit4.class) public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { + @Before + public void disableProcessCaches() { + PropertyInvalidatedCache.disableForTestMode(); + } + @Test public void testCreatePasswordPrimaryUser() throws RemoteException { testCreateCredential(PRIMARY_USER_ID, newPassword("password")); diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java b/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java index 9c0239b2b6a6e..c2f94e202ca3e 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java @@ -24,6 +24,7 @@ import static com.android.internal.widget.LockPatternUtils.USER_FRP; import static org.junit.Assert.assertEquals; +import android.app.PropertyInvalidatedCache; import android.app.admin.DevicePolicyManager; import androidx.test.runner.AndroidJUnit4; @@ -46,6 +47,11 @@ public class LockscreenFrpTest extends BaseLockSettingsServiceTests { mSettings.setDeviceProvisioned(false); } + @Before + public void disableProcessCaches() { + PropertyInvalidatedCache.disableForTestMode(); + } + @Test public void testFrpCredential_setPin() { mService.setLockCredential(newPin("1234"), nonePassword(), PRIMARY_USER_ID); diff --git a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java index c0a38b8749143..58e1c4378c418 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -36,6 +36,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.app.admin.PasswordMetrics; +import android.app.PropertyInvalidatedCache; import android.os.RemoteException; import android.os.UserHandle; import android.platform.test.annotations.Presubmit; @@ -49,6 +50,7 @@ import com.android.server.locksettings.SyntheticPasswordManager.AuthenticationRe import com.android.server.locksettings.SyntheticPasswordManager.AuthenticationToken; import com.android.server.locksettings.SyntheticPasswordManager.PasswordData; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.ArgumentCaptor; @@ -68,6 +70,11 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { public static final byte[] PAYLOAD = new byte[] {1, 2, -1, -2, 55}; public static final byte[] PAYLOAD2 = new byte[] {2, 3, -2, -3, 44, 1}; + @Before + public void disableProcessCaches() { + PropertyInvalidatedCache.disableForTestMode(); + } + @Test public void testPasswordBasedSyntheticPassword() throws RemoteException { final int USER_ID = 10;