From 5a1acefb88a991819c6423ba931a3990491f5b3d Mon Sep 17 00:00:00 2001
From: Robert Berry <robertberry@google.com>
Date: Mon, 26 Mar 2018 14:41:30 +0100
Subject: [PATCH] Fix setRecoverySecretTypes to not always set snapshot pending

Only updates should set snapshot pending. Setting the secret types for
the first time should not set snapshot pending. If it did, then just
initializing the recovery agent would cause a snapshot to be made, even
if it contained no keys. Also, setting the secret types to the same value
as it was previously should not set snapshot pending, for the exact same
reason. If the secret types were to change, however, for some reason,
then a new snapshot must be made, as it may have additional or fewer
layers of protection.

Bug: 74949975
Test: runtest frameworks-services -p \
      com.android.server.locksettings.recoverablekeystore

Change-Id: Ib29d56d5c46e730d9ed457f2d516f84ecb9e53b8
---
 .../RecoverableKeyStoreManager.java           | 22 +++++++++--
 .../RecoverableKeyStoreManagerTest.java       | 37 ++++++++++++++++++-
 2 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
index f0bcb815ebe21..e0487ea28ddb3 100644
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
@@ -382,10 +382,26 @@ public class RecoverableKeyStoreManager {
         Preconditions.checkNotNull(secretTypes, "secretTypes is null");
         int userId = UserHandle.getCallingUserId();
         int uid = Binder.getCallingUid();
-        long updatedRows = mDatabase.setRecoverySecretTypes(userId, uid, secretTypes);
-        if (updatedRows > 0) {
-            mDatabase.setShouldCreateSnapshot(userId, uid, true);
+
+        int[] currentSecretTypes = mDatabase.getRecoverySecretTypes(userId, uid);
+        if (Arrays.equals(secretTypes, currentSecretTypes)) {
+            Log.v(TAG, "Not updating secret types - same as old value.");
+            return;
         }
+
+        long updatedRows = mDatabase.setRecoverySecretTypes(userId, uid, secretTypes);
+        if (updatedRows < 1) {
+            throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR,
+                    "Database error trying to set secret types.");
+        }
+
+        if (currentSecretTypes.length == 0) {
+            Log.i(TAG, "Initialized secret types.");
+            return;
+        }
+
+        Log.i(TAG, "Updated secret types. Snapshot pending.");
+        mDatabase.setShouldCreateSnapshot(userId, uid, true);
     }
 
     /**
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java
index 445e50a4f8198..d4c3381d0ca39 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java
@@ -941,7 +941,7 @@ public class RecoverableKeyStoreManagerTest {
     }
 
     @Test
-    public void setRecoverySecretTypes() throws Exception {
+    public void setRecoverySecretTypes_updatesSecretTypes() throws Exception {
         int[] types1 = new int[]{11, 2000};
         int[] types2 = new int[]{1, 2, 3};
         int[] types3 = new int[]{};
@@ -959,6 +959,41 @@ public class RecoverableKeyStoreManagerTest {
                 types3);
     }
 
+    @Test
+    public void setRecoverySecretTypes_doesNotSetSnapshotPendingIfIniting() throws Exception {
+        int uid = Binder.getCallingUid();
+        int userId = UserHandle.getCallingUserId();
+        int[] secretTypes = new int[] { 101 };
+
+        mRecoverableKeyStoreManager.setRecoverySecretTypes(secretTypes);
+
+        assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse();
+    }
+
+    @Test
+    public void setRecoverySecretTypes_doesNotSetSnapshotPendingIfSettingSameValue()
+            throws Exception {
+        int uid = Binder.getCallingUid();
+        int userId = UserHandle.getCallingUserId();
+        int[] secretTypes = new int[] { 101 };
+
+        mRecoverableKeyStoreManager.setRecoverySecretTypes(secretTypes);
+        mRecoverableKeyStoreManager.setRecoverySecretTypes(secretTypes);
+
+        assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isFalse();
+    }
+
+    @Test
+    public void setRecoverySecretTypes_setsSnapshotPendingIfUpdatingValue() throws Exception {
+        int uid = Binder.getCallingUid();
+        int userId = UserHandle.getCallingUserId();
+
+        mRecoverableKeyStoreManager.setRecoverySecretTypes(new int[] { 101 });
+        mRecoverableKeyStoreManager.setRecoverySecretTypes(new int[] { 102 });
+
+        assertThat(mRecoverableKeyStoreDb.getShouldCreateSnapshot(userId, uid)).isTrue();
+    }
+
     @Test
     public void setRecoverySecretTypes_throwsIfNullTypes() throws Exception {
         try {