Remove package name parameter from setRecoveryStatus

Package name is implicit. Recovery agent can only act for the same uid. Bug: 73757432 Test: runtest frameworks-services -p com.android.server.locksettings.recoverablekeystore Change-Id: I45abf4b956fa4e97d981614d9e61295e85d5669e
2018-02-20 19:47:43 +00:00
parent 515f5c20f8
commit bbe02ae8a3
8 changed files with 42 additions and 87 deletions
--- a/api/system-current.txt
+++ b/api/system-current.txt
@@ -4298,10 +4298,9 @@ package android.security.keystore.recovery {
    method public void recoverySecretAvailable(android.security.keystore.recovery.KeyChainProtectionParams) throws android.security.keystore.recovery.InternalRecoveryServiceException;
    method public void removeKey(java.lang.String) throws android.security.keystore.recovery.InternalRecoveryServiceException;
    method public void setRecoverySecretTypes(int[]) throws android.security.keystore.recovery.InternalRecoveryServiceException;
-    method public void setRecoveryStatus(java.lang.String, java.lang.String, int) throws android.security.keystore.recovery.InternalRecoveryServiceException, android.content.pm.PackageManager.NameNotFoundException;
+    method public void setRecoveryStatus(java.lang.String, int) throws android.security.keystore.recovery.InternalRecoveryServiceException;
    method public void setServerParams(byte[]) throws android.security.keystore.recovery.InternalRecoveryServiceException;
    method public void setSnapshotCreatedPendingIntent(android.app.PendingIntent) throws android.security.keystore.recovery.InternalRecoveryServiceException;
-    field public static final int RECOVERY_STATUS_MISSING_ACCOUNT = 2; // 0x2
    field public static final int RECOVERY_STATUS_PERMANENT_FAILURE = 3; // 0x3
    field public static final int RECOVERY_STATUS_SYNCED = 0; // 0x0
    field public static final int RECOVERY_STATUS_SYNC_IN_PROGRESS = 1; // 0x1
--- a/api/system-removed.txt
+++ b/api/system-removed.txt
@@ -91,6 +91,14 @@ package android.os {

 }

+package android.security.keystore.recovery {
+
+  public class RecoveryController {
+    method public deprecated void setRecoveryStatus(java.lang.String, java.lang.String, int) throws android.security.keystore.recovery.InternalRecoveryServiceException, android.content.pm.PackageManager.NameNotFoundException;
+  }
+
+}
+
 package android.service.notification {

  public abstract class NotificationListenerService extends android.app.Service {
--- a/core/java/android/security/keystore/RecoveryController.java
+++ b/core/java/android/security/keystore/RecoveryController.java
@@ -47,6 +47,7 @@ import java.util.Map;
 * <p>A recovery agent requires the privileged permission
 * {@code android.Manifest.permission#RECOVER_KEYSTORE}.
 *
+ * @deprecated Use {@link android.security.keystore.recovery.RecoveryController}.
 * @hide
 */
 public class RecoveryController {
@@ -259,7 +260,9 @@ public class RecoveryController {
            @NonNull String packageName, @Nullable String[] aliases, int status)
            throws NameNotFoundException, InternalRecoveryServiceException {
        try {
-            mBinder.setRecoveryStatus(packageName, aliases, status);
+            for (String alias : aliases) {
+                mBinder.setRecoveryStatus(alias, status);
+            }
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        } catch (ServiceSpecificException e) {
--- a/core/java/android/security/keystore/recovery/RecoveryController.java
+++ b/core/java/android/security/keystore/recovery/RecoveryController.java
@@ -65,8 +65,6 @@ public class RecoveryController {
    public static final int RECOVERY_STATUS_SYNCED = 0;
    /** Waiting for recovery agent to sync the key. */
    public static final int RECOVERY_STATUS_SYNC_IN_PROGRESS = 1;
-    /** Recovery account is not available. */
-    public static final int RECOVERY_STATUS_MISSING_ACCOUNT = 2;
    /** Key cannot be synced. */
    public static final int RECOVERY_STATUS_PERMANENT_FAILURE = 3;

@@ -290,24 +288,33 @@ public class RecoveryController {
    }

    /**
-     * Updates recovery status for given key. It is used to notify keystore that key was
-     * successfully stored on the server or there were an error. Application can check this value
-     * using {@code getRecoveyStatus}.
-     *
-     * @param packageName Application whose recoverable key's status are to be updated.
-     * @param alias Application-specific key alias.
-     * @param status Status specific to recovery agent.
-     * @throws InternalRecoveryServiceException if an unexpected error occurred in the recovery
-     *     service.
+     * @deprecated Use {@link #setRecoveryStatus(String, int)}
+     * @removed
     */
+    @Deprecated
    @RequiresPermission(android.Manifest.permission.RECOVER_KEYSTORE)
    public void setRecoveryStatus(
            @NonNull String packageName, String alias, int status)
            throws NameNotFoundException, InternalRecoveryServiceException {
+        setRecoveryStatus(alias, status);
+    }
+
+    /**
+     * Sets the recovery status for given key. It is used to notify the keystore that the key was
+     * successfully stored on the server or that there was an error. An application can check this
+     * value using {@link #getRecoveryStatus(String, String)}.
+     *
+     * @param alias The alias of the key whose status to set.
+     * @param status The status of the key. One of {@link #RECOVERY_STATUS_SYNCED},
+     *     {@link #RECOVERY_STATUS_SYNC_IN_PROGRESS} or {@link #RECOVERY_STATUS_PERMANENT_FAILURE}.
+     * @throws InternalRecoveryServiceException if an unexpected error occurred in the recovery
+     *     service.
+     */
+    @RequiresPermission(android.Manifest.permission.RECOVER_KEYSTORE)
+    public void setRecoveryStatus(String alias, int status)
+            throws InternalRecoveryServiceException {
        try {
-            // TODO: update aidl
-            String[] aliases = alias == null ? null : new String[]{alias};
-            mBinder.setRecoveryStatus(packageName, aliases, status);
+            mBinder.setRecoveryStatus(alias, status);
        } catch (RemoteException e) {
            throw e.rethrowFromSystemServer();
        } catch (ServiceSpecificException e) {
--- a/core/java/com/android/internal/widget/ILockSettings.aidl
+++ b/core/java/com/android/internal/widget/ILockSettings.aidl
@@ -72,7 +72,7 @@ interface ILockSettings {
    void setSnapshotCreatedPendingIntent(in PendingIntent intent);
    Map getRecoverySnapshotVersions();
    void setServerParams(in byte[] serverParams);
-    void setRecoveryStatus(in String packageName, in String[] aliases, int status);
+    void setRecoveryStatus(in String alias, int status);
    Map getRecoveryStatus(in String packageName);
    void setRecoverySecretTypes(in int[] secretTypes);
    int[] getRecoverySecretTypes();
--- a/services/core/java/com/android/server/locksettings/LockSettingsService.java
+++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java
@@ -2001,9 +2001,8 @@ public class LockSettingsService extends ILockSettings.Stub {
    }

    @Override
-    public void setRecoveryStatus(@NonNull String packageName, @Nullable String[] aliases,
-            int status) throws RemoteException {
-        mRecoverableKeyStoreManager.setRecoveryStatus(packageName, aliases, status);
+    public void setRecoveryStatus(String alias, int status) throws RemoteException {
+        mRecoverableKeyStoreManager.setRecoveryStatus(alias, status);
    }

    public Map getRecoveryStatus(@Nullable String packageName) throws RemoteException {
--- a/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
+++ b/services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java
@@ -277,29 +277,11 @@ public class RecoverableKeyStoreManager {
    }

    /**
-     * Updates recovery status for the application given its {@code packageName}.
-     *
-     * @param packageName which recoverable key statuses will be returned
-     * @param aliases - KeyStore aliases or {@code null} for all aliases of the app
-     * @param status - new status
+     * Sets the recovery status of key with {@code alias} to {@code status}.
     */
-    public void setRecoveryStatus(
-            @NonNull String packageName, @Nullable String[] aliases, int status)
-            throws RemoteException {
+    public void setRecoveryStatus(String alias, int status) throws RemoteException {
        checkRecoverKeyStorePermission();
-        int uid = Binder.getCallingUid();
-        if (packageName != null) {
-            // TODO: get uid for package name, when many apps are supported.
-        }
-        if (aliases == null) {
-            // Get all keys for the app.
-            Map<String, Integer> allKeys = mDatabase.getStatusForAllKeys(uid);
-            aliases = new String[allKeys.size()];
-            allKeys.keySet().toArray(aliases);
-        }
-        for (String alias: aliases) {
-            mDatabase.setRecoveryStatus(uid, alias, status);
-        }
+        mDatabase.setRecoveryStatus(Binder.getCallingUid(), alias, status);
    }

    /**
--- a/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java
@@ -639,7 +639,7 @@ public class RecoverableKeyStoreManagerTest {
    }

    @Test
-    public void setRecoveryStatus_forOneAlias() throws Exception {
+    public void setRecoveryStatus() throws Exception {
        int userId = UserHandle.getCallingUserId();
        int uid = Binder.getCallingUid();
        int status = 100;
@@ -652,55 +652,12 @@ public class RecoverableKeyStoreManagerTest {
        assertThat(statuses).hasSize(1);
        assertThat(statuses).containsEntry(alias, status);

-        mRecoverableKeyStoreManager.setRecoveryStatus(
-                /*packageName=*/ null, new String[] {alias}, status2);
+        mRecoverableKeyStoreManager.setRecoveryStatus(alias, status2);
        statuses = mRecoverableKeyStoreManager.getRecoveryStatus(/*packageName=*/ null);
        assertThat(statuses).hasSize(1);
        assertThat(statuses).containsEntry(alias, status2); // updated
    }

-    @Test
-    public void setRecoveryStatus_for2Aliases() throws Exception {
-        int userId = UserHandle.getCallingUserId();
-        int uid = Binder.getCallingUid();
-        int status = 100;
-        int status2 = 200;
-        int status3 = 300;
-        String alias = "key1";
-        String alias2 = "key2";
-        WrappedKey wrappedKey = new WrappedKey(NONCE, KEY_MATERIAL, GENERATION_ID, status);
-        mRecoverableKeyStoreDb.insertKey(userId, uid, alias, wrappedKey);
-        mRecoverableKeyStoreDb.insertKey(userId, uid, alias2, wrappedKey);
-        Map<String, Integer> statuses =
-                mRecoverableKeyStoreManager.getRecoveryStatus(/*packageName=*/ null);
-        assertThat(statuses).hasSize(2);
-        assertThat(statuses).containsEntry(alias, status);
-        assertThat(statuses).containsEntry(alias2, status);
-
-        mRecoverableKeyStoreManager.setRecoveryStatus(
-                /*packageName=*/ null, /*aliases=*/ null, status2);
-        statuses = mRecoverableKeyStoreManager.getRecoveryStatus(/*packageName=*/ null);
-        assertThat(statuses).hasSize(2);
-        assertThat(statuses).containsEntry(alias, status2); // updated
-        assertThat(statuses).containsEntry(alias2, status2); // updated
-
-        mRecoverableKeyStoreManager.setRecoveryStatus(
-                /*packageName=*/ null, new String[] {alias2}, status3);
-
-        statuses = mRecoverableKeyStoreManager.getRecoveryStatus(/*packageName=*/ null);
-        assertThat(statuses).hasSize(2);
-        assertThat(statuses).containsEntry(alias, status2);
-        assertThat(statuses).containsEntry(alias2, status3); // updated
-
-        mRecoverableKeyStoreManager.setRecoveryStatus(
-                /*packageName=*/ null, new String[] {alias, alias2}, status);
-
-        statuses = mRecoverableKeyStoreManager.getRecoveryStatus(/*packageName=*/ null);
-        assertThat(statuses).hasSize(2);
-        assertThat(statuses).containsEntry(alias, status); // updated
-        assertThat(statuses).containsEntry(alias2, status); // updated
-    }
-
    private static byte[] encryptedApplicationKey(
            SecretKey recoveryKey, byte[] applicationKey) throws Exception {
        return KeySyncUtils.encryptKeysWithRecoveryKey(recoveryKey, ImmutableMap.of(