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

Commit 07b312c8 authored by Dmitry Dementyev's avatar Dmitry Dementyev
Browse files

Use a copy of lock screen secret in recoverablekeystore. 

LockscreenCredential removes secret from memory.
KeySyncTask.run is delayed and has risk of using array with all zeros.

Bug: 159914786
Test: atest com.google.android.gts.recoverablekeystore
Test: atest com.android.server.locksettings.recoverablekeystore
Change-Id: I0ed183eb3a30c16db77cb12b2347794796dd88d8
parent 46404010

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

+6 −2
Original line number Original line 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 userId The uid of the user whose profile has been unlocked.

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

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

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

     * @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 platformKeyManager platform key manager

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

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

     */

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

        mRecoverableKeyStoreDb = recoverableKeyStoreDb;

        mUserId = userId;

        mUserId = userId;

        mCredentialType = credentialType;

        mCredentialType = credentialType;

        mCredential = credential;

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

        mCredentialUpdated = credentialUpdated;

        mCredentialUpdated = credentialUpdated;

        mPlatformKeyManager = platformKeyManager;

        mPlatformKeyManager = platformKeyManager;

        mRecoverySnapshotStorage = snapshotStorage;

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

            }

        } catch (Exception e) {

        } catch (Exception e) {

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

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

        } finally {

            if (mCredential != null) {

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

            }

        }

        }

    }

    }

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

+61 −4
Original line number Original line 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.anyInt;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.atLeast;

import static org.mockito.Mockito.atLeast;

import static org.mockito.Mockito.doAnswer;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;

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

                TestData.getInsecureCertPathForEndpoint1());

        addApplicationKey(TEST_USER_ID, TEST_RECOVERY_AGENT_UID, appKeyAlias);

        addApplicationKey(TEST_USER_ID, TEST_RECOVERY_AGENT_UID, appKeyAlias);





        setExpectedScryptArgument(password.getBytes());



        mKeySyncTask.run();

        mKeySyncTask.run();





        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);

        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);

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

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

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

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

                isEqualTo(UI_FORMAT_PASSWORD);

                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_N), eq(KeySyncTask.SCRYPT_PARAM_R),

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

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

        KeyDerivationParams keyDerivationParams =

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

        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

    @Test

    public void run_useSha256ToHashPatternInProdMode() throws Exception {

    public void run_useSha256ToHashPatternInProdMode() throws Exception {

        String pattern = "123456";

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

        mRecoverableKeyStoreDb.setRecoveryServiceCertPath(

                TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_ROOT_CERT_ALIAS, TestData.CERT_PATH_1);

                TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_ROOT_CERT_ALIAS, TestData.CERT_PATH_1);





        setExpectedScryptArgument(shortPassword.getBytes());



        mKeySyncTask.run();

        mKeySyncTask.run();





        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);

        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);

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

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

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

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

                isEqualTo(UI_FORMAT_PASSWORD);

                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_N), eq(KeySyncTask.SCRYPT_PARAM_R),

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

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

        KeyDerivationParams keyDerivationParams =

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

        when(mSnapshotListenersStorage.hasListener(TEST_RECOVERY_AGENT_UID)).thenReturn(true);

        addApplicationKey(TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_APP_KEY_ALIAS);

        addApplicationKey(TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_APP_KEY_ALIAS);





        setExpectedScryptArgument(password.getBytes());



        mKeySyncTask.run();

        mKeySyncTask.run();





        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);

        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);

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

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

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

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

                isEqualTo(UI_FORMAT_PASSWORD);

                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_N), eq(KeySyncTask.SCRYPT_PARAM_R),

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

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

        when(mSnapshotListenersStorage.hasListener(TEST_RECOVERY_AGENT_UID)).thenReturn(true);

        addApplicationKey(TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_APP_KEY_ALIAS);

        addApplicationKey(TEST_USER_ID, TEST_RECOVERY_AGENT_UID, TEST_APP_KEY_ALIAS);





        setExpectedScryptArgument(pin.getBytes());



        mKeySyncTask.run();

        mKeySyncTask.run();





        KeyChainSnapshot keyChainSnapshot = mRecoverySnapshotStorage.get(TEST_RECOVERY_AGENT_UID);

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

        // Password with only digits is changed to pin.

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

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

                isEqualTo(UI_FORMAT_PIN);

                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_N), eq(KeySyncTask.SCRYPT_PARAM_R),

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

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

    }

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

        new Random().nextBytes(bytes);

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



    }

}
 
}