From 171eeee60d2b7a42495660c51aba10510162dc87 Mon Sep 17 00:00:00 2001 From: Eran Messeri Date: Wed, 24 Feb 2021 15:51:33 +0000 Subject: [PATCH] Implement Device Policy upgrade flow Implement an upgrade flow in the DevicePolicyManagerService for admin-set policies. The purpose is to allow graceful, rather than ad-hoc, setting of default values for new policies added to DevicePolicyData or ActiveAdmin. This change also cleans up the lowest of hanging fruits - presence of password properties metrics, by upgrading from version 0 (nonexistent) to version 1. The versioning mechanism is meant for handling additional upgrade scenarios. Test: Manual upgrade, flash to a build without a version and check that the device boots and the version file is created. Test: atest FrameworksServicesTests:PolicyVersionUpgraderTest Bug: 181009337 Change-Id: Iac7d345b34beba00d6d5fb10d6fb0a2c745592e0 --- .../server/devicepolicy/DevicePolicyData.java | 9 +- .../DevicePolicyManagerService.java | 69 +++++-- .../PolicyUpgraderDataProvider.java | 63 +++++++ .../devicepolicy/PolicyVersionUpgrader.java | 168 +++++++++++++++++ .../device_policies.xml | 9 + .../PolicyVersionUpgraderTest.java | 174 ++++++++++++++++++ 6 files changed, 474 insertions(+), 18 deletions(-) create mode 100644 services/devicepolicy/java/com/android/server/devicepolicy/PolicyUpgraderDataProvider.java create mode 100644 services/devicepolicy/java/com/android/server/devicepolicy/PolicyVersionUpgrader.java create mode 100644 services/tests/servicestests/assets/PolicyVersionUpgraderTest/device_policies.xml create mode 100644 services/tests/servicestests/src/com/android/server/devicepolicy/PolicyVersionUpgraderTest.java diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java index ba3ae45e99f88..c0b2ed4cc955f 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyData.java @@ -399,7 +399,7 @@ class DevicePolicyData { * @param adminInfoSupplier function that queries DeviceAdminInfo from PackageManager * @param ownerComponent device or profile owner component if any. */ - static boolean load(DevicePolicyData policy, boolean isFdeDevice, JournaledFile journaledFile, + static void load(DevicePolicyData policy, boolean isFdeDevice, JournaledFile journaledFile, Function adminInfoSupplier, ComponentName ownerComponent) { FileInputStream stream = null; @@ -407,7 +407,7 @@ class DevicePolicyData { if (VERBOSE_LOG) { Slog.v(TAG, "Loading data for user " + policy.mUserId + " from " + file); } - boolean needsRewrite = false; + try { stream = new FileInputStream(file); TypedXmlPullParser parser = Xml.resolvePullParser(stream); @@ -541,10 +541,6 @@ class DevicePolicyData { policy.mAdminBroadcastPending = Boolean.toString(true).equals(pending); } else if (TAG_INITIALIZATION_BUNDLE.equals(tag)) { policy.mInitBundle = PersistableBundle.restoreFromXml(parser); - } else if ("active-password".equals(tag)) { - // Remove password metrics from saved settings, as we no longer wish to store - // these on disk - needsRewrite = true; } else if (TAG_PASSWORD_VALIDITY.equals(tag)) { if (isFdeDevice) { // This flag is only used for FDE devices @@ -584,7 +580,6 @@ class DevicePolicyData { // Generate a list of admins from the admin map policy.mAdminList.addAll(policy.mAdminMap.values()); - return needsRewrite; } void validatePasswordOwner() { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 28e9acf8d883f..2e66b7c2650a9 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -371,9 +371,11 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { protected static final String LOG_TAG = "DevicePolicyManager"; - private static final boolean VERBOSE_LOG = false; // DO NOT SUBMIT WITH TRUE + static final boolean VERBOSE_LOG = false; // DO NOT SUBMIT WITH TRUE - private static final String DEVICE_POLICIES_XML = "device_policies.xml"; + static final String DEVICE_POLICIES_XML = "device_policies.xml"; + + static final String POLICIES_VERSION_XML = "device_policies_version"; private static final String TRANSFER_OWNERSHIP_PARAMETERS_XML = "transfer-ownership-parameters.xml"; @@ -466,8 +468,12 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { private static final Set GLOBAL_SETTINGS_DEPRECATED; private static final Set SYSTEM_SETTINGS_ALLOWLIST; private static final Set DA_DISALLOWED_POLICIES; - // A collection of user restrictions that are deprecated and should simply be ignored. private static final String AB_DEVICE_KEY = "ro.build.ab_update"; + // The version of the current DevicePolicyManagerService data. This version is used + // to decide whether an existing policy in the {@link #DEVICE_POLICIES_XML} needs to + // be upgraded. See {@link PolicyVersionUpgrader} on instructions how to add an upgrade + // step. + static final int DPMS_VERSION = 1; static { SECURE_SETTINGS_ALLOWLIST = new ArraySet<>(); @@ -2760,8 +2766,8 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { : mInjector.environmentGetUserSystemDirectory(userId); } - private JournaledFile makeJournaledFile(@UserIdInt int userId) { - final String base = new File(getPolicyFileDirectory(userId), DEVICE_POLICIES_XML) + private JournaledFile makeJournaledFile(@UserIdInt int userId, String fileName) { + final String base = new File(getPolicyFileDirectory(userId), fileName) .getAbsolutePath(); if (VERBOSE_LOG) { Log.v(LOG_TAG, "Opening " + base); @@ -2769,6 +2775,10 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { return new JournaledFile(new File(base), new File(base + ".tmp")); } + private JournaledFile makeJournaledFile(@UserIdInt int userId) { + return makeJournaledFile(userId, DEVICE_POLICIES_XML); + } + /** * Persist modified values to disk by calling {@link #saveSettingsLocked} for each * affected user ID. @@ -2797,18 +2807,13 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { } private void loadSettingsLocked(DevicePolicyData policy, int userHandle) { - boolean needsRewrite = DevicePolicyData.load(policy, + DevicePolicyData.load(policy, !mInjector.storageManagerIsFileBasedEncryptionEnabled(), makeJournaledFile(userHandle), component -> findAdmin( component, userHandle, /* throwForMissingPermission= */ false), getOwnerComponent(userHandle)); - // Might need to upgrade the file by rewriting it - if (needsRewrite) { - saveSettingsLocked(userHandle); - } - policy.validatePasswordOwner(); updateMaximumTimeToLockLocked(userHandle); updateLockTaskPackagesLocked(policy.mLockTaskPackages, userHandle); @@ -2922,6 +2927,7 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { private void onLockSettingsReady() { synchronized (getLockObject()) { migrateUserRestrictionsIfNecessaryLocked(); + performPolicyVersionUpgrade(); } getUserData(UserHandle.USER_SYSTEM); cleanUpOldUsers(); @@ -2962,6 +2968,47 @@ public class DevicePolicyManagerService extends BaseIDevicePolicyManager { updateUsbDataSignal(); } + private class DpmsUpgradeDataProvider implements PolicyUpgraderDataProvider { + @Override + public boolean isUserDeviceOwner(int userId) { + return mOwners.isDeviceOwnerUserId(userId); + } + + @Override + public boolean storageManagerIsFileBasedEncryptionEnabled() { + return mInjector.storageManagerIsFileBasedEncryptionEnabled(); + } + + @Override + public JournaledFile makeDevicePoliciesJournaledFile(int userId) { + return DevicePolicyManagerService.this.makeJournaledFile(userId, DEVICE_POLICIES_XML); + } + + @Override + public JournaledFile makePoliciesVersionJournaledFile(int userId) { + return DevicePolicyManagerService.this.makeJournaledFile(userId, POLICIES_VERSION_XML); + } + + @Override + public ComponentName getOwnerComponent(int userId) { + return DevicePolicyManagerService.this.getOwnerComponent(userId); + } + + @Override + public Function getAdminInfoSupplier(int userId) { + return component -> findAdmin(component, userId, /* throwForMissingPermission= */ + false); + } + } + + private void performPolicyVersionUpgrade() { + List allUsers = mUserManager.getUsers(); + PolicyVersionUpgrader upgrader = new PolicyVersionUpgrader( + new DpmsUpgradeDataProvider()); + + upgrader.upgradePolicy(allUsers.stream().mapToInt(u -> u.id).toArray(), DPMS_VERSION); + } + private void revertTransferOwnershipIfNecessaryLocked() { if (!mTransferOwnershipMetadataManager.metadataFileExists()) { return; diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyUpgraderDataProvider.java b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyUpgraderDataProvider.java new file mode 100644 index 0000000000000..b6420f8ff4bc3 --- /dev/null +++ b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyUpgraderDataProvider.java @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.devicepolicy; + +import android.annotation.Nullable; +import android.app.admin.DeviceAdminInfo; +import android.content.ComponentName; + +import com.android.internal.util.JournaledFile; + +import java.util.function.Function; + +/** + * An interface for providing the {@code PolicyVersionUpgrader} with all the data necessary + * to go through the upgrade process. + */ +public interface PolicyUpgraderDataProvider { + /** + * Returns true if the provided {@code userId} is a device owner. May affect some policy + * defaults. + */ + boolean isUserDeviceOwner(int userId); + + /** + * Returns true if the storage manager indicates file-based encryption is enabled. + */ + boolean storageManagerIsFileBasedEncryptionEnabled(); + + /** + * Returns the journaled policies file for a given user. + */ + JournaledFile makeDevicePoliciesJournaledFile(int userId); + + /** + * Returns the journaled policy version file for a given user. + */ + JournaledFile makePoliciesVersionJournaledFile(int userId); + + /** + * Returns the {@code ComponentName} of the owner component for a user. + */ + @Nullable ComponentName getOwnerComponent(int userId); + + /** + * Returns a function which provides the component name and device admin info for a given + * user. + */ + Function getAdminInfoSupplier(int userId); +} diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/PolicyVersionUpgrader.java b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyVersionUpgrader.java new file mode 100644 index 0000000000000..cea08634910cb --- /dev/null +++ b/services/devicepolicy/java/com/android/server/devicepolicy/PolicyVersionUpgrader.java @@ -0,0 +1,168 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.devicepolicy; + +import android.os.UserHandle; +import android.util.Slog; +import android.util.SparseArray; + +import com.android.internal.util.JournaledFile; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.file.Files; + +/** + * Class for dealing with Device Policy Manager Service version upgrades. + * Initially, this class is responsible for upgrading the "device_policies.xml" file upon + * platform version upgrade. + * + * It is useful for policies which have a different default for an upgrading device than a + * newly-configured device (for example, the admin can grant sensors-related permissions by + * default on existing fully-managed devices that upgrade to Android S, but on devices set up + * with Android S the value of the policy is set explicitly during set-up). + * + * Practically, it's useful for changes to the data model of the {@code DevicePolicyData} and + * {@code ActiveAdmin} classes. + * + * To add a new upgrade step: + * (1) Increase the {@code DPMS_VERSION} constant in {@code DevicePolicyManagerService} by one. + * (2) Add an if statement in {@code upgradePolicy} comparing the version, performing the upgrade + * step and setting the value of {@code currentVersion} to the newly-incremented version. + * (3) Add a test in {@code PolicyVersionUpgraderTest}. + */ +public class PolicyVersionUpgrader { + private static final String LOG_TAG = "DevicePolicyManager"; + private static final boolean VERBOSE_LOG = DevicePolicyManagerService.VERBOSE_LOG; + private final PolicyUpgraderDataProvider mProvider; + + public PolicyVersionUpgrader(PolicyUpgraderDataProvider provider) { + mProvider = provider; + } + + /** + * Performs the upgrade steps for all users on the system. + * + * @param allUsers List of all user IDs on the system, including disabled users, as well as + * managed profile user IDs. + * @param dpmsVersion The version to upgrade to. + */ + public void upgradePolicy(int[] allUsers, int dpmsVersion) { + int oldVersion = readVersion(); + if (oldVersion >= dpmsVersion) { + Slog.i(LOG_TAG, String.format("Current version %d, latest version %d, not upgrading.", + oldVersion, dpmsVersion)); + return; + } + + //NOTE: The current version is provided in case the XML file format changes in a + // non-backwards-compatible way, so that DeviceAdminData could load it with + // old tags, for example. + final SparseArray allUsersData = loadAllUsersData(allUsers, oldVersion); + + int currentVersion = oldVersion; + if (currentVersion == 0) { + Slog.i(LOG_TAG, String.format("Upgrading from version %d", currentVersion)); + // The first upgrade (from no version to version 1) is to overwrite + // the "active-password" tag in case it was left around. + currentVersion = 1; + } + + writePoliciesAndVersion(allUsers, allUsersData, currentVersion); + } + + private void writePoliciesAndVersion(int[] allUsers, SparseArray allUsersData, + int currentVersion) { + boolean allWritesSuccessful = true; + for (int user : allUsers) { + allWritesSuccessful = allWritesSuccessful && writeDataForUser(user, + allUsersData.get(user)); + } + + if (allWritesSuccessful) { + writeVersion(currentVersion); + } else { + Slog.e(LOG_TAG, String.format("Error: Failed upgrading policies to version %d", + currentVersion)); + } + } + + private SparseArray loadAllUsersData(int[] allUsers, int loadVersion) { + final SparseArray allUsersData = new SparseArray<>(); + for (int user: allUsers) { + allUsersData.append(user, loadDataForUser(user, loadVersion)); + } + return allUsersData; + } + + private DevicePolicyData loadDataForUser(int userId, int loadVersion) { + DevicePolicyData policy = new DevicePolicyData(userId); + DevicePolicyData.load(policy, + !mProvider.storageManagerIsFileBasedEncryptionEnabled(), + mProvider.makeDevicePoliciesJournaledFile(userId), + mProvider.getAdminInfoSupplier(userId), + mProvider.getOwnerComponent(userId)); + return policy; + } + + private boolean writeDataForUser(int userId, DevicePolicyData policy) { + return DevicePolicyData.store( + policy, + mProvider.makeDevicePoliciesJournaledFile(userId), + !mProvider.storageManagerIsFileBasedEncryptionEnabled()); + } + + private JournaledFile getVersionFile() { + return mProvider.makePoliciesVersionJournaledFile(UserHandle.USER_SYSTEM); + } + + private int readVersion() { + JournaledFile versionFile = getVersionFile(); + + File file = versionFile.chooseForRead(); + if (VERBOSE_LOG) { + Slog.v(LOG_TAG, "Loading version from " + file); + } + try { + String versionString = Files.readAllLines( + file.toPath(), Charset.defaultCharset()).get(0); + return Integer.parseInt(versionString); + } catch (IOException | NumberFormatException | IndexOutOfBoundsException e) { + Slog.e(LOG_TAG, "Error reading version", e); + return 0; + } + } + + private void writeVersion(int version) { + JournaledFile versionFile = getVersionFile(); + + File file = versionFile.chooseForWrite(); + if (VERBOSE_LOG) { + Slog.v(LOG_TAG, String.format("Writing new version to: %s", file)); + } + + try { + byte[] versionBytes = String.format("%d", version).getBytes(); + Files.write(file.toPath(), versionBytes); + versionFile.commit(); + } catch (IOException e) { + Slog.e(LOG_TAG, String.format("Writing version %d failed: %s", version), e); + versionFile.rollback(); + } + } +} diff --git a/services/tests/servicestests/assets/PolicyVersionUpgraderTest/device_policies.xml b/services/tests/servicestests/assets/PolicyVersionUpgraderTest/device_policies.xml new file mode 100644 index 0000000000000..d6ad99d7965d5 --- /dev/null +++ b/services/tests/servicestests/assets/PolicyVersionUpgraderTest/device_policies.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/PolicyVersionUpgraderTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/PolicyVersionUpgraderTest.java new file mode 100644 index 0000000000000..f94b800afbef5 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/PolicyVersionUpgraderTest.java @@ -0,0 +1,174 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.devicepolicy; + +import static com.google.common.truth.Truth.assertThat; + +import android.app.admin.DeviceAdminInfo; +import android.content.ComponentName; +import android.content.Context; + +import androidx.test.InstrumentationRegistry; + +import com.android.internal.util.JournaledFile; + +import com.google.common.io.Files; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.Charset; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Function; + +@RunWith(JUnit4.class) +public class PolicyVersionUpgraderTest { + // NOTE: Only change this value if the corresponding CL also adds a test to test the upgrade + // to the new version. + private static final int LATEST_TESTED_VERSION = 1; + + private static class FakePolicyUpgraderDataProvider implements PolicyUpgraderDataProvider { + int mDeviceOwnerUserId; + boolean mIsFileBasedEncryptionEnabled; + Map mUserToComponent = new HashMap<>(); + Map mComponentToDeviceAdminInfo; + File mDataDir; + + @Override + public boolean isUserDeviceOwner(int userId) { + return userId == mDeviceOwnerUserId; + } + + @Override + public boolean storageManagerIsFileBasedEncryptionEnabled() { + return mIsFileBasedEncryptionEnabled; + } + + private JournaledFile makeJournaledFile(int userId, String fileName) { + File parentDir = new File(mDataDir, String.format("user%d", userId)); + if (!parentDir.exists()) { + parentDir.mkdirs(); + } + + final String base = new File(parentDir, fileName).getAbsolutePath(); + return new JournaledFile(new File(base), new File(base + ".tmp")); + } + + @Override + public JournaledFile makeDevicePoliciesJournaledFile(int userId) { + return makeJournaledFile(userId, DevicePolicyManagerService.DEVICE_POLICIES_XML); + } + + @Override + public JournaledFile makePoliciesVersionJournaledFile(int userId) { + return makeJournaledFile(userId, DevicePolicyManagerService.POLICIES_VERSION_XML); + } + + @Override + public ComponentName getOwnerComponent(int userId) { + return mUserToComponent.get(userId); + } + + @Override + public Function getAdminInfoSupplier(int userId) { + return componentName -> mComponentToDeviceAdminInfo.get(componentName); + } + } + + private final Context mRealTestContext = InstrumentationRegistry.getTargetContext(); + private FakePolicyUpgraderDataProvider mProvider; + private PolicyVersionUpgrader mUpgrader; + private File mDataDir; + + @Before + public void setUp() { + mProvider = new FakePolicyUpgraderDataProvider(); + mUpgrader = new PolicyVersionUpgrader(mProvider); + mDataDir = new File(mRealTestContext.getCacheDir(), "test-data"); + mDataDir.getParentFile().mkdirs(); + mProvider.mDataDir = mDataDir; + } + + @Test + public void testSameVersionDoesNothing() throws IOException { + int[] users = new int[] {0}; + writeVersionToXml(DevicePolicyManagerService.DPMS_VERSION); + preparePoliciesFile(users[0]); + String oldContents = readPoliciesFile(0); + + mUpgrader.upgradePolicy(users, DevicePolicyManagerService.DPMS_VERSION); + + String newContents = readPoliciesFile(0); + assertThat(newContents).isEqualTo(oldContents); + } + + @Test + public void testUpgrade0To1RemovesPasswordMetrics() throws IOException { + int[] users = new int[] {0, 10}; + writeVersionToXml(0); + for (int userId : users) { + preparePoliciesFile(userId); + } + + String oldContents = readPoliciesFile(0); + assertThat(oldContents).contains("active-password"); + + mUpgrader.upgradePolicy(users, 1); + + assertThat(readVersionFromXml()).isEqualTo(1); + assertThat(readPoliciesFile(users[0])).doesNotContain("active-password"); + assertThat(readPoliciesFile(users[1])).doesNotContain("active-password"); + } + + @Test + public void isLatestVersionTested() { + assertThat(DevicePolicyManagerService.DPMS_VERSION).isEqualTo(LATEST_TESTED_VERSION); + } + + private void writeVersionToXml(int dpmsVersion) throws IOException { + JournaledFile versionFile = mProvider.makePoliciesVersionJournaledFile(0); + Files.asCharSink(versionFile.chooseForWrite(), Charset.defaultCharset()).write( + String.format("%d\n", dpmsVersion)); + versionFile.commit(); + } + + private int readVersionFromXml() throws IOException { + File versionFile = mProvider.makePoliciesVersionJournaledFile(0).chooseForRead(); + String versionString = Files.asCharSource(versionFile, + Charset.defaultCharset()).readFirstLine(); + return Integer.parseInt(versionString); + } + + private void preparePoliciesFile(int userId) throws IOException { + JournaledFile policiesFile = mProvider.makeDevicePoliciesJournaledFile(userId); + DpmTestUtils.writeToFile( + policiesFile.chooseForWrite(), + DpmTestUtils.readAsset(mRealTestContext, + "PolicyVersionUpgraderTest/device_policies.xml")); + policiesFile.commit(); + } + + private String readPoliciesFile(int userId) throws IOException { + File policiesFile = mProvider.makeDevicePoliciesJournaledFile(userId).chooseForRead(); + return new String(Files.asByteSource(policiesFile).read()); + } +}