diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java index 934f9481f7c8d..5e7f984d67267 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java @@ -16,7 +16,6 @@ package com.android.server.devicepolicy; -import android.annotation.NonNull; import android.annotation.UserIdInt; import android.app.admin.DeviceAdminInfo; import android.app.admin.DevicePolicyManager; @@ -25,7 +24,6 @@ import android.os.FileUtils; import android.os.PersistableBundle; import android.util.ArrayMap; import android.util.ArraySet; -import android.util.DebugUtils; import android.util.IndentingPrintWriter; import android.util.Slog; import android.util.TypedXmlPullParser; @@ -87,15 +85,6 @@ class DevicePolicyData { static final String NEW_USER_DISCLAIMER_NOT_NEEDED = "not_needed"; static final String NEW_USER_DISCLAIMER_NEEDED = "needed"; - private static final String ATTR_FACTORY_RESET_FLAGS = "factory-reset-flags"; - private static final String ATTR_FACTORY_RESET_REASON = "factory-reset-reason"; - - // NOTE: must be public because of DebugUtils.flagsToString() - public static final int FACTORY_RESET_FLAG_ON_BOOT = 1; - public static final int FACTORY_RESET_FLAG_WIPE_EXTERNAL_STORAGE = 2; - public static final int FACTORY_RESET_FLAG_WIPE_EUICC = 4; - public static final int FACTORY_RESET_FLAG_WIPE_FACTORY_RESET_PROTECTION = 8; - private static final String TAG = DevicePolicyManagerService.LOG_TAG; private static final boolean VERBOSE_LOG = false; // DO NOT SUBMIT WITH TRUE @@ -110,9 +99,6 @@ class DevicePolicyData { int mUserProvisioningState; int mPermissionPolicy; - int mFactoryResetFlags; - String mFactoryResetReason; - boolean mDeviceProvisioningConfigApplied = false; final ArrayMap mAdminMap = new ArrayMap<>(); @@ -214,17 +200,6 @@ class DevicePolicyData { out.attribute(null, ATTR_NEW_USER_DISCLAIMER, policyData.mNewUserDisclaimer); } - if (policyData.mFactoryResetFlags != 0) { - if (VERBOSE_LOG) { - Slog.v(TAG, "Storing factory reset flags for user " + policyData.mUserId + ": " - + factoryResetFlagsToString(policyData.mFactoryResetFlags)); - } - out.attributeInt(null, ATTR_FACTORY_RESET_FLAGS, policyData.mFactoryResetFlags); - } - if (policyData.mFactoryResetReason != null) { - out.attribute(null, ATTR_FACTORY_RESET_REASON, policyData.mFactoryResetReason); - } - // Serialize delegations. for (int i = 0; i < policyData.mDelegationMap.size(); ++i) { final String delegatePackage = policyData.mDelegationMap.keyAt(i); @@ -452,14 +427,6 @@ class DevicePolicyData { } policy.mNewUserDisclaimer = parser.getAttributeValue(null, ATTR_NEW_USER_DISCLAIMER); - policy.mFactoryResetFlags = parser.getAttributeInt(null, ATTR_FACTORY_RESET_FLAGS, 0); - if (VERBOSE_LOG) { - Slog.v(TAG, "Restored factory reset flags for user " + policy.mUserId + ": " - + factoryResetFlagsToString(policy.mFactoryResetFlags)); - } - policy.mFactoryResetReason = parser.getAttributeValue(null, ATTR_FACTORY_RESET_REASON); - - parser.next(); int outerDepth = parser.getDepth(); policy.mLockTaskPackages.clear(); policy.mAdminList.clear(); @@ -605,22 +572,6 @@ class DevicePolicyData { } } - void setDelayedFactoryReset(@NonNull String reason, boolean wipeExtRequested, boolean wipeEuicc, - boolean wipeResetProtectionData) { - mFactoryResetReason = reason; - - mFactoryResetFlags = FACTORY_RESET_FLAG_ON_BOOT; - if (wipeExtRequested) { - mFactoryResetFlags |= FACTORY_RESET_FLAG_WIPE_EXTERNAL_STORAGE; - } - if (wipeEuicc) { - mFactoryResetFlags |= FACTORY_RESET_FLAG_WIPE_EUICC; - } - if (wipeResetProtectionData) { - mFactoryResetFlags |= FACTORY_RESET_FLAG_WIPE_FACTORY_RESET_PROTECTION; - } - } - void dump(IndentingPrintWriter pw) { pw.println(); pw.println("Enabled Device Admins (User " + mUserId + ", provisioningState: " @@ -652,19 +603,6 @@ class DevicePolicyData { pw.print("mUserSetupComplete="); pw.println(mUserSetupComplete); pw.print("mAffiliationIds="); pw.println(mAffiliationIds); pw.print("mNewUserDisclaimer="); pw.println(mNewUserDisclaimer); - if (mFactoryResetFlags != 0) { - pw.print("mFactoryResetFlags="); pw.print(mFactoryResetFlags); - pw.print(" ("); - pw.print(factoryResetFlagsToString(mFactoryResetFlags)); - pw.println(')'); - } - if (mFactoryResetReason != null) { - pw.print("mFactoryResetReason="); pw.println(mFactoryResetReason); - } pw.decreaseIndent(); } - - static String factoryResetFlagsToString(int flags) { - return DebugUtils.flagsToString(DevicePolicyData.class, "FACTORY_RESET_FLAG_", flags); - } } diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 83c587027bfa3..8b575fb16a2a7 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -1317,11 +1317,12 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { mContext.getSystemService(PowerManager.class).reboot(reason); } - boolean recoverySystemRebootWipeUserData(boolean shutdown, String reason, boolean force, + void recoverySystemRebootWipeUserData(boolean shutdown, String reason, boolean force, boolean wipeEuicc, boolean wipeExtRequested, boolean wipeResetProtectionData) throws IOException { - return FactoryResetter.newBuilder(mContext).setSafetyChecker(mSafetyChecker) - .setReason(reason).setShutdown(shutdown).setForce(force).setWipeEuicc(wipeEuicc) + FactoryResetter.newBuilder(mContext).setSafetyChecker(mSafetyChecker) + .setReason(reason).setShutdown(shutdown) + .setForce(force).setWipeEuicc(wipeEuicc) .setWipeAdoptableStorage(wipeExtRequested) .setWipeFactoryResetProtection(wipeResetProtectionData) .build().factoryReset(); @@ -2783,10 +2784,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { maybeStartSecurityLogMonitorOnActivityManagerReady(); break; case SystemService.PHASE_BOOT_COMPLETED: - // Ideally it should be done earlier, but currently it relies on RecoverySystem, - // which would hang on earlier phases - factoryResetIfDelayedEarlier(); - ensureDeviceOwnerUserStarted(); // TODO Consider better place to do this. break; } @@ -6220,24 +6217,10 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { boolean wipeResetProtectionData) { wtfIfInLock(); boolean success = false; - try { - boolean delayed = !mInjector.recoverySystemRebootWipeUserData( + mInjector.recoverySystemRebootWipeUserData( /* shutdown= */ false, reason, /* force= */ true, /* wipeEuicc= */ wipeEuicc, wipeExtRequested, wipeResetProtectionData); - if (delayed) { - // Persist the request so the device is automatically factory-reset on next start if - // the system crashes or reboots before the {@code DevicePolicySafetyChecker} calls - // its callback. - Slog.i(LOG_TAG, String.format("Persisting factory reset request as it could be " - + "delayed by %s", mSafetyChecker)); - synchronized (getLockObject()) { - DevicePolicyData policy = getUserData(UserHandle.USER_SYSTEM); - policy.setDelayedFactoryReset(reason, wipeExtRequested, wipeEuicc, - wipeResetProtectionData); - saveSettingsLocked(UserHandle.USER_SYSTEM); - } - } success = true; } catch (IOException | SecurityException e) { Slog.w(LOG_TAG, "Failed requesting data wipe", e); @@ -6246,40 +6229,6 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } } - private void factoryResetIfDelayedEarlier() { - synchronized (getLockObject()) { - DevicePolicyData policy = getUserData(UserHandle.USER_SYSTEM); - - if (policy.mFactoryResetFlags == 0) return; - - if (policy.mFactoryResetReason == null) { - // Shouldn't happen. - Slog.e(LOG_TAG, "no persisted reason for factory resetting"); - policy.mFactoryResetReason = "requested before boot"; - } - FactoryResetter factoryResetter = FactoryResetter.newBuilder(mContext) - .setReason(policy.mFactoryResetReason).setForce(true) - .setWipeEuicc((policy.mFactoryResetFlags & DevicePolicyData - .FACTORY_RESET_FLAG_WIPE_EUICC) != 0) - .setWipeAdoptableStorage((policy.mFactoryResetFlags & DevicePolicyData - .FACTORY_RESET_FLAG_WIPE_EXTERNAL_STORAGE) != 0) - .setWipeFactoryResetProtection((policy.mFactoryResetFlags & DevicePolicyData - .FACTORY_RESET_FLAG_WIPE_FACTORY_RESET_PROTECTION) != 0) - .build(); - Slog.i(LOG_TAG, "Factory resetting on boot using " + factoryResetter); - try { - if (!factoryResetter.factoryReset()) { - // Shouldn't happen because FactoryResetter was created without a - // DevicePolicySafetyChecker. - Slog.wtf(LOG_TAG, "Factory reset using " + factoryResetter + " failed."); - } - } catch (IOException e) { - // Shouldn't happen. - Slog.wtf(LOG_TAG, "Could not factory reset using " + factoryResetter, e); - } - } - } - private void forceWipeUser(int userId, String wipeReasonForUser, boolean wipeSilently) { boolean success = false; try { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/FactoryResetter.java b/services/devicepolicy/java/com/android/server/devicepolicy/FactoryResetter.java index 457255bd9a730..564950bef4f58 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/FactoryResetter.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/FactoryResetter.java @@ -52,17 +52,14 @@ public final class FactoryResetter { /** * Factory reset the device according to the builder's arguments. - * - * @return {@code true} if device was factory reset, or {@code false} if it was delayed by the - * {@link DevicePolicySafetyChecker}. */ - public boolean factoryReset() throws IOException { + public void factoryReset() throws IOException { Preconditions.checkCallAuthorization(mContext.checkCallingOrSelfPermission( android.Manifest.permission.MASTER_CLEAR) == PackageManager.PERMISSION_GRANTED); if (mSafetyChecker == null) { factoryResetInternalUnchecked(); - return true; + return; } IResultReceiver receiver = new IResultReceiver.Stub() { @@ -80,36 +77,6 @@ public final class FactoryResetter { }; Slog.i(TAG, String.format("Delaying factory reset until %s confirms", mSafetyChecker)); mSafetyChecker.onFactoryReset(receiver); - return false; - } - - @Override - public String toString() { - StringBuilder builder = new StringBuilder("FactoryResetter["); - if (mReason == null) { - builder.append("no_reason"); - } else { - builder.append("reason='").append(mReason).append("'"); - } - if (mSafetyChecker != null) { - builder.append(",hasSafetyChecker"); - } - if (mShutdown) { - builder.append(",shutdown"); - } - if (mForce) { - builder.append(",force"); - } - if (mWipeEuicc) { - builder.append(",wipeEuicc"); - } - if (mWipeAdoptableStorage) { - builder.append(",wipeAdoptableStorage"); - } - if (mWipeFactoryResetProtection) { - builder.append(",ipeFactoryResetProtection"); - } - return builder.append(']').toString(); } private void factoryResetInternalUnchecked() throws IOException { diff --git a/services/tests/mockingservicestests/src/com/android/server/devicepolicy/FactoryResetterTest.java b/services/tests/mockingservicestests/src/com/android/server/devicepolicy/FactoryResetterTest.java index 589a3497435e9..c4d14f947cdae 100644 --- a/services/tests/mockingservicestests/src/com/android/server/devicepolicy/FactoryResetterTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/devicepolicy/FactoryResetterTest.java @@ -20,8 +20,6 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.doAnswer; import static com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession; import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify; -import static com.google.common.truth.Truth.assertThat; - import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.Mockito.never; @@ -160,11 +158,10 @@ public final class FactoryResetterTest { public void testFactoryReset_storageOnly() throws Exception { allowFactoryReset(); - boolean success = FactoryResetter.newBuilder(mContext) + FactoryResetter.newBuilder(mContext) .setWipeAdoptableStorage(true).build() .factoryReset(); - assertThat(success).isTrue(); verifyWipeAdoptableStorageCalled(); verifyWipeFactoryResetProtectionNotCalled(); verifyRebootWipeUserDataMinimumArgsCalled(); @@ -174,11 +171,10 @@ public final class FactoryResetterTest { public void testFactoryReset_frpOnly() throws Exception { allowFactoryReset(); - boolean success = FactoryResetter.newBuilder(mContext) + FactoryResetter.newBuilder(mContext) .setWipeFactoryResetProtection(true) .build().factoryReset(); - assertThat(success).isTrue(); verifyWipeAdoptableStorageNotCalled(); verifyWipeFactoryResetProtectionCalled(); verifyRebootWipeUserDataMinimumArgsCalled(); @@ -188,7 +184,7 @@ public final class FactoryResetterTest { public void testFactoryReset_allArgs() throws Exception { allowFactoryReset(); - boolean success = FactoryResetter.newBuilder(mContext) + FactoryResetter.newBuilder(mContext) .setReason(REASON) .setForce(true) .setShutdown(true) @@ -197,7 +193,6 @@ public final class FactoryResetterTest { .setWipeFactoryResetProtection(true) .build().factoryReset(); - assertThat(success).isTrue(); verifyWipeAdoptableStorageCalled(); verifyWipeFactoryResetProtectionCalled(); verifyRebootWipeUserDataAllArgsCalled(); @@ -207,10 +202,9 @@ public final class FactoryResetterTest { public void testFactoryReset_minimumArgs_safetyChecker_neverReplied() throws Exception { allowFactoryReset(); - boolean success = FactoryResetter.newBuilder(mContext) - .setSafetyChecker(mSafetyChecker).build().factoryReset(); + FactoryResetter.newBuilder(mContext).setSafetyChecker(mSafetyChecker).build() + .factoryReset(); - assertThat(success).isFalse(); verifyWipeAdoptableStorageNotCalled(); verifyWipeFactoryResetProtectionNotCalled(); verifyRebootWipeUserDataNotCalled(); @@ -227,7 +221,7 @@ public final class FactoryResetterTest { return null; }).when(mSafetyChecker).onFactoryReset(any()); - boolean success = FactoryResetter.newBuilder(mContext) + FactoryResetter.newBuilder(mContext) .setSafetyChecker(mSafetyChecker) .setReason(REASON) .setForce(true) @@ -237,7 +231,6 @@ public final class FactoryResetterTest { .setWipeFactoryResetProtection(true) .build().factoryReset(); - assertThat(success).isFalse(); verifyWipeAdoptableStorageCalled(); verifyWipeFactoryResetProtectionCalled(); verifyRebootWipeUserDataAllArgsCalled(); diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java index f3576af916554..9c28c99fcc071 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerServiceTestable.java @@ -319,10 +319,10 @@ public class DevicePolicyManagerServiceTestable extends DevicePolicyManagerServi } @Override - boolean recoverySystemRebootWipeUserData(boolean shutdown, String reason, boolean force, + void recoverySystemRebootWipeUserData(boolean shutdown, String reason, boolean force, boolean wipeEuicc, boolean wipeExtRequested, boolean wipeResetProtectionData) throws IOException { - return services.recoverySystem.rebootWipeUserData(shutdown, reason, force, wipeEuicc, + services.recoverySystem.rebootWipeUserData(shutdown, reason, force, wipeEuicc, wipeExtRequested, wipeResetProtectionData); } diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java b/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java index d8e0c5cade773..7d1de866cb449 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/MockSystemServices.java @@ -392,10 +392,9 @@ public class MockSystemServices { } public static class RecoverySystemForMock { - public boolean rebootWipeUserData(boolean shutdown, String reason, boolean force, + public void rebootWipeUserData(boolean shutdown, String reason, boolean force, boolean wipeEuicc, boolean wipeExtRequested, boolean wipeResetProtectionData) throws IOException { - return false; } }