From 2a4fff77e03ff7bf27e9db85900d44121aa1544e Mon Sep 17 00:00:00 2001 From: kumarashishg Date: Mon, 17 Jul 2023 12:01:18 +0000 Subject: [PATCH 01/26] Resolve custom printer icon boundary exploit. Because Settings grants the INTERACT_ACROSS_USERS_FULL permission, an exploit is possible where the third party print plugin service can pass other's User Icon URI. This CL provides a lightweight solution for parsing the image URI to detect profile exploitation. Bug: 281525042 Test: Build and flash the code. Try to reproduce the issue with mentioned steps in the bug (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0e0693ca9cb408d0dc82f6c6b3feb453fc8ddd83) Merged-In: Iaaa6fe2a627a265c4d1d7b843a033a132e1fe2ce Change-Id: Iaaa6fe2a627a265c4d1d7b843a033a132e1fe2ce --- .../server/print/PrintManagerService.java | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/services/print/java/com/android/server/print/PrintManagerService.java b/services/print/java/com/android/server/print/PrintManagerService.java index c9b9f3e6bd48..2bed4b5a81f5 100644 --- a/services/print/java/com/android/server/print/PrintManagerService.java +++ b/services/print/java/com/android/server/print/PrintManagerService.java @@ -252,12 +252,44 @@ public Icon getCustomPrinterIcon(PrinterId printerId, int userId) { } final long identity = Binder.clearCallingIdentity(); try { - return userState.getCustomPrinterIcon(printerId); + Icon icon = userState.getCustomPrinterIcon(printerId); + return validateIconUserBoundary(icon); } finally { Binder.restoreCallingIdentity(identity); } } + /** + * Validates the custom printer icon to see if it's not in the calling user space. + * If the condition is not met, return null. Otherwise, return the original icon. + * + * @param icon + * @return icon (validated) + */ + private Icon validateIconUserBoundary(Icon icon) { + // Refer to Icon#getUriString for context. The URI string is invalid for icons of + // incompatible types. + if (icon != null && (icon.getType() == Icon.TYPE_URI)) { + String encodedUser = icon.getUri().getEncodedUserInfo(); + + // If there is no encoded user, the URI is calling into the calling user space + if (encodedUser != null) { + int userId = Integer.parseInt(encodedUser); + // resolve encoded user + final int resolvedUserId = resolveCallingUserEnforcingPermissions(userId); + + synchronized (mLock) { + // Only the current group members can get the printer icons. + if (resolveCallingProfileParentLocked(resolvedUserId) + != getCurrentUserId()) { + return null; + } + } + } + } + return icon; + } + @Override public void cancelPrintJob(PrintJobId printJobId, int appId, int userId) { if (printJobId == null) { From 0038840cd4caca806340b4e9687b89e947f73b11 Mon Sep 17 00:00:00 2001 From: Winson Date: Tue, 26 May 2020 10:52:23 -0700 Subject: [PATCH 02/26] 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 | 34 +++- .../permission/PermissionManagerService.java | 14 ++ .../PermissionManagerServiceInternal.java | 7 + 8 files changed, 332 insertions(+), 6 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 b44b6d90811e..fce60faf6170 100644 --- a/core/java/android/content/pm/PackageInstaller.java +++ b/core/java/android/content/pm/PackageInstaller.java @@ -1277,6 +1277,13 @@ public static class SessionParams implements Parcelable { /** {@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; @@ -1450,6 +1457,8 @@ public void setAppIcon(@Nullable Bitmap appIcon) { /** * 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; @@ -1519,7 +1528,8 @@ public void setGrantedRuntimePermissions(String[] permissions) { * *

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 aa8e84262049..7cef05ac8733 100644 --- a/core/java/android/content/pm/PackageItemInfo.java +++ b/core/java/android/content/pm/PackageItemInfo.java @@ -48,8 +48,16 @@ * 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 = 1000; + + /** + * 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 000000000000..a564d30f3008 --- /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: "test_current", + test_suites: ["device-tests"], +} diff --git a/core/tests/PackageInstallerSessions/AndroidManifest.xml b/core/tests/PackageInstallerSessions/AndroidManifest.xml new file mode 100644 index 000000000000..5b22d2b4f3e3 --- /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 000000000000..494c92a8aa3f --- /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 f37362a34659..62ac26821986 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -41,6 +41,7 @@ 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; @@ -125,8 +126,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; /** Destroy sessions older than this on storage free request */ @@ -535,6 +538,20 @@ private int createSessionInternal(SessionParams params, String installerPackageN throw new SecurityException("User restriction prevents installing"); } + // 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)) { params.installFlags |= PackageManager.INSTALL_FROM_ADB; @@ -638,12 +655,23 @@ private int createSessionInternal(SessionParams params, String installerPackageN } } + 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 4ece487bf870..f163969b3005 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -3469,5 +3469,19 @@ public void removeOnRuntimePermissionStateChangedListener( PermissionManagerService.this.removeOnRuntimePermissionStateChangedListener( listener); } + + @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(); + } + } + } + } } } 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 a2f64eafe151..39ae8f589db2 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 @@ * TODO: Should be merged into PermissionManagerInternal, but currently uses internal classes. */ public abstract class PermissionManagerServiceInternal extends PermissionManagerInternal { + /** * Callbacks invoked when interesting actions have been taken on a permission. *

@@ -212,4 +213,10 @@ public abstract void enforceCrossUserPermission(int callingUid, int userId, /** Get all permission that have a certain protection level */ public abstract @NonNull ArrayList getAllPermissionWithProtectionLevel( @PermissionInfo.Protection int protectionLevel); + + /** + * 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); } From c3017ebf3a1fefc99e9e133fc39a7f7bbad715cd Mon Sep 17 00:00:00 2001 From: Alex Buynytskyy Date: Thu, 2 Nov 2023 15:15:48 -0700 Subject: [PATCH 03/26] Validate package names passed to the installer. Bug: 308989388 Bug: 307532206 Test: atest android.content.pm.cts.PackageManagerTest (cherry picked from commit 1f445474cd1b902b2e7292a0d24e58f020fd51e7) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:a7e48c8d7e00962d335b0076266a5df98d41a21c) Merged-In: I840c9c9af5752b3901d4719a13e7908faa43ab04 Change-Id: I840c9c9af5752b3901d4719a13e7908faa43ab04 --- .../android/content/pm/PackageParser.java | 2 +- .../server/pm/PackageInstallerService.java | 30 +++++++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/core/java/android/content/pm/PackageParser.java b/core/java/android/content/pm/PackageParser.java index cd9a7081bd5c..aac2275cfe94 100644 --- a/core/java/android/content/pm/PackageParser.java +++ b/core/java/android/content/pm/PackageParser.java @@ -1644,7 +1644,7 @@ private static ApkLite parseApkLiteInner(File apkFile, FileDescriptor fd, String } } - private static String validateName(String name, boolean requireSeparator, + public static String validateName(String name, boolean requireSeparator, boolean requireFilename) { final int N = name.length(); boolean hasSep = false; diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index 62ac26821986..4d77598b74c3 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -20,6 +20,7 @@ import static org.xmlpull.v1.XmlPullParser.START_TAG; import android.Manifest; +import android.annotation.NonNull; import android.app.ActivityManager; import android.app.AppGlobals; import android.app.AppOpsManager; @@ -43,6 +44,7 @@ import android.content.pm.PackageInstaller.SessionParams; import android.content.pm.PackageItemInfo; import android.content.pm.PackageManager; +import android.content.pm.PackageParser; import android.content.pm.ParceledListSlice; import android.content.pm.VersionedPackage; import android.graphics.Bitmap; @@ -540,17 +542,22 @@ private int createSessionInternal(SessionParams params, String installerPackageN // 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) { + if (params.appPackageName != null && !isValidPackageName(params.appPackageName)) { 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; + // Validate installer package name. + if (params.installerPackageName != null && !isValidPackageName( + params.installerPackageName)) { + params.installerPackageName = null; + } + + String requestedInstallerPackageName = + params.installerPackageName != null ? params.installerPackageName + : installerPackageName; if ((callingUid == Process.SHELL_UID) || (callingUid == Process.ROOT_UID)) { params.installFlags |= PackageManager.INSTALL_FROM_ADB; @@ -804,6 +811,19 @@ private int allocateSessionIdLocked() { throw new IllegalStateException("Failed to allocate session ID"); } + private static boolean isValidPackageName(@NonNull String packageName) { + if (packageName.length() > SessionParams.MAX_PACKAGE_NAME_LENGTH) { + return false; + } + // "android" is a valid package name + String errorMessage = PackageParser.validateName( + packageName, /* requireSeparator= */ false, /* requireFilename */ true); + if (errorMessage != null) { + return false; + } + return true; + } + private File getTmpSessionDir(String volumeUuid) { return Environment.getDataAppDirectory(volumeUuid); } From e112de71d950f360088957016b606a17394d137d Mon Sep 17 00:00:00 2001 From: Alex Buynytskyy Date: Wed, 20 Dec 2023 01:50:36 +0000 Subject: [PATCH 04/26] Disallow system apps to be installed/updated as instant. Bug: 299441833 Test: atest android.content.pm.cts.PackageManagerTest (cherry picked from commit 496e78a1951f2ed69290f03c5625c0f8382f4d31) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0d0f185c0d526c1dac0a8894b2c2f2e378328d73) Merged-In: Idd89a6dd72f0e68259095f677185f0494391025c Change-Id: Idd89a6dd72f0e68259095f677185f0494391025c --- .../core/java/com/android/server/pm/PackageManagerService.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 948085270856..ab1c7e1a85ce 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -13724,6 +13724,9 @@ int installExistingPackageAsUser(@Nullable String packageName, @UserIdInt int us if (pkgSetting == null) { return PackageManager.INSTALL_FAILED_INVALID_URI; } + if (instantApp && (pkgSetting.isSystem() || isUpdatedSystemApp(pkgSetting))) { + return PackageManager.INSTALL_FAILED_INVALID_URI; + } if (!canViewInstantApps(callingUid, UserHandle.getUserId(callingUid))) { // only allow the existing package to be used if it's installed as a full // application for at least one user From 4a98702ebec4675d9cc9484a51590003d55b6dee Mon Sep 17 00:00:00 2001 From: Dmitry Dementyev Date: Wed, 3 Jan 2024 09:26:56 -0800 Subject: [PATCH 05/26] Close AccountManagerService.session after timeout. Bug: 303905130 Bug: 316893159 Test: manual (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:bb53f192e0ceaa026a083da156ef0cb0140f0c09) Merged-In: Ib4cebf1750fc6324dc1c8853e0d716ea5e8ec073 Change-Id: Ib4cebf1750fc6324dc1c8853e0d716ea5e8ec073 --- .../android/server/accounts/AccountManagerService.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java index a3c7159c2b05..f81dbd3c4483 100644 --- a/services/core/java/com/android/server/accounts/AccountManagerService.java +++ b/services/core/java/com/android/server/accounts/AccountManagerService.java @@ -182,6 +182,7 @@ public void onStopUser(int userHandle) { final MessageHandler mHandler; + private static final int TIMEOUT_DELAY_MS = 1000 * 60 * 15; // Messages that can be sent on mHandler private static final int MESSAGE_TIMED_OUT = 3; private static final int MESSAGE_COPY_SHARED_ACCOUNT = 4; @@ -4774,6 +4775,7 @@ public Session(UserAccounts accounts, IAccountManagerResponse response, String a synchronized (mSessions) { mSessions.put(toString(), this); } + scheduleTimeout(); if (response != null) { try { response.asBinder().linkToDeath(this, 0 /* flags */); @@ -4940,6 +4942,11 @@ private void unbind() { } } + private void scheduleTimeout() { + mHandler.sendMessageDelayed( + mHandler.obtainMessage(MESSAGE_TIMED_OUT, this), TIMEOUT_DELAY_MS); + } + public void cancelTimeout() { mHandler.removeMessages(MESSAGE_TIMED_OUT, this); } @@ -4976,6 +4983,9 @@ public void onServiceDisconnected(ComponentName name) { public void onTimedOut() { IAccountManagerResponse response = getResponseAndClose(); + if (Log.isLoggable(TAG, Log.VERBOSE)) { + Log.v(TAG, "Session.onTimedOut"); + } if (response != null) { try { response.onError(AccountManager.ERROR_CODE_REMOTE_EXCEPTION, From b3fb018d56d90a023f48feb35815d0231871d03a Mon Sep 17 00:00:00 2001 From: Beverly Date: Thu, 18 Jan 2024 20:13:52 +0000 Subject: [PATCH 06/26] isUserInLockDown can be true when there are other strong auth requirements Bug: 315206668 Bug: 218495634 Flag: None Test: manual, atest LockPatternUtilsTest (cherry picked from commit d341f1ecdb011d24b17358f115391b3f997cb179) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ba8dfc68aada76127abafdb17d0f0896cc14447a) Merged-In: I5e979a7822dd7254b4579ab28ecf96df1db44179 Change-Id: I5e979a7822dd7254b4579ab28ecf96df1db44179 --- .../internal/widget/LockPatternUtils.java | 4 +- .../internal/util/LockPatternUtilsTest.java | 40 ++++++++++++++++--- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index 43e12b0129e1..459f1db00be9 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -1782,8 +1782,8 @@ public boolean isBiometricAllowedForUser(int userId) { } public boolean isUserInLockdown(int userId) { - return getStrongAuthForUser(userId) - == StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN; + return (getStrongAuthForUser(userId) + & StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN) != 0; } private ICheckCredentialProgressCallback wrapCallback( diff --git a/core/tests/utiltests/src/com/android/internal/util/LockPatternUtilsTest.java b/core/tests/utiltests/src/com/android/internal/util/LockPatternUtilsTest.java index 9913531cdf13..433a35bffeb8 100644 --- a/core/tests/utiltests/src/com/android/internal/util/LockPatternUtilsTest.java +++ b/core/tests/utiltests/src/com/android/internal/util/LockPatternUtilsTest.java @@ -19,6 +19,9 @@ import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_MANAGED; import static android.app.admin.DevicePolicyManager.PASSWORD_QUALITY_UNSPECIFIED; +import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STRONG_AUTH_NOT_REQUIRED; +import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN; + import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.doReturn; @@ -48,12 +51,15 @@ @SmallTest public class LockPatternUtilsTest { + private ILockSettings mLockSettings; + private static final int USER_ID = 1; private static final int DEMO_USER_ID = 5; private LockPatternUtils mLockPatternUtils; private void configureTest(boolean isSecure, boolean isDemoUser, int deviceDemoMode) throws Exception { + mLockSettings = Mockito.mock(ILockSettings.class); final Context context = spy(new ContextWrapper(InstrumentationRegistry.getTargetContext())); final MockContentResolver cr = new MockContentResolver(context); @@ -61,13 +67,12 @@ private void configureTest(boolean isSecure, boolean isDemoUser, int deviceDemoM when(context.getContentResolver()).thenReturn(cr); Settings.Global.putInt(cr, Settings.Global.DEVICE_DEMO_MODE, deviceDemoMode); - final ILockSettings ils = Mockito.mock(ILockSettings.class); - when(ils.havePassword(DEMO_USER_ID)).thenReturn(isSecure); - when(ils.getLong("lockscreen.password_type", PASSWORD_QUALITY_UNSPECIFIED, DEMO_USER_ID)) - .thenReturn((long) PASSWORD_QUALITY_MANAGED); + when(mLockSettings.havePassword(DEMO_USER_ID)).thenReturn(isSecure); + when(mLockSettings.getLong("lockscreen.password_type", PASSWORD_QUALITY_UNSPECIFIED, + DEMO_USER_ID)).thenReturn((long) PASSWORD_QUALITY_MANAGED); // TODO(b/63758238): stop spying the class under test mLockPatternUtils = spy(new LockPatternUtils(context)); - when(mLockPatternUtils.getLockSettings()).thenReturn(ils); + when(mLockPatternUtils.getLockSettings()).thenReturn(mLockSettings); doReturn(true).when(mLockPatternUtils).hasSecureLockScreen(); final UserInfo userInfo = Mockito.mock(UserInfo.class); @@ -77,6 +82,31 @@ private void configureTest(boolean isSecure, boolean isDemoUser, int deviceDemoM when(context.getSystemService(Context.USER_SERVICE)).thenReturn(um); } + @Test + public void isUserInLockDown() throws Exception { + configureTest(true, false, 2); + + // GIVEN strong auth not required + when(mLockSettings.getStrongAuthForUser(USER_ID)).thenReturn(STRONG_AUTH_NOT_REQUIRED); + + // THEN user isn't in lockdown + assertFalse(mLockPatternUtils.isUserInLockdown(USER_ID)); + + // GIVEN lockdown + when(mLockSettings.getStrongAuthForUser(USER_ID)).thenReturn( + STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN); + + // THEN user is in lockdown + assertTrue(mLockPatternUtils.isUserInLockdown(USER_ID)); + + // GIVEN lockdown and lockout + when(mLockSettings.getStrongAuthForUser(USER_ID)).thenReturn( + STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN | STRONG_AUTH_REQUIRED_AFTER_LOCKOUT); + + // THEN user is in lockdown + assertTrue(mLockPatternUtils.isUserInLockdown(USER_ID)); + } + @Test public void isLockScreenDisabled_isDemoUser_true() throws Exception { configureTest(false, true, 2); From 525fe1b77b1dec4a77a6e9eddaa6b9aa32ad34cf Mon Sep 17 00:00:00 2001 From: Tetiana Meronyk Date: Wed, 10 Jan 2024 16:25:13 +0000 Subject: [PATCH 07/26] Fix security vulnerability that creates user with no restrictions when accountOptions are too long. Bug: 293602970 Test: atest UserManagerTest#testAddUserAccountData_validStringValuesAreSaved_validBundleIsSaved && atest UserManagerTest#testAddUserAccountData_invalidStringValuesAreTruncated_invalidBundleIsDropped (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:8dc6feaee7c0a5cea093b5280acaad862921cf3e) Merged-In: I23c971f671546ac085060add89485cfac6691ca3 Change-Id: I23c971f671546ac085060add89485cfac6691ca3 --- core/java/android/os/PersistableBundle.java | 37 +++++++ core/java/android/os/UserManager.java | 23 +++- .../app/ConfirmUserCreationActivity.java | 12 ++ .../android/server/pm/UserManagerService.java | 29 +++-- .../android/server/pm/UserManagerTest.java | 103 ++++++++++++++++++ 5 files changed, 188 insertions(+), 16 deletions(-) diff --git a/core/java/android/os/PersistableBundle.java b/core/java/android/os/PersistableBundle.java index 3e6312754359..bf584c957aa0 100644 --- a/core/java/android/os/PersistableBundle.java +++ b/core/java/android/os/PersistableBundle.java @@ -268,6 +268,43 @@ public void saveToXml(XmlSerializer out) throws IOException, XmlPullParserExcept XmlUtils.writeMapXml(mMap, out, this); } + /** + * Checks whether all keys and values are within the given character limit. + * Note: Maximum character limit of String that can be saved to XML as part of bundle is 65535. + * Otherwise IOException is thrown. + * @param limit length of String keys and values in the PersistableBundle, including nested + * PersistableBundles to check against. + * + * @hide + */ + public boolean isBundleContentsWithinLengthLimit(int limit) { + unparcel(); + if (mMap == null) { + return true; + } + for (int i = 0; i < mMap.size(); i++) { + if (mMap.keyAt(i) != null && mMap.keyAt(i).length() > limit) { + return false; + } + final Object value = mMap.valueAt(i); + if (value instanceof String && ((String) value).length() > limit) { + return false; + } else if (value instanceof String[]) { + String[] stringArray = (String[]) value; + for (int j = 0; j < stringArray.length; j++) { + if (stringArray[j] != null + && stringArray[j].length() > limit) { + return false; + } + } + } else if (value instanceof PersistableBundle + && !((PersistableBundle) value).isBundleContentsWithinLengthLimit(limit)) { + return false; + } + } + return true; + } + /** @hide */ static class MyReadMapCallback implements XmlUtils.ReadMapCallback { @Override diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java index da41478e91a6..fc714923bf41 100644 --- a/core/java/android/os/UserManager.java +++ b/core/java/android/os/UserManager.java @@ -77,6 +77,21 @@ public class UserManager { private Boolean mIsManagedProfileCached; + /** Maximum length of username. + * @hide + */ + public static final int MAX_USER_NAME_LENGTH = 100; + + /** Maximum length of user property String value. + * @hide + */ + public static final int MAX_ACCOUNT_STRING_LENGTH = 500; + + /** Maximum length of account options String values. + * @hide + */ + public static final int MAX_ACCOUNT_OPTIONS_LENGTH = 1000; + /** * @hide * No user restriction. @@ -2199,15 +2214,15 @@ public UserInfo createRestrictedProfile(String name) { * time, the preferred user name and account information are used by the setup process for that * user. * - * @param userName Optional name to assign to the user. + * @param userName Optional name to assign to the user. Character limit is 100. * @param accountName Optional account name that will be used by the setup wizard to initialize - * the user. + * the user. Character limit is 500. * @param accountType Optional account type for the account to be created. This is required - * if the account name is specified. + * if the account name is specified. Character limit is 500. * @param accountOptions Optional bundle of data to be passed in during account creation in the * new user via {@link AccountManager#addAccount(String, String, String[], * Bundle, android.app.Activity, android.accounts.AccountManagerCallback, - * Handler)}. + * Handler)}. Character limit is 1000. * @return An Intent that can be launched from an Activity. * @see #USER_CREATION_FAILED_NOT_PERMITTED * @see #USER_CREATION_FAILED_NO_MORE_USERS diff --git a/core/java/com/android/internal/app/ConfirmUserCreationActivity.java b/core/java/com/android/internal/app/ConfirmUserCreationActivity.java index 03da9bc939ec..74dedc38a922 100644 --- a/core/java/com/android/internal/app/ConfirmUserCreationActivity.java +++ b/core/java/com/android/internal/app/ConfirmUserCreationActivity.java @@ -110,6 +110,14 @@ private String checkUserCreationRequirements() { if (cantCreateUser) { setResult(UserManager.USER_CREATION_FAILED_NOT_PERMITTED); return null; + } else if (!(isUserPropertyWithinLimit(mUserName, UserManager.MAX_USER_NAME_LENGTH) + && isUserPropertyWithinLimit(mAccountName, UserManager.MAX_ACCOUNT_STRING_LENGTH) + && isUserPropertyWithinLimit(mAccountType, UserManager.MAX_ACCOUNT_STRING_LENGTH)) + || (mAccountOptions != null && !mAccountOptions.isBundleContentsWithinLengthLimit( + UserManager.MAX_ACCOUNT_OPTIONS_LENGTH))) { + setResult(UserManager.USER_CREATION_FAILED_NOT_PERMITTED); + Log.i(TAG, "User properties must not exceed their character limits"); + return null; } else if (cantCreateAnyMoreUsers) { setResult(UserManager.USER_CREATION_FAILED_NO_MORE_USERS); return null; @@ -137,4 +145,8 @@ public void onClick(DialogInterface dialog, int which) { } finish(); } + + private boolean isUserPropertyWithinLimit(String property, int limit) { + return property == null || property.length() <= limit; + } } diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java index 318c11141cfe..645ee1a2f12e 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -225,8 +225,6 @@ public class UserManagerService extends IUserManager.Stub { private static final int USER_VERSION = 7; - private static final int MAX_USER_STRING_LENGTH = 500; - private static final long EPOCH_PLUS_30_YEARS = 30L * 365 * 24 * 60 * 60 * 1000L; // ms // Maximum number of managed profiles permitted per user is 1. This cannot be increased @@ -2420,16 +2418,18 @@ void writeUserLP(UserData userData, OutputStream os) if (userData.persistSeedData) { if (userData.seedAccountName != null) { serializer.attribute(null, ATTR_SEED_ACCOUNT_NAME, - truncateString(userData.seedAccountName)); + truncateString(userData.seedAccountName, + UserManager.MAX_ACCOUNT_STRING_LENGTH)); } if (userData.seedAccountType != null) { serializer.attribute(null, ATTR_SEED_ACCOUNT_TYPE, - truncateString(userData.seedAccountType)); + truncateString(userData.seedAccountType, + UserManager.MAX_ACCOUNT_STRING_LENGTH)); } } if (userInfo.name != null) { serializer.startTag(null, TAG_NAME); - serializer.text(truncateString(userInfo.name)); + serializer.text(truncateString(userInfo.name, UserManager.MAX_USER_NAME_LENGTH)); serializer.endTag(null, TAG_NAME); } synchronized (mRestrictionsLock) { @@ -2470,11 +2470,11 @@ void writeUserLP(UserData userData, OutputStream os) serializer.endDocument(); } - private String truncateString(String original) { - if (original == null || original.length() <= MAX_USER_STRING_LENGTH) { + private String truncateString(String original, int limit) { + if (original == null || original.length() <= limit) { return original; } - return original.substring(0, MAX_USER_STRING_LENGTH); + return original.substring(0, limit); } /* @@ -2819,7 +2819,7 @@ private UserInfo createUserInternalUnchecked(@Nullable String name, @UserInfoFla private UserInfo createUserInternalUncheckedNoTracing(@Nullable String name, @UserInfoFlag int flags, @UserIdInt int parentId, boolean preCreate, @Nullable String[] disallowedPackages, @NonNull TimingsTraceLog t) { - String truncatedName = truncateString(name); + String truncatedName = truncateString(name, UserManager.MAX_USER_NAME_LENGTH); // First try to use a pre-created user (if available). // NOTE: currently we don't support pre-created managed profiles if (!preCreate && (parentId < 0 && !UserInfo.isManagedProfile(flags))) { @@ -3877,9 +3877,14 @@ public void setSeedAccountData(int userId, String accountName, String accountTyp Slog.e(LOG_TAG, "No such user for settings seed data u=" + userId); return; } - userData.seedAccountName = truncateString(accountName); - userData.seedAccountType = truncateString(accountType); - userData.seedAccountOptions = accountOptions; + userData.seedAccountName = truncateString(accountName, + UserManager.MAX_ACCOUNT_STRING_LENGTH); + userData.seedAccountType = truncateString(accountType, + UserManager.MAX_ACCOUNT_STRING_LENGTH); + if (accountOptions != null && accountOptions.isBundleContentsWithinLengthLimit( + UserManager.MAX_ACCOUNT_OPTIONS_LENGTH)) { + userData.seedAccountOptions = accountOptions; + } userData.persistSeedData = persist; } if (persist) { diff --git a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java index e9edba58a3dd..69548f839c1e 100644 --- a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java @@ -16,6 +16,8 @@ package com.android.server.pm; +import static org.junit.Assert.assertTrue; + import android.app.ActivityManager; import android.content.BroadcastReceiver; import android.content.Context; @@ -24,6 +26,7 @@ import android.content.pm.PackageManager; import android.content.pm.UserInfo; import android.os.Bundle; +import android.os.PersistableBundle; import android.os.UserHandle; import android.os.UserManager; import android.provider.Settings; @@ -601,6 +604,106 @@ public void testConcurrentUserCreate() throws Exception { assertEquals(canBeCreatedCount, created.get()); } + @Test + public void testAddUserAccountData_validStringValuesAreSaved_validBundleIsSaved() { + assumeManagedUsersSupported(); + + String userName = "User"; + String accountName = "accountName"; + String accountType = "accountType"; + String arrayKey = "StringArrayKey"; + String stringKey = "StringKey"; + String intKey = "IntKey"; + String nestedBundleKey = "PersistableBundleKey"; + String value1 = "Value 1"; + String value2 = "Value 2"; + String value3 = "Value 3"; + + UserInfo userInfo = mUserManager.createUser(userName, + UserManager.USER_TYPE_FULL_SECONDARY, 0); + + PersistableBundle accountOptions = new PersistableBundle(); + String[] stringArray = {value1, value2}; + accountOptions.putInt(intKey, 1234); + PersistableBundle nested = new PersistableBundle(); + nested.putString(stringKey, value3); + accountOptions.putPersistableBundle(nestedBundleKey, nested); + accountOptions.putStringArray(arrayKey, stringArray); + + mUserManager.clearSeedAccountData(); + mUserManager.setSeedAccountData(mContext.getUserId(), accountName, + accountType, accountOptions); + + //assert userName accountName and accountType were saved correctly + assertTrue(mUserManager.getUserInfo(userInfo.id).name.equals(userName)); + assertTrue(mUserManager.getSeedAccountName().equals(accountName)); + assertTrue(mUserManager.getSeedAccountType().equals(accountType)); + + //assert bundle with correct values was added + assertThat(mUserManager.getSeedAccountOptions().containsKey(arrayKey)).isTrue(); + assertThat(mUserManager.getSeedAccountOptions().getPersistableBundle(nestedBundleKey) + .getString(stringKey)).isEqualTo(value3); + assertThat(mUserManager.getSeedAccountOptions().getStringArray(arrayKey)[0]) + .isEqualTo(value1); + + mUserManager.removeUser(userInfo.id); + } + + @Test + public void testAddUserAccountData_invalidStringValuesAreTruncated_invalidBundleIsDropped() { + assumeManagedUsersSupported(); + + String tooLongString = generateLongString(); + String userName = "User " + tooLongString; + String accountType = "Account Type " + tooLongString; + String accountName = "accountName " + tooLongString; + String arrayKey = "StringArrayKey"; + String stringKey = "StringKey"; + String intKey = "IntKey"; + String nestedBundleKey = "PersistableBundleKey"; + String value1 = "Value 1"; + String value2 = "Value 2"; + + UserInfo userInfo = mUserManager.createUser(userName, + UserManager.USER_TYPE_FULL_SECONDARY, 0); + + PersistableBundle accountOptions = new PersistableBundle(); + String[] stringArray = {value1, value2}; + accountOptions.putInt(intKey, 1234); + PersistableBundle nested = new PersistableBundle(); + nested.putString(stringKey, tooLongString); + accountOptions.putPersistableBundle(nestedBundleKey, nested); + accountOptions.putStringArray(arrayKey, stringArray); + mUserManager.clearSeedAccountData(); + mUserManager.setSeedAccountData(mContext.getUserId(), accountName, + accountType, accountOptions); + + //assert userName was truncated + assertTrue(mUserManager.getUserInfo(userInfo.id).name.length() + == UserManager.MAX_USER_NAME_LENGTH); + + //assert accountName and accountType got truncated + assertTrue(mUserManager.getSeedAccountName().length() + == UserManager.MAX_ACCOUNT_STRING_LENGTH); + assertTrue(mUserManager.getSeedAccountType().length() + == UserManager.MAX_ACCOUNT_STRING_LENGTH); + + //assert bundle with invalid values was dropped + assertThat(mUserManager.getSeedAccountOptions() == null).isTrue(); + + mUserManager.removeUser(userInfo.id); + } + + private String generateLongString() { + String partialString = "Test Name Test Name Test Name Test Name Test Name Test Name Test " + + "Name Test Name Test Name Test Name "; //String of length 100 + StringBuilder resultString = new StringBuilder(); + for (int i = 0; i < 600; i++) { + resultString.append(partialString); + } + return resultString.toString(); + } + private boolean isPackageInstalledForUser(String packageName, int userId) { try { return mPackageManager.getPackageInfoAsUser(packageName, 0, userId) != null; From bc6193c2a8e96ae9ac5dfecfa567f9d29191da7e Mon Sep 17 00:00:00 2001 From: Valentin Iftime Date: Mon, 16 Oct 2023 09:29:17 +0200 Subject: [PATCH 08/26] [BACKPORT] Prioritize system toasts Insert toasts from system packages at the front of the queue to ensure that apps can't spam with toast to delay system toasts from showing. Test: atest NotificationManagerServiceTest Bug: 293301736 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:67721fcfb3198f220c90c976f870407a0bb8d6c6) Merged-In: I13547f853476bc88d12026c545aba9f857ce8724 Change-Id: I13547f853476bc88d12026c545aba9f857ce8724 --- .../NotificationManagerService.java | 40 +++++++++-- .../NotificationManagerServiceTest.java | 68 +++++++++++++++++++ 2 files changed, 104 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 0837adc770c7..36a285820f6f 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -751,17 +751,21 @@ private void writePolicyXml(OutputStream stream, boolean forBackup, int userId) private static final class ToastRecord { + final int uid; final int pid; final String pkg; + final boolean isSystemToast; final ITransientNotification callback; int duration; int displayId; Binder token; - ToastRecord(int pid, String pkg, ITransientNotification callback, int duration, + ToastRecord(int uid, int pid, String pkg, boolean isSystemToast, ITransientNotification callback, int duration, Binder token, int displayId) { + this.uid = uid; this.pid = pid; this.pkg = pkg; + this.isSystemToast = isSystemToast; this.callback = callback; this.duration = duration; this.token = token; @@ -2524,10 +2528,21 @@ record = mToastQueue.get(index); Binder token = new Binder(); mWindowManagerInternal.addWindowToken(token, TYPE_TOAST, displayId); - record = new ToastRecord(callingPid, pkg, callback, duration, token, + record = new ToastRecord(callingUid, callingPid, pkg, isSystemToast, callback, duration, token, displayId); - mToastQueue.add(record); - index = mToastQueue.size() - 1; + + // Insert system toasts at the front of the queue + int systemToastInsertIdx = mToastQueue.size(); + if (isSystemToast) { + systemToastInsertIdx = getInsertIndexForSystemToastLocked(); + } + if (systemToastInsertIdx < mToastQueue.size()) { + index = systemToastInsertIdx; + mToastQueue.add(index, record); + } else { + mToastQueue.add(record); + index = mToastQueue.size() - 1; + } keepProcessAliveIfNeededLocked(callingPid); } // If it's at index 0, it's the current toast. It doesn't matter if it's @@ -2543,6 +2558,23 @@ record = new ToastRecord(callingPid, pkg, callback, duration, token, } } + @GuardedBy("mToastQueue") + private int getInsertIndexForSystemToastLocked() { + // If there are other system toasts: insert after the last one + int idx = 0; + for (ToastRecord r : mToastQueue) { + if (idx == 0 && mIsCurrentToastShown) { + idx++; + continue; + } + if (!r.isSystemToast) { + return idx; + } + idx++; + } + return idx; + } + @Override public void cancelToast(String pkg, ITransientNotification callback) { Slog.i(TAG, "cancelToast pkg=" + pkg + " callback=" + callback); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index 5c2e27080986..c6f0b8cdb5bd 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -4480,6 +4480,74 @@ public void testAlwaysAllowSystemToasts() throws Exception { assertEquals(1, mService.mToastQueue.size()); } + @Test + public void testPrioritizeSystemToasts() throws Exception { + // Insert non-system toasts + final String testPackage = "testPackageName"; + assertEquals(0, mService.mToastQueue.size()); + mService.isSystemUid = false; + mService.isSystemAppId = false; + setToastRateIsWithinQuota(true); + setIfPackageHasPermissionToAvoidToastRateLimiting(testPackage, false); + + // package is not suspended + when(mPackageManager.isPackageSuspendedForUser(testPackage, UserHandle.getUserId(mUid))) + .thenReturn(false); + + INotificationManager nmService = (INotificationManager) mService.mService; + + // Enqueue maximum number of toasts for test package + for (int i = 0; i < NotificationManagerService.MAX_PACKAGE_TOASTS; i++) { + nmService.enqueueTextToast(testPackage, new Binder(), "Text", 2000, 0, null); + } + + // Enqueue system toast + final String testPackageSystem = "testPackageNameSystem"; + mService.isSystemUid = true; + setIfPackageHasPermissionToAvoidToastRateLimiting(testPackageSystem, false); + when(mPackageManager.isPackageSuspendedForUser(testPackageSystem, UserHandle.getUserId(mUid))) + .thenReturn(false); + + nmService.enqueueToast(testPackageSystem, new Binder(), new TestableToastCallback(), 2000, 0); + + // System toast is inserted at the front of the queue, behind current showing toast + assertEquals(testPackageSystem, mService.mToastQueue.get(1).pkg); + } + + @Test + public void testPrioritizeSystemToasts_enqueueAfterExistingSystemToast() throws Exception { + // Insert system toasts + final String testPackageSystem1 = "testPackageNameSystem1"; + assertEquals(0, mService.mToastQueue.size()); + mService.isSystemUid = true; + setToastRateIsWithinQuota(true); + setIfPackageHasPermissionToAvoidToastRateLimiting(testPackageSystem1, false); + + // package is not suspended + when(mPackageManager.isPackageSuspendedForUser(testPackageSystem1, UserHandle.getUserId(mUid))) + .thenReturn(false); + + INotificationManager nmService = (INotificationManager) mService.mService; + + // Enqueue maximum number of toasts for test package + for (int i = 0; i < NotificationManagerService.MAX_PACKAGE_TOASTS; i++) { + nmService.enqueueTextToast(testPackageSystem1, new Binder(), "Text", 2000, 0, null); + } + + // Enqueue another system toast + final String testPackageSystem2 = "testPackageNameSystem2"; + mService.isSystemUid = true; + setIfPackageHasPermissionToAvoidToastRateLimiting(testPackageSystem2, false); + when(mPackageManager.isPackageSuspendedForUser(testPackageSystem2, UserHandle.getUserId(mUid))) + .thenReturn(false); + + nmService.enqueueToast(testPackageSystem2, new Binder(), new TestableToastCallback(), 2000, 0); + + // System toast is inserted at the back of the queue, after the other system toasts + assertEquals(testPackageSystem2, + mService.mToastQueue.get(mService.mToastQueue.size() - 1).pkg); + } + @Test public void testOnNotificationSmartReplySent() { final int replyIndex = 2; From ac7dcf0944d9b3b33a16dc587e5e7bb5f577b9dd Mon Sep 17 00:00:00 2001 From: Jan Tomljanovic Date: Fri, 6 Nov 2020 11:28:09 +0000 Subject: [PATCH 09/26] Don't try to show the current toast again while it's showing. By doing this we avoid a few bad things: - mechanism that hides the current toast by trying to show it again - delaying the call to hide and remove the current toast from the queue when it's duration expires (which in the case of repeated calls can delay this indefinitely) Test: atest NotificationManagerServiceTest Test: atest android.widget.cts.ToastTest Bug: 167672740 Change-Id: Ie4953109314113efae49fa0c5e0c236e6e0dbb23 --- .../NotificationManagerService.java | 13 ++++++++++ .../NotificationManagerServiceTest.java | 26 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 36a285820f6f..3457bc27cab1 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -444,6 +444,10 @@ public class NotificationManagerService extends SystemService { private KeyguardManager mKeyguardManager; + // True if the toast that's on top of the queue is being shown at the moment. + @GuardedBy("mToastQueue") + private boolean mIsCurrentToastShown = false; + // The last key in this list owns the hardware. ArrayList mLights = new ArrayList<>(); @@ -6517,12 +6521,17 @@ public void run() { @GuardedBy("mToastQueue") void showNextToastLocked() { + if (mIsCurrentToastShown) { + return; // Don't show the same toast twice. + } + ToastRecord record = mToastQueue.get(0); while (record != null) { if (DBG) Slog.d(TAG, "Show pkg=" + record.pkg + " callback=" + record.callback); try { record.callback.show(record.token); scheduleDurationReachedLocked(record); + mIsCurrentToastShown = true; return; } catch (RemoteException e) { Slog.w(TAG, "Object died trying to show notification " + record.callback @@ -6554,6 +6563,10 @@ void cancelToastLocked(int index) { // the list anyway } + if (index == 0) { + mIsCurrentToastShown = false; + } + ToastRecord lastToast = mToastQueue.remove(index); mWindowManagerInternal.removeWindowToken(lastToast.token, false /* removeWindows */, diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index c6f0b8cdb5bd..20fe61418d4f 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -4415,6 +4415,32 @@ public void testAllowForegroundToasts() throws Exception { } @Test + public void testDontCallShowToastAgainOnTheSameTextToast() throws Exception { + final String testPackage = "testPackageName"; + assertEquals(0, mService.mToastQueue.size()); + mService.isSystemUid = false; + + // package is not suspended + when(mPackageManager.isPackageSuspendedForUser(testPackage, UserHandle.getUserId(mUid))) + .thenReturn(false); + + setAppInForegroundForToasts(mUid, true); + + Binder token = new Binder(); + INotificationManager nmService = (INotificationManager) mService.mService; + + // first time trying to show the toast, showToast gets called + nmService.enqueueTextToast(testPackage, token, "Text", 2000, 0, null); + verify(mStatusBar, times(1)) + .showToast(anyInt(), any(), any(), any(), any(), anyInt(), any()); + + // second time trying to show the same toast, showToast isn't called again (total number of + // invocations stays at one) + nmService.enqueueTextToast(testPackage, token, "Text", 2000, 0, null); + verify(mStatusBar, times(1)) + .showToast(anyInt(), any(), any(), any(), any(), anyInt(), any()); + } + public void testDisallowToastsFromSuspendedPackages() throws Exception { final String testPackage = "testPackageName"; assertEquals(0, mService.mToastQueue.size()); From 062cd07e4dd4d8de98fe1ddabb88ae17a7ba7ee9 Mon Sep 17 00:00:00 2001 From: Jing Ji Date: Tue, 25 Oct 2022 22:39:52 -0700 Subject: [PATCH 10/26] DO NOT MERGE: ActivityManager#killBackgroundProcesses can kill caller's own app only unless it's a system app. Bug: 239423414 Bug: 223376078 Test: atest CtsAppTestCases:ActivityManagerTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:d1c95670b248df945784b0f2830acf83b5682de3) Merged-In: Iac6baa889965b8ffecd9a43179a4c96632ad1d02 AOSP-Change-Id: Iac6baa889965b8ffecd9a43179a4c96632ad1d02 Change-Id: I3a39b5e2b2ff0c314972ddeccb012894de704de8 --- .../server/am/ActivityManagerService.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 23590169c5bc..7ddc98c9abad 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -4357,6 +4357,22 @@ void killAllBackgroundProcessesExcept(int minTargetSdk, int maxProcState) { throw new SecurityException(msg); } + final int callingUid = Binder.getCallingUid(); + final int callingPid = Binder.getCallingPid(); + + ProcessRecord proc; + synchronized (mPidsSelfLocked) { + proc = mPidsSelfLocked.get(callingPid); + } + if (callingUid >= FIRST_APPLICATION_UID + && (proc == null || !proc.info.isSystemApp())) { + final String msg = "Permission Denial: killAllBackgroundProcesses() from pid=" + + callingPid + ", uid=" + callingUid + " is not allowed"; + Slog.w(TAG, msg); + // Silently return to avoid existing apps from crashing. + return; + } + final long callingId = Binder.clearCallingIdentity(); try { synchronized (this) { From c894a301c14fecd2f0d9fea00f541c94a04e9a90 Mon Sep 17 00:00:00 2001 From: Jing Ji Date: Thu, 19 Oct 2023 14:22:58 -0700 Subject: [PATCH 11/26] DO NOT MERGE: Fix ActivityManager#killBackgroundProcesses permissions In the pevious CL, we incorrectly added the permission check in the killBackgroundProcessesExcept. Now fix this issue. Bug: 239423414 Bug: 223376078 Test: atest CtsAppTestCases:ActivityManagerTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:140fce861944419a375c669010c6c47cd7ff5b37) Merged-In: I9471a77188ee63ec32cd0c81569193e4ccad885b AOSP-Change-Id: I9471a77188ee63ec32cd0c81569193e4ccad885b Change-Id: I1b1e683b6a92b0fa2a844a99bedcccac8c980e58 --- .../server/am/ActivityManagerService.java | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 7ddc98c9abad..23590169c5bc 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -4357,22 +4357,6 @@ void killAllBackgroundProcessesExcept(int minTargetSdk, int maxProcState) { throw new SecurityException(msg); } - final int callingUid = Binder.getCallingUid(); - final int callingPid = Binder.getCallingPid(); - - ProcessRecord proc; - synchronized (mPidsSelfLocked) { - proc = mPidsSelfLocked.get(callingPid); - } - if (callingUid >= FIRST_APPLICATION_UID - && (proc == null || !proc.info.isSystemApp())) { - final String msg = "Permission Denial: killAllBackgroundProcesses() from pid=" - + callingPid + ", uid=" + callingUid + " is not allowed"; - Slog.w(TAG, msg); - // Silently return to avoid existing apps from crashing. - return; - } - final long callingId = Binder.clearCallingIdentity(); try { synchronized (this) { From 8d4281281b98fe004c3eb200191cb904b370b753 Mon Sep 17 00:00:00 2001 From: Valentin Iftime Date: Thu, 1 Feb 2024 13:58:49 +0100 Subject: [PATCH 12/26] Verify URI permission for channel sound update from NotificationListenerService Check that a privileged NotificationListenerService (CDM) has the permission to access the sound URI when updating a notification channel. Test: atest com.android.server.notification.NotificationManagerServiceTest#testUpdateNotificationChannelFromPrivilegedListener_noSoundUriPermission Bug: 317357401 (cherry picked from commit 9b7bbbf5ad542ecf9ecbf8cd819b468791b443c0) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f090c0538a27d8658d8a860046d5c5e931302341) Merged-In: Ic7d2e96e43565e98d2aa29b8f2ba35c142387ba9 Change-Id: Ic7d2e96e43565e98d2aa29b8f2ba35c142387ba9 --- .../NotificationManagerService.java | 22 ++++++ .../NotificationManagerServiceTest.java | 67 +++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java index 3457bc27cab1..777ed41c4079 100755 --- a/services/core/java/com/android/server/notification/NotificationManagerService.java +++ b/services/core/java/com/android/server/notification/NotificationManagerService.java @@ -4376,6 +4376,10 @@ public void updateNotificationChannelFromPrivilegedListener(INotificationListene Preconditions.checkNotNull(user); verifyPrivilegedListener(token, user, false); + + final NotificationChannel originalChannel = mPreferencesHelper.getNotificationChannel( + pkg, getUidForPackageAndUser(pkg, user), channel.getId(), true); + verifyPrivilegedListenerUriPermission(Binder.getCallingUid(), channel, originalChannel); updateNotificationChannelInt(pkg, getUidForPackageAndUser(pkg, user), channel, true); } @@ -4455,6 +4459,24 @@ private void verifyPrivilegedListener(INotificationListener token, UserHandle us } } + private void verifyPrivilegedListenerUriPermission(int sourceUid, + @NonNull NotificationChannel updateChannel, + @Nullable NotificationChannel originalChannel) { + // Check that the NLS has the required permissions to access the channel + final Uri soundUri = updateChannel.getSound(); + final Uri originalSoundUri = + (originalChannel != null) ? originalChannel.getSound() : null; + if (soundUri != null && !Objects.equals(originalSoundUri, soundUri)) { + Binder.withCleanCallingIdentity(() -> { + mUgmInternal.checkGrantUriPermission(sourceUid, null, + ContentProvider.getUriWithoutUserId(soundUri), + Intent.FLAG_GRANT_READ_URI_PERMISSION, + ContentProvider.getUserIdFromUri(soundUri, + UserHandle.getUserId(sourceUid))); + }); + } + } + private int getUidForPackageAndUser(String pkg, UserHandle user) throws RemoteException { int uid = 0; long identity = Binder.clearCallingIdentity(); diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java index 20fe61418d4f..b6fd7823b173 100755 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java @@ -2037,6 +2037,73 @@ public void testUpdateNotificationChannelFromPrivilegedListener_badUser() throws eq(NotificationListenerService.NOTIFICATION_CHANNEL_OR_GROUP_UPDATED)); } + @Test + public void testUpdateNotificationChannelFromPrivilegedListener_noSoundUriPermission() + throws Exception { + mService.setPreferencesHelper(mPreferencesHelper); + List associations = new ArrayList<>(); + associations.add("a"); + when(mCompanionMgr.getAssociations(PKG, UserHandle.getUserId(mUid))) + .thenReturn(associations); + when(mPreferencesHelper.getNotificationChannel(eq(PKG), anyInt(), + eq(mTestNotificationChannel.getId()), anyBoolean())) + .thenReturn(mTestNotificationChannel); + + final Uri soundUri = Uri.parse("content://media/test/sound/uri"); + final NotificationChannel updatedNotificationChannel = new NotificationChannel( + TEST_CHANNEL_ID, TEST_CHANNEL_ID, IMPORTANCE_DEFAULT); + updatedNotificationChannel.setSound(soundUri, + updatedNotificationChannel.getAudioAttributes()); + + doThrow(new SecurityException("no access")).when(mUgmInternal) + .checkGrantUriPermission(eq(Process.myUid()), any(), eq(soundUri), + anyInt(), eq(Process.myUserHandle().getIdentifier())); + + assertThrows(SecurityException.class, + () -> mBinderService.updateNotificationChannelFromPrivilegedListener(null, PKG, + Process.myUserHandle(), updatedNotificationChannel)); + + verify(mPreferencesHelper, never()).updateNotificationChannel( + anyString(), anyInt(), any(), anyBoolean()); + + verify(mListeners, never()).notifyNotificationChannelChanged(eq(PKG), + eq(Process.myUserHandle()), eq(mTestNotificationChannel), + eq(NotificationListenerService.NOTIFICATION_CHANNEL_OR_GROUP_UPDATED)); + } + + @Test + public void testUpdateNotificationChannelFromPrivilegedListener_noSoundUriPermission_sameSound() + throws Exception { + mService.setPreferencesHelper(mPreferencesHelper); + List associations = new ArrayList<>(); + associations.add("a"); + when(mCompanionMgr.getAssociations(PKG, UserHandle.getUserId(mUid))) + .thenReturn(associations); + when(mPreferencesHelper.getNotificationChannel(eq(PKG), anyInt(), + eq(mTestNotificationChannel.getId()), anyBoolean())) + .thenReturn(mTestNotificationChannel); + + final Uri soundUri = Settings.System.DEFAULT_NOTIFICATION_URI; + final NotificationChannel updatedNotificationChannel = new NotificationChannel( + TEST_CHANNEL_ID, TEST_CHANNEL_ID, IMPORTANCE_DEFAULT); + updatedNotificationChannel.setSound(soundUri, + updatedNotificationChannel.getAudioAttributes()); + + doThrow(new SecurityException("no access")).when(mUgmInternal) + .checkGrantUriPermission(eq(Process.myUid()), any(), eq(soundUri), + anyInt(), eq(Process.myUserHandle().getIdentifier())); + + mBinderService.updateNotificationChannelFromPrivilegedListener( + null, PKG, Process.myUserHandle(), updatedNotificationChannel); + + verify(mPreferencesHelper, times(1)).updateNotificationChannel( + anyString(), anyInt(), any(), anyBoolean()); + + verify(mListeners, never()).notifyNotificationChannelChanged(eq(PKG), + eq(Process.myUserHandle()), eq(mTestNotificationChannel), + eq(NotificationListenerService.NOTIFICATION_CHANNEL_OR_GROUP_UPDATED)); + } + @Test public void testGetNotificationChannelFromPrivilegedListener_cdm_success() throws Exception { mService.setPreferencesHelper(mPreferencesHelper); From 02cec37c759f08f4bc385fcbd983fab58e65deff Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Thu, 30 Nov 2023 23:12:39 +0000 Subject: [PATCH 13/26] Added throttle when reporting shortcut usage Bug: 304290201 Test: manual (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:76121eb73d4c40829d5513b073871333520fe0a2) Merged-In: I96370cbd4f6a55f894c1a93307e5f82dfd394652 Change-Id: I96370cbd4f6a55f894c1a93307e5f82dfd394652 --- .../android/server/pm/ShortcutPackage.java | 35 +++++++++++++++++++ .../android/server/pm/ShortcutService.java | 12 +++---- .../server/pm/ShortcutManagerTest2.java | 2 ++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/pm/ShortcutPackage.java b/services/core/java/com/android/server/pm/ShortcutPackage.java index c6bc7576147f..da018ad04179 100644 --- a/services/core/java/com/android/server/pm/ShortcutPackage.java +++ b/services/core/java/com/android/server/pm/ShortcutPackage.java @@ -19,6 +19,7 @@ import android.annotation.Nullable; import android.annotation.UserIdInt; import android.app.Person; +import android.app.usage.UsageStatsManagerInternal; import android.content.ComponentName; import android.content.Intent; import android.content.IntentFilter; @@ -28,12 +29,14 @@ import android.content.pm.ShortcutManager; import android.content.res.Resources; import android.os.PersistableBundle; +import android.os.SystemClock; import android.text.format.Formatter; import android.util.ArrayMap; import android.util.ArraySet; import android.util.Log; import android.util.Slog; +import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.ArrayUtils; import com.android.internal.util.Preconditions; @@ -119,6 +122,11 @@ class ShortcutPackage extends ShortcutPackageItem { private static final String KEY_BITMAPS = "bitmaps"; private static final String KEY_BITMAP_BYTES = "bitmapBytes"; + @VisibleForTesting + public static final int REPORT_USAGE_BUFFER_SIZE = 3; + + private final Object mLock = new Object(); + /** * All the shortcuts from the package, keyed on IDs. */ @@ -143,6 +151,9 @@ class ShortcutPackage extends ShortcutPackageItem { private long mLastKnownForegroundElapsedTime; + @GuardedBy("mLock") + private List mLastReportedTime = new ArrayList<>(); + private ShortcutPackage(ShortcutUser shortcutUser, int packageUserId, String packageName, ShortcutPackageInfo spi) { super(shortcutUser, packageUserId, packageName, @@ -1352,6 +1363,30 @@ public boolean hasNonManifestShortcuts() { return false; } + void reportShortcutUsed(@NonNull final UsageStatsManagerInternal usageStatsManagerInternal, + @NonNull final String shortcutId) { + synchronized (mLock) { + final long currentTS = SystemClock.elapsedRealtime(); + final ShortcutService s = mShortcutUser.mService; + if (mLastReportedTime.isEmpty() + || mLastReportedTime.size() < REPORT_USAGE_BUFFER_SIZE) { + mLastReportedTime.add(currentTS); + } else if (currentTS - mLastReportedTime.get(0) > s.mSaveDelayMillis) { + mLastReportedTime.remove(0); + mLastReportedTime.add(currentTS); + } else { + return; + } + final long token = s.injectClearCallingIdentity(); + try { + usageStatsManagerInternal.reportShortcutUsage(getPackageName(), shortcutId, + getUser().getUserId()); + } finally { + s.injectRestoreCallingIdentity(token); + } + } + } + public void dump(@NonNull PrintWriter pw, @NonNull String prefix, DumpFilter filter) { pw.println(); diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index 2e2883dcb2a5..c18cdcb89140 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -315,7 +315,7 @@ public boolean test(PackageInfo pi) { private CompressFormat mIconPersistFormat; private int mIconPersistQuality; - private int mSaveDelayMillis; + int mSaveDelayMillis; private final IPackageManager mIPackageManager; private final PackageManagerInternal mPackageManagerInternal; @@ -2285,10 +2285,11 @@ public void reportShortcutUsed(String packageName, String shortcutId, int userId shortcutId, packageName, userId)); } + final ShortcutPackage ps; synchronized (mLock) { throwIfUserLockedL(userId); - final ShortcutPackage ps = getPackageShortcutsForPublisherLocked(packageName, userId); + ps = getPackageShortcutsForPublisherLocked(packageName, userId); if (ps.findShortcutById(shortcutId) == null) { Log.w(TAG, String.format("reportShortcutUsed: package %s doesn't have shortcut %s", @@ -2297,12 +2298,7 @@ public void reportShortcutUsed(String packageName, String shortcutId, int userId } } - final long token = injectClearCallingIdentity(); - try { - mUsageStatsManagerInternal.reportShortcutUsage(packageName, shortcutId, userId); - } finally { - injectRestoreCallingIdentity(token); - } + ps.reportShortcutUsed(mUsageStatsManagerInternal, shortcutId); } @Override diff --git a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java index 18970322d854..27cf3502d489 100644 --- a/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java +++ b/services/tests/servicestests/src/com/android/server/pm/ShortcutManagerTest2.java @@ -1940,6 +1940,8 @@ public void testThrottling_resetByInternalCall() throws Exception { public void testReportShortcutUsed() { mRunningUsers.put(USER_10, true); + mService.updateConfigurationLocked( + ShortcutService.ConfigConstants.KEY_SAVE_DELAY_MILLIS + "=1"); runWithCaller(CALLING_PACKAGE_1, USER_10, () -> { reset(mMockUsageStatsManagerInternal); From 912922272fc8cbe51573ffe9b58dbf869c9394eb Mon Sep 17 00:00:00 2001 From: Pinyao Ting Date: Tue, 20 Jul 2021 00:01:29 +0000 Subject: [PATCH 14/26] Prevend user spoofing in isRequestPinItemSupported This CL ensure the caller process is from the same user when calling ShortcutService#isRequestPinItemSupported. Bug: 191772737 Test: atest ShortcutManagerTest1 ShortcutManagerTest2 ShortcutManagerTest3 ShortcutManagerTest4 ShortcutManagerTest5 ShortcutManagerTest6 ShortcutManagerTest7 ShortcutManagerTest8 ShortcutManagerTest9 ShortcutManagerTest10 ShortcutManagerTest11 ShortcutManagerTest12 Test: atest CtsShortcutManagerTestCases Change-Id: Icab7cdf25b870b88ecfde9b99e107bbeda0eb485 --- .../com/android/server/pm/ShortcutService.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/services/core/java/com/android/server/pm/ShortcutService.java b/services/core/java/com/android/server/pm/ShortcutService.java index c18cdcb89140..f4c812743918 100644 --- a/services/core/java/com/android/server/pm/ShortcutService.java +++ b/services/core/java/com/android/server/pm/ShortcutService.java @@ -1566,6 +1566,19 @@ void injectEnforceCallingPermission( mContext.enforceCallingPermission(permission, message); } + private void verifyCallerUserId(@UserIdInt int userId) { + if (isCallerSystem()) { + return; // no check + } + + final int callingUid = injectBinderCallingUid(); + + // Otherwise, make sure the arguments are valid. + if (UserHandle.getUserId(callingUid) != userId) { + throw new SecurityException("Invalid user-ID"); + } + } + private void verifyCaller(@NonNull String packageName, @UserIdInt int userId) { Preconditions.checkStringNotEmpty(packageName, "packageName"); @@ -2303,6 +2316,8 @@ public void reportShortcutUsed(String packageName, String shortcutId, int userId @Override public boolean isRequestPinItemSupported(int callingUserId, int requestType) { + verifyCallerUserId(callingUserId); + final long token = injectClearCallingIdentity(); try { return mShortcutRequestPinProcessor From 098bec83a9dfa26261976045f8bc3630fd0268cb Mon Sep 17 00:00:00 2001 From: Valentin Iftime Date: Thu, 22 Feb 2024 10:51:58 +0100 Subject: [PATCH 15/26] Check for NLS bind permission when rebinding services Also, after updating packages with NLS components, check the approved services and remove from approved list if missing permissions. Test: atest ManagedServicesTest Bug: 321707289 (cherry picked from commit 24b13a64f9f5e5aa7f45a2132806d6c74e2c62dc) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0c15cdfdd4720efb72c3244a044bb27e2c286c4b) Merged-In: I11901755ec430c6e3145def9d67e4e63cda00806 Change-Id: I11901755ec430c6e3145def9d67e4e63cda00806 --- .../server/notification/ManagedServices.java | 108 ++++++++++++++---- .../notification/ManagedServicesTest.java | 54 +++++++++ 2 files changed, 139 insertions(+), 23 deletions(-) diff --git a/services/core/java/com/android/server/notification/ManagedServices.java b/services/core/java/com/android/server/notification/ManagedServices.java index 4828bbfff676..2d9cfcb3ebb5 100644 --- a/services/core/java/com/android/server/notification/ManagedServices.java +++ b/services/core/java/com/android/server/notification/ManagedServices.java @@ -141,7 +141,9 @@ abstract public class ManagedServices { // List of approved packages or components (by user, then by primary/secondary) that are // allowed to be bound as managed services. A package or component appearing in this list does // not mean that we are currently bound to said package/component. - private ArrayMap>> mApproved = new ArrayMap<>(); + @GuardedBy("mApproved") + protected final ArrayMap>> mApproved = + new ArrayMap<>(); // True if approved services are stored in xml, not settings. private boolean mUseXml; @@ -573,6 +575,23 @@ protected boolean isPackageOrComponentAllowed(String pkgOrComponent, int userId) return false; } + protected boolean isPackageOrComponentAllowedWithPermission(ComponentName component, + int userId) { + if (!(isPackageOrComponentAllowed(component.flattenToString(), userId) + || isPackageOrComponentAllowed(component.getPackageName(), userId))) { + return false; + } + return componentHasBindPermission(component, userId); + } + + private boolean componentHasBindPermission(ComponentName component, int userId) { + ServiceInfo info = getServiceInfo(component, userId); + if (info == null) { + return false; + } + return mConfig.bindPermission.equals(info.permission); + } + protected boolean isPackageAllowed(String pkg, int userId) { if (pkg == null) { return false; @@ -623,6 +642,7 @@ public void onPackagesChanged(boolean removingPackage, String[] pkgList, int[] u for (int uid : uidList) { if (isPackageAllowed(pkgName, UserHandle.getUserId(uid))) { anyServicesInvolved = true; + trimApprovedListsForInvalidServices(pkgName, UserHandle.getUserId(uid)); } } } @@ -749,8 +769,7 @@ protected void setComponentState(ComponentName component, boolean enabled) { for (int i = 0; i < userIds.size(); i++) { final int userId = userIds.get(i); if (enabled) { - if (isPackageOrComponentAllowed(component.flattenToString(), userId) - || isPackageOrComponentAllowed(component.getPackageName(), userId)) { + if (isPackageOrComponentAllowedWithPermission(component, userId)) { registerServiceLocked(component, userId); } else { Slog.d(TAG, component + " no longer has permission to be bound"); @@ -889,6 +908,33 @@ private boolean removeUninstalledItemsFromApprovedLists(int uninstalledUserId, S return removed; } + private void trimApprovedListsForInvalidServices(String packageName, int userId) { + synchronized (mApproved) { + final ArrayMap> approvedByType = mApproved.get(userId); + if (approvedByType == null) { + return; + } + for (int i = 0; i < approvedByType.size(); i++) { + final ArraySet approved = approvedByType.valueAt(i); + for (int j = approved.size() - 1; j >= 0; j--) { + final String approvedPackageOrComponent = approved.valueAt(j); + if (TextUtils.equals(getPackageName(approvedPackageOrComponent), packageName)) { + final ComponentName component = ComponentName.unflattenFromString( + approvedPackageOrComponent); + if (component != null && !componentHasBindPermission(component, userId)) { + approved.removeAt(j); + if (DEBUG) { + Slog.v(TAG, "Removing " + approvedPackageOrComponent + + " from approved list; no bind permission found " + + mConfig.bindPermission); + } + } + } + } + } + } + } + protected String getPackageName(String packageOrComponent) { final ComponentName component = ComponentName.unflattenFromString(packageOrComponent); if (component != null) { @@ -1048,26 +1094,20 @@ private void bindToServices(SparseArray> componentsToBind) { final int userId = componentsToBind.keyAt(i); final Set add = componentsToBind.get(userId); for (ComponentName component : add) { - try { - ServiceInfo info = mPm.getServiceInfo(component, - PackageManager.MATCH_DIRECT_BOOT_AWARE - | PackageManager.MATCH_DIRECT_BOOT_UNAWARE, userId); - if (info == null) { - Slog.w(TAG, "Not binding " + getCaption() + " service " + component - + ": service not found"); - continue; - } - if (!mConfig.bindPermission.equals(info.permission)) { - Slog.w(TAG, "Not binding " + getCaption() + " service " + component - + ": it does not require the permission " + mConfig.bindPermission); - continue; - } - Slog.v(TAG, - "enabling " + getCaption() + " for " + userId + ": " + component); - registerService(component, userId); - } catch (RemoteException e) { - e.rethrowFromSystemServer(); + ServiceInfo info = getServiceInfo(component, userId); + if (info == null) { + Slog.w(TAG, "Not binding " + getCaption() + " service " + component + + ": service not found"); + continue; } + if (!mConfig.bindPermission.equals(info.permission)) { + Slog.w(TAG, "Not binding " + getCaption() + " service " + component + + ": it does not require the permission " + mConfig.bindPermission); + continue; + } + Slog.v(TAG, + "enabling " + getCaption() + " for " + userId + ": " + component); + registerService(component, userId); } } } @@ -1081,6 +1121,15 @@ private void registerService(final ComponentName name, final int userid) { } } + @VisibleForTesting + void reregisterService(final ComponentName cn, final int userId) { + // If rebinding a package that died, ensure it still has permission + // after the rebind delay + if (isPackageOrComponentAllowedWithPermission(cn, userId)) { + registerService(cn, userId); + } + } + /** * Inject a system service into the management list. */ @@ -1181,7 +1230,7 @@ public void onBindingDied(ComponentName name) { mHandler.postDelayed(new Runnable() { @Override public void run() { - registerService(name, userid); + reregisterService(name, userid); } }, ON_BINDING_DIED_REBIND_DELAY_MS); } else { @@ -1313,6 +1362,19 @@ private void unbindService(ServiceConnection connection, ComponentName component } } + private ServiceInfo getServiceInfo(ComponentName component, int userId) { + try { + return mPm.getServiceInfo(component, + PackageManager.GET_META_DATA + | PackageManager.MATCH_DIRECT_BOOT_AWARE + | PackageManager.MATCH_DIRECT_BOOT_UNAWARE, + userId); + } catch (RemoteException e) { + e.rethrowFromSystemServer(); + } + return null; + } + public class ManagedServiceInfo implements IBinder.DeathRecipient { public IInterface service; public ComponentName component; diff --git a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java index 8aaf29a11033..cac620f409f3 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/ManagedServicesTest.java @@ -28,8 +28,10 @@ import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -624,6 +626,58 @@ public void testUpgradeAppBindsNewServices() throws Exception { } } + @Test + public void testUpgradeAppNoPermissionNoRebind() throws Exception { + Context context = spy(getContext()); + doReturn(true).when(context).bindServiceAsUser(any(), any(), anyInt(), any()); + + ManagedServices service = new TestManagedServices(context, mLock, mUserProfiles, + mIpm, + APPROVAL_BY_COMPONENT); + + List packages = new ArrayList<>(); + packages.add("package"); + addExpectedServices(service, packages, 0); + + final ComponentName unapprovedComponent = ComponentName.unflattenFromString("package/C1"); + final ComponentName approvedComponent = ComponentName.unflattenFromString("package/C2"); + + // Both components are approved initially + mExpectedPrimaryComponentNames.clear(); + mExpectedPrimaryPackages.clear(); + mExpectedPrimaryComponentNames.put(0, "package/C1:package/C2"); + mExpectedSecondaryComponentNames.clear(); + mExpectedSecondaryPackages.clear(); + + loadXml(service); + + //Component package/C1 loses bind permission + when(mIpm.getServiceInfo(any(), anyInt(), anyInt())).thenAnswer( + (Answer) invocation -> { + ComponentName invocationCn = invocation.getArgument(0); + if (invocationCn != null) { + ServiceInfo serviceInfo = new ServiceInfo(); + serviceInfo.packageName = invocationCn.getPackageName(); + serviceInfo.name = invocationCn.getClassName(); + if (invocationCn.equals(unapprovedComponent)) { + serviceInfo.permission = "none"; + } else { + serviceInfo.permission = service.getConfig().bindPermission; + } + serviceInfo.metaData = null; + return serviceInfo; + } + return null; + } + ); + + // Trigger package update + service.onPackagesChanged(false, new String[]{"package"}, new int[]{0}); + + assertFalse(service.isComponentEnabledForCurrentProfiles(unapprovedComponent)); + assertTrue(service.isComponentEnabledForCurrentProfiles(approvedComponent)); + } + @Test public void testSetPackageOrComponentEnabled() throws Exception { for (int approvalLevel : new int[] {APPROVAL_BY_COMPONENT, APPROVAL_BY_PACKAGE}) { From 66f73bd990fb337b4ffc307509de86cd04dfb39d Mon Sep 17 00:00:00 2001 From: Yi-an Chen Date: Tue, 20 Feb 2024 04:34:57 +0000 Subject: [PATCH 16/26] Fix error handling for non-dynamic permissions We only allow removing dynamic permissions. When removePermission() is called for a non-dynamic permission, in addition to logging it, we should also return early to avoid the removePermission() call. Test: manual Bug: 321555066 Fixes: 321711213 (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:2b5d63b64b2b8208ccc4f62eac3d8962f981dbf8) Merged-In: I7336f2fc78804f26e4b2a329870ecdea776595d8 Change-Id: I7336f2fc78804f26e4b2a329870ecdea776595d8 --- .../android/server/pm/permission/PermissionManagerService.java | 1 + 1 file changed, 1 insertion(+) 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 f163969b3005..989eba9ed99b 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -1015,6 +1015,7 @@ private void removeDynamicPermission( // TODO: switch this back to SecurityException Slog.wtf(TAG, "Not allowed to modify non-dynamic permission " + permName); + return; } mSettings.removePermissionLocked(permName); if (callback != null) { From 9185f42b915f4b3b9dc6a297bf50f4597d9aaa20 Mon Sep 17 00:00:00 2001 From: Dmitry Dementyev Date: Tue, 26 Mar 2024 10:31:44 -0700 Subject: [PATCH 17/26] Add more checkKeyIntent checks to AccountManagerService. Another verification is needed after Bundle modification. Bug: 321941232 Test: manual (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:36db8a1d61a881f89fdd3911886adcda6e1f0d7f) Merged-In: I9e45d758a2320328da5664b6341eafe6f285f297 Change-Id: I9e45d758a2320328da5664b6341eafe6f285f297 --- .../android/server/accounts/AccountManagerService.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java index f81dbd3c4483..75ee995f7c74 100644 --- a/services/core/java/com/android/server/accounts/AccountManagerService.java +++ b/services/core/java/com/android/server/accounts/AccountManagerService.java @@ -3486,6 +3486,11 @@ public void onResult(Bundle result) { // Strip auth token from result. result.remove(AccountManager.KEY_AUTHTOKEN); + if (!checkKeyIntent(Binder.getCallingUid(), result)) { + onError(AccountManager.ERROR_CODE_INVALID_RESPONSE, + "invalid intent in bundle returned"); + return; + } if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.v(TAG, @@ -5070,6 +5075,11 @@ public void onResult(Bundle result) { } else { if (mStripAuthTokenFromResult) { result.remove(AccountManager.KEY_AUTHTOKEN); + if (!checkKeyIntent(Binder.getCallingUid(), result)) { + onError(AccountManager.ERROR_CODE_INVALID_RESPONSE, + "invalid intent in bundle returned"); + return; + } } if (Log.isLoggable(TAG, Log.VERBOSE)) { Log.v(TAG, getClass().getSimpleName() From 8706bda4c2700064e85b3d35275fc8776feb59f1 Mon Sep 17 00:00:00 2001 From: Haoran Zhang Date: Wed, 13 Mar 2024 17:08:00 +0000 Subject: [PATCH 18/26] [DO NOT MERGE][Autofill Framework] Add in check for intent filter when setting/updating service For test, I registered two tests around on ABTD. CtsAutoFillServiceTestCases module is passing except three known failures: Test run link: - https://android-build.corp.google.com/builds/abtd/run/L33300030002610600 - https://android-build.corp.google.com/builds/abtd/run/L58100030002616607 Bug: b/324874908 Test: atest CtsAutoFillServiceTestCases (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:141d9d050346bfc4673c429382deb1b3d210f6ad) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:51d64705ab70788a536c26d4df5e63f0952ec98f) Merged-In: I51c2e3788ac29ff4d6b86aa2a735ff2ea1463a77 Change-Id: I51c2e3788ac29ff4d6b86aa2a735ff2ea1463a77 --- .../autofill/AutofillManagerServiceImpl.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java index 1bd5201f5b26..58a1064682d3 100644 --- a/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java +++ b/services/autofill/java/com/android/server/autofill/AutofillManagerServiceImpl.java @@ -32,8 +32,10 @@ import android.app.ActivityTaskManager; import android.app.IActivityTaskManager; import android.content.ComponentName; +import android.content.Intent; import android.content.pm.PackageManager; import android.content.pm.PackageManager.NameNotFoundException; +import android.content.pm.ResolveInfo; import android.content.pm.ServiceInfo; import android.graphics.Rect; import android.metrics.LogMaker; @@ -214,6 +216,31 @@ protected boolean updateLocked(boolean disabled) { @Override // from PerUserSystemService protected ServiceInfo newServiceInfoLocked(@NonNull ComponentName serviceComponent) throws NameNotFoundException { + final List resolveInfos = + getContext().getPackageManager().queryIntentServicesAsUser( + new Intent(AutofillService.SERVICE_INTERFACE), + // The MATCH_INSTANT flag is added because curret autofill CTS module is + // defined in one apk, which makes the test autofill service installed in a + // instant app when the CTS tests are running in instant app mode. + // TODO: Remove MATCH_INSTANT flag after completing refactoring the CTS module + // to make the test autofill service a separate apk. + PackageManager.GET_META_DATA | PackageManager.MATCH_INSTANT, + mUserId); + boolean serviceHasAutofillIntentFilter = false; + for (ResolveInfo resolveInfo : resolveInfos) { + final ServiceInfo serviceInfo = resolveInfo.serviceInfo; + if (serviceInfo.getComponentName().equals(serviceComponent)) { + serviceHasAutofillIntentFilter = true; + break; + } + } + if (!serviceHasAutofillIntentFilter) { + Slog.w(TAG, + "Autofill service from '" + serviceComponent.getPackageName() + "' does" + + "not have intent filter " + AutofillService.SERVICE_INTERFACE); + throw new SecurityException("Service does not declare intent filter " + + AutofillService.SERVICE_INTERFACE); + } mInfo = new AutofillServiceInfo(getContext(), serviceComponent, mUserId); return mInfo.getServiceInfo(); } From 0a1707a985c7375f298035ab80442ec5ebb0c306 Mon Sep 17 00:00:00 2001 From: Hans Boehm Date: Tue, 2 Jan 2024 16:53:13 -0800 Subject: [PATCH 19/26] Check hidden API exemptions Refuse to deal with newlines and null characters in HiddenApiSettings.update(). Also disallow nulls in process start arguments. Bug: 316153291 Test: Treehugger for now (cherry picked from commit 7ba059e2cf0a2c20f9a849719cdc32b12c933a44) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:60669aa49aba34c0950d6246bd95b54f91a3c8e8) Merged-In: I83cd60e46407a4a082f9f3c80e937dbd522dbac4 Change-Id: I83cd60e46407a4a082f9f3c80e937dbd522dbac4 --- core/java/android/os/ZygoteProcess.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/core/java/android/os/ZygoteProcess.java b/core/java/android/os/ZygoteProcess.java index c923f8e7b5ff..0fafe2c7e23d 100644 --- a/core/java/android/os/ZygoteProcess.java +++ b/core/java/android/os/ZygoteProcess.java @@ -411,6 +411,8 @@ private Process.ProcessStartResult zygoteSendArgsAndGetResult( throw new ZygoteStartFailedEx("Embedded newlines not allowed"); } else if (arg.indexOf('\r') >= 0) { throw new ZygoteStartFailedEx("Embedded carriage returns not allowed"); + } else if (arg.indexOf('\u0000') >= 0) { + throw new ZygoteStartFailedEx("Embedded nulls not allowed"); } } @@ -869,6 +871,14 @@ private boolean maybeSetApiBlacklistExemptions(ZygoteState state, boolean sendIf return true; } + for (/* NonNull */ String s : mApiBlacklistExemptions) { + // indexOf() is intrinsified and faster than contains(). + if (s.indexOf('\n') >= 0 || s.indexOf('\r') >= 0 || s.indexOf('\u0000') >= 0) { + Slog.e(LOG_TAG, "Failed to set API denylist exemptions: Bad character"); + mApiBlacklistExemptions = Collections.emptyList(); + return false; + } + } try { state.mZygoteOutputWriter.write(Integer.toString(mApiBlacklistExemptions.size() + 1)); state.mZygoteOutputWriter.newLine(); From 79f4cac5b77977d1e5ca2b09162bc4bd20999e42 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Mon, 13 Apr 2020 11:03:44 -0400 Subject: [PATCH 20/26] Add StatusBarNotification::getNormalizedUserId Required for ASB 2024-06 Cherry-picked from I9b2ae1ecd1cc8b42ab715ee033879f295949a9ba Change-Id: Ife602cee53c303dd3f841004d8ffc84b38c7677b --- .../service/notification/StatusBarNotification.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/core/java/android/service/notification/StatusBarNotification.java b/core/java/android/service/notification/StatusBarNotification.java index 905c7811e457..39395074b916 100644 --- a/core/java/android/service/notification/StatusBarNotification.java +++ b/core/java/android/service/notification/StatusBarNotification.java @@ -273,6 +273,18 @@ public int getUserId() { return this.user.getIdentifier(); } + /** + * Like {@link #getUserId()} but handles special users. + * @hide + */ + public int getNormalizedUserId() { + int userId = getUserId(); + if (userId == UserHandle.USER_ALL) { + userId = UserHandle.USER_SYSTEM; + } + return userId; + } + /** The package that the notification belongs to. */ public String getPackageName() { return pkg; From 73f6c1eee14de4f3836be5a279b512e762a20857 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Wed, 9 Oct 2019 15:33:11 -0700 Subject: [PATCH 21/26] Add Context.createContextAsUser() Without it, apps (mainline modules) will need to use createPackageContext..., which is a bit painful. Bug: 142472686 Test: atest android.content.cts.ContextTest#testCreateContextAsUser Change-Id: Id640e03862462724df1a4a3101f0b08faafba22f --- api/system-current.txt | 3 ++- api/test-current.txt | 3 ++- core/java/android/app/ContextImpl.java | 9 +++++++++ core/java/android/content/Context.java | 20 ++++++++++++++++++- core/java/android/content/ContextWrapper.java | 6 ++++++ .../src/android/test/mock/MockContext.java | 6 ++++++ 6 files changed, 44 insertions(+), 3 deletions(-) diff --git a/api/system-current.txt b/api/system-current.txt index d3785a68c692..82aa4e91fd5c 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -1346,8 +1346,9 @@ package android.content { public abstract class Context { method @RequiresPermission(android.Manifest.permission.INTERACT_ACROSS_USERS) public boolean bindServiceAsUser(@RequiresPermission android.content.Intent, android.content.ServiceConnection, int, android.os.UserHandle); + method @NonNull public android.content.Context createContextAsUser(@NonNull android.os.UserHandle); method public abstract android.content.Context createCredentialProtectedStorageContext(); - method public android.content.Context createPackageContextAsUser(String, int, android.os.UserHandle) throws android.content.pm.PackageManager.NameNotFoundException; + method @NonNull public android.content.Context createPackageContextAsUser(@NonNull String, int, @NonNull android.os.UserHandle) throws android.content.pm.PackageManager.NameNotFoundException; method @Nullable public abstract java.io.File getPreloadsFileCache(); method public abstract boolean isCredentialProtectedStorage(); method public abstract void sendBroadcast(android.content.Intent, @Nullable String, @Nullable android.os.Bundle); diff --git a/api/test-current.txt b/api/test-current.txt index d3bea18fb944..5f53b2a40a49 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -638,7 +638,8 @@ package android.content { } public abstract class Context { - method public android.content.Context createPackageContextAsUser(String, int, android.os.UserHandle) throws android.content.pm.PackageManager.NameNotFoundException; + method @NonNull public android.content.Context createContextAsUser(@NonNull android.os.UserHandle); + method @NonNull public android.content.Context createPackageContextAsUser(@NonNull String, int, @NonNull android.os.UserHandle) throws android.content.pm.PackageManager.NameNotFoundException; method public abstract android.view.Display getDisplay(); method public abstract int getDisplayId(); method public android.os.UserHandle getUser(); diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java index 41a4fba0434c..1f3c3a46792a 100644 --- a/core/java/android/app/ContextImpl.java +++ b/core/java/android/app/ContextImpl.java @@ -2200,6 +2200,15 @@ public Context createPackageContextAsUser(String packageName, int flags, UserHan "Application package " + packageName + " not found"); } + @Override + public Context createContextAsUser(UserHandle user) { + try { + return createPackageContextAsUser(getPackageName(), mFlags, user); + } catch (NameNotFoundException e) { + throw new IllegalStateException("Own package not found: package=" + getPackageName()); + } + } + @Override public Context createContextForSplit(String splitName) throws NameNotFoundException { if (!mPackageInfo.getApplicationInfo().requestsIsolatedSplitLoading()) { diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index d8235cf8734a..e5dcb5dc423b 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -5232,8 +5232,9 @@ public abstract Context createPackageContext(String packageName, */ @SystemApi @TestApi + @NonNull public Context createPackageContextAsUser( - String packageName, @CreatePackageOptions int flags, UserHandle user) + @NonNull String packageName, @CreatePackageOptions int flags, @NonNull UserHandle user) throws PackageManager.NameNotFoundException { if (Build.IS_ENG) { throw new IllegalStateException("createPackageContextAsUser not overridden!"); @@ -5241,6 +5242,23 @@ public Context createPackageContextAsUser( return this; } + /** + * Similar to {@link #createPackageContext(String, int)}, but for the own package with a + * different {@link UserHandle}. For example, {@link #getContentResolver()} + * will open any {@link Uri} as the given user. + * + * @hide + */ + @SystemApi + @TestApi + @NonNull + public Context createContextAsUser(@NonNull UserHandle user) { + if (Build.IS_ENG) { + throw new IllegalStateException("createContextAsUser not overridden!"); + } + return this; + } + /** * Creates a context given an {@link android.content.pm.ApplicationInfo}. * diff --git a/core/java/android/content/ContextWrapper.java b/core/java/android/content/ContextWrapper.java index 0859f97e81a1..f7cd51e7ffbc 100644 --- a/core/java/android/content/ContextWrapper.java +++ b/core/java/android/content/ContextWrapper.java @@ -883,6 +883,12 @@ public Context createPackageContextAsUser(String packageName, int flags, UserHan return mBase.createPackageContextAsUser(packageName, flags, user); } + /** @hide */ + @Override + public Context createContextAsUser(UserHandle user) { + return mBase.createContextAsUser(user); + } + /** @hide */ @Override @UnsupportedAppUsage diff --git a/test-mock/src/android/test/mock/MockContext.java b/test-mock/src/android/test/mock/MockContext.java index a95b6f11e98a..fcd4701c7630 100644 --- a/test-mock/src/android/test/mock/MockContext.java +++ b/test-mock/src/android/test/mock/MockContext.java @@ -756,6 +756,12 @@ public Context createPackageContextAsUser(String packageName, int flags, UserHan throw new UnsupportedOperationException(); } + /** {@hide} */ + @Override + public Context createContextAsUser(UserHandle user) { + throw new UnsupportedOperationException(); + } + /** {@hide} */ @Override public int getUserId() { From 2eb6cfbd1a82c8a40e848f2d2fa65ed4bcb28872 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Fri, 11 Oct 2019 20:19:58 -0700 Subject: [PATCH 22/26] Explicitly take flags in createContextAsUser() Bug: 142472686 Test: atest android.content.cts.ContextTest#testCreateContextAsUser Change-Id: Id2e3d5ffe5887a4916e0872a7e85d62cbb439744 --- api/system-current.txt | 2 +- api/test-current.txt | 2 +- core/java/android/app/ContextImpl.java | 4 ++-- core/java/android/content/Context.java | 2 +- core/java/android/content/ContextWrapper.java | 4 ++-- test-mock/src/android/test/mock/MockContext.java | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/api/system-current.txt b/api/system-current.txt index 82aa4e91fd5c..a33e927f49bc 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -1346,7 +1346,7 @@ package android.content { public abstract class Context { method @RequiresPermission(android.Manifest.permission.INTERACT_ACROSS_USERS) public boolean bindServiceAsUser(@RequiresPermission android.content.Intent, android.content.ServiceConnection, int, android.os.UserHandle); - method @NonNull public android.content.Context createContextAsUser(@NonNull android.os.UserHandle); + method @NonNull public android.content.Context createContextAsUser(@NonNull android.os.UserHandle, int); method public abstract android.content.Context createCredentialProtectedStorageContext(); method @NonNull public android.content.Context createPackageContextAsUser(@NonNull String, int, @NonNull android.os.UserHandle) throws android.content.pm.PackageManager.NameNotFoundException; method @Nullable public abstract java.io.File getPreloadsFileCache(); diff --git a/api/test-current.txt b/api/test-current.txt index 5f53b2a40a49..29c580da327f 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -638,7 +638,7 @@ package android.content { } public abstract class Context { - method @NonNull public android.content.Context createContextAsUser(@NonNull android.os.UserHandle); + method @NonNull public android.content.Context createContextAsUser(@NonNull android.os.UserHandle, int); method @NonNull public android.content.Context createPackageContextAsUser(@NonNull String, int, @NonNull android.os.UserHandle) throws android.content.pm.PackageManager.NameNotFoundException; method public abstract android.view.Display getDisplay(); method public abstract int getDisplayId(); diff --git a/core/java/android/app/ContextImpl.java b/core/java/android/app/ContextImpl.java index 1f3c3a46792a..9c46b23d8df8 100644 --- a/core/java/android/app/ContextImpl.java +++ b/core/java/android/app/ContextImpl.java @@ -2201,9 +2201,9 @@ public Context createPackageContextAsUser(String packageName, int flags, UserHan } @Override - public Context createContextAsUser(UserHandle user) { + public Context createContextAsUser(UserHandle user, @CreatePackageOptions int flags) { try { - return createPackageContextAsUser(getPackageName(), mFlags, user); + return createPackageContextAsUser(getPackageName(), flags, user); } catch (NameNotFoundException e) { throw new IllegalStateException("Own package not found: package=" + getPackageName()); } diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index e5dcb5dc423b..6e0e41b110d6 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -5252,7 +5252,7 @@ public Context createPackageContextAsUser( @SystemApi @TestApi @NonNull - public Context createContextAsUser(@NonNull UserHandle user) { + public Context createContextAsUser(@NonNull UserHandle user, @CreatePackageOptions int flags) { if (Build.IS_ENG) { throw new IllegalStateException("createContextAsUser not overridden!"); } diff --git a/core/java/android/content/ContextWrapper.java b/core/java/android/content/ContextWrapper.java index f7cd51e7ffbc..7993ea192424 100644 --- a/core/java/android/content/ContextWrapper.java +++ b/core/java/android/content/ContextWrapper.java @@ -885,8 +885,8 @@ public Context createPackageContextAsUser(String packageName, int flags, UserHan /** @hide */ @Override - public Context createContextAsUser(UserHandle user) { - return mBase.createContextAsUser(user); + public Context createContextAsUser(UserHandle user, @CreatePackageOptions int flags) { + return mBase.createContextAsUser(user, flags); } /** @hide */ diff --git a/test-mock/src/android/test/mock/MockContext.java b/test-mock/src/android/test/mock/MockContext.java index fcd4701c7630..5053ceedc703 100644 --- a/test-mock/src/android/test/mock/MockContext.java +++ b/test-mock/src/android/test/mock/MockContext.java @@ -758,7 +758,7 @@ public Context createPackageContextAsUser(String packageName, int flags, UserHan /** {@hide} */ @Override - public Context createContextAsUser(UserHandle user) { + public Context createContextAsUser(UserHandle user, @CreatePackageOptions int flags) { throw new UnsupportedOperationException(); } From 98bb229f741e3ad694b907161a98bc9d87518a6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Hern=C3=A1ndez?= Date: Fri, 22 Mar 2024 14:26:23 +0100 Subject: [PATCH 23/26] Resolve message/conversation image Uris with the correct user id Bug: 317503801 Test: atest ExpandableNotificationRowTest (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3b913c4237993525d2435a2d1082c6af8997168d) Merged-In: I11c5b39f2d9d8f0788acab43640a6d4abcd5a179 Change-Id: I11c5b39f2d9d8f0788acab43640a6d4abcd5a179 --- .../row/ExpandableNotificationRow.java | 13 +++++++-- .../row/NotificationInlineImageResolver.java | 8 +++++- .../systemui/SysuiTestableContext.java | 23 ++++++++++++++++ .../row/ExpandableNotificationRowTest.java | 27 +++++++++++++++++++ 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java index 91c3bcb7225b..a781da06aef8 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRow.java @@ -47,6 +47,7 @@ import android.os.Build; import android.os.Bundle; import android.os.SystemClock; +import android.os.UserHandle; import android.service.notification.StatusBarNotification; import android.util.ArraySet; import android.util.AttributeSet; @@ -454,6 +455,8 @@ private void setIconRunning(ImageView imageView, boolean running) { public void setEntry(@NonNull NotificationEntry entry) { mEntry = entry; mStatusBarNotification = entry.notification; + mImageResolver = new NotificationInlineImageResolver(userContextForEntry(mContext, entry), + new NotificationInlineImageCache()); if (mStatusBarNotification != null) { updateAlarmOrCall(); AppLockManager appLockManager = (AppLockManager) mContext @@ -1665,8 +1668,6 @@ public ExpandableNotificationRow(Context context, AttributeSet attrs) { mFalsingManager = Dependency.get(FalsingManager.class); // TODO: inject into a controller. mNotificationInflater = new NotificationContentInflater(this); mMenuRow = new NotificationMenuRow(mContext); - mImageResolver = new NotificationInlineImageResolver(context, - new NotificationInlineImageCache()); mMediaManager = Dependency.get(NotificationMediaManager.class); initDimens(); } @@ -1679,6 +1680,14 @@ public void setStatusBarStateController(StatusBarStateController statusBarStateC mStatusbarStateController = statusBarStateController; } + private static Context userContextForEntry(Context base, NotificationEntry entry) { + if (base.getUserId() == entry.notification.getNormalizedUserId()) { + return base; + } + return base.createContextAsUser( + UserHandle.of(entry.notification.getNormalizedUserId()), /* flags= */ 0); + } + private void initDimens() { mNotificationMinHeightBeforeN = NotificationUtils.getFontScaledHeight(mContext, R.dimen.notification_min_height_legacy); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationInlineImageResolver.java b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationInlineImageResolver.java index 466be072afdb..885b28aebc89 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationInlineImageResolver.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/notification/row/NotificationInlineImageResolver.java @@ -26,6 +26,7 @@ import android.os.SystemClock; import android.util.Log; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.widget.ImageResolver; import com.android.internal.widget.LocalImageResolver; import com.android.internal.widget.MessagingMessage; @@ -54,7 +55,7 @@ public class NotificationInlineImageResolver implements ImageResolver { * @param imageCache The implementation of internal cache. */ public NotificationInlineImageResolver(Context context, ImageCache imageCache) { - mContext = context.getApplicationContext(); + mContext = context; mImageCache = imageCache; if (mImageCache != null) { @@ -62,6 +63,11 @@ public NotificationInlineImageResolver(Context context, ImageCache imageCache) { } } + @VisibleForTesting + public Context getContext() { + return mContext; + } + /** * Check if this resolver has its internal cache implementation. * @return True if has its internal cache, false otherwise. diff --git a/packages/SystemUI/tests/src/com/android/systemui/SysuiTestableContext.java b/packages/SystemUI/tests/src/com/android/systemui/SysuiTestableContext.java index f792d7d11e15..6324569411ed 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/SysuiTestableContext.java +++ b/packages/SystemUI/tests/src/com/android/systemui/SysuiTestableContext.java @@ -14,15 +14,20 @@ package com.android.systemui; +import android.annotation.NonNull; import android.content.Context; import android.testing.LeakCheck; import android.testing.TestableContext; import android.util.ArrayMap; import android.view.Display; +import java.util.HashMap; +import java.util.Map; + public class SysuiTestableContext extends TestableContext implements SysUiServiceProvider { private ArrayMap, Object> mComponents; + private final Map mContextForUser = new HashMap<>(); public SysuiTestableContext(Context base) { super(base); @@ -59,4 +64,22 @@ public Context createDisplayContext(Display display) { new SysuiTestableContext(getBaseContext().createDisplayContext(display)); return context; } + + /** + * Sets a Context object that will be returned as the result of {@link #createContextAsUser} + * for a specific {@code user}. + */ + public void prepareCreateContextAsUser(UserHandle user, Context context) { + mContextForUser.put(user, context); + } + + @Override + @NonNull + public Context createContextAsUser(UserHandle user, int flags) { + Context userContext = mContextForUser.get(user); + if (userContext != null) { + return userContext; + } + return super.createContextAsUser(user, flags); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java index d526d104630e..c9b29fe7e490 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/notification/row/ExpandableNotificationRowTest.java @@ -21,6 +21,10 @@ import static com.android.systemui.statusbar.notification.row.NotificationContentInflater.FLAG_CONTENT_VIEW_ALL; import static com.android.systemui.statusbar.notification.row.NotificationContentInflater.FLAG_CONTENT_VIEW_HEADS_UP; import static com.android.systemui.statusbar.notification.row.NotificationContentInflater.FLAG_CONTENT_VIEW_PUBLIC; +import static com.android.systemui.statusbar.notification.row.NotificationTestHelper.PKG; +import static com.android.systemui.statusbar.notification.row.NotificationTestHelper.USER_HANDLE; + +import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -38,6 +42,8 @@ import android.app.AppOpsManager; import android.app.NotificationChannel; +import android.content.Context; +import android.os.UserHandle; import android.testing.AndroidTestingRunner; import android.testing.TestableLooper; import android.testing.TestableLooper.RunWithLooper; @@ -48,6 +54,7 @@ import androidx.test.filters.SmallTest; import com.android.systemui.SysuiTestCase; +import com.android.systemui.SysuiTestableContext; import com.android.systemui.plugins.statusbar.NotificationMenuRowPlugin; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.NotificationTestHelper; @@ -377,4 +384,24 @@ public void testGetIsNonblockable_criticalDeviceFunction() throws Exception { assertTrue(row.getIsNonblockable()); } + + @Test + public void imageResolver_sameNotificationUser_usesContext() throws Exception { + ExpandableNotificationRow row = mNotificationTestHelper.createRow(PKG, + USER_HANDLE.getUid(1234), USER_HANDLE); + + assertThat(row.getImageResolver().getContext()).isSameInstanceAs(mContext); + } + + @Test + public void imageResolver_differentNotificationUser_createsUserContext() throws Exception { + UserHandle user = new UserHandle(33); + Context userContext = new SysuiTestableContext(mContext); + mContext.prepareCreateContextAsUser(user, userContext); + + ExpandableNotificationRow row = mNotificationTestHelper.createRow(PKG, + user.getUid(1234), user); + + assertThat(row.getImageResolver().getContext()).isSameInstanceAs(userContext); + } } From 12c39911672575009ac0d77fab5229ec8ce5bdad Mon Sep 17 00:00:00 2001 From: Ameer Armaly Date: Fri, 8 Mar 2024 19:41:06 +0000 Subject: [PATCH 24/26] [RESTRICT AUTOMERGE] AccessibilityManagerService: remove uninstalled services from enabled list after service update. Bug: 326485767 Test: atest AccessibilityEndToEndTest#testUpdateServiceWithoutIntent_disablesService (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:5405514a23edcba0cf30e6ec78189e3f4e7d95cf) Merged-In: I5e59296fcad68e62b34c74ee5fd80b6ad6b46fa1 Change-Id: I5e59296fcad68e62b34c74ee5fd80b6ad6b46fa1 --- .../AccessibilityManagerService.java | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java index 194c90e125f9..dbc9d42a4adc 100644 --- a/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java +++ b/services/accessibility/java/com/android/server/accessibility/AccessibilityManagerService.java @@ -1623,10 +1623,13 @@ private void updateServicesLocked(UserState userState) { boolean isUnlockingOrUnlocked = LocalServices.getService(UserManagerInternal.class) .isUserUnlockingOrUnlocked(userState.mUserId); + // Store the list of installed services. + mTempComponentNameSet.clear(); for (int i = 0, count = userState.mInstalledServices.size(); i < count; i++) { AccessibilityServiceInfo installedService = userState.mInstalledServices.get(i); ComponentName componentName = ComponentName.unflattenFromString( installedService.getId()); + mTempComponentNameSet.add(componentName); AccessibilityServiceConnection service = componentNameToServiceMap.get(componentName); @@ -1673,6 +1676,25 @@ private void updateServicesLocked(UserState userState) { if (audioManager != null) { audioManager.setAccessibilityServiceUids(mTempIntArray); } + // If any services have been removed, remove them from the enabled list and the touch + // exploration granted list. + boolean anyServiceRemoved = + userState.mEnabledServices.removeIf((comp) -> !mTempComponentNameSet.contains(comp)) + || userState.mTouchExplorationGrantedServices.removeIf( + (comp) -> !mTempComponentNameSet.contains(comp)); + if (anyServiceRemoved) { + // Update the enabled services setting. + persistComponentNamesToSettingLocked( + Settings.Secure.ENABLED_ACCESSIBILITY_SERVICES, + userState.mEnabledServices, + userState.mUserId); + // Update the touch exploration granted services setting. + persistComponentNamesToSettingLocked( + Settings.Secure.TOUCH_EXPLORATION_GRANTED_ACCESSIBILITY_SERVICES, + userState.mTouchExplorationGrantedServices, + userState.mUserId); + } + mTempComponentNameSet.clear(); updateAccessibilityEnabledSetting(userState); } From d54b4900a54de6308c508b70ace0198e0d4737c8 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Thu, 29 Feb 2024 12:03:05 +0000 Subject: [PATCH 25/26] [BACKPORT] Verify UID of incoming Zygote connections. Only the system UID should be allowed to connect to the Zygote. While for generic Zygotes this is also covered by SELinux policy, this is not true for App Zygotes: the preload code running in an app zygote could connect to another app zygote socket, if it had access to its (random) socket address. On the Java layer, simply check the UID when the connection is made. In the native layer, this check was already present, but it actually didn't work in the case where we receive a new incoming connection on the socket, and receive a 'non-fork' command: in that case, we will simply exit the native loop, and let the Java layer handle the command, without any further UID checking. Modified the native logic to drop new connections with a mismatching UID, and to keep serving the existing connection (if it was still there). [Backport: No native layer for ZygoteCommandBuffer present] Bug: 319081336 Test: manual (cherry picked from commit 2ffc7cb220e4220b7e108c4043a3f0f2a85b6508) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:e397fd3d20c3f409311e411387ec1524ccecf085) Merged-In: I3f85a17107849e2cd3e82d6ef15c90b9e2f26532 Change-Id: I3f85a17107849e2cd3e82d6ef15c90b9e2f26532 --- core/java/com/android/internal/os/ZygoteConnection.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/java/com/android/internal/os/ZygoteConnection.java b/core/java/com/android/internal/os/ZygoteConnection.java index 52d0adba0a05..fe2ff54194fb 100644 --- a/core/java/com/android/internal/os/ZygoteConnection.java +++ b/core/java/com/android/internal/os/ZygoteConnection.java @@ -106,6 +106,9 @@ class ZygoteConnection { throw ex; } + if (peer.getUid() != Process.SYSTEM_UID) { + throw new ZygoteSecurityException("Only system UID is allowed to connect to Zygote."); + } isEof = false; } From 72f173fed98efabee3c3e62071aa1c5ce5e2fb41 Mon Sep 17 00:00:00 2001 From: Yi-an Chen Date: Tue, 23 Apr 2024 21:53:02 +0000 Subject: [PATCH 26/26] Fix security vulnerability of non-dynamic permission removal The original removePermission() code in PermissionManagerService missed a logical negation operator when handling non-dynamic permissions, causing both testPermissionPermission_nonDynamicPermission_permissionUnchanged and testRemovePermission_dynamicPermission_permissionRemoved tests in DynamicPermissionsTest to fail. The corresponding test DynamicPermissionsTest is also updated in the other CL: ag/27073864 Bug: 321711213 Test: DynamicPermissionsTest on sc-dev and tm-dev locally (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:35d77a77feef62dc108f6478cb9228cc6044f70d) Merged-In: Id573b75cdcfce3a1df5731ffb00c4228c513e686 Change-Id: Id573b75cdcfce3a1df5731ffb00c4228c513e686 --- .../android/server/pm/permission/PermissionManagerService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 989eba9ed99b..b47994d5aa22 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -1011,7 +1011,7 @@ private void removeDynamicPermission( if (bp == null) { return; } - if (bp.isDynamic()) { + if (!bp.isDynamic()) { // TODO: switch this back to SecurityException Slog.wtf(TAG, "Not allowed to modify non-dynamic permission " + permName);