From bbe02ae8a3dd07989d61bbb739bfd863123c5489 Mon Sep 17 00:00:00 2001 From: Robert Berry Date: Tue, 20 Feb 2018 19:47:43 +0000 Subject: [PATCH] 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 --- api/system-current.txt | 3 +- api/system-removed.txt | 8 ++++ .../security/keystore/RecoveryController.java | 5 ++- .../keystore/recovery/RecoveryController.java | 35 +++++++++------- .../com/android/internal/widget/ILockSettings.aidl | 2 +- .../server/locksettings/LockSettingsService.java | 5 +-- .../RecoverableKeyStoreManager.java | 24 ++--------- .../RecoverableKeyStoreManagerTest.java | 47 +--------------------- 8 files changed, 42 insertions(+), 87 deletions(-) diff --git a/api/system-current.txt b/api/system-current.txt index e0c447a1f506..8d5f2568e97d 100644 --- 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 diff --git a/api/system-removed.txt b/api/system-removed.txt index 48f43e0880da..ac40052b7e80 100644 --- 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 { diff --git a/core/java/android/security/keystore/RecoveryController.java b/core/java/android/security/keystore/RecoveryController.java index 98e6a2099012..786d45417f6f 100644 --- a/core/java/android/security/keystore/RecoveryController.java +++ b/core/java/android/security/keystore/RecoveryController.java @@ -47,6 +47,7 @@ import java.util.Map; *

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) { diff --git a/core/java/android/security/keystore/recovery/RecoveryController.java b/core/java/android/security/keystore/recovery/RecoveryController.java index 7cd08f76a47e..d57782c066e1 100644 --- 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) { diff --git a/core/java/com/android/internal/widget/ILockSettings.aidl b/core/java/com/android/internal/widget/ILockSettings.aidl index 732534ccbcbe..17e498ce5e50 100644 --- 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(); diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index 6deff3652944..838aa701ae4a 100644 --- 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 { 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 54607564f25e..35954a54e385 100644 --- 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 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); } /** 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 8bd0df492dee..3c35c5ba80d5 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 @@ -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 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( -- 2.11.0