From 6cd87c2245752ab6f66e9710eb3a8e379bfe9ace Mon Sep 17 00:00:00 2001 From: Rubin Xu Date: Thu, 20 Feb 2020 16:44:27 +0000 Subject: [PATCH] Enforce secure FRP mode while changing credentials Test: atest com.android.server.locksettings Bug: 149868029 Change-Id: I15fcca130c61a922a4079255329da6493a357edc --- .../server/locksettings/LockSettingsService.java | 13 +++++++++++++ .../android/server/locksettings/FakeSettings.java | 15 +++++++++++++++ .../locksettings/LockSettingsServiceTestable.java | 6 ++++++ .../locksettings/LockSettingsServiceTests.java | 9 +++++++++ 4 files changed, 43 insertions(+) diff --git a/services/core/java/com/android/server/locksettings/LockSettingsService.java b/services/core/java/com/android/server/locksettings/LockSettingsService.java index c1c37603b9c36..1b4ec8a8fa325 100644 --- a/services/core/java/com/android/server/locksettings/LockSettingsService.java +++ b/services/core/java/com/android/server/locksettings/LockSettingsService.java @@ -530,6 +530,11 @@ public class LockSettingsService extends ILockSettings.Stub { return Settings.Global.getInt(contentResolver, keyName, defaultValue); } + public int settingsSecureGetInt(ContentResolver contentResolver, String keyName, + int defaultValue, int userId) { + return Settings.Secure.getIntForUser(contentResolver, keyName, defaultValue, userId); + } + public @NonNull ManagedProfilePasswordCache getManagedProfilePasswordCache() { try { java.security.KeyStore ks = java.security.KeyStore.getInstance("AndroidKeyStore"); @@ -1010,6 +1015,13 @@ public class LockSettingsService extends ILockSettings.Stub { } } + private void enforceFrpResolved() { + if (mInjector.settingsSecureGetInt(mContext.getContentResolver(), + Settings.Secure.SECURE_FRP_MODE, 0, UserHandle.USER_SYSTEM) == 1) { + throw new SecurityException("Cannot change credential while FRP is not resolved yet"); + } + } + private final void checkWritePermission(int userId) { mContext.enforceCallingOrSelfPermission(PERMISSION, "LockSettingsWrite"); } @@ -1572,6 +1584,7 @@ public class LockSettingsService extends ILockSettings.Stub { "This operation requires secure lock screen feature"); } checkWritePermission(userId); + enforceFrpResolved(); // When changing credential for profiles with unified challenge, some callers // will pass in empty credential while others will pass in the credential of diff --git a/services/tests/servicestests/src/com/android/server/locksettings/FakeSettings.java b/services/tests/servicestests/src/com/android/server/locksettings/FakeSettings.java index 70a927c216ecd..c5e924be2612f 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/FakeSettings.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/FakeSettings.java @@ -15,16 +15,23 @@ */ package com.android.server.locksettings; +import android.content.ContentResolver; +import android.os.UserHandle; import android.provider.Settings; public class FakeSettings { private int mDeviceProvisioned; + private int mSecureFrpMode; public void setDeviceProvisioned(boolean provisioned) { mDeviceProvisioned = provisioned ? 1 : 0; } + public void setSecureFrpMode(boolean secure) { + mSecureFrpMode = secure ? 1 : 0; + } + public int globalGetInt(String keyName) { switch (keyName) { case Settings.Global.DEVICE_PROVISIONED: @@ -33,4 +40,12 @@ public class FakeSettings { throw new IllegalArgumentException("Unhandled global settings: " + keyName); } } + + public int secureGetInt(ContentResolver contentResolver, String keyName, int defaultValue, + int userId) { + if (Settings.Secure.SECURE_FRP_MODE.equals(keyName) && userId == UserHandle.USER_SYSTEM) { + return mSecureFrpMode; + } + return defaultValue; + } } 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 1ff451b50c427..4e1454bd0962a 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTestable.java @@ -123,6 +123,12 @@ public class LockSettingsServiceTestable extends LockSettingsService { return mSettings.globalGetInt(keyName); } + @Override + public int settingsSecureGetInt(ContentResolver contentResolver, String keyName, + int defaultValue, int userId) { + return mSettings.secureGetInt(contentResolver, keyName, defaultValue, userId); + } + @Override public UserManagerInternal getUserManagerInternal() { return mUserManagerInternal; diff --git a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java index 684bbd4fc8ebd..661ce113e81ea 100644 --- a/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java +++ b/services/tests/servicestests/src/com/android/server/locksettings/LockSettingsServiceTests.java @@ -416,6 +416,15 @@ public class LockSettingsServiceTests extends BaseLockSettingsServiceTests { eq(CREDENTIAL_TYPE_PASSWORD), any(), eq(MANAGED_PROFILE_USER_ID)); } + @Test + public void testCredentialChangeNotPossibleInSecureFrpMode() { + mSettings.setSecureFrpMode(true); + try { + mService.setLockCredential(newPassword("1234"), nonePassword(), PRIMARY_USER_ID); + fail("Password shouldn't be changeable before FRP unlock"); + } catch (SecurityException e) { } + } + private void testCreateCredential(int userId, LockscreenCredential credential) throws RemoteException { assertTrue(mService.setLockCredential(credential, nonePassword(), userId));