summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Biggers <ebiggers@google.com>2023-07-28 22:03:03 +0000
committerAndroid Build Coastguard Worker <android-build-coastguard-worker@google.com>2023-08-22 02:13:45 +0000
commit535988f4309790a420ed66d7e62e5eb0f647e1be (patch)
tree2485819f5a5c1c77f95d44ac7da29add77cb685e
parentde1f76fece78ee7bd7c436faa170f48371dc7520 (diff)
downloadbase-535988f4309790a420ed66d7e62e5eb0f647e1be.tar.gz
RESTRICT AUTOMERGE: SettingsProvider: exclude secure_frp_mode from resets
When RescueParty detects that a system process is crashing frequently, it tries to recover in various ways, such as by resetting all settings. Unfortunately, this included resetting the secure_frp_mode setting, which is the means by which the system keeps track of whether the Factory Reset Protection (FRP) challenge has been passed yet. With this setting reset, some FRP restrictions went away and it became possible to bypass FRP by setting a new lockscreen credential. Fix this by excluding secure_frp_mode from resets. Note: currently this bug isn't reproducible on 'main' due to ag/23727749 disabling much of RescueParty, but that is a temporary change. Bug: 253043065 Test: With ag/23727749 reverted and with my fix to prevent com.android.settings from crashing *not* applied, tried repeatedly setting lockscreen credential while in FRP mode, using the smartlock setup activity launched by intent via adb. Verified that although RescueParty is still triggered after 5 attempts, secure_frp_mode is no longer reset (its value remains "1"). Test: Verified that secure_frp_mode still gets changed from 1 to 0 when FRP is passed legitimately. Test: atest com.android.providers.settings.SettingsProviderTest Test: atest android.provider.SettingsProviderTest (cherry picked from commit 9890dd7f15c091f7d1a09e4fddb9f85d32015955) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:1c3d701c60015f379bc231450c8c8f86d463d6fd) Merged-In: Id95ed43b9cc2208090064392bcd5dc012710af93 Change-Id: Id95ed43b9cc2208090064392bcd5dc012710af93
-rw-r--r--packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java17
-rw-r--r--packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java25
2 files changed, 38 insertions, 4 deletions
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
index 23b63089849f..f60f8db5e773 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
@@ -3227,6 +3227,15 @@ public class SettingsProvider extends ContentProvider {
return settingsState.getSettingLocked(name);
}
+ private static boolean shouldExcludeSettingFromReset(Setting setting, String prefix) {
+ // If a prefix was specified, exclude settings whose names don't start with it.
+ if (prefix != null && !setting.getName().startsWith(prefix)) {
+ return true;
+ }
+ // Never reset SECURE_FRP_MODE, as it could be abused to bypass FRP via RescueParty.
+ return Global.SECURE_FRP_MODE.equals(setting.getName());
+ }
+
public void resetSettingsLocked(int type, int userId, String packageName, int mode,
String tag) {
resetSettingsLocked(type, userId, packageName, mode, tag, /*prefix=*/
@@ -3249,7 +3258,7 @@ public class SettingsProvider extends ContentProvider {
Setting setting = settingsState.getSettingLocked(name);
if (packageName.equals(setting.getPackageName())) {
if ((tag != null && !tag.equals(setting.getTag()))
- || (prefix != null && !setting.getName().startsWith(prefix))) {
+ || shouldExcludeSettingFromReset(setting, prefix)) {
continue;
}
if (settingsState.resetSettingLocked(name)) {
@@ -3270,7 +3279,7 @@ public class SettingsProvider extends ContentProvider {
Setting setting = settingsState.getSettingLocked(name);
if (!SettingsState.isSystemPackage(getContext(),
setting.getPackageName())) {
- if (prefix != null && !setting.getName().startsWith(prefix)) {
+ if (shouldExcludeSettingFromReset(setting, prefix)) {
continue;
}
if (settingsState.resetSettingLocked(name)) {
@@ -3291,7 +3300,7 @@ public class SettingsProvider extends ContentProvider {
Setting setting = settingsState.getSettingLocked(name);
if (!SettingsState.isSystemPackage(getContext(),
setting.getPackageName())) {
- if (prefix != null && !setting.getName().startsWith(prefix)) {
+ if (shouldExcludeSettingFromReset(setting, prefix)) {
continue;
}
if (setting.isDefaultFromSystem()) {
@@ -3316,7 +3325,7 @@ public class SettingsProvider extends ContentProvider {
for (String name : settingsState.getSettingNamesLocked()) {
Setting setting = settingsState.getSettingLocked(name);
boolean someSettingChanged = false;
- if (prefix != null && !setting.getName().startsWith(prefix)) {
+ if (shouldExcludeSettingFromReset(setting, prefix)) {
continue;
}
if (setting.isDefaultFromSystem()) {
diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java
index eaf0dcb9b4e7..a945c33bc20a 100644
--- a/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java
+++ b/packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java
@@ -464,6 +464,31 @@ public class SettingsProviderTest extends BaseSettingsProviderTest {
}
}
+ // To prevent FRP bypasses, the SECURE_FRP_MODE setting should not be reset when all other
+ // settings are reset. But it should still be possible to explicitly set its value.
+ @Test
+ public void testSecureFrpModeSettingCannotBeReset() throws Exception {
+ final String name = Settings.Global.SECURE_FRP_MODE;
+ final String origValue = getSetting(SETTING_TYPE_GLOBAL, name);
+ setSettingViaShell(SETTING_TYPE_GLOBAL, name, "1", false);
+ try {
+ assertEquals("1", getSetting(SETTING_TYPE_GLOBAL, name));
+ for (int type : new int[] { SETTING_TYPE_GLOBAL, SETTING_TYPE_SECURE }) {
+ resetSettingsViaShell(type, Settings.RESET_MODE_UNTRUSTED_DEFAULTS);
+ resetSettingsViaShell(type, Settings.RESET_MODE_UNTRUSTED_CHANGES);
+ resetSettingsViaShell(type, Settings.RESET_MODE_TRUSTED_DEFAULTS);
+ }
+ // The value should still be "1". It should not have been reset to null.
+ assertEquals("1", getSetting(SETTING_TYPE_GLOBAL, name));
+ // It should still be possible to explicitly set the value to "0".
+ setSettingViaShell(SETTING_TYPE_GLOBAL, name, "0", false);
+ assertEquals("0", getSetting(SETTING_TYPE_GLOBAL, name));
+ } finally {
+ setSettingViaShell(SETTING_TYPE_GLOBAL, name, origValue, false);
+ assertEquals(origValue, getSetting(SETTING_TYPE_GLOBAL, name));
+ }
+ }
+
private void doTestQueryStringInBracketsViaProviderApiForType(int type) {
// Make sure we have a clean slate.
deleteStringViaProviderApi(type, FAKE_SETTING_NAME);