Skip to content

Commit 72c2f96

Browse files
ebiggersAndroid Build Coastguard Worker
authored andcommitted
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) (changed Global.SECURE_FRP_MODE to Secure.SECURE_FRP_MODE, needed because this setting was moved in U) (removed static keyword from shouldExcludeSettingFromReset(), needed for compatibility with Java 15 and earlier) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:efc2b69c88c3b4686a521935d6e5e9884b9e6347) Merged-In: Id95ed43b9cc2208090064392bcd5dc012710af93 Change-Id: Id95ed43b9cc2208090064392bcd5dc012710af93
1 parent 1538790 commit 72c2f96

2 files changed

Lines changed: 38 additions & 4 deletions

File tree

packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3101,6 +3101,15 @@ public Setting getSettingLocked(int type, int userId, String name) {
31013101
return settingsState.getSettingLocked(name);
31023102
}
31033103

3104+
private boolean shouldExcludeSettingFromReset(Setting setting, String prefix) {
3105+
// If a prefix was specified, exclude settings whose names don't start with it.
3106+
if (prefix != null && !setting.getName().startsWith(prefix)) {
3107+
return true;
3108+
}
3109+
// Never reset SECURE_FRP_MODE, as it could be abused to bypass FRP via RescueParty.
3110+
return Secure.SECURE_FRP_MODE.equals(setting.getName());
3111+
}
3112+
31043113
public void resetSettingsLocked(int type, int userId, String packageName, int mode,
31053114
String tag) {
31063115
resetSettingsLocked(type, userId, packageName, mode, tag, /*prefix=*/
@@ -3123,7 +3132,7 @@ public void resetSettingsLocked(int type, int userId, String packageName, int mo
31233132
Setting setting = settingsState.getSettingLocked(name);
31243133
if (packageName.equals(setting.getPackageName())) {
31253134
if ((tag != null && !tag.equals(setting.getTag()))
3126-
|| (prefix != null && !setting.getName().startsWith(prefix))) {
3135+
|| shouldExcludeSettingFromReset(setting, prefix)) {
31273136
continue;
31283137
}
31293138
if (settingsState.resetSettingLocked(name)) {
@@ -3143,7 +3152,7 @@ public void resetSettingsLocked(int type, int userId, String packageName, int mo
31433152
Setting setting = settingsState.getSettingLocked(name);
31443153
if (!SettingsState.isSystemPackage(getContext(),
31453154
setting.getPackageName())) {
3146-
if (prefix != null && !setting.getName().startsWith(prefix)) {
3155+
if (shouldExcludeSettingFromReset(setting, prefix)) {
31473156
continue;
31483157
}
31493158
if (settingsState.resetSettingLocked(name)) {
@@ -3163,7 +3172,7 @@ public void resetSettingsLocked(int type, int userId, String packageName, int mo
31633172
Setting setting = settingsState.getSettingLocked(name);
31643173
if (!SettingsState.isSystemPackage(getContext(),
31653174
setting.getPackageName())) {
3166-
if (prefix != null && !setting.getName().startsWith(prefix)) {
3175+
if (shouldExcludeSettingFromReset(setting, prefix)) {
31673176
continue;
31683177
}
31693178
if (setting.isDefaultFromSystem()) {
@@ -3186,7 +3195,7 @@ public void resetSettingsLocked(int type, int userId, String packageName, int mo
31863195
for (String name : settingsState.getSettingNamesLocked()) {
31873196
Setting setting = settingsState.getSettingLocked(name);
31883197
boolean someSettingChanged = false;
3189-
if (prefix != null && !setting.getName().startsWith(prefix)) {
3198+
if (shouldExcludeSettingFromReset(setting, prefix)) {
31903199
continue;
31913200
}
31923201
if (setting.isDefaultFromSystem()) {

packages/SettingsProvider/test/src/com/android/providers/settings/SettingsProviderTest.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,31 @@ private void testResetModeTrustedDefaultsCommon(int type) throws Exception {
464464
}
465465
}
466466

467+
// To prevent FRP bypasses, the SECURE_FRP_MODE setting should not be reset when all other
468+
// settings are reset. But it should still be possible to explicitly set its value.
469+
@Test
470+
public void testSecureFrpModeSettingCannotBeReset() throws Exception {
471+
final String name = Settings.Secure.SECURE_FRP_MODE;
472+
final String origValue = getSetting(SETTING_TYPE_GLOBAL, name);
473+
setSettingViaShell(SETTING_TYPE_GLOBAL, name, "1", false);
474+
try {
475+
assertEquals("1", getSetting(SETTING_TYPE_GLOBAL, name));
476+
for (int type : new int[] { SETTING_TYPE_GLOBAL, SETTING_TYPE_SECURE }) {
477+
resetSettingsViaShell(type, Settings.RESET_MODE_UNTRUSTED_DEFAULTS);
478+
resetSettingsViaShell(type, Settings.RESET_MODE_UNTRUSTED_CHANGES);
479+
resetSettingsViaShell(type, Settings.RESET_MODE_TRUSTED_DEFAULTS);
480+
}
481+
// The value should still be "1". It should not have been reset to null.
482+
assertEquals("1", getSetting(SETTING_TYPE_GLOBAL, name));
483+
// It should still be possible to explicitly set the value to "0".
484+
setSettingViaShell(SETTING_TYPE_GLOBAL, name, "0", false);
485+
assertEquals("0", getSetting(SETTING_TYPE_GLOBAL, name));
486+
} finally {
487+
setSettingViaShell(SETTING_TYPE_GLOBAL, name, origValue, false);
488+
assertEquals(origValue, getSetting(SETTING_TYPE_GLOBAL, name));
489+
}
490+
}
491+
467492
private void doTestQueryStringInBracketsViaProviderApiForType(int type) {
468493
// Make sure we have a clean slate.
469494
deleteStringViaProviderApi(type, FAKE_SETTING_NAME);

0 commit comments

Comments
 (0)