Merge "Update RecoverableKeyStoreManager methods to throw NullPointerException... (364dbf1c) · Commits · e / os / android_frameworks_base

core/java/android/security/keystore/recovery/RecoveryController.java

+9 −8

Original line number	Diff line number	Diff line
		@@ -266,8 +266,8 @@ public class RecoveryController {

		/**
		* Sets a listener which notifies recovery agent that new recovery snapshot is available. {@link
		* #getRecoveryData} can be used to get the snapshot. Note that every recovery agent can have at
		* most one registered listener at any time.
		* #getKeyChainSnapshot} can be used to get the snapshot. Note that every recovery agent can
		* have at most one registered listener at any time.
		*
		* @param intent triggered when new snapshot is available. Unregisters listener if the value is
		* {@code null}.
		@@ -292,12 +292,13 @@ public class RecoveryController {
		* in vaultParams {@link RecoverySession#start(CertPath, byte[], byte[], List)}.
		*
		* @param serverParams included in recovery key blob.
		* @see #getRecoveryData
		* @see #getKeyChainSnapshot
		* @throws InternalRecoveryServiceException if an unexpected error occurred in the recovery
		* service.
		*/
		@RequiresPermission(android.Manifest.permission.RECOVER_KEYSTORE)
		public void setServerParams(byte[] serverParams) throws InternalRecoveryServiceException {
		public void setServerParams(@NonNull byte[] serverParams)
		throws InternalRecoveryServiceException {
		try {
		mBinder.setServerParams(serverParams);
		} catch (RemoteException e) {
		@@ -321,7 +322,7 @@ public class RecoveryController {
		* Returns a list of aliases of keys belonging to the application.
		*/
		@RequiresPermission(android.Manifest.permission.RECOVER_KEYSTORE)
		public List<String> getAliases() throws InternalRecoveryServiceException {
		public @NonNull List<String> getAliases() throws InternalRecoveryServiceException {
		try {
		Map<String, Integer> allStatuses = mBinder.getRecoveryStatus();
		return new ArrayList<>(allStatuses.keySet());
		@@ -355,7 +356,7 @@ public class RecoveryController {
		* service.
		*/
		@RequiresPermission(android.Manifest.permission.RECOVER_KEYSTORE)
		public void setRecoveryStatus(String alias, int status)
		public void setRecoveryStatus(@NonNull String alias, int status)
		throws InternalRecoveryServiceException {
		try {
		mBinder.setRecoveryStatus(alias, status);
		@@ -390,7 +391,7 @@ public class RecoveryController {
		* service.
		*/
		@RequiresPermission(android.Manifest.permission.RECOVER_KEYSTORE)
		public int getRecoveryStatus(String alias) throws InternalRecoveryServiceException {
		public int getRecoveryStatus(@NonNull String alias) throws InternalRecoveryServiceException {
		try {
		Map<String, Integer> allStatuses = mBinder.getRecoveryStatus();
		Integer status = allStatuses.get(alias);
		@@ -605,7 +606,7 @@ public class RecoveryController {
		* <p>A recovery session is required to restore keys from a remote store.
		*/
		@RequiresPermission(android.Manifest.permission.RECOVER_KEYSTORE)
		public RecoverySession createRecoverySession() {
		public @NonNull RecoverySession createRecoverySession() {
		return RecoverySession.newInstance(this);
		}

core/java/android/security/keystore/recovery/RecoverySession.java

+4 −3

Original line number	Diff line number	Diff line
		@@ -49,7 +49,8 @@ public class RecoverySession implements AutoCloseable {
		private final String mSessionId;
		private final RecoveryController mRecoveryController;

		private RecoverySession(RecoveryController recoveryController, String sessionId) {
		private RecoverySession(@NonNull RecoveryController recoveryController,
		@NonNull String sessionId) {
		mRecoveryController = recoveryController;
		mSessionId = sessionId;
		}
		@@ -58,14 +59,14 @@ public class RecoverySession implements AutoCloseable {
		* A new session, started by the {@link RecoveryController}.
		*/
		@RequiresPermission(android.Manifest.permission.RECOVER_KEYSTORE)
		static RecoverySession newInstance(RecoveryController recoveryController) {
		static @NonNull RecoverySession newInstance(RecoveryController recoveryController) {
		return new RecoverySession(recoveryController, newSessionId());
		}

		/**
		* Returns a new random session ID.
		*/
		private static String newSessionId() {
		private static @NonNull String newSessionId() {
		SecureRandom secureRandom = new SecureRandom();
		byte[] sessionId = new byte[SESSION_ID_LENGTH_BYTES];
		secureRandom.nextBytes(sessionId);

services/core/java/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManager.java

+31 −12

Original line number	Diff line number	Diff line
		@@ -47,6 +47,7 @@ import android.util.Log;

		import com.android.internal.annotations.VisibleForTesting;
		import com.android.internal.util.HexDump;
		import com.android.internal.util.Preconditions;
		import com.android.server.locksettings.recoverablekeystore.certificate.CertUtils;
		import com.android.server.locksettings.recoverablekeystore.certificate.SigXml;
		import com.android.server.locksettings.recoverablekeystore.storage.ApplicationKeyStorage;
		@@ -246,11 +247,11 @@ public class RecoverableKeyStoreManager {
		@NonNull String rootCertificateAlias, @NonNull byte[] recoveryServiceCertFile,
		@NonNull byte[] recoveryServiceSigFile)
		throws RemoteException {
		if (recoveryServiceCertFile == null \|\| recoveryServiceSigFile == null) {
		Log.d(TAG, "The given cert or sig file is null");
		throw new ServiceSpecificException(
		ERROR_BAD_CERTIFICATE_FORMAT, "The given cert or sig file is null.");
		if (rootCertificateAlias == null) {
		Log.e(TAG, "rootCertificateAlias is null");
		}
		Preconditions.checkNotNull(recoveryServiceCertFile, "recoveryServiceCertFile is null");
		Preconditions.checkNotNull(recoveryServiceSigFile, "recoveryServiceSigFile is null");

		SigXml sigXml;
		try {
		@@ -293,11 +294,11 @@ public class RecoverableKeyStoreManager {
		/**
		* Gets all data necessary to recover application keys on new device.
		*
		* @return recovery data
		* @return KeyChain Snapshot.
		* @throws ServiceSpecificException if no snapshot is pending.
		* @hide
		*/
		public @NonNull
		KeyChainSnapshot getKeyChainSnapshot()
		public @NonNull KeyChainSnapshot getKeyChainSnapshot()
		throws RemoteException {
		checkRecoverKeyStorePermission();
		int uid = Binder.getCallingUid();
		@@ -327,7 +328,7 @@ public class RecoverableKeyStoreManager {
		throw new UnsupportedOperationException();
		}

		public void setServerParams(byte[] serverParams) throws RemoteException {
		public void setServerParams(@NonNull byte[] serverParams) throws RemoteException {
		checkRecoverKeyStorePermission();
		int userId = UserHandle.getCallingUserId();
		int uid = Binder.getCallingUid();
		@@ -340,8 +341,9 @@ public class RecoverableKeyStoreManager {
		/**
		* Sets the recovery status of key with {@code alias} to {@code status}.
		*/
		public void setRecoveryStatus(String alias, int status) throws RemoteException {
		public void setRecoveryStatus(@NonNull String alias, int status) throws RemoteException {
		checkRecoverKeyStorePermission();
		Preconditions.checkNotNull(alias, "alias is null");
		mDatabase.setRecoveryStatus(Binder.getCallingUid(), alias, status);
		}

		@@ -366,6 +368,7 @@ public class RecoverableKeyStoreManager {
		@NonNull @KeyChainProtectionParams.UserSecretType int[] secretTypes)
		throws RemoteException {
		checkRecoverKeyStorePermission();
		Preconditions.checkNotNull(secretTypes, "secretTypes is null");
		int userId = UserHandle.getCallingUserId();
		int uid = Binder.getCallingUid();
		long updatedRows = mDatabase.setRecoverySecretTypes(userId, uid, secretTypes);
		@@ -497,7 +500,14 @@ public class RecoverableKeyStoreManager {
		@NonNull List<KeyChainProtectionParams> secrets)
		throws RemoteException {
		checkRecoverKeyStorePermission();

		if (rootCertificateAlias == null) {
		Log.e(TAG, "rootCertificateAlias is null");
		}
		Preconditions.checkNotNull(sessionId, "invalid session");
		Preconditions.checkNotNull(verifierCertPath, "verifierCertPath is null");
		Preconditions.checkNotNull(vaultParams, "vaultParams is null");
		Preconditions.checkNotNull(vaultChallenge, "vaultChallenge is null");
		Preconditions.checkNotNull(secrets, "secrets is null");
		CertPath certPath;
		try {
		certPath = verifierCertPath.getCertPath();
		@@ -543,6 +553,9 @@ public class RecoverableKeyStoreManager {
		@NonNull List<WrappedApplicationKey> applicationKeys)
		throws RemoteException {
		checkRecoverKeyStorePermission();
		Preconditions.checkNotNull(sessionId, "invalid session");
		Preconditions.checkNotNull(encryptedRecoveryKey, "encryptedRecoveryKey is null");
		Preconditions.checkNotNull(applicationKeys, "encryptedRecoveryKey is null");
		int uid = Binder.getCallingUid();
		RecoverySessionStorage.Entry sessionEntry = mRecoverySessionStorage.get(uid, sessionId);
		if (sessionEntry == null) {
		@@ -669,10 +682,13 @@ public class RecoverableKeyStoreManager {
		* Destroys the session with the given {@code sessionId}.
		*/
		public void closeSession(@NonNull String sessionId) throws RemoteException {
		checkRecoverKeyStorePermission();
		Preconditions.checkNotNull(sessionId, "invalid session");
		mRecoverySessionStorage.remove(Binder.getCallingUid(), sessionId);
		}

		public void removeKey(@NonNull String alias) throws RemoteException {
		Preconditions.checkNotNull(alias, "alias is null");
		int uid = Binder.getCallingUid();
		int userId = UserHandle.getCallingUserId();

		@@ -690,6 +706,7 @@ public class RecoverableKeyStoreManager {
		* @return grant alias, which caller can use to access the key.
		*/
		public String generateKey(@NonNull String alias) throws RemoteException {
		Preconditions.checkNotNull(alias, "alias is null");
		int uid = Binder.getCallingUid();
		int userId = UserHandle.getCallingUserId();

		@@ -728,8 +745,9 @@ public class RecoverableKeyStoreManager {
		*/
		public String importKey(@NonNull String alias, @NonNull byte[] keyBytes)
		throws RemoteException {
		if (keyBytes == null \|\|
		keyBytes.length != RecoverableKeyGenerator.KEY_SIZE_BITS / Byte.SIZE) {
		Preconditions.checkNotNull(alias, "alias is null");
		Preconditions.checkNotNull(keyBytes, "keyBytes is null");
		if (keyBytes.length != RecoverableKeyGenerator.KEY_SIZE_BITS / Byte.SIZE) {
		Log.e(TAG, "The given key for import doesn't have the required length "
		+ RecoverableKeyGenerator.KEY_SIZE_BITS);
		throw new ServiceSpecificException(ERROR_INVALID_KEY_FORMAT,
		@@ -772,6 +790,7 @@ public class RecoverableKeyStoreManager {
		* @return grant alias, which caller can use to access the key.
		*/
		public String getKey(@NonNull String alias) throws RemoteException {
		Preconditions.checkNotNull(alias, "alias is null");
		int uid = Binder.getCallingUid();
		int userId = UserHandle.getCallingUserId();
		return getAlias(userId, uid, alias);

services/tests/servicestests/src/com/android/server/locksettings/recoverablekeystore/RecoverableKeyStoreManagerTest.java

+34 −4

Original line number	Diff line number	Diff line
		@@ -242,8 +242,8 @@ public class RecoverableKeyStoreManagerTest {
		try {
		mRecoverableKeyStoreManager.importKey(TEST_ALIAS, /keyBytes=/ null);
		fail("should have thrown");
		} catch (ServiceSpecificException e) {
		assertThat(e.getMessage()).contains("not contain 256 bits");
		} catch (NullPointerException e) {
		assertThat(e.getMessage()).contains("is null");
		}
		}

		@@ -410,7 +410,7 @@ public class RecoverableKeyStoreManagerTest {
		ROOT_CERTIFICATE_ALIAS, /recoveryServiceCertFile=/ null,
		TestData.getSigXml());
		fail("should have thrown");
		} catch (ServiceSpecificException e) {
		} catch (NullPointerException e) {
		assertThat(e.getMessage()).contains("is null");
		}
		}
		@@ -422,7 +422,7 @@ public class RecoverableKeyStoreManagerTest {
		ROOT_CERTIFICATE_ALIAS, TestData.getCertXml(),
		/recoveryServiceSigFile=/ null);
		fail("should have thrown");
		} catch (ServiceSpecificException e) {
		} catch (NullPointerException e) {
		assertThat(e.getMessage()).contains("is null");
		}
		}
		@@ -574,6 +574,16 @@ public class RecoverableKeyStoreManagerTest {
		assertEquals(1, mRecoverySessionStorage.size());
		}

		@Test
		public void closeSession_throwsIfNullSession() throws Exception {
		try {
		mRecoverableKeyStoreManager.closeSession(/sessionId=/ null);
		fail("should have thrown");
		} catch (NullPointerException e) {
		assertThat(e.getMessage()).contains("invalid");
		}
		}

		@Test
		public void startRecoverySession_throwsIfBadNumberOfSecrets() throws Exception {
		try {
		@@ -896,6 +906,16 @@ public class RecoverableKeyStoreManagerTest {
		types3);
		}

		@Test
		public void setRecoverySecretTypes_throwsIfNullTypes() throws Exception {
		try {
		mRecoverableKeyStoreManager.setRecoverySecretTypes(/types=/ null);
		fail("should have thrown");
		} catch (NullPointerException e) {
		assertThat(e.getMessage()).contains("is null");
		}
		}

		@Test
		public void setRecoverySecretTypes_updatesShouldCreateSnapshot() throws Exception {
		int uid = Binder.getCallingUid();
		@@ -930,6 +950,16 @@ public class RecoverableKeyStoreManagerTest {
		assertThat(statuses).containsEntry(alias, status2); // updated
		}

		@Test
		public void setRecoveryStatus_throwsIfNullAlias() throws Exception {
		try {
		mRecoverableKeyStoreManager.setRecoveryStatus(/alias=/ null, /status=/ 100);
		fail("should have thrown");
		} catch (NullPointerException e) {
		assertThat(e.getMessage()).contains("is null");
		}
		}

		private static byte[] encryptedApplicationKey(
		SecretKey recoveryKey, byte[] applicationKey) throws Exception {
		return KeySyncUtils.encryptKeysWithRecoveryKey(recoveryKey, ImmutableMap.of(