From f29cab97376c3ec36da724bdd72d943866d71f35 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Fri, 19 May 2023 18:20:51 +0000 Subject: [PATCH] Automatically fix bad FRP block written by Android 14 Beta 2 Make LockSettingsService automatically fix the FRP persistent data block if it is in the "bad" format written by Android 14 Beta 2 which Android 13 can't understand. Previously, the FRP persistent data block only got fixed if the user happened to set, change, or clear their lockscreen credential. I.e., the code that wrote the FRP block was fixed, but there wasn't anything that made the FRP block be written when not otherwise needed. Bug: 276780938 Test: atest com.android.server.locksettings Test: Installed udc-beta2 and added Google account and PIN. Upgraded to build with this CL, without wiping. Then downgraded to tm-qpr2, with wiping (but not through Settings). Verified that the FRP challenge is presented in the Setup Wizard and can be passed. Without this CL, the bad FRP block would not have been fixed. Change-Id: Ifbe8646af679118a1f6bb696f1ede9a359cffdc2 --- .../locksettings/LockSettingsService.java | 25 ++++--- .../locksettings/LockSettingsStorage.java | 5 ++ .../SyntheticPasswordManager.java | 9 +++ .../locksettings/LockscreenFrpTest.java | 69 ++++++++++++++++++- .../locksettings/SyntheticPasswordTests.java | 2 + 5 files changed, 101 insertions(+), 9 deletions(-) diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 20f06971b6b23..2e62ef4f85666 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -889,22 +889,31 @@ public class LockSettingsService extends ILockSettings.Stub { } - private void migrateOldDataAfterSystemReady() { - // Migrate the FRP credential to the persistent data block + @VisibleForTesting + void migrateOldDataAfterSystemReady() { + // Write the FRP persistent data block if needed. + // + // The original purpose of this code was to write the FRP block for the first time, when + // upgrading from Android 8.1 or earlier which didn't use the FRP block. This code has + // since been repurposed to also fix the "bad" (non-forwards-compatible) FRP block written + // by Android 14 Beta 2. For this reason, the database key used here has been renamed from + // "migrated_frp" to "migrated_frp2" to cause migrateFrpCredential() to run again on devices + // where it had run before. if (LockPatternUtils.frpCredentialEnabled(mContext) - && !getBoolean("migrated_frp", false, 0)) { + && !getBoolean("migrated_frp2", false, 0)) { migrateFrpCredential(); - setBoolean("migrated_frp", true, 0); + setBoolean("migrated_frp2", true, 0); } } /** - * Migrate the credential for the FRP credential owner user if the following are satisfied: - * - the user has a secure credential - * - the FRP credential is not set up + * Write the FRP persistent data block if the following are satisfied: + * - the user who owns the FRP credential has a nonempty credential + * - the FRP persistent data block doesn't exist or uses the "bad" format from Android 14 Beta 2 */ private void migrateFrpCredential() { - if (mStorage.readPersistentDataBlock() != PersistentData.NONE) { + PersistentData data = mStorage.readPersistentDataBlock(); + if (data != PersistentData.NONE && !data.isBadFormatFromAndroid14Beta()) { return; } for (UserInfo userInfo : mUserManager.getUsers()) { diff --git a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java index 731ecadc1372f..2fa637e030f19 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsStorage.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsStorage.java @@ -606,6 +606,11 @@ class LockSettingsStorage { this.payload = payload; } + public boolean isBadFormatFromAndroid14Beta() { + return (this.type == TYPE_SP_GATEKEEPER || this.type == TYPE_SP_WEAVER) + && SyntheticPasswordManager.PasswordData.isBadFormatFromAndroid14Beta(this.payload); + } + public static PersistentData fromBytes(byte[] frpData) { if (frpData == null || frpData.length == 0) { return NONE; diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java index 65e7a00ad13f7..66f862ac92057 100644 --- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java +++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java @@ -370,6 +370,15 @@ class SyntheticPasswordManager { return result; } + /** + * Returns true if the given serialized PasswordData begins with the value 2 as a short. + * This detects the "bad" (non-forwards-compatible) PasswordData format that was temporarily + * used during development of Android 14. For more details, see fromBytes() below. + */ + public static boolean isBadFormatFromAndroid14Beta(byte[] data) { + return data != null && data.length >= 2 && data[0] == 0 && data[1] == 2; + } + public static PasswordData fromBytes(byte[] data) { PasswordData result = new PasswordData(); ByteBuffer buffer = ByteBuffer.allocate(data.length); 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 2b49b8ab64d25..a242cdec89db3 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockscreenFrpTest.java @@ -23,6 +23,8 @@ import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PIN; import static com.android.internal.widget.LockPatternUtils.USER_FRP; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import android.app.PropertyInvalidatedCache; import android.app.admin.DevicePolicyManager; @@ -38,8 +40,9 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import java.nio.ByteBuffer; -/** Test setting a lockscreen credential and then verify it under USER_FRP */ +/** Tests that involve the Factory Reset Protection (FRP) credential. */ @SmallTest @Presubmit @RunWith(AndroidJUnit4.class) @@ -148,4 +151,68 @@ public class LockscreenFrpTest extends BaseLockSettingsServiceTests { mService.verifyCredential(newPin("1234"), USER_FRP, 0 /* flags */) .getResponseCode()); } + + // The FRP block that gets written by the current version of Android must still be accepted by + // old versions of Android. This test tries to detect non-forward-compatible changes in + // PasswordData#toBytes(), which would break that. + @Test + public void testFrpBlock_isForwardsCompatible() { + mService.setLockCredential(newPin("1234"), nonePassword(), PRIMARY_USER_ID); + PersistentData data = mStorage.readPersistentDataBlock(); + ByteBuffer buffer = ByteBuffer.wrap(data.payload); + + final int credentialType = buffer.getInt(); + assertEquals(CREDENTIAL_TYPE_PIN, credentialType); + + final byte scryptLogN = buffer.get(); + assertTrue(scryptLogN >= 0); + + final byte scryptLogR = buffer.get(); + assertTrue(scryptLogR >= 0); + + final byte scryptLogP = buffer.get(); + assertTrue(scryptLogP >= 0); + + final int saltLength = buffer.getInt(); + assertTrue(saltLength > 0); + final byte[] salt = new byte[saltLength]; + buffer.get(salt); + + final int passwordHandleLength = buffer.getInt(); + assertTrue(passwordHandleLength > 0); + final byte[] passwordHandle = new byte[passwordHandleLength]; + buffer.get(passwordHandle); + } + + @Test + public void testFrpBlock_inBadAndroid14FormatIsAutomaticallyFixed() { + mService.setLockCredential(newPin("1234"), nonePassword(), PRIMARY_USER_ID); + + // Write a "bad" FRP block with PasswordData beginning with the bytes [0, 2]. + byte[] badPasswordData = new byte[] { + 0, 2, /* version 2 */ + 0, 3, /* CREDENTIAL_TYPE_PIN */ + 11, /* scryptLogN */ + 22, /* scryptLogR */ + 33, /* scryptLogP */ + 0, 0, 0, 5, /* salt.length */ + 1, 2, -1, -2, 55, /* salt */ + 0, 0, 0, 6, /* passwordHandle.length */ + 2, 3, -2, -3, 44, 1, /* passwordHandle */ + 0, 0, 0, 6, /* pinLength */ + }; + mStorage.writePersistentDataBlock(PersistentData.TYPE_SP_GATEKEEPER, PRIMARY_USER_ID, 0, + badPasswordData); + + // Execute the code that should fix the FRP block. + assertFalse(mStorage.getBoolean("migrated_frp2", false, 0)); + mService.migrateOldDataAfterSystemReady(); + assertTrue(mStorage.getBoolean("migrated_frp2", false, 0)); + + // Verify that the FRP block has been fixed. + PersistentData data = mStorage.readPersistentDataBlock(); + assertEquals(PersistentData.TYPE_SP_GATEKEEPER, data.type); + ByteBuffer buffer = ByteBuffer.wrap(data.payload); + assertEquals(CREDENTIAL_TYPE_PIN, buffer.getInt()); + } } 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 067feae4d36ae..ce0347dbe4acf 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java @@ -638,6 +638,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { 2, 3, -2, -3, 44, 1, /* passwordHandle */ 0, 0, 0, 6, /* pinLength */ }; + assertFalse(PasswordData.isBadFormatFromAndroid14Beta(serialized)); PasswordData deserialized = PasswordData.fromBytes(serialized); assertEquals(11, deserialized.scryptLogN); @@ -690,6 +691,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests { 2, 3, -2, -3, 44, 1, /* passwordHandle */ 0, 0, 0, 6, /* pinLength */ }; + assertTrue(PasswordData.isBadFormatFromAndroid14Beta(serialized)); PasswordData deserialized = PasswordData.fromBytes(serialized); assertEquals(11, deserialized.scryptLogN);