Merge "Fix downgrades on device with FRP enabled" into udc-dev

2 files changed, 74 insertions, 44 deletions

diff --git a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java
index 8b8c5f600255..65e7a00ad13f 100644
--- a/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java
+++ b/services/core/java/com/android/server/locksettings/SyntheticPasswordManager.java
@@ -152,9 +152,6 @@ class SyntheticPasswordManager {
     // The security strength of the synthetic password, in bytes
     private static final int SYNTHETIC_PASSWORD_SECURITY_STRENGTH = 256 / 8;
 
-    public static final short PASSWORD_DATA_V1 = 1;
-    public static final short PASSWORD_DATA_V2 = 2;
-
     private static final int PASSWORD_SCRYPT_LOG_N = 11;
     private static final int PASSWORD_SCRYPT_LOG_R = 3;
     private static final int PASSWORD_SCRYPT_LOG_P = 1;
@@ -379,21 +376,18 @@ class SyntheticPasswordManager {
             buffer.put(data, 0, data.length);
             buffer.flip();
 
-          /*
-           * Originally this file did not contain a version number. However, its first field was
-           * 'credentialType' as an 'int'. Since 'credentialType' could only be in the range
-           * [-1, 4] and this file uses big endian byte order, the first two bytes were redundant,
-           * and when interpreted as a 'short' could only contain -1 or 0. Therefore, we've now
-           * reclaimed these two bytes for a 'short' version number and shrunk 'credentialType'
-           * to a 'short'.
-           */
-            short version = buffer.getShort();
-            if (version == ((short) 0) || version == (short) -1) {
-                version = PASSWORD_DATA_V1;
-            } else if (version != PASSWORD_DATA_V2) {
-                throw new IllegalArgumentException("Unknown PasswordData version: " + version);
-            }
-            result.credentialType = buffer.getShort();
+            /*
+             * The serialized PasswordData is supposed to begin with credentialType as an int.
+             * However, all credentialType values fit in a short and the byte order is big endian,
+             * so the first two bytes don't convey any non-redundant information.  For this reason,
+             * temporarily during development of Android 14, the first two bytes were "stolen" from
+             * credentialType to use for a data format version number.
+             *
+             * However, this change was reverted as it was a non-forwards-compatible change.  (See
+             * toBytes() for why this data format needs to be forwards-compatible.)  Therefore,
+             * recover from this misstep by ignoring the first two bytes.
+             */
+            result.credentialType = (short) buffer.getInt();
             result.scryptLogN = buffer.get();
             result.scryptLogR = buffer.get();
             result.scryptLogP = buffer.get();
@@ -407,7 +401,7 @@ class SyntheticPasswordManager {
             } else {
                 result.passwordHandle = null;
             }
-            if (version == PASSWORD_DATA_V2) {
+            if (buffer.remaining() >= Integer.BYTES) {
                 result.pinLength = buffer.getInt();
             } else {
                 result.pinLength = PIN_LENGTH_UNAVAILABLE;
@@ -415,16 +409,25 @@ class SyntheticPasswordManager {
             return result;
         }
 
+        /**
+         * Serializes this PasswordData into a byte array.
+         * <p>
+         * Careful: all changes to the format of the serialized PasswordData must be forwards
+         * compatible.  I.e., older versions of Android must still accept the latest PasswordData.
+         * This is because a serialized PasswordData is stored in the Factory Reset Protection (FRP)
+         * persistent data block.  It's possible that a device has FRP set up on a newer version of
+         * Android, is factory reset, and then is set up with an older version of Android.
+         */
         public byte[] toBytes() {
 
-            ByteBuffer buffer = ByteBuffer.allocate(2 * Short.BYTES + 3 * Byte.BYTES
+            ByteBuffer buffer = ByteBuffer.allocate(Integer.BYTES + 3 * Byte.BYTES
                     + Integer.BYTES + salt.length + Integer.BYTES +
                     (passwordHandle != null ? passwordHandle.length : 0) + Integer.BYTES);
+            // credentialType must fit in a short.  For an explanation, see fromBytes().
             if (credentialType < Short.MIN_VALUE || credentialType > Short.MAX_VALUE) {
                 throw new IllegalArgumentException("Unknown credential type: " + credentialType);
             }
-            buffer.putShort(PASSWORD_DATA_V2);
-            buffer.putShort((short) credentialType);
+            buffer.putInt(credentialType);
             buffer.put(scryptLogN);
             buffer.put(scryptLogR);
             buffer.put(scryptLogP);
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 bfb6b0f1b6c7..067feae4d36a 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/SyntheticPasswordTests.java
@@ -23,6 +23,7 @@ import static android.content.pm.UserInfo.FLAG_PRIMARY;
 import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_NONE;
 import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PASSWORD;
 import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PASSWORD_OR_PIN;
+import static com.android.internal.widget.LockPatternUtils.CREDENTIAL_TYPE_PIN;
 import static com.android.internal.widget.LockPatternUtils.PIN_LENGTH_UNAVAILABLE;
 
 import static org.junit.Assert.assertEquals;
@@ -625,11 +626,9 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests {
     }
 
     @Test
-    public void testPasswordDataV2VersionCredentialTypePin_deserialize() {
-        // Test that we can deserialize existing PasswordData and don't inadvertently change the
-        // wire format.
+    public void testDeserializePasswordData_forPinWithLengthAvailable() {
         byte[] serialized = new byte[] {
-                0, 2, 0, 2, /* CREDENTIAL_TYPE_PASSWORD_OR_PIN */
+                0, 0, 0, 3, /* CREDENTIAL_TYPE_PIN */
                 11, /* scryptLogN */
                 22, /* scryptLogR */
                 33, /* scryptLogP */
@@ -637,25 +636,23 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests {
                 1, 2, -1, -2, 55, /* salt */
                 0, 0, 0, 6, /* passwordHandle.length */
                 2, 3, -2, -3, 44, 1, /* passwordHandle */
-                0, 0, 0, 5, /* pinLength */
+                0, 0, 0, 6, /* pinLength */
         };
         PasswordData deserialized = PasswordData.fromBytes(serialized);
 
         assertEquals(11, deserialized.scryptLogN);
         assertEquals(22, deserialized.scryptLogR);
         assertEquals(33, deserialized.scryptLogP);
-        assertEquals(5, deserialized.pinLength);
-        assertEquals(CREDENTIAL_TYPE_PASSWORD_OR_PIN, deserialized.credentialType);
+        assertEquals(CREDENTIAL_TYPE_PIN, deserialized.credentialType);
         assertArrayEquals(PAYLOAD, deserialized.salt);
         assertArrayEquals(PAYLOAD2, deserialized.passwordHandle);
+        assertEquals(6, deserialized.pinLength);
     }
 
     @Test
-    public void testPasswordDataV2VersionNegativePinLengthNoCredential_deserialize() {
-        // Test that we can deserialize existing PasswordData and don't inadvertently change the
-        // wire format.
+    public void testDeserializePasswordData_forPinWithLengthExplicitlyUnavailable() {
         byte[] serialized = new byte[] {
-                0, 2, -1, -1, /* CREDENTIAL_TYPE_NONE */
+                0, 0, 0, 3, /* CREDENTIAL_TYPE_PIN */
                 11, /* scryptLogN */
                 22, /* scryptLogR */
                 33, /* scryptLogP */
@@ -663,23 +660,52 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests {
                 1, 2, -1, -2, 55, /* salt */
                 0, 0, 0, 6, /* passwordHandle.length */
                 2, 3, -2, -3, 44, 1, /* passwordHandle */
-                -1, -1, -1, -2, /* pinLength */
+                -1, -1, -1, -1, /* pinLength */
         };
         PasswordData deserialized = PasswordData.fromBytes(serialized);
 
         assertEquals(11, deserialized.scryptLogN);
         assertEquals(22, deserialized.scryptLogR);
         assertEquals(33, deserialized.scryptLogP);
-        assertEquals(-2, deserialized.pinLength);
-        assertEquals(CREDENTIAL_TYPE_NONE, deserialized.credentialType);
+        assertEquals(CREDENTIAL_TYPE_PIN, deserialized.credentialType);
         assertArrayEquals(PAYLOAD, deserialized.salt);
         assertArrayEquals(PAYLOAD2, deserialized.passwordHandle);
+        assertEquals(PIN_LENGTH_UNAVAILABLE, deserialized.pinLength);
     }
 
     @Test
-    public void testPasswordDataV1VersionNoCredential_deserialize() {
-        // Test that we can deserialize existing PasswordData and don't inadvertently change the
-        // wire format.
+    public void testDeserializePasswordData_forPinWithVersionNumber() {
+        // Test deserializing a PasswordData that has a version number in the first two bytes.
+        // Files like this were created by some Android 14 beta versions.  This version number was a
+        // mistake and should be ignored by the deserializer.
+        byte[] serialized = 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 */
+        };
+        PasswordData deserialized = PasswordData.fromBytes(serialized);
+
+        assertEquals(11, deserialized.scryptLogN);
+        assertEquals(22, deserialized.scryptLogR);
+        assertEquals(33, deserialized.scryptLogP);
+        assertEquals(CREDENTIAL_TYPE_PIN, deserialized.credentialType);
+        assertArrayEquals(PAYLOAD, deserialized.salt);
+        assertArrayEquals(PAYLOAD2, deserialized.passwordHandle);
+        assertEquals(6, deserialized.pinLength);
+    }
+
+    @Test
+    public void testDeserializePasswordData_forNoneCred() {
+        // Test that a PasswordData that uses CREDENTIAL_TYPE_NONE and lacks the PIN length field
+        // can be deserialized.  Files like this were created by Android 13 and earlier.  Android 14
+        // and later no longer create PasswordData for CREDENTIAL_TYPE_NONE.
         byte[] serialized = new byte[] {
                 -1, -1, -1, -1, /* CREDENTIAL_TYPE_NONE */
                 11, /* scryptLogN */
@@ -695,16 +721,17 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests {
         assertEquals(11, deserialized.scryptLogN);
         assertEquals(22, deserialized.scryptLogR);
         assertEquals(33, deserialized.scryptLogP);
-        assertEquals(PIN_LENGTH_UNAVAILABLE, deserialized.pinLength);
         assertEquals(CREDENTIAL_TYPE_NONE, deserialized.credentialType);
         assertArrayEquals(PAYLOAD, deserialized.salt);
         assertArrayEquals(PAYLOAD2, deserialized.passwordHandle);
+        assertEquals(PIN_LENGTH_UNAVAILABLE, deserialized.pinLength);
     }
 
     @Test
-    public void testPasswordDataV1VersionCredentialTypePin_deserialize() {
-        // Test that we can deserialize existing PasswordData and don't inadvertently change the
-        // wire format.
+    public void testDeserializePasswordData_forPasswordOrPin() {
+        // Test that a PasswordData that uses CREDENTIAL_TYPE_PASSWORD_OR_PIN and lacks the PIN
+        // length field can be deserialized.  Files like this were created by Android 10 and
+        // earlier.  Android 11 eliminated CREDENTIAL_TYPE_PASSWORD_OR_PIN.
         byte[] serialized = new byte[] {
                 0, 0, 0, 2, /* CREDENTIAL_TYPE_PASSWORD_OR_PIN */
                 11, /* scryptLogN */
@@ -720,10 +747,10 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests {
         assertEquals(11, deserialized.scryptLogN);
         assertEquals(22, deserialized.scryptLogR);
         assertEquals(33, deserialized.scryptLogP);
-        assertEquals(PIN_LENGTH_UNAVAILABLE, deserialized.pinLength);
         assertEquals(CREDENTIAL_TYPE_PASSWORD_OR_PIN, deserialized.credentialType);
         assertArrayEquals(PAYLOAD, deserialized.salt);
         assertArrayEquals(PAYLOAD2, deserialized.passwordHandle);
+        assertEquals(PIN_LENGTH_UNAVAILABLE, deserialized.pinLength);
     }
 
     @Test