From 4da93f5a8191ee3943b1214a36ffa4283299bbad Mon Sep 17 00:00:00 2001 From: Kholoud Mohamed Date: Mon, 17 Apr 2023 15:43:56 +0000 Subject: [PATCH] cleanup up appRestrictions policy in the policy engine Bug: 269486068 Bug: 273494642 Test: btest a.d.c.ApplicationRestrictionsTest Test: btest a.d.c.DeviceManagementCoexistenceTest Change-Id: I38749166ccfa2a5d804431aa8e88cc403f3321e7 --- .../devicepolicy/BundlePolicySerializer.java | 110 +++--------------- .../DevicePolicyManagerService.java | 32 +++-- 2 files changed, 34 insertions(+), 108 deletions(-) diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/BundlePolicySerializer.java b/services/devicepolicy/java/com/android/server/devicepolicy/BundlePolicySerializer.java index ee73f8afabd2e..82f9aadba9f44 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/BundlePolicySerializer.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/BundlePolicySerializer.java @@ -17,50 +17,28 @@ package com.android.server.devicepolicy; import android.annotation.NonNull; -import android.annotation.Nullable; import android.app.admin.BundlePolicyValue; import android.app.admin.PackagePolicyKey; import android.app.admin.PolicyKey; import android.os.Bundle; -import android.os.Environment; import android.os.Parcelable; -import android.util.AtomicFile; -import android.util.Slog; -import android.util.Xml; +import android.util.Log; -import com.android.internal.annotations.GuardedBy; -import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.XmlUtils; import com.android.modules.utils.TypedXmlPullParser; import com.android.modules.utils.TypedXmlSerializer; -import libcore.io.IoUtils; - import org.xmlpull.v1.XmlPullParser; import org.xmlpull.v1.XmlPullParserException; -import java.io.File; -import java.io.FileInputStream; -import java.io.FileOutputStream; import java.io.IOException; import java.util.ArrayList; import java.util.Objects; -// TODO(b/266704763): clean this up and stop creating separate files for each value, the code here -// is copied from UserManagerService, however it doesn't currently handle setting different -// restrictions for the same package in different users, it also will not remove the files for -// outdated restrictions, this will all get fixed when we save it as part of the policies file -// rather than in its own files. final class BundlePolicySerializer extends PolicySerializer { private static final String TAG = "BundlePolicySerializer"; - private static final String ATTR_FILE_NAME = "file-name"; - - private static final String RESTRICTIONS_FILE_PREFIX = "AppRestrictions_"; - private static final String XML_SUFFIX = ".xml"; - - private static final String TAG_RESTRICTIONS = "restrictions"; private static final String TAG_ENTRY = "entry"; private static final String TAG_VALUE = "value"; private static final String ATTR_KEY = "key"; @@ -83,62 +61,26 @@ final class BundlePolicySerializer extends PolicySerializer { throw new IllegalArgumentException("policyKey is not of type " + "PackagePolicyKey"); } - String packageName = ((PackagePolicyKey) policyKey).getPackageName(); - String fileName = packageToRestrictionsFileName(packageName, value); - writeApplicationRestrictionsLAr(fileName, value); - serializer.attribute(/* namespace= */ null, ATTR_FILE_NAME, fileName); + writeBundle(value, serializer); } - @Nullable @Override BundlePolicyValue readFromXml(TypedXmlPullParser parser) { - String fileName = parser.getAttributeValue(/* namespace= */ null, ATTR_FILE_NAME); - - return new BundlePolicyValue(readApplicationRestrictions(fileName)); - } - - private static String packageToRestrictionsFileName(String packageName, Bundle restrictions) { - return RESTRICTIONS_FILE_PREFIX + packageName + Objects.hash(restrictions) + XML_SUFFIX; - } - - @GuardedBy("mAppRestrictionsLock") - private static Bundle readApplicationRestrictions(String fileName) { - AtomicFile restrictionsFile = - new AtomicFile(new File(Environment.getDataSystemDirectory(), fileName)); - return readApplicationRestrictions(restrictionsFile); - } - - @VisibleForTesting - @GuardedBy("mAppRestrictionsLock") - static Bundle readApplicationRestrictions(AtomicFile restrictionsFile) { - final Bundle restrictions = new Bundle(); - final ArrayList values = new ArrayList<>(); - if (!restrictionsFile.getBaseFile().exists()) { - return restrictions; - } - - FileInputStream fis = null; + Bundle bundle = new Bundle(); + ArrayList values = new ArrayList<>(); try { - fis = restrictionsFile.openRead(); - final TypedXmlPullParser parser = Xml.resolvePullParser(fis); - XmlUtils.nextElement(parser); - if (parser.getEventType() != XmlPullParser.START_TAG) { - Slog.e(TAG, "Unable to read restrictions file " - + restrictionsFile.getBaseFile()); - return restrictions; + final int outerDepth = parser.getDepth(); + while (XmlUtils.nextElementWithin(parser, outerDepth)) { + readBundle(bundle, values, parser); } - while (parser.next() != XmlPullParser.END_DOCUMENT) { - readEntry(restrictions, values, parser); - } - } catch (IOException | XmlPullParserException e) { - Slog.w(TAG, "Error parsing " + restrictionsFile.getBaseFile(), e); - } finally { - IoUtils.closeQuietly(fis); + } catch (XmlPullParserException | IOException e) { + Log.e(TAG, "Error parsing Bundle policy.", e); + return null; } - return restrictions; + return new BundlePolicyValue(bundle); } - private static void readEntry(Bundle restrictions, ArrayList values, + private static void readBundle(Bundle restrictions, ArrayList values, TypedXmlPullParser parser) throws XmlPullParserException, IOException { int type = parser.getEventType(); if (type == XmlPullParser.START_TAG && parser.getName().equals(TAG_ENTRY)) { @@ -186,37 +128,11 @@ final class BundlePolicySerializer extends PolicySerializer { Bundle childBundle = new Bundle(); int outerDepth = parser.getDepth(); while (XmlUtils.nextElementWithin(parser, outerDepth)) { - readEntry(childBundle, values, parser); + readBundle(childBundle, values, parser); } return childBundle; } - private static void writeApplicationRestrictionsLAr(String fileName, Bundle restrictions) { - AtomicFile restrictionsFile = new AtomicFile( - new File(Environment.getDataSystemDirectory(), fileName)); - writeApplicationRestrictionsLAr(restrictions, restrictionsFile); - } - - static void writeApplicationRestrictionsLAr(Bundle restrictions, AtomicFile restrictionsFile) { - FileOutputStream fos = null; - try { - fos = restrictionsFile.startWrite(); - final TypedXmlSerializer serializer = Xml.resolveSerializer(fos); - serializer.startDocument(null, true); - serializer.setFeature("http://xmlpull.org/v1/doc/features.html#indent-output", true); - - serializer.startTag(null, TAG_RESTRICTIONS); - writeBundle(restrictions, serializer); - serializer.endTag(null, TAG_RESTRICTIONS); - - serializer.endDocument(); - restrictionsFile.finishWrite(fos); - } catch (Exception e) { - restrictionsFile.failWrite(fos); - Slog.e(TAG, "Error writing application restrictions list", e); - } - } - private static void writeBundle(Bundle restrictions, TypedXmlSerializer serializer) throws IOException { for (String key : restrictions.keySet()) { diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 18fcafa84c240..7e5d5aae06e4c 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -11053,7 +11053,6 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { } if (!isPermissionCheckFlagEnabled() && !isPolicyEngineForFinanceFlagEnabled()) { - // TODO: Figure out if something like this needs to be restored for policy engine final ComponentName profileOwner = getProfileOwnerAsUser(userId); if (profileOwner == null) { return false; @@ -11640,7 +11639,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { caller.getUserId()); } setBackwardsCompatibleAppRestrictions( - packageName, restrictions, caller.getUserHandle()); + caller, packageName, restrictions, caller.getUserHandle()); } else { Preconditions.checkCallAuthorization((caller.hasAdminComponent() && (isProfileOwner(caller) || isDefaultDeviceOwner(caller))) @@ -11661,17 +11660,28 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { } /** - * Set app restrictions in user manager to keep backwards compatibility for the old - * getApplicationRestrictions API. + * Set app restrictions in user manager for DPC callers only to keep backwards compatibility + * for the old getApplicationRestrictions API. */ private void setBackwardsCompatibleAppRestrictions( - String packageName, Bundle restrictions, UserHandle userHandle) { - Bundle restrictionsToApply = restrictions == null || restrictions.isEmpty() - ? getAppRestrictionsSetByAnyAdmin(packageName, userHandle) - : restrictions; - mInjector.binderWithCleanCallingIdentity(() -> { - mUserManager.setApplicationRestrictions(packageName, restrictionsToApply, userHandle); - }); + CallerIdentity caller, String packageName, Bundle restrictions, UserHandle userHandle) { + if ((caller.hasAdminComponent() && (isProfileOwner(caller) || isDefaultDeviceOwner(caller))) + || (caller.hasPackage() && isCallerDelegate(caller, DELEGATION_APP_RESTRICTIONS))) { + Bundle restrictionsToApply = restrictions == null || restrictions.isEmpty() + ? getAppRestrictionsSetByAnyAdmin(packageName, userHandle) + : restrictions; + mInjector.binderWithCleanCallingIdentity(() -> { + mUserManager.setApplicationRestrictions(packageName, restrictionsToApply, + userHandle); + }); + } else { + // Notify package of changes via an intent - only sent to explicitly registered + // receivers. Sending here because For DPCs, this is being sent in UMS. + final Intent changeIntent = new Intent(Intent.ACTION_APPLICATION_RESTRICTIONS_CHANGED); + changeIntent.setPackage(packageName); + changeIntent.addFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY); + mContext.sendBroadcastAsUser(changeIntent, userHandle); + } } private Bundle getAppRestrictionsSetByAnyAdmin(String packageName, UserHandle userHandle) {