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
diff --git a/keystore/java/android/security/AndroidKeyStoreMaintenance.java b/keystore/java/android/security/AndroidKeyStoreMaintenance.java
index 24aea37..ecf4eb4 100644
--- a/keystore/java/android/security/AndroidKeyStoreMaintenance.java
+++ b/keystore/java/android/security/AndroidKeyStoreMaintenance.java
@@ -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;
@@ -112,29 +111,6 @@
}
/**
- * 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.
*
diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java
index ac29f85..7157646 100644
--- a/services/core/java/com/android/server/locksettings/LockSettingsService.java
+++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java
@@ -252,9 +252,6 @@
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 @@
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 @@
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,32 +1079,14 @@
// 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);
- synchronized (mSpManager) {
- migrateUserToSpWithBoundKeysLocked(user.id);
- }
+ if (!getBoolean(MIGRATED_SP_FULL, false, 0)) {
+ for (UserInfo user : mUserManager.getAliveUsers()) {
+ removeStateForReusedUserIdIfNecessary(user.id, user.serialNumber);
+ synchronized (mSpManager) {
+ migrateUserToSpWithBoundKeysLocked(user.id);
}
- 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);
- }
+ setBoolean(MIGRATED_SP_FULL, true, 0);
}
mThirdPartyAppsStarted = true;
@@ -1122,30 +1094,6 @@
}
@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)) {
Slogf.d(TAG, "User %d is secured; no migration needed", userId);
@@ -1454,11 +1402,6 @@
}
@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();
try {
@@ -2195,9 +2138,7 @@
return;
}
onSyntheticPasswordUnlocked(userId, result.syntheticPassword);
- if (FIX_UNLOCKED_DEVICE_REQUIRED_KEYS) {
- unlockKeystore(userId, result.syntheticPassword);
- }
+ unlockKeystore(userId, result.syntheticPassword);
unlockCeStorage(userId, result.syntheticPassword);
}
}
@@ -2503,9 +2444,7 @@
// 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);
- }
+ AndroidKeyStoreMaintenance.onUserAdded(userId);
synchronized (mUserCreationAndRemovalLock) {
// During early boot, don't actually create the synthetic password yet, but rather
@@ -2931,9 +2870,7 @@
LockscreenCredential.createNone(), sp, userId);
setCurrentLskfBasedProtectorId(protectorId, userId);
setCeStorageProtection(userId, sp);
- if (FIX_UNLOCKED_DEVICE_REQUIRED_KEYS) {
- initKeystoreSuperKeys(userId, sp, /* allowExisting= */ false);
- }
+ 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 @@
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 @@
gateKeeperClearSecureUserId(userId);
unlockCeStorage(userId, sp);
unlockKeystore(userId, sp);
- if (FIX_UNLOCKED_DEVICE_REQUIRED_KEYS) {
- AndroidKeyStoreMaintenance.onUserLskfRemoved(userId);
- } else {
- setKeystorePassword(null, userId);
- }
+ AndroidKeyStoreMaintenance.onUserLskfRemoved(userId);
removeBiometricsForUser(userId);
}
setCurrentLskfBasedProtectorId(newProtectorId, userId);
diff --git a/services/core/java/com/android/server/trust/TrustManagerService.java b/services/core/java/com/android/server/trust/TrustManagerService.java
index 3976283..b3c31a9 100644
--- a/services/core/java/com/android/server/trust/TrustManagerService.java
+++ b/services/core/java/com/android/server/trust/TrustManagerService.java
@@ -849,12 +849,7 @@
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,19 +910,15 @@
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
- // biometric SIDs and weak unlock methods, not the profile's.
- int authUserId = mLockPatternUtils.isProfileWithUnifiedChallenge(userId)
- ? resolveProfileParent(userId) : userId;
+ // 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
+ // biometric SIDs and weak unlock methods, not the profile's.
+ int authUserId = mLockPatternUtils.isProfileWithUnifiedChallenge(userId)
+ ? resolveProfileParent(userId) : userId;
- mKeyStoreAuthorization.onDeviceLocked(userId, getBiometricSids(authUserId),
- isWeakUnlockMethodEnabled(authUserId));
- } else {
- mKeyStoreAuthorization.onDeviceLocked(userId, getBiometricSids(userId), false);
- }
+ mKeyStoreAuthorization.onDeviceLocked(userId, getBiometricSids(authUserId),
+ isWeakUnlockMethodEnabled(authUserId));
} 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
diff --git a/services/tests/mockingservicestests/src/com/android/server/trust/TrustManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/trust/TrustManagerServiceTest.java
index 2a67029..7aec42b 100644
--- a/services/tests/mockingservicestests/src/com/android/server/trust/TrustManagerServiceTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/trust/TrustManagerServiceTest.java
@@ -72,9 +72,6 @@
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;
@@ -124,9 +121,6 @@
.build();
@Rule
- public final CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule();
-
- @Rule
public final MockContext mMockContext = new MockContext(
ApplicationProvider.getApplicationContext());
@@ -418,7 +412,6 @@
// 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 @@
}
@Test
- @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)
public void testKeystoreWeakUnlockEnabled_whenWeakFingerprintIsSetupAndAllowed()
throws Exception {
setupStrongAuthTrackerToAllowEverything();
@@ -626,7 +618,6 @@
}
@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 @@
}
@Test
- @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)
public void testKeystoreWeakUnlockEnabled_whenConvenienceFingerprintIsSetupAndAllowed()
throws Exception {
setupStrongAuthTrackerToAllowEverything();
@@ -643,7 +633,6 @@
}
@Test
- @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)
public void testKeystoreWeakUnlockEnabled_whenConvenienceFaceIsSetupAndAllowed()
throws Exception {
setupStrongAuthTrackerToAllowEverything();
@@ -652,7 +641,6 @@
}
@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 @@
}
@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 @@
}
@Test
- @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)
public void testKeystoreWeakUnlockDisabled_whenWeakFingerprintSensorIsPresentButNotEnrolled()
throws Exception {
setupStrongAuthTrackerToAllowEverything();
@@ -678,7 +664,6 @@
}
@Test
- @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)
public void testKeystoreWeakUnlockDisabled_whenWeakFaceSensorIsPresentButNotEnrolled()
throws Exception {
setupStrongAuthTrackerToAllowEverything();
@@ -687,7 +672,6 @@
}
@Test
- @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)
public void
testKeystoreWeakUnlockDisabled_whenWeakFingerprintIsSetupButForbiddenByDevicePolicy()
throws Exception {
@@ -699,7 +683,6 @@
}
@Test
- @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)
public void testKeystoreWeakUnlockDisabled_whenWeakFaceIsSetupButForbiddenByDevicePolicy()
throws Exception {
setupStrongAuthTrackerToAllowEverything();
@@ -710,7 +693,6 @@
}
@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 @@
}
@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 @@
}
@Test
- @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)
public void testKeystoreWeakUnlockDisabled_whenNoBiometricsAreSetup() throws Exception {
setupStrongAuthTrackerToAllowEverything();
verifyWeakUnlockDisabled();
diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java
index f9077c4..93fc071a 100644
--- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java
+++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java
@@ -196,11 +196,6 @@
}
@Override
- void setKeystorePassword(byte[] password, int userHandle) {
-
- }
-
- @Override
void initKeystoreSuperKeys(int userId, SyntheticPassword sp, boolean allowExisting) {
}
diff --git a/tests/TrustTests/src/android/trust/test/GrantAndRevokeTrustTest.kt b/tests/TrustTests/src/android/trust/test/GrantAndRevokeTrustTest.kt
index d0e5626..0c3c7e2 100644
--- a/tests/TrustTests/src/android/trust/test/GrantAndRevokeTrustTest.kt
+++ b/tests/TrustTests/src/android/trust/test/GrantAndRevokeTrustTest.kt
@@ -17,9 +17,6 @@
package android.trust.test
import android.content.pm.PackageManager
-import android.platform.test.annotations.RequiresFlagsDisabled
-import android.platform.test.annotations.RequiresFlagsEnabled
-import android.platform.test.flag.junit.DeviceFlagsValueProvider
import android.service.trust.GrantTrustResult
import android.trust.BaseTrustAgentService
import android.trust.TrustTestActivity
@@ -58,7 +55,6 @@
.around(ScreenLockRule())
.around(lockStateTrackingRule)
.around(trustAgentRule)
- .around(DeviceFlagsValueProvider.createCheckFlagsRule())
@Before
fun manageTrust() {
@@ -93,7 +89,6 @@
}
@Test
- @RequiresFlagsEnabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)
fun grantCannotActivelyUnlockDevice() {
// On automotive, trust agents can actively unlock the device.
assumeFalse(packageManager.hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE))
@@ -120,24 +115,6 @@
}
@Test
- @RequiresFlagsDisabled(android.security.Flags.FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2)
- fun grantCouldCauseWrongDeviceLockedStateDueToBug() {
- // On automotive, trust agents can actively unlock the device.
- assumeFalse(packageManager.hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE))
-
- // Verify that b/296464083 exists. That is, when the device is locked
- // and a trust agent grants trust, the deviceLocked state incorrectly
- // becomes false even though the device correctly remains locked.
- uiDevice.sleep()
- lockStateTrackingRule.assertLocked()
- trustAgentRule.agent.grantTrust(GRANT_MESSAGE, 10000, 0) {}
- uiDevice.wakeUp()
- uiDevice.sleep()
- await()
- lockStateTrackingRule.assertUnlockedButNotReally()
- }
-
- @Test
fun grantDoesNotCallBack() {
val callback = mock<(GrantTrustResult) -> Unit>()
trustAgentRule.agent.grantTrust(GRANT_MESSAGE, 0, 0, callback)
diff --git a/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt b/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt
index 0121809..80d7947 100644
--- a/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt
+++ b/tests/TrustTests/src/android/trust/test/lib/LockStateTrackingRule.kt
@@ -64,13 +64,6 @@
wait("not trusted") { trustState.trusted == false }
}
- // TODO(b/299298338) remove this when removing FLAG_FIX_UNLOCKED_DEVICE_REQUIRED_KEYS_V2
- fun assertUnlockedButNotReally() {
- wait("device unlocked") { !keyguardManager.isDeviceLocked }
- wait("not trusted") { trustState.trusted == false }
- wait("keyguard locked") { windowManager.isKeyguardLocked }
- }
-
fun assertUnlockedAndTrusted() {
wait("device unlocked") { !keyguardManager.isDeviceLocked }
wait("trusted") { trustState.trusted == true }