From 10d51880e261ab8561a9eea22c1a05ae915424ca Mon Sep 17 00:00:00 2001 From: Winson Date: Tue, 26 May 2020 10:52:23 -0700 Subject: [PATCH] Add PackageInstaller SessionParams restrictions To mitigate a boot loop with reading a massive install_sessions.xml file, this restricts the amount of data that can be written by limiting the size of unbounded parameters like package name and app label. This introduces a lowered max session count. 50 for general applications without the INSTALL_PACKAGES permission, and the same 1024 for those with the permission. Also truncates labels read from PackageItemInfo to 1000 characters, which is probably enough. These changes restrict a malicious third party app to ~0.15 MB written to disk, and a valid installer to ~3.6 MB, as opposed to the >1000 MB previously allowed. These numbers assume no install granted runtime permissions. Those were not restricted since there's no good way to do so, but it's assumed that any installer with that permission is highly privleged and doesn't need to be limited. Along the same lines, DataLoaderParams are also not restricted. This will have to be added if that API is ever made public. However, installer package was restricted, even though the API is hidden. It was an easy add and may have some effect since the value is derived from other data and passed through by other system components. It's still possible to inflate the file size if a lot of different apps attempt to install a large number of packages, but that would require thousands of malicious apps to be installed. Bug: 157224146 Test: atest android.content.pm.PackageSessionTests Change-Id: Iec42bee08d19d4ac53b361a92be6bc1401d9efc8 --- .../android/content/pm/PackageInstaller.java | 12 +- .../android/content/pm/PackageItemInfo.java | 12 +- .../tests/PackageInstallerSessions/Android.bp | 42 ++++ .../AndroidManifest.xml | 29 +++ .../android/content/pm/PackageSessionTests.kt | 188 ++++++++++++++++++ .../server/pm/PackageInstallerService.java | 33 ++- .../permission/PermissionManagerService.java | 14 ++ .../PermissionManagerServiceInternal.java | 7 + 8 files changed, 330 insertions(+), 7 deletions(-) create mode 100644 core/tests/PackageInstallerSessions/Android.bp create mode 100644 core/tests/PackageInstallerSessions/AndroidManifest.xml create mode 100644 core/tests/PackageInstallerSessions/src/android/content/pm/PackageSessionTests.kt diff --git a/core/java/android/content/pm/PackageInstaller.java b/core/java/android/content/pm/PackageInstaller.java index ed75504529b92..fc4ccd072e758 100644 --- a/core/java/android/content/pm/PackageInstaller.java +++ b/core/java/android/content/pm/PackageInstaller.java @@ -1449,6 +1449,13 @@ public class PackageInstaller { /** {@hide} */ public static final int UID_UNKNOWN = -1; + /** + * This value is derived from the maximum file name length. No package above this limit + * can ever be successfully installed on the device. + * @hide + */ + public static final int MAX_PACKAGE_NAME_LENGTH = 255; + /** {@hide} */ @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023) public int mode = MODE_INVALID; @@ -1642,6 +1649,8 @@ public class PackageInstaller { /** * Optionally set a label representing the app being installed. + * + * This value will be trimmed to the first 1000 characters. */ public void setAppLabel(@Nullable CharSequence appLabel) { this.appLabel = (appLabel != null) ? appLabel.toString() : null; @@ -1711,7 +1720,8 @@ public class PackageInstaller { * *

Initially, all restricted permissions are whitelisted but you can change * which ones are whitelisted by calling this method or the corresponding ones - * on the {@link PackageManager}. + * on the {@link PackageManager}. Only soft or hard restricted permissions on the current + * Android version are supported and any invalid entries will be removed. * * @see PackageManager#addWhitelistedRestrictedPermission(String, String, int) * @see PackageManager#removeWhitelistedRestrictedPermission(String, String, int) diff --git a/core/java/android/content/pm/PackageItemInfo.java b/core/java/android/content/pm/PackageItemInfo.java index f354bdb5a08b4..65ce1e7ef0796 100644 --- a/core/java/android/content/pm/PackageItemInfo.java +++ b/core/java/android/content/pm/PackageItemInfo.java @@ -49,8 +49,16 @@ import java.util.Objects; * in the implementation of Parcelable in subclasses. */ public class PackageItemInfo { - /** The maximum length of a safe label, in characters */ - private static final int MAX_SAFE_LABEL_LENGTH = 50000; + + /** + * The maximum length of a safe label, in characters + * + * TODO(b/157997155): It may make sense to expose this publicly so that apps can check for the + * value and truncate the strings/use a different label, without having to hardcode and make + * assumptions about the value. + * @hide + */ + public static final int MAX_SAFE_LABEL_LENGTH = 1000; /** @hide */ public static final float DEFAULT_MAX_LABEL_SIZE_PX = 500f; diff --git a/core/tests/PackageInstallerSessions/Android.bp b/core/tests/PackageInstallerSessions/Android.bp new file mode 100644 index 0000000000000..e74f30ee10a46 --- /dev/null +++ b/core/tests/PackageInstallerSessions/Android.bp @@ -0,0 +1,42 @@ +// +// Copyright 2020 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. +// + +android_test { + name: "FrameworksCorePackageInstallerSessionsTests", + + srcs: [ + "src/**/*.kt", + ], + static_libs: [ + "androidx.test.rules", + "compatibility-device-util-axt", + "frameworks-base-testutils", + "platform-test-annotations", + "testng", + "truth-prebuilt", + ], + + libs: [ + "android.test.runner", + "android.test.base", + "framework", + "framework-res", + ], + + platform_apis: true, + sdk_version: "core_platform", + test_suites: ["device-tests"], +} diff --git a/core/tests/PackageInstallerSessions/AndroidManifest.xml b/core/tests/PackageInstallerSessions/AndroidManifest.xml new file mode 100644 index 0000000000000..5b22d2b4f3e37 --- /dev/null +++ b/core/tests/PackageInstallerSessions/AndroidManifest.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + diff --git a/core/tests/PackageInstallerSessions/src/android/content/pm/PackageSessionTests.kt b/core/tests/PackageInstallerSessions/src/android/content/pm/PackageSessionTests.kt new file mode 100644 index 0000000000000..494c92a8aa3f1 --- /dev/null +++ b/core/tests/PackageInstallerSessions/src/android/content/pm/PackageSessionTests.kt @@ -0,0 +1,188 @@ +/* + * Copyright (C) 2020 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 android.content.pm + +import android.content.Context +import android.content.pm.PackageInstaller.SessionParams +import android.platform.test.annotations.Presubmit +import androidx.test.InstrumentationRegistry +import androidx.test.filters.LargeTest +import com.android.compatibility.common.util.ShellIdentityUtils +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.testng.Assert.assertThrows +import kotlin.random.Random + +/** + * For verifying public [PackageInstaller] session APIs. This differs from + * [com.android.server.pm.PackageInstallerSessionTest] in services because that mocks the session, + * whereas this test uses the installer on device. + */ +@Presubmit +class PackageSessionTests { + + companion object { + /** + * Permissions marked "hardRestricted" or "softRestricted" in core/res/AndroidManifest.xml. + */ + private val RESTRICTED_PERMISSIONS = listOf( + "android.permission.SEND_SMS", + "android.permission.RECEIVE_SMS", + "android.permission.READ_SMS", + "android.permission.RECEIVE_WAP_PUSH", + "android.permission.RECEIVE_MMS", + "android.permission.READ_CELL_BROADCASTS", + "android.permission.ACCESS_BACKGROUND_LOCATION", + "android.permission.READ_CALL_LOG", + "android.permission.WRITE_CALL_LOG", + "android.permission.PROCESS_OUTGOING_CALLS" + ) + } + + private val context: Context = InstrumentationRegistry.getContext() + + private val installer = context.packageManager.packageInstaller + + @Before + @After + fun abandonAllSessions() { + installer.mySessions.asSequence() + .map { it.sessionId } + .forEach { + try { + installer.abandonSession(it) + } catch (ignored: Exception) { + // Querying for sessions checks by calling package name, but abandoning + // checks by UID, which won't match if this test failed to clean up + // on a previous install + run + uninstall, so ignore these failures. + } + } + } + + @Test + fun truncateAppLabel() { + val longLabel = invalidAppLabel() + val params = SessionParams(SessionParams.MODE_FULL_INSTALL).apply { + setAppLabel(longLabel) + } + + createSession(params) { + assertThat(installer.getSessionInfo(it)?.appLabel) + .isEqualTo(longLabel.take(PackageItemInfo.MAX_SAFE_LABEL_LENGTH)) + } + } + + @Test + fun removeInvalidAppPackageName() { + val longName = invalidPackageName() + val params = SessionParams(SessionParams.MODE_FULL_INSTALL).apply { + setAppPackageName(longName) + } + + createSession(params) { + assertThat(installer.getSessionInfo(it)?.appPackageName) + .isEqualTo(null) + } + } + + @Test + fun removeInvalidInstallerPackageName() { + val longName = invalidPackageName() + val params = SessionParams(SessionParams.MODE_FULL_INSTALL).apply { + setInstallerPackageName(longName) + } + + createSession(params) { + // If a custom installer name is dropped, it defaults to the caller + assertThat(installer.getSessionInfo(it)?.installerPackageName) + .isEqualTo(context.packageName) + } + } + + @Test + fun truncateWhitelistPermissions() { + val params = SessionParams(SessionParams.MODE_FULL_INSTALL).apply { + setWhitelistedRestrictedPermissions(invalidPermissions()) + } + + createSession(params) { + assertThat(installer.getSessionInfo(it)?.whitelistedRestrictedPermissions!!) + .containsExactlyElementsIn(RESTRICTED_PERMISSIONS) + } + } + + @LargeTest + @Test + fun allocateMaxSessionsWithPermission() { + ShellIdentityUtils.invokeWithShellPermissions { + repeat(1024) { createDummySession() } + assertThrows(IllegalStateException::class.java) { createDummySession() } + } + } + + @LargeTest + @Test + fun allocateMaxSessionsNoPermission() { + repeat(50) { createDummySession() } + assertThrows(IllegalStateException::class.java) { createDummySession() } + } + + private fun createDummySession() { + installer.createSession(SessionParams(SessionParams.MODE_FULL_INSTALL) + .apply { + setAppPackageName(invalidPackageName()) + setAppLabel(invalidAppLabel()) + setWhitelistedRestrictedPermissions(invalidPermissions()) + }) + } + + private fun invalidPackageName(maxLength: Int = SessionParams.MAX_PACKAGE_NAME_LENGTH): String { + return (0 until (maxLength + 10)) + .asSequence() + .mapIndexed { index, _ -> + // A package name needs at least one separator + if (index == 2) { + '.' + } else { + Random.nextInt('z' - 'a').toChar() + 'a'.toInt() + } + } + .joinToString(separator = "") + } + + private fun invalidAppLabel() = (0 until PackageItemInfo.MAX_SAFE_LABEL_LENGTH + 10) + .asSequence() + .map { Random.nextInt(Char.MAX_VALUE.toInt()).toChar() } + .joinToString(separator = "") + + private fun invalidPermissions() = RESTRICTED_PERMISSIONS.toMutableSet() + .apply { + // Add some invalid permission names + repeat(10) { add(invalidPackageName(300)) } + } + + private fun createSession(params: SessionParams, block: (Int) -> Unit = {}) { + val sessionId = installer.createSession(params) + try { + block(sessionId) + } finally { + installer.abandonSession(sessionId) + } + } +} diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index 236a6816b3e38..f827721be3b7b 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -40,6 +40,7 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageInstaller; import android.content.pm.PackageInstaller.SessionInfo; import android.content.pm.PackageInstaller.SessionParams; +import android.content.pm.PackageItemInfo; import android.content.pm.PackageManager; import android.content.pm.ParceledListSlice; import android.content.pm.VersionedPackage; @@ -126,8 +127,10 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements private static final long MAX_AGE_MILLIS = 3 * DateUtils.DAY_IN_MILLIS; /** Automatically destroy staged sessions that have not changed state in this time */ private static final long MAX_TIME_SINCE_UPDATE_MILLIS = 7 * DateUtils.DAY_IN_MILLIS; - /** Upper bound on number of active sessions for a UID */ - private static final long MAX_ACTIVE_SESSIONS = 1024; + /** Upper bound on number of active sessions for a UID that has INSTALL_PACKAGES */ + private static final long MAX_ACTIVE_SESSIONS_WITH_PERMISSION = 1024; + /** Upper bound on number of active sessions for a UID without INSTALL_PACKAGES */ + private static final long MAX_ACTIVE_SESSIONS_NO_PERMISSION = 50; /** Upper bound on number of historical sessions for a UID */ private static final long MAX_HISTORICAL_SESSIONS = 1048576; @@ -503,7 +506,18 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements + "to use a data loader"); } - String requestedInstallerPackageName = params.installerPackageName != null + // App package name and label length is restricted so that really long strings aren't + // written to disk. + if (params.appPackageName != null + && params.appPackageName.length() > SessionParams.MAX_PACKAGE_NAME_LENGTH) { + params.appPackageName = null; + } + + params.appLabel = TextUtils.trimToSize(params.appLabel, + PackageItemInfo.MAX_SAFE_LABEL_LENGTH); + + String requestedInstallerPackageName = (params.installerPackageName != null + && params.installerPackageName.length() < SessionParams.MAX_PACKAGE_NAME_LENGTH) ? params.installerPackageName : installerPackageName; if ((callingUid == Process.SHELL_UID) || (callingUid == Process.ROOT_UID)) { @@ -635,12 +649,23 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements } } + if (params.whitelistedRestrictedPermissions != null) { + mPermissionManager.retainHardAndSoftRestrictedPermissions( + params.whitelistedRestrictedPermissions); + } + final int sessionId; final PackageInstallerSession session; synchronized (mSessions) { // Sanity check that installer isn't going crazy final int activeCount = getSessionCount(mSessions, callingUid); - if (activeCount >= MAX_ACTIVE_SESSIONS) { + if (mContext.checkCallingOrSelfPermission(Manifest.permission.INSTALL_PACKAGES) + == PackageManager.PERMISSION_GRANTED) { + if (activeCount >= MAX_ACTIVE_SESSIONS_WITH_PERMISSION) { + throw new IllegalStateException( + "Too many active sessions for UID " + callingUid); + } + } else if (activeCount >= MAX_ACTIVE_SESSIONS_NO_PERMISSION) { throw new IllegalStateException( "Too many active sessions for UID " + callingUid); } diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index b0d4d957fc21d..c6af616e5f949 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -4948,6 +4948,20 @@ public class PermissionManagerService extends IPermissionManager.Stub { StorageManager.UUID_PRIVATE_INTERNAL, true, mDefaultPermissionCallback); } } + + @Override + public void retainHardAndSoftRestrictedPermissions(@NonNull List permissions) { + synchronized (mLock) { + Iterator iterator = permissions.iterator(); + while (iterator.hasNext()) { + String permission = iterator.next(); + BasePermission basePermission = mSettings.mPermissions.get(permission); + if (basePermission == null || !basePermission.isHardOrSoftRestricted()) { + iterator.remove(); + } + } + } + } } private static final class OnPermissionChangeListeners extends Handler { diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java index 57a25eddf7ce6..4412162a5cc8a 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerServiceInternal.java @@ -36,6 +36,7 @@ import java.util.function.Consumer; * TODO: Should be merged into PermissionManagerInternal, but currently uses internal classes. */ public abstract class PermissionManagerServiceInternal extends PermissionManagerInternal { + /** * Provider for package names. */ @@ -455,4 +456,10 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager /** Called when a new user has been created. */ public abstract void onNewUserCreated(@UserIdInt int userId); + + /** + * Removes invalid permissions which are not {@link PermissionInfo#FLAG_HARD_RESTRICTED} or + * {@link PermissionInfo#FLAG_SOFT_RESTRICTED} from the input. + */ + public abstract void retainHardAndSoftRestrictedPermissions(@NonNull List permissions); }