From 745555592daa8a7ed8955c0a9c8a04fe7b826f7f Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 2 Dec 2022 22:53:38 +0000 Subject: [PATCH] Fix FRP credential overwritten before it is verified When initializing the synthetic password of the user that will own the FRP credential, don't clear the FRP credential if the device is not yet provisioned. In addition, prevent the Weaver slot used by the FRP credential from being overwritten while the device is not yet provisioned. This fixes commit 78e245a21a11 ("Give all users SP-based credentials"). Test: atest com.android.server.locksettings Test: Manually tested FRP Bug: 254378141 Change-Id: I55d916d43fc30e8c76c3d85a09fad14b7946b53d --- .../locksettings/LockSettingsService.java | 1 + .../locksettings/LockSettingsStorage.java | 2 +- .../SyntheticPasswordManager.java | 73 ++++++++++++++++--- .../locksettings/LockscreenFrpTest.java | 1 + .../locksettings/SyntheticPasswordTests.java | 37 ++++++++++ .../WeaverBasedSyntheticPasswordTests.java | 38 ++++++++++ 6 files changed, 139 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index d19ea790821bb..0ae3a02fb80a2 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -3272,6 +3272,7 @@ public class LockSettingsService extends ILockSettings.Stub { for (UserInfo user : users) { if (userOwnsFrpCredential(mContext, user)) { if (!isUserSecure(user.id)) { + Slogf.d(TAG, "Clearing FRP credential tied to user %d", user.id); mStorage.writePersistentDataBlock(PersistentData.TYPE_NONE, user.id, 0, null); } diff --git a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java index 807ba3cf4463b..473c4b6e0e759 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java @@ -550,7 +550,7 @@ class LockSettingsStorage { mCache.clear(); } - @Nullable @VisibleForTesting + @Nullable PersistentDataBlockManagerInternal getPersistentDataBlockManager() { if (mPersistentDataBlockManagerInternal == null) { mPersistentDataBlockManagerInternal = diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java index 73a16fdbc7c96..acd7cc1f0ddba 100644 --- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java +++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java @@ -32,6 +32,7 @@ import android.hardware.weaver.V1_0.WeaverStatus; import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.UserManager; +import android.provider.Settings; import android.security.GateKeeper; import android.security.Scrypt; import android.service.gatekeeper.GateKeeperResponse; @@ -457,6 +458,11 @@ public class SyntheticPasswordManager { mPasswordSlotManager = passwordSlotManager; } + private boolean isDeviceProvisioned() { + return Settings.Global.getInt(mContext.getContentResolver(), + Settings.Global.DEVICE_PROVISIONED, 0) != 0; + } + @VisibleForTesting protected IWeaver getWeaverService() throws RemoteException { try { @@ -770,6 +776,17 @@ public class SyntheticPasswordManager { private int getNextAvailableWeaverSlot() { Set usedSlots = getUsedWeaverSlots(); usedSlots.addAll(mPasswordSlotManager.getUsedSlots()); + // If the device is not yet provisioned, then the Weaver slot used by the FRP credential may + // be still needed and must not be reused yet. (This *should* instead check "has FRP been + // resolved yet?", which would allow reusing the slot a bit earlier. However, the + // SECURE_FRP_MODE setting gets set to 1 too late for it to be used here.) + if (!isDeviceProvisioned()) { + PersistentData persistentData = mStorage.readPersistentDataBlock(); + if (persistentData != null && persistentData.type == PersistentData.TYPE_SP_WEAVER) { + int slot = persistentData.userId; // Note: field name is misleading + usedSlots.add(slot); + } + } for (int i = 0; i < mWeaverConfig.slots; i++) { if (!usedSlots.contains(i)) { return i; @@ -814,9 +831,14 @@ public class SyntheticPasswordManager { protectorSecret = transformUnderWeaverSecret(stretchedLskf, weaverSecret); } else { - // Weaver is unavailable, so make the protector use Gatekeeper to verify the LSKF - // instead. However, skip Gatekeeper when the LSKF is empty, since it wouldn't give any - // benefit in that case as Gatekeeper isn't expected to provide secure deletion. + // Weaver is unavailable, so make the protector use Gatekeeper (GK) to verify the LSKF. + // + // However, skip GK when the LSKF is empty. There are two reasons for this, one + // performance and one correctness. The performance reason is that GK wouldn't give any + // benefit with an empty LSKF anyway, since GK isn't expected to provide secure + // deletion. The correctness reason is that it is unsafe to enroll a password in the + // 'fakeUserId' GK range on an FRP-protected device that is in the setup wizard with FRP + // not passed yet, as that may overwrite the enrollment used by the FRP credential. if (!credential.isNone()) { // In case GK enrollment leaves persistent state around (in RPMB), this will nuke // them to prevent them from accumulating and causing problems. @@ -908,12 +930,40 @@ public class SyntheticPasswordManager { } } + private static boolean isNoneCredential(PasswordData pwd) { + return pwd == null || pwd.credentialType == LockPatternUtils.CREDENTIAL_TYPE_NONE; + } + + private boolean shouldSynchronizeFrpCredential(@Nullable PasswordData pwd, int userId) { + if (mStorage.getPersistentDataBlockManager() == null) { + return false; + } + UserInfo userInfo = mUserManager.getUserInfo(userId); + if (!LockPatternUtils.userOwnsFrpCredential(mContext, userInfo)) { + return false; + } + // When initializing the synthetic password of the user that will own the FRP credential, + // the FRP data block must not be cleared if the device isn't provisioned yet, since in this + // case the old value of the block may still be needed for the FRP authentication step. The + // FRP data block will instead be cleared later, by + // LockSettingsService.DeviceProvisionedObserver.clearFrpCredentialIfOwnerNotSecure(). + // + // Don't check the SECURE_FRP_MODE setting here, as it gets set to 1 too late. + // + // Don't delay anything for a nonempty credential. A nonempty credential can be set before + // the device has been provisioned, but it's guaranteed to be after FRP was resolved. + if (isNoneCredential(pwd) && !isDeviceProvisioned()) { + Slog.d(TAG, "Not clearing FRP credential yet because device is not yet provisioned"); + return false; + } + return true; + } + private void synchronizeFrpPassword(@Nullable PasswordData pwd, int requestedQuality, int userId) { - if (mStorage.getPersistentDataBlockManager() != null - && LockPatternUtils.userOwnsFrpCredential(mContext, - mUserManager.getUserInfo(userId))) { - if (pwd != null && pwd.credentialType != LockPatternUtils.CREDENTIAL_TYPE_NONE) { + if (shouldSynchronizeFrpCredential(pwd, userId)) { + Slogf.d(TAG, "Syncing Gatekeeper-based FRP credential tied to user %d", userId); + if (!isNoneCredential(pwd)) { mStorage.writePersistentDataBlock(PersistentData.TYPE_SP, userId, requestedQuality, pwd.toBytes()); } else { @@ -924,10 +974,9 @@ public class SyntheticPasswordManager { private void synchronizeWeaverFrpPassword(@Nullable PasswordData pwd, int requestedQuality, int userId, int weaverSlot) { - if (mStorage.getPersistentDataBlockManager() != null - && LockPatternUtils.userOwnsFrpCredential(mContext, - mUserManager.getUserInfo(userId))) { - if (pwd != null && pwd.credentialType != LockPatternUtils.CREDENTIAL_TYPE_NONE) { + if (shouldSynchronizeFrpCredential(pwd, userId)) { + Slogf.d(TAG, "Syncing Weaver-based FRP credential tied to user %d", userId); + if (!isNoneCredential(pwd)) { mStorage.writePersistentDataBlock(PersistentData.TYPE_SP_WEAVER, weaverSlot, requestedQuality, pwd.toBytes()); } else { @@ -1058,7 +1107,7 @@ public class SyntheticPasswordManager { AuthenticationResult result = new AuthenticationResult(); if (protectorId == SyntheticPasswordManager.NULL_PROTECTOR_ID) { - // This should never happen, due to the migration done in LSS.bootCompleted(). + // This should never happen, due to the migration done in LSS.onThirdPartyAppsStarted(). Slogf.wtf(TAG, "Synthetic password not found for user %d", userId); result.gkResponse = VerifyCredentialResponse.ERROR; return result; 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 d05af76f8f5ad..fc0ca7eda2438 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java @@ -98,6 +98,7 @@ public class LockscreenFrpTest extends BaseLockSettingsServiceTests { mService.setLockCredential(newPassword("1234"), nonePassword(), PRIMARY_USER_ID); assertEquals(CREDENTIAL_TYPE_PASSWORD, mService.getCredentialType(USER_FRP)); + setDeviceProvisioned(true); mService.setLockCredential(nonePassword(), newPassword("1234"), PRIMARY_USER_ID); assertEquals(CREDENTIAL_TYPE_NONE, mService.getCredentialType(USER_FRP)); } 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 3f4bec6530914..b9cafa4e24239 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -136,6 +136,43 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { assertTrue(mService.isSyntheticPasswordBasedCredential(userId)); } + protected void initializeSyntheticPassword(int userId) { + synchronized (mService.mSpManager) { + mService.initializeSyntheticPasswordLocked(userId); + } + } + + // Tests that the FRP credential is updated when an LSKF-based protector is created for the user + // that owns the FRP credential, if the device is already provisioned. + @Test + public void testFrpCredentialSyncedIfDeviceProvisioned() throws RemoteException { + setDeviceProvisioned(true); + initializeSyntheticPassword(PRIMARY_USER_ID); + verify(mStorage.mPersistentDataBlockManager).setFrpCredentialHandle(any()); + } + + // Tests that the FRP credential is not updated when an LSKF-based protector is created for the + // user that owns the FRP credential, if the new credential is empty and the device is not yet + // provisioned. + @Test + public void testEmptyFrpCredentialNotSyncedIfDeviceNotProvisioned() throws RemoteException { + setDeviceProvisioned(false); + initializeSyntheticPassword(PRIMARY_USER_ID); + verify(mStorage.mPersistentDataBlockManager, never()).setFrpCredentialHandle(any()); + } + + // Tests that the FRP credential is updated when an LSKF-based protector is created for the user + // that owns the FRP credential, if the new credential is nonempty and the device is not yet + // provisioned. + @Test + public void testNonEmptyFrpCredentialSyncedIfDeviceNotProvisioned() throws RemoteException { + setDeviceProvisioned(false); + initializeSyntheticPassword(PRIMARY_USER_ID); + verify(mStorage.mPersistentDataBlockManager, never()).setFrpCredentialHandle(any()); + mService.setLockCredential(newPassword("password"), nonePassword(), PRIMARY_USER_ID); + verify(mStorage.mPersistentDataBlockManager).setFrpCredentialHandle(any()); + } + @Test public void testChangeCredential() throws RemoteException { final LockscreenCredential password = newPassword("password"); diff --git a/services/tests/servicestests/src/com/android/server/locksettings/WeaverBasedSyntheticPasswordTests.java b/services/tests/servicestests/src/com/android/server/locksettings/WeaverBasedSyntheticPasswordTests.java index a3ac5153a03d5..6c13a6fe04a04 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/WeaverBasedSyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/WeaverBasedSyntheticPasswordTests.java @@ -1,11 +1,17 @@ package com.android.server.locksettings; +import static org.junit.Assert.assertEquals; + import android.platform.test.annotations.Presubmit; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.server.locksettings.LockSettingsStorage.PersistentData; +import com.google.android.collect.Sets; + import org.junit.Before; +import org.junit.Test; import org.junit.runner.RunWith; @SmallTest @@ -17,4 +23,36 @@ public class WeaverBasedSyntheticPasswordTests extends SyntheticPasswordTests { public void enableWeaver() throws Exception { mSpManager.enableWeaver(); } + + // Tests that if the device is not yet provisioned and the FRP credential uses Weaver, then the + // Weaver slot of the FRP credential is not reused. Assumes that Weaver slots are allocated + // sequentially, starting at slot 0. + @Test + public void testFrpWeaverSlotNotReused() { + final int userId = 10; + final int frpWeaverSlot = 0; + + setDeviceProvisioned(false); + assertEquals(Sets.newHashSet(), mPasswordSlotManager.getUsedSlots()); + mStorage.writePersistentDataBlock(PersistentData.TYPE_SP_WEAVER, frpWeaverSlot, 0, + new byte[1]); + initializeSyntheticPassword(userId); // This should allocate a Weaver slot. + assertEquals(Sets.newHashSet(1), mPasswordSlotManager.getUsedSlots()); + } + + // Tests that if the device is already provisioned and the FRP credential uses Weaver, then the + // Weaver slot of the FRP credential is reused. This is not a very interesting test by itself; + // it's here as a control for testFrpWeaverSlotNotReused(). + @Test + public void testFrpWeaverSlotReused() { + final int userId = 10; + final int frpWeaverSlot = 0; + + setDeviceProvisioned(true); + assertEquals(Sets.newHashSet(), mPasswordSlotManager.getUsedSlots()); + mStorage.writePersistentDataBlock(PersistentData.TYPE_SP_WEAVER, frpWeaverSlot, 0, + new byte[1]); + initializeSyntheticPassword(userId); // This should allocate a Weaver slot. + assertEquals(Sets.newHashSet(0), mPasswordSlotManager.getUsedSlots()); + } }