diff --git a/core/java/android/app/admin/DevicePolicyManager.java b/core/java/android/app/admin/DevicePolicyManager.java index 27f5545d8f03a..e0364cbe1d69d 100644 --- a/core/java/android/app/admin/DevicePolicyManager.java +++ b/core/java/android/app/admin/DevicePolicyManager.java @@ -13401,11 +13401,10 @@ public class DevicePolicyManager { /** * Called by a device admin or holder of the permission * {@link android.Manifest.permission#MANAGE_DEVICE_POLICY_SUPPORT_MESSAGE} to set the short - * support message. This will be displayed to the user - * in settings screens where funtionality has been disabled by the admin. The message should be - * limited to a short statement such as "This setting is disabled by your administrator. Contact - * someone@example.com for support." If the message is longer than 200 characters it may be - * truncated. + * support message. This will be displayed to the user in settings screens where functionality + * has been disabled by the admin. The message should be limited to a short statement such as + * "This setting is disabled by your administrator. Contact someone@example.com for support." + * If the message is longer than 200 characters it may be truncated. *

* If the short support message needs to be localized, it is the responsibility of the * {@link DeviceAdminReceiver} to listen to the {@link Intent#ACTION_LOCALE_CHANGED} broadcast @@ -13460,7 +13459,8 @@ public class DevicePolicyManager { /** * Called by a device admin to set the long support message. This will be displayed to the user - * in the device administators settings screen. + * in the device administrators settings screen. If the message is longer than 20000 characters + * it may be truncated. *

* If the long support message needs to be localized, it is the responsibility of the * {@link DeviceAdminReceiver} to listen to the {@link Intent#ACTION_LOCALE_CHANGED} broadcast diff --git a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java index 0a4cd7a9a8a42..3bb5f4af93ead 100644 --- a/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java +++ b/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java @@ -39,7 +39,6 @@ import static android.Manifest.permission.MANAGE_DEVICE_POLICY_DEFAULT_SMS; import static android.Manifest.permission.MANAGE_DEVICE_POLICY_DISPLAY; import static android.Manifest.permission.MANAGE_DEVICE_POLICY_FACTORY_RESET; import static android.Manifest.permission.MANAGE_DEVICE_POLICY_FUN; -import static android.Manifest.permission.MANAGE_DEVICE_POLICY_INPUT_METHODS; import static android.Manifest.permission.MANAGE_DEVICE_POLICY_INSTALL_UNKNOWN_SOURCES; import static android.Manifest.permission.MANAGE_DEVICE_POLICY_KEYGUARD; import static android.Manifest.permission.MANAGE_DEVICE_POLICY_LOCALE; @@ -520,6 +519,7 @@ import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.text.DateFormat; import java.time.LocalDate; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -532,6 +532,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Objects; +import java.util.Queue; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -567,7 +568,15 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { private static final int REQUEST_PROFILE_OFF_DEADLINE = 5572; + // Binary XML serializer doesn't support longer strings + private static final int MAX_POLICY_STRING_LENGTH = 65535; + // FrameworkParsingPackageUtils#MAX_FILE_NAME_SIZE, Android packages are used in dir names. + private static final int MAX_PACKAGE_NAME_LENGTH = 223; + private static final int MAX_PROFILE_NAME_LENGTH = 200; + private static final int MAX_LONG_SUPPORT_MESSAGE_LENGTH = 20000; + private static final int MAX_SHORT_SUPPORT_MESSAGE_LENGTH = 200; + private static final int MAX_ORG_NAME_LENGTH = 200; private static final long MS_PER_DAY = TimeUnit.DAYS.toMillis(1); @@ -11729,7 +11738,15 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { } Objects.requireNonNull(agent, "agent is null"); - int userHandle = UserHandle.getCallingUserId(); + + enforceMaxPackageNameLength(agent.getPackageName()); + final String agentAsString = agent.flattenToString(); + enforceMaxStringLength(agentAsString, "agent name"); + if (args != null) { + enforceMaxStringLength(args, "args"); + } + + int userHandle = mInjector.userHandleGetCallingUserId(); synchronized (getLockObject()) { ActiveAdmin ap; if (isPermissionCheckFlagEnabled()) { @@ -11746,7 +11763,7 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { checkCanExecuteOrThrowUnsafe( DevicePolicyManager.OPERATION_SET_TRUST_AGENT_CONFIGURATION); - ap.trustAgentInfos.put(agent.flattenToString(), new TrustAgentInfo(args)); + ap.trustAgentInfos.put(agentAsString, new TrustAgentInfo(args)); saveSettingsLocked(userHandle); } } @@ -12016,6 +12033,10 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { isDeviceOwner(caller) || isProfileOwner(caller)); if (packageList != null) { + for (String pkg : packageList) { + enforceMaxPackageNameLength(pkg); + } + int userId = caller.getUserId(); final List enabledServices; long id = mInjector.binderClearCallingIdentity(); @@ -12197,6 +12218,10 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { } if (packageList != null) { + for (String pkg : packageList) { + enforceMaxPackageNameLength(pkg); + } + List enabledImes = mInjector.binderWithCleanCallingIdentity(() -> InputMethodManagerInternal.get().getEnabledInputMethodListAsUser(userId)); if (enabledImes != null) { @@ -14081,6 +14106,9 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { if (!mHasFeature) { return; } + + enforceMaxStringLength(accountType, "account type"); + CallerIdentity caller; if (isPolicyEngineForFinanceFlagEnabled()) { caller = getCallerIdentity(who, callerPackageName); @@ -14771,6 +14799,10 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { public void setLockTaskPackages(ComponentName who, String callerPackageName, String[] packages) throws SecurityException { Objects.requireNonNull(packages, "packages is null"); + for (String pkg : packages) { + enforceMaxPackageNameLength(pkg); + } + CallerIdentity caller; if (isPolicyEngineForFinanceFlagEnabled()) { caller = getCallerIdentity(who, callerPackageName); @@ -17373,6 +17405,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { CallerIdentity caller; ActiveAdmin admin; + message = truncateIfLonger(message, MAX_SHORT_SUPPORT_MESSAGE_LENGTH); + if (isPermissionCheckFlagEnabled()) { caller = getCallerIdentity(who, callerPackageName); EnforcingAdmin enforcingAdmin = enforcePermissionAndGetEnforcingAdmin( @@ -17433,6 +17467,9 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { if (!mHasFeature) { return; } + + message = truncateIfLonger(message, MAX_LONG_SUPPORT_MESSAGE_LENGTH); + Objects.requireNonNull(who, "ComponentName is null"); final CallerIdentity caller = getCallerIdentity(who); synchronized (getLockObject()) { @@ -17597,6 +17634,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { Preconditions.checkCallAuthorization(isDeviceOwner(caller) || isProfileOwner(caller)); } + text = truncateIfLonger(text, MAX_ORG_NAME_LENGTH); + synchronized (getLockObject()) { if (!isPermissionCheckFlagEnabled()) { admin = getProfileOwnerOrDeviceOwnerLocked(caller.getUserId()); @@ -17877,9 +17916,8 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { throw new IllegalArgumentException("ids must not be null"); } for (String id : ids) { - if (TextUtils.isEmpty(id)) { - throw new IllegalArgumentException("ids must not contain empty string"); - } + Preconditions.checkArgument(!TextUtils.isEmpty(id), "ids must not have empty string"); + enforceMaxStringLength(id, "affiliation id"); } final Set affiliationIds = new ArraySet<>(ids); @@ -19392,6 +19430,9 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { "Provided administrator and target are the same object."); Preconditions.checkArgument(!admin.getPackageName().equals(target.getPackageName()), "Provided administrator and target have the same package name."); + if (bundle != null) { + enforceMaxStringLength(bundle, "bundle"); + } final CallerIdentity caller = getCallerIdentity(admin); Preconditions.checkCallAuthorization( @@ -24091,6 +24132,53 @@ public class DevicePolicyManagerService extends IDevicePolicyManager.Stub { }); } + /** + * Truncates char sequence to maximum length, nulls are ignored. + */ + private static CharSequence truncateIfLonger(CharSequence input, int maxLength) { + return input == null || input.length() <= maxLength + ? input + : input.subSequence(0, maxLength); + } + + /** + * Throw if string argument is too long to be serialized. + */ + private static void enforceMaxStringLength(String str, String argName) { + Preconditions.checkArgument( + str.length() <= MAX_POLICY_STRING_LENGTH, argName + " loo long"); + } + + private static void enforceMaxPackageNameLength(String pkg) { + Preconditions.checkArgument( + pkg.length() <= MAX_PACKAGE_NAME_LENGTH, "Package name too long"); + } + + /** + * Throw if persistable bundle contains any string that we can't serialize. + */ + private static void enforceMaxStringLength(PersistableBundle bundle, String argName) { + // Persistable bundles can have other persistable bundles as values, traverse with a queue. + Queue queue = new ArrayDeque<>(); + queue.add(bundle); + while (!queue.isEmpty()) { + PersistableBundle current = queue.remove(); + for (String key : current.keySet()) { + enforceMaxStringLength(key, "key in " + argName); + Object value = current.get(key); + if (value instanceof String) { + enforceMaxStringLength((String) value, "string value in " + argName); + } else if (value instanceof String[]) { + for (String str : (String[]) value) { + enforceMaxStringLength(str, "string value in " + argName); + } + } else if (value instanceof PersistableBundle) { + queue.add((PersistableBundle) value); + } + } + } + } + private ActiveAdmin getActiveAdminForCaller(@Nullable ComponentName who, CallerIdentity caller) { synchronized (getLockObject()) { diff --git a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java index 34b88b03547b4..57755a9525fca 100644 --- a/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/devicepolicy/DevicePolicyManagerTest.java @@ -130,6 +130,7 @@ import android.os.Build; import android.os.Build.VERSION_CODES; import android.os.Bundle; import android.os.IpcDataCache; +import android.os.PersistableBundle; import android.os.Process; import android.os.UserHandle; import android.os.UserManager; @@ -8539,6 +8540,46 @@ public class DevicePolicyManagerTest extends DpmTestBase { eq(FUSED_PROVIDER), any(), eq(getServices().executor), any()); } + /** + * Verifies that bundles with tons of moderately long strings are persisted correctly. + * + * Policy is serialized into binary XML and there is a limit on the max string length: 65535. + * This test ensures that as long as each string in the trust agent configuration is below this + * limit, the policy can be serialized and deserialized correctly, even when the total length + * of the configuration is above that limit. This should be the case because PersistableBundle + * contents are stored as XML subtrees rather than as strings. + */ + @Test + public void testSetTrustAgentConfiguration_largeBundlePersisted() { + setAsProfileOwner(admin1); + + ComponentName agent = new ComponentName("some.trust.agent", "some.trust.agent.Agent"); + PersistableBundle configIn = new PersistableBundle(); + String kilobyteString = new String(new char[1024]).replace('\0', 'A'); + for (int i = 0; i < 1024; i++) { + configIn.putString("key-" + i, kilobyteString); + } + + runAsCaller(mAdmin1Context, dpms, dpm -> { + dpm.setTrustAgentConfiguration(admin1, agent, configIn); + }); + + // Re-read policies to see if they were serialized/deserialized correctly. + initializeDpms(); + + List configsOut = new ArrayList<>(); + runAsCaller(mAdmin1Context, dpms, dpm -> { + configsOut.addAll(dpm.getTrustAgentConfiguration(admin1, agent)); + }); + + assertThat(configsOut.size()).isEqualTo(1); + PersistableBundle configOut = configsOut.get(0); + assertThat(configOut.size()).isEqualTo(1024); + for (int i = 0; i < 1024; i++) { + assertThat(configOut.getString("key-" + i, null)).isEqualTo(kilobyteString); + } + } + private void setupVpnAuthorization(String userVpnPackage, int userVpnUid) { final AppOpsManager.PackageOps vpnOp = new AppOpsManager.PackageOps(userVpnPackage, userVpnUid, List.of(new AppOpsManager.OpEntry(