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

Commit ec6110bf authored by Eric Biggers's avatar Eric Biggers Committed by Gerrit Code Review
Browse files

Merge changes from topic "locksettings-aosp-5" into main 

* changes:
  Fix system process crash
  Change log priority for ERROR_KEY_DOES_NOT_EXIST in RecoverableKeyStore
  Use a copy of lock screen secret in recoverablekeystore.
  Throw InsecureUserException when LSKF is not set in recoverablekeystore.
parents 5402a800 34a64515

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

+6 −2
Original line number Diff line number Diff line
@@ -123,7 +123,7 @@ public class KeySyncTask implements Runnable { 
     * @param userId The uid of the user whose profile has been unlocked.

     * @param credentialType The type of credential as defined in {@code LockPatternUtils}

     * @param credential The credential, encoded as a byte array

     * @param credentialUpdated signals weather credentials were updated.

     * @param credentialUpdated indicates credentials change.

     * @param platformKeyManager platform key manager

     * @param testOnlyInsecureCertificateHelper utility class used for end-to-end tests

     */
@@ -143,7 +143,7 @@ public class KeySyncTask implements Runnable { 
        mRecoverableKeyStoreDb = recoverableKeyStoreDb;

        mUserId = userId;

        mCredentialType = credentialType;

        mCredential = credential;

        mCredential = credential != null ? Arrays.copyOf(credential, credential.length) : null;

        mCredentialUpdated = credentialUpdated;

        mPlatformKeyManager = platformKeyManager;

        mRecoverySnapshotStorage = snapshotStorage;
@@ -160,6 +160,10 @@ public class KeySyncTask implements Runnable { 
            }

        } catch (Exception e) {

            Log.e(TAG, "Unexpected exception thrown during KeySyncTask", e);

        } finally {

            if (mCredential != null) {

                Arrays.fill(mCredential, (byte) 0); // no longer needed.

            }

        }

    }

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

+32 −17
Original line number Diff line number Diff line
@@ -166,6 +166,7 @@ public class PlatformKeyManager { 
     * @param userId The ID of the user to whose lock screen the platform key must be bound.

     * @throws NoSuchAlgorithmException if AES is unavailable - should never happen.

     * @throws KeyStoreException if there is an error in AndroidKeyStore.

     * @throws InsecureUserException if the user does not have a lock screen set.

     * @throws IOException if there was an issue with local database update.

     * @throws RemoteException if there was an issue communicating with {@link IGateKeeperService}.

     *
@@ -174,7 +175,7 @@ public class PlatformKeyManager { 
    @VisibleForTesting

    void regenerate(int userId)

            throws NoSuchAlgorithmException, KeyStoreException, IOException,

                    RemoteException {

                    RemoteException, InsecureUserException {

        int generationId = getGenerationId(userId);

        int nextId;

        if (generationId == -1) {
@@ -195,13 +196,14 @@ public class PlatformKeyManager { 
     * @throws UnrecoverableKeyException if the key could not be recovered.

     * @throws NoSuchAlgorithmException if AES is unavailable - should never occur.

     * @throws IOException if there was an issue with local database update.

     * @throws InsecureUserException if the user does not have a lock screen set.

     * @throws RemoteException if there was an issue communicating with {@link IGateKeeperService}.

     *

     * @hide

     */

    public PlatformEncryptionKey getEncryptKey(int userId)

            throws KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException,

                    IOException, RemoteException {

                    IOException, RemoteException, InsecureUserException {

        init(userId);

        try {

            // Try to see if the decryption key is still accessible before using the encryption key.
@@ -254,7 +256,7 @@ public class PlatformKeyManager { 
     */

    public PlatformDecryptionKey getDecryptKey(int userId)

            throws KeyStoreException, UnrecoverableKeyException, NoSuchAlgorithmException,

                    IOException, RemoteException {

                    IOException, InsecureUserException, RemoteException {

        init(userId);

        try {

            PlatformDecryptionKey decryptionKey = getDecryptKeyInternal(userId);
@@ -328,7 +330,7 @@ public class PlatformKeyManager { 
     */

    void init(int userId)

            throws KeyStoreException, NoSuchAlgorithmException, IOException,

                    RemoteException {

                    RemoteException, InsecureUserException {

        int generationId = getGenerationId(userId);

        if (isKeyLoaded(userId, generationId)) {

            Log.i(TAG, String.format(
@@ -414,7 +416,8 @@ public class PlatformKeyManager { 
     * @throws RemoteException if there was an issue communicating with {@link IGateKeeperService}.

     */

    private void generateAndLoadKey(int userId, int generationId)

            throws NoSuchAlgorithmException, KeyStoreException, IOException, RemoteException {

            throws NoSuchAlgorithmException, KeyStoreException, IOException, RemoteException,

                InsecureUserException {

        String encryptAlias = getEncryptAlias(userId, generationId);

        String decryptAlias = getDecryptAlias(userId, generationId);

        // SecretKey implementation doesn't provide reliable way to destroy the secret
@@ -427,11 +430,13 @@ public class PlatformKeyManager { 
                    .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE);

        // Skip UserAuthenticationRequired for main user

        if (userId ==  UserHandle.USER_SYSTEM) {

            // attempt to store key will fail if screenlock is not set.

            decryptionKeyProtection.setUnlockedDeviceRequired(true);

        } else {

            // Don't set protection params to prevent losing key.

        }

        // Store decryption key first since it is more likely to fail.

        try {

            mKeyStore.setEntry(

                    decryptAlias,

                    new KeyStore.SecretKeyEntry(secretKey),
@@ -443,7 +448,13 @@ public class PlatformKeyManager { 
                        .setBlockModes(KeyProperties.BLOCK_MODE_GCM)

                        .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_NONE)

                        .build());



        } catch (KeyStoreException e) {

            if (!isDeviceSecure(userId)) {

                throw new InsecureUserException("Screenlock is not set");

            } else {

                throw e;

            }

        }

        setGenerationId(userId, generationId);

    }
@@ -477,4 +488,8 @@ public class PlatformKeyManager { 
        return keyStore;

    }



    private boolean isDeviceSecure(int userId) {

        return mContext.getSystemService(KeyguardManager.class).isDeviceSecure(userId);

    }



}

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

+11 −2
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package com.android.server.locksettings.recoverablekeystore; 
import static android.security.keystore.recovery.RecoveryController.ERROR_BAD_CERTIFICATE_FORMAT;

import static android.security.keystore.recovery.RecoveryController.ERROR_DECRYPTION_FAILED;

import static android.security.keystore.recovery.RecoveryController.ERROR_DOWNGRADE_CERTIFICATE;

import static android.security.keystore.recovery.RecoveryController.ERROR_INSECURE_USER;

import static android.security.keystore.recovery.RecoveryController.ERROR_INVALID_CERTIFICATE;

import static android.security.keystore.recovery.RecoveryController.ERROR_INVALID_KEY_FORMAT;

import static android.security.keystore.recovery.RecoveryController.ERROR_NO_SNAPSHOT_PENDING;
@@ -194,8 +195,12 @@ public class RecoverableKeyStoreManager { 
        mApplicationKeyStorage = applicationKeyStorage;

        mTestCertHelper = testOnlyInsecureCertificateHelper;

        mCleanupManager = cleanupManager;

        try {

            // Clears data for removed users.

            mCleanupManager.verifyKnownUsers();

        } catch (Exception e) {

            Log.e(TAG, "Failed to verify known users", e);

        }

        try {

            mRecoverableKeyGenerator = RecoverableKeyGenerator.newInstance(mDatabase);

        } catch (NoSuchAlgorithmException e) {
@@ -750,6 +755,8 @@ public class RecoverableKeyStoreManager { 
            throw new RuntimeException(e);

        } catch (KeyStoreException | UnrecoverableKeyException | IOException e) {

            throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR, e.getMessage());

        } catch (InsecureUserException e) {

            throw new ServiceSpecificException(ERROR_INSECURE_USER, e.getMessage());

        }



        try {
@@ -817,6 +824,8 @@ public class RecoverableKeyStoreManager { 
            throw new RuntimeException(e);

        } catch (KeyStoreException | UnrecoverableKeyException | IOException e) {

            throw new ServiceSpecificException(ERROR_SERVICE_INTERNAL_ERROR, e.getMessage());

        } catch (InsecureUserException e) {

            throw new ServiceSpecificException(ERROR_INSECURE_USER, e.getMessage());

        }



        try {

services/core/java/com/android/server/locksettings/recoverablekeystore/storage/ApplicationKeyStorage.java

+1 −1
Original line number Diff line number Diff line
@@ -138,7 +138,7 @@ public class ApplicationKeyStorage { 
        } catch (android.security.KeyStoreException e) {

            if (e.getNumericErrorCode()

                    == android.security.KeyStoreException.ERROR_KEY_DOES_NOT_EXIST) {

                Log.e(TAG, "Failed to get grant for KeyStore key - key not found", e);

                Log.w(TAG, "Failed to get grant for KeyStore key - key not found");

                throw new ServiceSpecificException(ERROR_KEY_NOT_FOUND, e.getMessage());

            }

            Log.e(TAG, "Failed to get grant for KeyStore key.", e);

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

+61 −4
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import static org.mockito.ArgumentMatchers.any; 
import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.atLeast;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;
@@ -297,13 +298,15 @@ public class KeySyncTaskTest { 
                TestData.getInsecureCertPathForEndpoint1());

        addApplicationKey(TEST_USER_ID, TEST_RECOVERY_AGENT_UID, appKeyAlias);



        setExpectedScryptArgument(password.getBytes());



        mKeySyncTask.run();



        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);

        assertThat(keyChainSnapshot.getKeyChainProtectionParams()).hasSize(1);

        assertThat(keyChainSnapshot.getKeyChainProtectionParams().get(0).getLockScreenUiFormat()).

                isEqualTo(UI_FORMAT_PASSWORD);

        verify(mMockScrypt).scrypt(eq(password.getBytes()), any(),

        verify(mMockScrypt).scrypt(any(), any(),

                eq(KeySyncTask.SCRYPT_PARAM_N), eq(KeySyncTask.SCRYPT_PARAM_R),

                eq(KeySyncTask.SCRYPT_PARAM_P), eq(KeySyncTask.SCRYPT_PARAM_OUTLEN_BYTES));

        KeyDerivationParams keyDerivationParams =
@@ -313,6 +316,44 @@ public class KeySyncTaskTest { 
        assertThat(keyDerivationParams.getMemoryDifficulty()).isEqualTo(KeySyncTask.SCRYPT_PARAM_N);

    }



    @Test

    public void run_zeroizedCredential() throws Exception {

        String password = TrustedRootCertificates.INSECURE_PASSWORD_PREFIX + "123";

        String appKeyAlias = TrustedRootCertificates.INSECURE_KEY_ALIAS_PREFIX + "alias";

        byte[] zeroizedCredential = password.getBytes();

        mKeySyncTask = new KeySyncTask(

                mRecoverableKeyStoreDb,

                mRecoverySnapshotStorage,

                mSnapshotListenersStorage,

                TEST_USER_ID,

                CREDENTIAL_TYPE_PASSWORD,

                /*credential=*/ zeroizedCredential,

                /*credentialUpdated=*/ false,

                mPlatformKeyManager,

                mTestOnlyInsecureCertificateHelper,

                mMockScrypt);

        mRecoverableKeyStoreDb.setServerParams(

                TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_VAULT_HANDLE);

        mRecoverableKeyStoreDb.setPlatformKeyGenerationId(TEST_USER_ID, TEST_GENERATION_ID);

        mRecoverableKeyStoreDb.setActiveRootOfTrust(TEST_USER_ID, TEST_RECOVERY_AGENT_UID,

                TrustedRootCertificates.TEST_ONLY_INSECURE_CERTIFICATE_ALIAS);

        mRecoverableKeyStoreDb.setRecoveryServiceCertPath(

                TEST_USER_ID, TEST_RECOVERY_AGENT_UID,

                TrustedRootCertificates.TEST_ONLY_INSECURE_CERTIFICATE_ALIAS,

                TestData.getInsecureCertPathForEndpoint1());

        addApplicationKey(TEST_USER_ID, TEST_RECOVERY_AGENT_UID, appKeyAlias);



        // Need to check array value during method call since it is modified later.

        setExpectedScryptArgument(password.getBytes());



        Arrays.fill(zeroizedCredential, (byte) 0);

        mKeySyncTask.run();



        verify(mMockScrypt).scrypt(any(), any(),

                eq(KeySyncTask.SCRYPT_PARAM_N), eq(KeySyncTask.SCRYPT_PARAM_R),

                eq(KeySyncTask.SCRYPT_PARAM_P), eq(KeySyncTask.SCRYPT_PARAM_OUTLEN_BYTES));

    }



    @Test

    public void run_useSha256ToHashPatternInProdMode() throws Exception {

        String pattern = "123456";
@@ -368,13 +409,15 @@ public class KeySyncTaskTest { 
        mRecoverableKeyStoreDb.setRecoveryServiceCertPath(

                TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_ROOT_CERT_ALIAS, TestData.CERT_PATH_1);



        setExpectedScryptArgument(shortPassword.getBytes());



        mKeySyncTask.run();



        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);

        assertThat(keyChainSnapshot.getKeyChainProtectionParams()).hasSize(1);

        assertThat(keyChainSnapshot.getKeyChainProtectionParams().get(0).getLockScreenUiFormat()).

                isEqualTo(UI_FORMAT_PASSWORD);

        verify(mMockScrypt).scrypt(eq(shortPassword.getBytes()), any(),

        verify(mMockScrypt).scrypt(any(), any(),

                eq(KeySyncTask.SCRYPT_PARAM_N), eq(KeySyncTask.SCRYPT_PARAM_R),

                eq(KeySyncTask.SCRYPT_PARAM_P), eq(KeySyncTask.SCRYPT_PARAM_OUTLEN_BYTES));

        KeyDerivationParams keyDerivationParams =
@@ -650,13 +693,15 @@ public class KeySyncTaskTest { 
        when(mSnapshotListenersStorage.hasListener(TEST_RECOVERY_AGENT_UID)).thenReturn(true);

        addApplicationKey(TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_APP_KEY_ALIAS);



        setExpectedScryptArgument(password.getBytes());



        mKeySyncTask.run();



        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);

        assertThat(keyChainSnapshot.getKeyChainProtectionParams()).hasSize(1);

        assertThat(keyChainSnapshot.getKeyChainProtectionParams().get(0).getLockScreenUiFormat()).

                isEqualTo(UI_FORMAT_PASSWORD);

        verify(mMockScrypt).scrypt(eq(password.getBytes()), any(),

        verify(mMockScrypt).scrypt(any(), any(),

                eq(KeySyncTask.SCRYPT_PARAM_N), eq(KeySyncTask.SCRYPT_PARAM_R),

                eq(KeySyncTask.SCRYPT_PARAM_P), eq(KeySyncTask.SCRYPT_PARAM_OUTLEN_BYTES));

    }
@@ -681,6 +726,8 @@ public class KeySyncTaskTest { 
        when(mSnapshotListenersStorage.hasListener(TEST_RECOVERY_AGENT_UID)).thenReturn(true);

        addApplicationKey(TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_APP_KEY_ALIAS);



        setExpectedScryptArgument(pin.getBytes());



        mKeySyncTask.run();



        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);
@@ -688,7 +735,7 @@ public class KeySyncTaskTest { 
        // Password with only digits is changed to pin.

        assertThat(keyChainSnapshot.getKeyChainProtectionParams().get(0).getLockScreenUiFormat()).

                isEqualTo(UI_FORMAT_PIN);

        verify(mMockScrypt).scrypt(eq(pin.getBytes()), any(),

        verify(mMockScrypt).scrypt(any(), any(),

                eq(KeySyncTask.SCRYPT_PARAM_N), eq(KeySyncTask.SCRYPT_PARAM_R),

                eq(KeySyncTask.SCRYPT_PARAM_P), eq(KeySyncTask.SCRYPT_PARAM_OUTLEN_BYTES));

    }
@@ -868,4 +915,14 @@ public class KeySyncTaskTest { 
        new Random().nextBytes(bytes);

        return bytes;

    }



    private void setExpectedScryptArgument(byte[] credentials) {

        doAnswer(invocation -> {

            assertThat((byte[]) invocation.getArguments()[0]).isEqualTo(credentials);

            return invocation.callRealMethod();

        }).when(mMockScrypt).scrypt(any(), any(),

                eq(KeySyncTask.SCRYPT_PARAM_N), eq(KeySyncTask.SCRYPT_PARAM_R),

                eq(KeySyncTask.SCRYPT_PARAM_P), eq(KeySyncTask.SCRYPT_PARAM_OUTLEN_BYTES));



    }

}