Fix bug where PlatformKeyManager did not save generation ID

This caused a new platform key to be repeatedly generated. Also fix an issue
where you had to have the RECOVER_KEYSTORE permission to check the status of
your own keys. This does not make sense.

Test: adb shell am instrument -w -e package com.android.server.locksettings.recoverablekeystore com.android.frameworks.servicestests/android.support.test.runner.AndroidJUnitRunner
Change-Id: I51aa4e1fe1a96b79bb9b6ae249d29311808134f1

diff --git a/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java b/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java
index 95f5cb7..1719710 100644 (file)
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/PlatformKeyManager.java
@@ -115,16 +115,12 @@ public class PlatformKeyManager {
     /**
      * Returns the current generation ID of the platform key. This increments whenever a platform
      * key has to be replaced. (e.g., because the user has removed and then re-added their lock
-     * screen).
+     * screen). Returns -1 if no key has been generated yet.
      *
      * @hide
      */
     public int getGenerationId() {
-        int generationId = mDatabase.getPlatformKeyGenerationId(mUserId);
-        if (generationId == -1) {
-            return 1;
-        }
-        return generationId;
+        return mDatabase.getPlatformKeyGenerationId(mUserId);
     }
 
     /**
@@ -207,14 +203,22 @@ public class PlatformKeyManager {
                     Locale.US, "Platform key generation %d exists already.", generationId));
             return;
         }
-        if (generationId == 1) {
+        if (generationId == -1) {
             Log.i(TAG, "Generating initial platform ID.");
         } else {
             Log.w(TAG, String.format(Locale.US, "Platform generation ID was %d but no "
                     + "entry was present in AndroidKeyStore. Generating fresh key.", generationId));
         }
 
+        if (generationId == -1) {
+            generationId = 1;
+        } else {
+            // Had to generate a fresh key, bump the generation id
+            generationId++;
+        }
+
         generateAndLoadKey(generationId);
+        mDatabase.setPlatformKeyGenerationId(mUserId, generationId);
     }
 
     /**

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 fe1cad4..eccf241 100644 (file)
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
@@ -216,7 +216,6 @@ public class RecoverableKeyStoreManager {
         // Any application should be able to check status for its own keys.
         // If caller is a recovery agent it can check statuses for other packages, but
         // only for recoverable keys it manages.
-        checkRecoverKeyStorePermission();
         return mDatabase.getStatusForAllKeys(Binder.getCallingUid());
     }
 

diff --git a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/PlatformKeyManagerTest.java b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/PlatformKeyManagerTest.java
index 6f13a98..97fbca2 100644 (file)
--- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/PlatformKeyManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/PlatformKeyManagerTest.java
@@ -205,6 +205,14 @@ public class PlatformKeyManagerTest {
     }
 
     @Test
+    public void init_savesGenerationIdToDatabase() throws Exception {
+        mPlatformKeyManager.init();
+
+        assertEquals(1,
+                mRecoverableKeyStoreDb.getPlatformKeyGenerationId(USER_ID_FIXTURE));
+    }
+
+    @Test
     public void init_setsGenerationIdTo1() throws Exception {
         mPlatformKeyManager.init();
 
@@ -212,7 +220,38 @@ public class PlatformKeyManagerTest {
     }
 
     @Test
+    public void init_incrementsGenerationIdIfKeyIsUnavailable() throws Exception {
+        mPlatformKeyManager.init();
+
+        mPlatformKeyManager.init();
+
+        assertEquals(2, mPlatformKeyManager.getGenerationId());
+    }
+
+    @Test
+    public void init_doesNotIncrementGenerationIdIfKeyAvailable() throws Exception {
+        mPlatformKeyManager.init();
+        when(mKeyStoreProxy
+                .containsAlias("com.android.server.locksettings.recoverablekeystore/"
+                        + "platform/42/1/decrypt")).thenReturn(true);
+        when(mKeyStoreProxy
+                .containsAlias("com.android.server.locksettings.recoverablekeystore/"
+                        + "platform/42/1/encrypt")).thenReturn(true);
+
+        mPlatformKeyManager.init();
+
+        assertEquals(1, mPlatformKeyManager.getGenerationId());
+    }
+
+    @Test
+    public void getGenerationId_returnsMinusOneIfNotInitialized() throws Exception {
+        assertEquals(-1, mPlatformKeyManager.getGenerationId());
+    }
+
+    @Test
     public void getDecryptKey_getsDecryptKeyWithCorrectAlias() throws Exception {
+        mPlatformKeyManager.init();
+
         mPlatformKeyManager.getDecryptKey();
 
         verify(mKeyStoreProxy).getKey(
@@ -222,6 +261,8 @@ public class PlatformKeyManagerTest {
 
     @Test
     public void getEncryptKey_getsDecryptKeyWithCorrectAlias() throws Exception {
+        mPlatformKeyManager.init();
+
         mPlatformKeyManager.getEncryptKey();
 
         verify(mKeyStoreProxy).getKey(