Merge "Update RecoverableKeyStoreManager methods to throw NullPointerException when null is passed as @NonNull argument." into pi-dev

am: 364dbf1c9e

Change-Id: I6ad01aa0dace567c6c6da67435213bbe8a9e63f3
This commit is contained in:
Dmitry Dementyev
2018-03-23 17:22:26 +00:00
committed by android-build-merger
4 changed files with 78 additions and 27 deletions

View File

@@ -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);
}

View File

@@ -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);

View File

@@ -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);

View File

@@ -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(