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

Commit 0038733c authored by Eric Biggers's avatar Eric Biggers Committed by Android (Google) Code Review
Browse files

Merge "Make recoverablekeystore use LockscreenCredential instead of (type, bytes)" into main

parents 4f22799e 7defb8b5
Loading
Loading
Loading
Loading
+2 −6
Original line number Diff line number Diff line
@@ -1783,8 +1783,7 @@ public class LockSettingsService extends ILockSettings.Stub {

        // Send credentials for the user and any child profiles that share its lock screen.
        for (int profileId : getProfilesWithSameLockScreen(userId)) {
            mRecoverableKeyStoreManager.lockScreenSecretAvailable(
                    credential.getType(), credential.getCredential(), profileId);
            mRecoverableKeyStoreManager.lockScreenSecretAvailable(credential, profileId);
        }
    }

@@ -1801,12 +1800,9 @@ public class LockSettingsService extends ILockSettings.Stub {
            return;
        }

        // RecoverableKeyStoreManager expects null for empty credential.
        final byte[] secret = credential.isNone() ? null : credential.getCredential();
        // Send credentials for the user and any child profiles that share its lock screen.
        for (int profileId : getProfilesWithSameLockScreen(userId)) {
            mRecoverableKeyStoreManager.lockScreenSecretChanged(
                    credential.getType(), secret, profileId);
            mRecoverableKeyStoreManager.lockScreenSecretChanged(credential, profileId);
        }
    }

+27 −30
Original line number Diff line number Diff line
@@ -32,6 +32,7 @@ import android.util.Pair;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.ArrayUtils;
import com.android.internal.widget.LockPatternUtils;
import com.android.internal.widget.LockscreenCredential;
import com.android.server.locksettings.recoverablekeystore.storage.RecoverableKeyStoreDb;
import com.android.server.locksettings.recoverablekeystore.storage.RecoverySnapshotStorage;

@@ -50,7 +51,6 @@ import java.security.UnrecoverableKeyException;
import java.security.cert.CertPath;
import java.security.cert.CertificateException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;

@@ -84,8 +84,7 @@ public class KeySyncTask implements Runnable {

    private final RecoverableKeyStoreDb mRecoverableKeyStoreDb;
    private final int mUserId;
    private final int mCredentialType;
    private final byte[] mCredential;
    private final LockscreenCredential mCredential;
    private final boolean mCredentialUpdated;
    private final PlatformKeyManager mPlatformKeyManager;
    private final RecoverySnapshotStorage mRecoverySnapshotStorage;
@@ -99,8 +98,7 @@ public class KeySyncTask implements Runnable {
            RecoverySnapshotStorage snapshotStorage,
            RecoverySnapshotListenersStorage recoverySnapshotListenersStorage,
            int userId,
            int credentialType,
            byte[] credential,
            LockscreenCredential credential,
            boolean credentialUpdated
    ) throws NoSuchAlgorithmException, KeyStoreException, InsecureUserException {
        return new KeySyncTask(
@@ -108,7 +106,6 @@ public class KeySyncTask implements Runnable {
                snapshotStorage,
                recoverySnapshotListenersStorage,
                userId,
                credentialType,
                credential,
                credentialUpdated,
                PlatformKeyManager.getInstance(context, recoverableKeyStoreDb),
@@ -121,8 +118,7 @@ public class KeySyncTask implements Runnable {
     *
     * @param recoverableKeyStoreDb Database where the keys are stored.
     * @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 credential The lockscreen credential
     * @param credentialUpdated indicates credentials change.
     * @param platformKeyManager platform key manager
     * @param testOnlyInsecureCertificateHelper utility class used for end-to-end tests
@@ -133,8 +129,7 @@ public class KeySyncTask implements Runnable {
            RecoverySnapshotStorage snapshotStorage,
            RecoverySnapshotListenersStorage recoverySnapshotListenersStorage,
            int userId,
            int credentialType,
            byte[] credential,
            LockscreenCredential credential,
            boolean credentialUpdated,
            PlatformKeyManager platformKeyManager,
            TestOnlyInsecureCertificateHelper testOnlyInsecureCertificateHelper,
@@ -142,8 +137,9 @@ public class KeySyncTask implements Runnable {
        mSnapshotListenersStorage = recoverySnapshotListenersStorage;
        mRecoverableKeyStoreDb = recoverableKeyStoreDb;
        mUserId = userId;
        mCredentialType = credentialType;
        mCredential = credential != null ? Arrays.copyOf(credential, credential.length) : null;
        // This is an async task, so duplicate the credential in case the calling code later
        // zeroizes it. KeySyncTask zeroizes the duplicate copy when it finishes.
        mCredential = credential.duplicate();
        mCredentialUpdated = credentialUpdated;
        mPlatformKeyManager = platformKeyManager;
        mRecoverySnapshotStorage = snapshotStorage;
@@ -161,14 +157,13 @@ public class KeySyncTask implements Runnable {
        } catch (Exception e) {
            Log.e(TAG, "Unexpected exception thrown during KeySyncTask", e);
        } finally {
            if (mCredential != null) {
                ArrayUtils.zeroize(mCredential); // no longer needed.
            }
            mCredential.zeroize(); // no longer needed
        }
    }

    private void syncKeys() throws RemoteException {
        if (mCredential != null && mCredential.length >= 80) {
    @VisibleForTesting
    void syncKeys() throws RemoteException {
        if (mCredential.size() >= 80) {
            // The value is likely a randomly generated profile password
            // It doesn't match string typed by the user.
            Log.e(TAG, "Unexpected credential length for user " + mUserId);
@@ -177,7 +172,7 @@ public class KeySyncTask implements Runnable {
            mRecoverableKeyStoreDb.setBadRemoteGuessCounter(mUserId, 0);
        }
        int generation = mPlatformKeyManager.getGenerationId(mUserId);
        if (mCredentialType == LockPatternUtils.CREDENTIAL_TYPE_NONE) {
        if (mCredential.isNone()) {
            // Application keys for the user will not be available for sync.
            Log.w(TAG, "Credentials are not set for user " + mUserId);
            if (generation < PlatformKeyManager.MIN_GENERATION_ID_FOR_UNLOCKED_DEVICE_REQUIRED) {
@@ -187,7 +182,11 @@ public class KeySyncTask implements Runnable {
            return;
        }
        if (isCustomLockScreen()) {
            Log.w(TAG, "Unsupported credential type " + mCredentialType + " for user " + mUserId);
            // This code is unreachable in AOSP, since LockscreenCredential can be constructed only
            // with a known credential type. Keep it here in case a customization adds a custom
            // credential type.
            final int type = mCredential.getType();
            Log.w(TAG, "Unsupported credential type " + type + " for user " + mUserId);
            // Keys will be synced when user starts using non custom screen lock.
            if (generation < PlatformKeyManager.MIN_GENERATION_ID_FOR_UNLOCKED_DEVICE_REQUIRED) {
                mRecoverableKeyStoreDb.invalidateKeysForUserIdOnCustomScreenLock(mUserId);
@@ -213,10 +212,10 @@ public class KeySyncTask implements Runnable {
    }

    private boolean isCustomLockScreen() {
        return mCredentialType != LockPatternUtils.CREDENTIAL_TYPE_NONE
            && mCredentialType != LockPatternUtils.CREDENTIAL_TYPE_PATTERN
            && mCredentialType != LockPatternUtils.CREDENTIAL_TYPE_PIN
            && mCredentialType != LockPatternUtils.CREDENTIAL_TYPE_PASSWORD;
        return !mCredential.isNone()
                && !mCredential.isPattern()
                && !mCredential.isPin()
                && !mCredential.isPassword();
    }

    private void syncKeysForAgent(int recoveryAgentUid) throws IOException, RemoteException {
@@ -263,8 +262,7 @@ public class KeySyncTask implements Runnable {
        if (mTestOnlyInsecureCertificateHelper.isTestOnlyCertificateAlias(rootCertAlias)) {
            Log.w(TAG, "Insecure root certificate is used by recovery agent "
                    + recoveryAgentUid);
            if (mTestOnlyInsecureCertificateHelper.doesCredentialSupportInsecureMode(
                    mCredentialType, mCredential)) {
            if (mTestOnlyInsecureCertificateHelper.doesCredentialSupportInsecureMode(mCredential)) {
                Log.w(TAG, "Whitelisted credential is used to generate snapshot by "
                        + "recovery agent "+ recoveryAgentUid);
            } else {
@@ -278,9 +276,9 @@ public class KeySyncTask implements Runnable {
        byte[] salt = generateSalt();
        byte[] localLskfHash;
        if (useScryptToHashCredential) {
            localLskfHash = hashCredentialsByScrypt(salt, mCredential);
            localLskfHash = hashCredentialsByScrypt(salt, mCredential.getCredential());
        } else {
            localLskfHash = hashCredentialsBySaltedSha256(salt, mCredential);
            localLskfHash = hashCredentialsBySaltedSha256(salt, mCredential.getCredential());
        }

        Map<String, Pair<SecretKey, byte[]>> rawKeysWithMetadata;
@@ -368,7 +366,7 @@ public class KeySyncTask implements Runnable {
        }
        KeyChainProtectionParams keyChainProtectionParams = new KeyChainProtectionParams.Builder()
                .setUserSecretType(TYPE_LOCKSCREEN)
                .setLockScreenUiFormat(getUiFormat(mCredentialType))
                .setLockScreenUiFormat(getUiFormat(mCredential.getType()))
                .setKeyDerivationParams(keyDerivationParams)
                .setSecret(new byte[0])
                .build();
@@ -546,7 +544,6 @@ public class KeySyncTask implements Runnable {
    }

    private boolean shouldUseScryptToHashCredential() {
        return mCredentialType == LockPatternUtils.CREDENTIAL_TYPE_PASSWORD
                || mCredentialType == LockPatternUtils.CREDENTIAL_TYPE_PIN;
        return mCredential.isPassword() || mCredential.isPin();
    }
}
+4 −12
Original line number Diff line number Diff line
@@ -941,13 +941,11 @@ public class RecoverableKeyStoreManager {
    /**
     * This function can only be used inside LockSettingsService.
     *
     * @param credentialType the type of credential, as defined in {@code LockPatternUtils}
     * @param credential the credential, encoded as a byte array
     * @param credential the lockscreen credential (not CREDENTIAL_TYPE_NONE)
     * @param userId the ID of the user to whom the credential belongs
     * @hide
     */
    public void lockScreenSecretAvailable(
            int credentialType, @NonNull byte[] credential, int userId) {
    public void lockScreenSecretAvailable(@NonNull LockscreenCredential credential, int userId) {
        // So as not to block the critical path unlocking the phone, defer to another thread.
        try {
            mExecutorService.schedule(KeySyncTask.newInstance(
@@ -956,7 +954,6 @@ public class RecoverableKeyStoreManager {
                    mSnapshotStorage,
                    mListenersStorage,
                    userId,
                    credentialType,
                    credential,
                    /*credentialUpdated=*/ false),
                    SYNC_DELAY_MILLIS,
@@ -974,15 +971,11 @@ public class RecoverableKeyStoreManager {
    /**
     * This function can only be used inside LockSettingsService.
     *
     * @param credentialType the type of the new credential, as defined in {@code LockPatternUtils}
     * @param credential the new credential, encoded as a byte array
     * @param credential the new lockscreen credential (possibly CREDENTIAL_TYPE_NONE)
     * @param userId the ID of the user whose credential was changed
     * @hide
     */
    public void lockScreenSecretChanged(
            int credentialType,
            @Nullable byte[] credential,
            int userId) {
    public void lockScreenSecretChanged(@NonNull LockscreenCredential credential, int userId) {
        // So as not to block the critical path unlocking the phone, defer to another thread.
        try {
            mExecutorService.schedule(KeySyncTask.newInstance(
@@ -991,7 +984,6 @@ public class RecoverableKeyStoreManager {
                    mSnapshotStorage,
                    mListenersStorage,
                    userId,
                    credentialType,
                    credential,
                    /*credentialUpdated=*/ true),
                    SYNC_DELAY_MILLIS,
+8 −9
Original line number Diff line number Diff line
@@ -26,7 +26,7 @@ import android.security.keystore.recovery.TrustedRootCertificates;
import android.util.Log;
import android.util.Pair;

import com.android.internal.widget.LockPatternUtils;
import com.android.internal.widget.LockscreenCredential;

import java.security.cert.X509Certificate;
import java.util.Date;
@@ -101,26 +101,25 @@ public class TestOnlyInsecureCertificateHelper {
    }

    /**
     * Checks whether a password is in "Insecure mode"
     * @param credentialType the type of credential, e.g. pattern and password
     * @param credential the pattern or password
     * Checks whether a lockscreen credential is in "Insecure mode"
     * @param credential the lockscreen credential
     * @return true, if the credential is in "Insecure mode"
     */
    public boolean doesCredentialSupportInsecureMode(int credentialType, byte[] credential) {
    public boolean doesCredentialSupportInsecureMode(LockscreenCredential credential) {
        if (credential == null) {
            return false;
        }
        if (credentialType != LockPatternUtils.CREDENTIAL_TYPE_PASSWORD
                && credentialType != LockPatternUtils.CREDENTIAL_TYPE_PIN) {
        if (!credential.isPassword() && !credential.isPin()) {
            return false;
        }
        byte[] insecurePasswordPrefixBytes =
                TrustedRootCertificates.INSECURE_PASSWORD_PREFIX.getBytes();
        if (credential.length < insecurePasswordPrefixBytes.length) {
        if (credential.size() < insecurePasswordPrefixBytes.length) {
            return false;
        }
        byte[] credentialBytes = credential.getCredential();
        for (int i = 0; i < insecurePasswordPrefixBytes.length; i++) {
            if (credential[i] != insecurePasswordPrefixBytes[i]) {
            if (credentialBytes[i] != insecurePasswordPrefixBytes[i]) {
                return false;
            }
        }
+27 −37
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.AdditionalMatchers.not;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
@@ -253,10 +254,9 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {

    @Test
    public void testSetLockCredential_forPrimaryUser_sendsCredentials() throws Exception {
        setCredential(PRIMARY_USER_ID, newPassword("password"));
        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretChanged(CREDENTIAL_TYPE_PASSWORD, "password".getBytes(),
                        PRIMARY_USER_ID);
        LockscreenCredential password = newPassword("password");
        setCredential(PRIMARY_USER_ID, password);
        verify(mRecoverableKeyStoreManager).lockScreenSecretChanged(password, PRIMARY_USER_ID);
    }

    @Test
@@ -280,21 +280,21 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {
    @Test
    public void testSetLockCredential_forProfileWithSeparateChallenge_sendsCredentials()
            throws Exception {
        setCredential(MANAGED_PROFILE_USER_ID, newPattern("12345"));
        LockscreenCredential pattern = newPattern("12345");
        setCredential(MANAGED_PROFILE_USER_ID, pattern);
        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretChanged(CREDENTIAL_TYPE_PATTERN, "12345".getBytes(),
                        MANAGED_PROFILE_USER_ID);
                .lockScreenSecretChanged(pattern, MANAGED_PROFILE_USER_ID);
    }

    @Test
    public void testSetLockCredential_forProfileWithSeparateChallenge_updatesCredentials()
            throws Exception {
        LockscreenCredential cred1 = newPattern("12345");
        LockscreenCredential cred2 = newPassword("newPassword");
        mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, true, null);
        setCredential(MANAGED_PROFILE_USER_ID, newPattern("12345"));
        setCredential(MANAGED_PROFILE_USER_ID, newPassword("newPassword"), newPattern("12345"));
        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretChanged(CREDENTIAL_TYPE_PASSWORD, "newPassword".getBytes(),
                        MANAGED_PROFILE_USER_ID);
        setCredential(MANAGED_PROFILE_USER_ID, cred1);
        setCredential(MANAGED_PROFILE_USER_ID, cred2, cred1);
        verify(mRecoverableKeyStoreManager).lockScreenSecretChanged(cred2, MANAGED_PROFILE_USER_ID);
    }

    @Test
@@ -310,11 +310,11 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {
    @Test
    public void testSetLockCredential_forProfileWithUnifiedChallenge_doesNotSendRandomCredential()
            throws Exception {
        LockscreenCredential pattern = newPattern("12345");
        mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, false, null);
        setCredential(PRIMARY_USER_ID, newPattern("12345"));
        setCredential(PRIMARY_USER_ID, pattern);
        verify(mRecoverableKeyStoreManager, never())
                .lockScreenSecretChanged(
                        eq(CREDENTIAL_TYPE_PASSWORD), any(), eq(MANAGED_PROFILE_USER_ID));
                .lockScreenSecretChanged(not(eq(pattern)), eq(MANAGED_PROFILE_USER_ID));
    }

    @Test
@@ -327,12 +327,9 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {
        mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, false, null);
        setCredential(PRIMARY_USER_ID, newCredential, oldCredential);

        verify(mRecoverableKeyStoreManager).lockScreenSecretChanged(newCredential, PRIMARY_USER_ID);
        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretChanged(CREDENTIAL_TYPE_PASSWORD, newCredential.getCredential(),
                        PRIMARY_USER_ID);
        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretChanged(CREDENTIAL_TYPE_PASSWORD, newCredential.getCredential(),
                        MANAGED_PROFILE_USER_ID);
                .lockScreenSecretChanged(newCredential, MANAGED_PROFILE_USER_ID);
    }


@@ -383,14 +380,15 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {
    public void
            testSetLockCredential_forPrimaryUserWithUnifiedChallengeProfile_removesBothCredentials()
                    throws Exception {
        LockscreenCredential noneCredential = nonePassword();
        setCredential(PRIMARY_USER_ID, newPassword("oldPassword"));
        mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, false, null);
        clearCredential(PRIMARY_USER_ID, newPassword("oldPassword"));

        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretChanged(CREDENTIAL_TYPE_NONE, null, PRIMARY_USER_ID);
                .lockScreenSecretChanged(noneCredential, PRIMARY_USER_ID);
        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretChanged(CREDENTIAL_TYPE_NONE, null, MANAGED_PROFILE_USER_ID);
                .lockScreenSecretChanged(noneCredential, MANAGED_PROFILE_USER_ID);
    }

    @Test
@@ -437,8 +435,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {
        mService.setSeparateProfileChallengeEnabled(MANAGED_PROFILE_USER_ID, false, null);
        setCredential(MANAGED_PROFILE_USER_ID, profilePassword);
        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretChanged(CREDENTIAL_TYPE_PASSWORD, profilePassword.getCredential(),
                        MANAGED_PROFILE_USER_ID);
                .lockScreenSecretChanged(profilePassword, MANAGED_PROFILE_USER_ID);
    }

    @Test
@@ -458,7 +455,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {
        // Called once for setting the initial separate profile credentials and not again during
        // unification.
        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretChanged(anyInt(), any(), eq(MANAGED_PROFILE_USER_ID));
                .lockScreenSecretChanged(any(), eq(MANAGED_PROFILE_USER_ID));
    }

    @Test
@@ -469,9 +466,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {

        mService.verifyCredential(password, PRIMARY_USER_ID, 0 /* flags */);

        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretAvailable(
                        CREDENTIAL_TYPE_PASSWORD, password.getCredential(), PRIMARY_USER_ID);
        verify(mRecoverableKeyStoreManager).lockScreenSecretAvailable(password, PRIMARY_USER_ID);
    }

    @Test
@@ -484,8 +479,7 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {
        mService.verifyCredential(pattern, MANAGED_PROFILE_USER_ID, 0 /* flags */);

        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretAvailable(
                        CREDENTIAL_TYPE_PATTERN, pattern.getCredential(), MANAGED_PROFILE_USER_ID);
                .lockScreenSecretAvailable(pattern, MANAGED_PROFILE_USER_ID);
    }

    @Test
@@ -499,16 +493,12 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests {
        mService.verifyCredential(pattern, PRIMARY_USER_ID, 0 /* flags */);

        // Parent sends its credentials for both the parent and profile.
        verify(mRecoverableKeyStoreManager).lockScreenSecretAvailable(pattern, PRIMARY_USER_ID);
        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretAvailable(
                        CREDENTIAL_TYPE_PATTERN, pattern.getCredential(), PRIMARY_USER_ID);
        verify(mRecoverableKeyStoreManager)
                .lockScreenSecretAvailable(
                        CREDENTIAL_TYPE_PATTERN, pattern.getCredential(), MANAGED_PROFILE_USER_ID);
                .lockScreenSecretAvailable(pattern, MANAGED_PROFILE_USER_ID);
        // Profile doesn't send its own random credentials.
        verify(mRecoverableKeyStoreManager, never())
                .lockScreenSecretAvailable(
                        eq(CREDENTIAL_TYPE_PASSWORD), any(), eq(MANAGED_PROFILE_USER_ID));
                .lockScreenSecretAvailable(not(eq(pattern)), eq(MANAGED_PROFILE_USER_ID));
    }

    @Test
Loading