Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1e6a9dce authored by Dmitry Dementyev's avatar Dmitry Dementyev
Browse files

Update RecoverableKeyStoreManager methods to throw NullPointerException when... 

Update RecoverableKeyStoreManager methods to throw NullPointerException when null is passed as @NonNull argument.

Mark serverParams as nullable. Null value can be used to prevent new
snapshots creation.

Bug: 73959762
Test: Test: adb shell am instrument \
-w -e package com.android.server.locksettings.recoverablekeystore \
com.android.frameworks.servicestests/android.support.test.runner.AndroidJUnitRunner
Change-Id: I5c6ddd696b2882b3d27978b0146ff419bedaf5ee
parent 3a31f6c5

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);
@@ -651,7 +652,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;
@@ -244,11 +245,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 {
@@ -291,11 +292,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();
@@ -325,7 +326,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();
@@ -338,8 +339,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);

    }
@@ -364,6 +366,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);
@@ -495,7 +498,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();
@@ -541,6 +551,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) {
@@ -667,10 +680,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();
@@ -688,6 +704,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();
@@ -726,8 +743,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,
@@ -770,6 +788,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");

        }

    }
@@ -393,7 +393,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");

        }

    }
@@ -405,7 +405,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");

        }

    }
@@ -557,6 +557,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 {
@@ -879,6 +889,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();
@@ -913,6 +933,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(