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

Commit 9c0a23c1 authored by Eric Biggers's avatar Eric Biggers
Browse files

Make lockUser() handle resetting strong auth flags and password metrics

Currently, a user's strong auth flags are set to the default value
(usually STRONG_AUTH_REQUIRED_AFTER_BOOT) by onUserStopped() if the user
doesn't allow delayed locking, or by lockUser() if the user allows
delayed locking.  But, when a user that doesn't allow delayed locking is
stopped, by definition they are locked right away, i.e. onUserStopped()
is followed by lockUser().  Let's simplify by making lockUser() handle
the strong auth flag reset for all users.  This is an action associated
with the user being locked, not stopped per se, so this makes sense.

Do the same for the password metrics removal too.  This was actually
missing for users that allow delayed locking.

Also remove the unnecessary asynchronous processing on the handler
thread.  requireStrongAuth() itself is already asynchronous.

Bug: 319142556
Flag: android.security.reset_auth_flags_and_metrics_in_lock_user
Test: atest FrameworksServicesTests:com.android.server.locksettings
Change-Id: I01b3da11ddcb7d067c0edd63df82adbf20978dcc
parent e3c0540a
Loading
Loading
Loading
Loading
+7 −0
Original line number Diff line number Diff line
@@ -49,6 +49,13 @@ flag {
    bug: "325129836"
}

flag {
    name: "reset_auth_flags_and_metrics_in_lock_user"
    namespace: "security"
    description: "Make LockSettingsService#lockUser() always handle resetting strong auth flags and removing user password metrics"
    bug: "319142556"
}

flag {
    name: "scrypt_parameter_change"
    namespace: "security"
+36 −15
Original line number Diff line number Diff line
@@ -59,7 +59,6 @@ import static com.android.server.locksettings.UnifiedProfilePasswordCrypto.remov
import android.Manifest;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.annotation.RequiresPermission;
import android.annotation.UserIdInt;
import android.app.ActivityManager;
import android.app.IActivityManager;
@@ -307,9 +306,10 @@ public class LockSettingsService extends ILockSettings.Stub {
    private ArrayList<Pair<Long, Integer>> mProtectorsToDestroyOnBootCompleted = new ArrayList<>();

    // Current password metrics for all secured users on the device. Updated when user unlocks the
    // device or changes password. Removed if user is stopped with its CE key evicted.
    // device or changes password. Removed when user is locked.
    @GuardedBy("this")
    private final SparseArray<PasswordMetrics> mUserPasswordMetrics = new SparseArray<>();

    @VisibleForTesting
    protected boolean mHasSecureLockScreen;

@@ -860,13 +860,15 @@ public class LockSettingsService extends ILockSettings.Stub {
    }

    @VisibleForTesting
    @RequiresPermission(anyOf = {
            android.Manifest.permission.MANAGE_USERS,
            android.Manifest.permission.QUERY_USERS,
            android.Manifest.permission.INTERACT_ACROSS_USERS}, conditional = true)
    void onUserStopped(int userId) {
        hideEncryptionNotification(new UserHandle(userId));

        if (android.security.Flags.resetAuthFlagsAndMetricsInLockUser()) {
            // Don't perform any locking actions (e.g. locking CE storage) here, since the user is
            // not necessarily being locked. These actions happen in lockUser() instead.
            return;
        }

        // Normally, CE storage is locked when a user is stopped, and restarting the user requires
        // strong auth.  Therefore, reset the user's strong auth flags.  The exception is users that
        // allow delayed locking; under some circumstances, biometric authentication is allowed to
@@ -3496,6 +3498,13 @@ public class LockSettingsService extends ILockSettings.Stub {
        return true;
    }

    /*
     * Try to lock, evict, and/or zeroize all user secrets that were unlocked by primary
     * authentication, such that they will become available only via primary authentication again.
     *
     * Ideally, the result would be identical to the boot-time state. In reality, that state is not
     * truly reached, and we just do the best we can.
     */
    private void lockUser(@UserIdInt int userId) {
        // Lock the user's credential-encrypted storage.
        try {
@@ -3510,17 +3519,29 @@ public class LockSettingsService extends ILockSettings.Stub {
            lockKeystore(userId);
        }

        // Reset user's strong auth flags
        mHandler.post(() -> {
        if (android.security.Flags.resetAuthFlagsAndMetricsInLockUser()) {
            // Reset the user's strong auth flags.
            int strongAuthFlags = LockPatternUtils.StrongAuthTracker.getDefaultFlags(mContext);
            requireStrongAuth(strongAuthFlags, userId);

            // Remove the user's password metrics.
            synchronized (this) {
                mUserPasswordMetrics.remove(userId);
            }
        } else {
            // Reset the user's strong auth flags, if it wasn't already done by onUserStopped().
            mHandler.post(
                    () -> {
                        UserProperties userProperties = getUserProperties(userId);
            if (userProperties != null && userProperties
                    .getAllowStoppingUserWithDelayedLocking()) {
                int strongAuthRequired = LockPatternUtils.StrongAuthTracker
                        .getDefaultFlags(mContext);
                        if (userProperties != null
                                && userProperties.getAllowStoppingUserWithDelayedLocking()) {
                            int strongAuthRequired =
                                    LockPatternUtils.StrongAuthTracker.getDefaultFlags(mContext);
                            requireStrongAuth(strongAuthRequired, userId);
                        }
                    });
        }
    }

    @Override
    public boolean tryUnlockWithCachedUnifiedChallenge(int userId) {
+18 −3
Original line number Diff line number Diff line
@@ -828,8 +828,7 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests {
        verify(mAuthSecretService, never()).setPrimaryUserCredential(any(byte[].class));
    }

    @Test
    public void testUnlockUserWithToken() throws Exception {
    private void testUnlockUserWithToken_helper() throws Exception {
        LockscreenCredential password = newPassword("password");
        byte[] token = "some-high-entropy-secure-token".getBytes();
        initSpAndSetCredential(PRIMARY_USER_ID, password);
@@ -841,7 +840,11 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests {
        assertTrue(mService.verifyCredential(password, PRIMARY_USER_ID, 0 /* flags */).isMatched());
        assertTrue(mLocalService.isEscrowTokenActive(handle, PRIMARY_USER_ID));

        if (android.security.Flags.resetAuthFlagsAndMetricsInLockUser()) {
            mLocalService.lockUser(PRIMARY_USER_ID);
        } else {
            mService.onUserStopped(PRIMARY_USER_ID);
        }
        assertNull(mLocalService.getUserPasswordMetrics(PRIMARY_USER_ID));

        assertTrue(mLocalService.unlockUserWithToken(handle, token, PRIMARY_USER_ID));
@@ -849,6 +852,18 @@ public class SyntheticPasswordTests extends BaseLockSettingsServiceTests {
                mLocalService.getUserPasswordMetrics(PRIMARY_USER_ID));
    }

    @Test
    @DisableFlags(android.security.Flags.FLAG_RESET_AUTH_FLAGS_AND_METRICS_IN_LOCK_USER)
    public void testUnlockUserWithToken_orig() throws Exception {
        testUnlockUserWithToken_helper();
    }

    @Test
    @EnableFlags(android.security.Flags.FLAG_RESET_AUTH_FLAGS_AND_METRICS_IN_LOCK_USER)
    public void testUnlockUserWithToken() throws Exception {
        testUnlockUserWithToken_helper();
    }

    @Test
    public void testPasswordChange_NoOrphanedFilesLeft() throws Exception {
        LockscreenCredential password = newPassword("password");
+24 −3
Original line number Diff line number Diff line
@@ -30,7 +30,10 @@ import android.app.admin.PasswordMetrics;
import android.content.pm.PackageManager;
import android.os.IBinder;
import android.os.RemoteException;
import android.platform.test.annotations.DisableFlags;
import android.platform.test.annotations.EnableFlags;
import android.platform.test.annotations.Presubmit;
import android.platform.test.flag.junit.SetFlagsRule;

import androidx.test.filters.SmallTest;
import androidx.test.runner.AndroidJUnit4;
@@ -40,6 +43,7 @@ import com.android.internal.widget.IWeakEscrowTokenRemovedListener;
import com.android.internal.widget.LockscreenCredential;

import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

@@ -49,6 +53,8 @@ import org.junit.runner.RunWith;
@RunWith(AndroidJUnit4.class)
public class WeakEscrowTokenTests extends BaseLockSettingsServiceTests{

    @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();

    @Before
    public void setUp() {
        mService.initializeSyntheticPassword(PRIMARY_USER_ID);
@@ -148,8 +154,7 @@ public class WeakEscrowTokenTests extends BaseLockSettingsServiceTests{
        verify(mockRemoveListener, never()).onWeakEscrowTokenRemoved(handle1, PRIMARY_USER_ID);
    }

    @Test
    public void testUnlockUserWithToken_weakEscrowToken() throws Exception {
    private void testUnlockUserWithToken_weakEscrowToken_helper() throws Exception {
        mockAutoHardware();
        IWeakEscrowTokenActivatedListener mockActivateListener =
                mock(IWeakEscrowTokenActivatedListener.Stub.class);
@@ -165,7 +170,11 @@ public class WeakEscrowTokenTests extends BaseLockSettingsServiceTests{
        assertTrue(mService.isWeakEscrowTokenActive(handle, PRIMARY_USER_ID));
        assertTrue(mService.isWeakEscrowTokenValid(handle, token, PRIMARY_USER_ID));

        if (android.security.Flags.resetAuthFlagsAndMetricsInLockUser()) {
            mLocalService.lockUser(PRIMARY_USER_ID);
        } else {
            mService.onUserStopped(PRIMARY_USER_ID);
        }
        assertNull(mLocalService.getUserPasswordMetrics(PRIMARY_USER_ID));

        assertTrue(mLocalService.unlockUserWithToken(handle, token, PRIMARY_USER_ID));
@@ -173,6 +182,18 @@ public class WeakEscrowTokenTests extends BaseLockSettingsServiceTests{
                mLocalService.getUserPasswordMetrics(PRIMARY_USER_ID));
    }

    @Test
    @DisableFlags(android.security.Flags.FLAG_RESET_AUTH_FLAGS_AND_METRICS_IN_LOCK_USER)
    public void testUnlockUserWithToken_weakEscrowToken_orig() throws Exception {
        testUnlockUserWithToken_weakEscrowToken_helper();
    }

    @Test
    @EnableFlags(android.security.Flags.FLAG_RESET_AUTH_FLAGS_AND_METRICS_IN_LOCK_USER)
    public void testUnlockUserWithToken_weakEscrowToken() throws Exception {
        testUnlockUserWithToken_weakEscrowToken_helper();
    }

    private void mockAutoHardware() {
        when(mPackageManager.hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE)).thenReturn(true);
    }