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

Commit 1583c6a6 authored by Eric Biggers's avatar Eric Biggers
Browse files

system_server: make UnlockedDeviceRequired fix unconditional 

Make the fix unconditional and remove all superseded code.

Bug: 299298338
Test: atest -p --include-subdirs system/security/keystore2
Test: atest CtsKeystoreTestCases
Test: atest com.android.server.locksettings
Test: atest TrustManagerServiceTest
Test: atest TrustTests
Change-Id: I78dd3268b2b98873a4020a9b6fe0348e935eb5aa
parent 057e0e1f

keystore/java/android/security/AndroidKeyStoreMaintenance.java

+0 −24
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@ 
package android.security;



import android.annotation.NonNull;

import android.annotation.Nullable;

import android.os.RemoteException;

import android.os.ServiceManager;

import android.os.ServiceSpecificException;
@@ -111,29 +110,6 @@ public class AndroidKeyStoreMaintenance { 
        }

    }



    /**

     * Informs Keystore 2.0 about changing user's password

     *

     * @param userId   - Android user id of the user

     * @param password - a secret derived from the synthetic password provided by the

     *                 LockSettingsService

     * @return 0 if successful or a {@code ResponseCode}

     * @hide

     */

    public static int onUserPasswordChanged(int userId, @Nullable byte[] password) {

        StrictMode.noteDiskWrite();

        try {

            getService().onUserPasswordChanged(userId, password);

            return 0;

        } catch (ServiceSpecificException e) {

            Log.e(TAG, "onUserPasswordChanged failed", e);

            return e.errorCode;

        } catch (Exception e) {

            Log.e(TAG, "Can not connect to keystore", e);

            return SYSTEM_ERROR;

        }

    }



    /**

     * Tells Keystore that a user's LSKF is being removed, ie the user's lock screen is changing to

     * Swipe or None.  Keystore uses this notification to delete the user's auth-bound keys.

services/core/java/com/android/server/locksettings/LockSettingsService.java

+11 −81
Original line number Diff line number Diff line
@@ -252,9 +252,6 @@ public class LockSettingsService extends ILockSettings.Stub { 
    private static final String MIGRATED_SP_CE_ONLY = "migrated_all_users_to_sp_and_bound_ce";

    private static final String MIGRATED_SP_FULL = "migrated_all_users_to_sp_and_bound_keys";



    private static final boolean FIX_UNLOCKED_DEVICE_REQUIRED_KEYS =

            android.security.Flags.fixUnlockedDeviceRequiredKeysV2();



    // Duration that LockSettingsService will store the gatekeeper password for. This allows

    // multiple biometric enrollments without prompting the user to enter their password via

    // ConfirmLockPassword/ConfirmLockPattern multiple times. This needs to be at least the duration
@@ -662,7 +659,6 @@ public class LockSettingsService extends ILockSettings.Stub { 
        mActivityManager = injector.getActivityManager();



        IntentFilter filter = new IntentFilter();

        filter.addAction(Intent.ACTION_USER_ADDED);

        filter.addAction(Intent.ACTION_USER_STARTING);

        filter.addAction(Intent.ACTION_LOCALE_CHANGED);

        injector.getContext().registerReceiverAsUser(mBroadcastReceiver, UserHandle.ALL, filter,
@@ -899,13 +895,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
    private final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() {

        @Override

        public void onReceive(Context context, Intent intent) {

            if (Intent.ACTION_USER_ADDED.equals(intent.getAction())) {

                if (!FIX_UNLOCKED_DEVICE_REQUIRED_KEYS) {

                    // Notify keystore that a new user was added.

                    final int userHandle = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, 0);

                    AndroidKeyStoreMaintenance.onUserAdded(userHandle);

                }

            } else if (Intent.ACTION_USER_STARTING.equals(intent.getAction())) {

            if (Intent.ACTION_USER_STARTING.equals(intent.getAction())) {

                final int userHandle = intent.getIntExtra(Intent.EXTRA_USER_HANDLE, 0);

                mStorage.prefetchUser(userHandle);

            } else if (Intent.ACTION_LOCALE_CHANGED.equals(intent.getAction())) {
@@ -1089,7 +1079,6 @@ public class LockSettingsService extends ILockSettings.Stub { 
            // Note: if this migration gets interrupted (e.g. by the device powering off), there

            // shouldn't be a problem since this will run again on the next boot, and

            // setCeStorageProtection() and initKeystoreSuperKeys(..., true) are idempotent.

            if (FIX_UNLOCKED_DEVICE_REQUIRED_KEYS) {

            if (!getBoolean(MIGRATED_SP_FULL, false, 0)) {

                for (UserInfo user : mUserManager.getAliveUsers()) {

                    removeStateForReusedUserIdIfNecessary(user.id, user.serialNumber);
@@ -1099,52 +1088,11 @@ public class LockSettingsService extends ILockSettings.Stub { 
                }

                setBoolean(MIGRATED_SP_FULL, true, 0);

            }

            } else {

                if (getString(MIGRATED_SP_CE_ONLY, null, 0) == null) {

                    for (UserInfo user : mUserManager.getAliveUsers()) {

                        removeStateForReusedUserIdIfNecessary(user.id, user.serialNumber);

                        synchronized (mSpManager) {

                            migrateUserToSpWithBoundCeKeyLocked(user.id);

                        }

                    }

                    setString(MIGRATED_SP_CE_ONLY, "true", 0);

                }



                if (getBoolean(MIGRATED_SP_FULL, false, 0)) {

                    // The FIX_UNLOCKED_DEVICE_REQUIRED_KEYS flag was enabled but then got disabled.

                    // Ensure the full migration runs again the next time the flag is enabled...

                    setBoolean(MIGRATED_SP_FULL, false, 0);

                }

            }



            mThirdPartyAppsStarted = true;

        }

    }



    @GuardedBy("mSpManager")

    private void migrateUserToSpWithBoundCeKeyLocked(@UserIdInt int userId) {

        if (isUserSecure(userId)) {

            Slogf.d(TAG, "User %d is secured; no migration needed", userId);

            return;

        }

        long protectorId = getCurrentLskfBasedProtectorId(userId);

        if (protectorId == SyntheticPasswordManager.NULL_PROTECTOR_ID) {

            Slogf.i(TAG, "Migrating unsecured user %d to SP-based credential", userId);

            initializeSyntheticPassword(userId);

        } else {

            Slogf.i(TAG, "Existing unsecured user %d has a synthetic password; re-encrypting CE " +

                    "key with it", userId);

            AuthenticationResult result = mSpManager.unlockLskfBasedProtector(

                    getGateKeeperService(), protectorId, LockscreenCredential.createNone(), userId,

                    null);

            if (result.syntheticPassword == null) {

                Slogf.wtf(TAG, "Failed to unwrap synthetic password for unsecured user %d", userId);

                return;

            }

            setCeStorageProtection(userId, result.syntheticPassword);

        }

    }



    @GuardedBy("mSpManager")

    private void migrateUserToSpWithBoundKeysLocked(@UserIdInt int userId) {

        if (isUserSecure(userId)) {
@@ -1453,11 +1401,6 @@ public class LockSettingsService extends ILockSettings.Stub { 
        return getCredentialTypeInternal(userId) != CREDENTIAL_TYPE_NONE;

    }



    @VisibleForTesting /** Note: this method is overridden in unit tests */

    void setKeystorePassword(byte[] password, int userHandle) {

        AndroidKeyStoreMaintenance.onUserPasswordChanged(userHandle, password);

    }



    @VisibleForTesting /** Note: this method is overridden in unit tests */

    void initKeystoreSuperKeys(@UserIdInt int userId, SyntheticPassword sp, boolean allowExisting) {

        final byte[] password = sp.deriveKeyStorePassword();
@@ -2195,9 +2138,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
                return;

            }

            onSyntheticPasswordUnlocked(userId, result.syntheticPassword);

            if (FIX_UNLOCKED_DEVICE_REQUIRED_KEYS) {

            unlockKeystore(userId, result.syntheticPassword);

            }

            unlockCeStorage(userId, result.syntheticPassword);

        }

    }
@@ -2503,9 +2444,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
        // long time, so for now we keep doing it just in case it's ever important.  Don't wait

        // until initKeystoreSuperKeys() to do this; that can be delayed if the user is being

        // created during early boot, and maybe something will use Keystore before then.

        if (FIX_UNLOCKED_DEVICE_REQUIRED_KEYS) {

        AndroidKeyStoreMaintenance.onUserAdded(userId);

        }



        synchronized (mUserCreationAndRemovalLock) {

            // During early boot, don't actually create the synthetic password yet, but rather
@@ -2931,9 +2870,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
                    LockscreenCredential.createNone(), sp, userId);

            setCurrentLskfBasedProtectorId(protectorId, userId);

            setCeStorageProtection(userId, sp);

            if (FIX_UNLOCKED_DEVICE_REQUIRED_KEYS) {

            initKeystoreSuperKeys(userId, sp, /* allowExisting= */ false);

            }

            onSyntheticPasswordCreated(userId, sp);

            Slogf.i(TAG, "Successfully initialized synthetic password for user %d", userId);

            return sp;
@@ -3048,9 +2985,6 @@ public class LockSettingsService extends ILockSettings.Stub { 
            if (!mSpManager.hasSidForUser(userId)) {

                mSpManager.newSidForUser(getGateKeeperService(), sp, userId);

                mSpManager.verifyChallenge(getGateKeeperService(), sp, 0L, userId);

                if (!FIX_UNLOCKED_DEVICE_REQUIRED_KEYS) {

                    setKeystorePassword(sp.deriveKeyStorePassword(), userId);

                }

            }

        } else {

            // Cache all profile password if they use unified challenge. This will later be used to
@@ -3061,11 +2995,7 @@ public class LockSettingsService extends ILockSettings.Stub { 
            gateKeeperClearSecureUserId(userId);

            unlockCeStorage(userId, sp);

            unlockKeystore(userId, sp);

            if (FIX_UNLOCKED_DEVICE_REQUIRED_KEYS) {

            AndroidKeyStoreMaintenance.onUserLskfRemoved(userId);

            } else {

                setKeystorePassword(null, userId);

            }

            removeBiometricsForUser(userId);

        }

        setCurrentLskfBasedProtectorId(newProtectorId, userId);

services/core/java/com/android/server/trust/TrustManagerService.java

+10 −19
Original line number Diff line number Diff line
@@ -849,12 +849,7 @@ public class TrustManagerService extends SystemService { 
                continue;

            }



            final boolean trusted;

            if (android.security.Flags.fixUnlockedDeviceRequiredKeysV2()) {

                trusted = getUserTrustStateInner(id) == TrustState.TRUSTED;

            } else {

                trusted = aggregateIsTrusted(id);

            }

            final boolean trusted = getUserTrustStateInner(id) == TrustState.TRUSTED;

            boolean showingKeyguard = true;

            boolean biometricAuthenticated = false;

            boolean currentUserIsUnlocked = false;
@@ -915,7 +910,6 @@ public class TrustManagerService extends SystemService { 


    private void notifyKeystoreOfDeviceLockState(int userId, boolean isLocked) {

        if (isLocked) {

            if (android.security.Flags.fixUnlockedDeviceRequiredKeysV2()) {

            // A profile with unified challenge is unlockable not by its own biometrics and

            // trust agents, but rather by those of the parent user.  Therefore, when protecting

            // the profile's UnlockedDeviceRequired keys, we must use the parent's list of
@@ -925,9 +919,6 @@ public class TrustManagerService extends SystemService { 


            mKeyStoreAuthorization.onDeviceLocked(userId, getBiometricSids(authUserId),

                    isWeakUnlockMethodEnabled(authUserId));

            } else {

                mKeyStoreAuthorization.onDeviceLocked(userId, getBiometricSids(userId), false);

            }

        } else {

            // Notify Keystore that the device is now unlocked for the user.  Note that for unlocks

            // with LSKF, this is redundant with the call from LockSettingsService which provides

services/tests/mockingservicestests/src/com/android/server/trust/TrustManagerServiceTest.java

+0 −20
Original line number Diff line number Diff line
@@ -72,9 +72,6 @@ import android.os.RemoteException; 
import android.os.ServiceManager;

import android.os.UserHandle;

import android.os.UserManager;

import android.platform.test.annotations.RequiresFlagsEnabled;

import android.platform.test.flag.junit.CheckFlagsRule;

import android.platform.test.flag.junit.DeviceFlagsValueProvider;

import android.provider.Settings;

import android.security.KeyStoreAuthorization;

import android.service.trust.GrantTrustResult;
@@ -123,9 +120,6 @@ public class TrustManagerServiceTest { 
            .mockStatic(WindowManagerGlobal.class)

            .build();



    @Rule

    public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule();



    @Rule

    public final MockContext mMockContext = new MockContext(

            ApplicationProvider.getApplicationContext());
@@ -418,7 +412,6 @@ public class TrustManagerServiceTest { 
    // user, not the profile.  This matches the authentication that is needed to unlock the device

    // for the profile again.

    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testLockDeviceForManagedProfileWithUnifiedChallenge_usesParentBiometricSids()

            throws Exception {

        setupMocksForProfile(/* unifiedChallenge= */ true);
@@ -617,7 +610,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockEnabled_whenWeakFingerprintIsSetupAndAllowed()

            throws Exception {

        setupStrongAuthTrackerToAllowEverything();
@@ -626,7 +618,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockEnabled_whenWeakFaceIsSetupAndAllowed() throws Exception {

        setupStrongAuthTrackerToAllowEverything();

        setupFace(SensorProperties.STRENGTH_WEAK);
@@ -634,7 +625,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockEnabled_whenConvenienceFingerprintIsSetupAndAllowed()

            throws Exception {

        setupStrongAuthTrackerToAllowEverything();
@@ -643,7 +633,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockEnabled_whenConvenienceFaceIsSetupAndAllowed()

            throws Exception {

        setupStrongAuthTrackerToAllowEverything();
@@ -652,7 +641,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockDisabled_whenStrongAuthRequired() throws Exception {

        setupStrongAuthTracker(StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN, true);

        setupFace(SensorProperties.STRENGTH_WEAK);
@@ -660,7 +648,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockDisabled_whenNonStrongBiometricNotAllowed() throws Exception {

        setupStrongAuthTracker(StrongAuthTracker.STRONG_AUTH_NOT_REQUIRED,

                /* isNonStrongBiometricAllowed= */ false);
@@ -669,7 +656,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockDisabled_whenWeakFingerprintSensorIsPresentButNotEnrolled()

            throws Exception {

        setupStrongAuthTrackerToAllowEverything();
@@ -678,7 +664,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockDisabled_whenWeakFaceSensorIsPresentButNotEnrolled()

            throws Exception {

        setupStrongAuthTrackerToAllowEverything();
@@ -687,7 +672,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void

            testKeystoreWeakUnlockDisabled_whenWeakFingerprintIsSetupButForbiddenByDevicePolicy()

            throws Exception {
@@ -699,7 +683,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockDisabled_whenWeakFaceIsSetupButForbiddenByDevicePolicy()

            throws Exception {

        setupStrongAuthTrackerToAllowEverything();
@@ -710,7 +693,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockDisabled_whenOnlyStrongFingerprintIsSetup() throws Exception {

        setupStrongAuthTrackerToAllowEverything();

        setupFingerprint(SensorProperties.STRENGTH_STRONG);
@@ -718,7 +700,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockDisabled_whenOnlyStrongFaceIsSetup() throws Exception {

        setupStrongAuthTrackerToAllowEverything();

        setupFace(SensorProperties.STRENGTH_STRONG);
@@ -726,7 +707,6 @@ public class TrustManagerServiceTest { 
    }



    @Test

    @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)

    public void testKeystoreWeakUnlockDisabled_whenNoBiometricsAreSetup() throws Exception {

        setupStrongAuthTrackerToAllowEverything();

        verifyWeakUnlockDisabled();

services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java

+0 −5
Original line number Diff line number Diff line
@@ -195,11 +195,6 @@ public class LockSettingsServiceTestable extends LockSettingsService { 
        }

    }



    @Override

    void setKeystorePassword(byte[] password, int userHandle) {



    }



    @Override

    void initKeystoreSuperKeys(int userId, SyntheticPassword sp, boolean allowExisting) {

    }