From 7d2a43c3a5d23d496e037fc434152973d3d8d7a7 Mon Sep 17 00:00:00 2001 From: Joanne Chung Date: Fri, 7 Jul 2023 10:37:08 +0000 Subject: [PATCH] Restricted by package visibility rule for PackageMonitorCallbackHelper Bug: 289567218 Test: PackageMonitorCallbackHelper --iterations 100 Test: manual. The current registered clients can still receice callbacks, e.g. system, systemui Change-Id: If56201f782bd4dbb76a53121df03c884205b6099 --- .../server/pm/PackageManagerService.java | 13 ++-- .../pm/PackageMonitorCallbackHelper.java | 70 +++++++++++++++---- .../server/pm/SuspendPackageHelper.java | 2 +- .../pm/PackageMonitorCallbackHelperTest.java | 40 +++++++---- 4 files changed, 88 insertions(+), 37 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 3e4830420f8a1..4cdfa5d921a63 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -2998,14 +2998,14 @@ public class PackageManagerService implements PackageSender, TestUtilityService // action. When the targetPkg is set, it sends the broadcast to specific app, e.g. // installer app or null for registered apps. The callback only need to send back to the // registered apps so we check the null condition here. - notifyPackageMonitor(action, pkg, extras, userIds, instantUserIds); + notifyPackageMonitor(action, pkg, extras, userIds, instantUserIds, broadcastAllowList); } } void notifyPackageMonitor(String action, String pkg, Bundle extras, int[] userIds, - int[] instantUserIds) { + int[] instantUserIds, SparseArray broadcastAllowList) { mPackageMonitorCallbackHelper.notifyPackageMonitor(action, pkg, extras, userIds, - instantUserIds); + instantUserIds, broadcastAllowList); } void notifyResourcesChanged(boolean mediaStatus, boolean replacing, @@ -3061,7 +3061,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService mHandler.post(() -> mBroadcastHelper.sendPackageAddedForNewUsers( packageName, appId, userIds, instantUserIds, dataLoaderType, broadcastAllowList)); mPackageMonitorCallbackHelper.notifyPackageAddedForNewUsers(packageName, appId, userIds, - instantUserIds, dataLoaderType); + instantUserIds, dataLoaderType, broadcastAllowList); if (sendBootCompleted && !ArrayUtils.isEmpty(userIds)) { mHandler.post(() -> { for (int userId : userIds) { @@ -4067,7 +4067,7 @@ public class PackageManagerService implements PackageSender, TestUtilityService packageName, dontKillApp, componentNames, packageUid, reason, userIds, instantUserIds, broadcastAllowList)); mPackageMonitorCallbackHelper.notifyPackageChanged(packageName, dontKillApp, componentNames, - packageUid, reason, userIds, instantUserIds); + packageUid, reason, userIds, instantUserIds, broadcastAllowList); } /** @@ -6237,7 +6237,8 @@ public class PackageManagerService implements PackageSender, TestUtilityService @Override public void registerPackageMonitorCallback(@NonNull IRemoteCallback callback, int userId) { - mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, userId); + int uid = Binder.getCallingUid(); + mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, userId, uid); } @Override diff --git a/services/core/java/com/android/server/pm/PackageMonitorCallbackHelper.java b/services/core/java/com/android/server/pm/PackageMonitorCallbackHelper.java index 1e3fab61d0212..a7715025d0244 100644 --- a/services/core/java/com/android/server/pm/PackageMonitorCallbackHelper.java +++ b/services/core/java/com/android/server/pm/PackageMonitorCallbackHelper.java @@ -29,10 +29,13 @@ import android.net.Uri; import android.os.Bundle; import android.os.Handler; import android.os.IRemoteCallback; +import android.os.Process; import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.UserHandle; import android.text.TextUtils; +import android.util.Slog; +import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; import com.android.internal.util.ArrayUtils; @@ -42,6 +45,9 @@ import java.util.ArrayList; /** Helper class to handle PackageMonitorCallback and notify the registered client. This is mainly * used by PackageMonitor to improve the broadcast latency. */ class PackageMonitorCallbackHelper { + + private static final boolean DEBUG = false; + @NonNull private final Object mLock = new Object(); final IActivityManager mActivityManager = ActivityManager.getService(); @@ -56,9 +62,9 @@ class PackageMonitorCallbackHelper { @GuardedBy("mLock") private final RemoteCallbackList mCallbacks = new RemoteCallbackList<>(); - public void registerPackageMonitorCallback(IRemoteCallback callback, int userId) { + public void registerPackageMonitorCallback(IRemoteCallback callback, int userId, int uid) { synchronized (mLock) { - mCallbacks.register(callback, userId); + mCallbacks.register(callback, new RegisterUser(userId, uid)); } } @@ -73,8 +79,9 @@ class PackageMonitorCallbackHelper { synchronized (mLock) { int registerCount = mCallbacks.getRegisteredCallbackCount(); for (int i = 0; i < registerCount; i++) { - int registerUserId = (int) mCallbacks.getRegisteredCallbackCookie(i); - if (registerUserId == userId) { + RegisterUser registerUser = + (RegisterUser) mCallbacks.getRegisteredCallbackCookie(i); + if (registerUser.getUserId() == userId) { IRemoteCallback callback = mCallbacks.getRegisteredCallbackItem(i); if (targetUnRegisteredCallbacks == null) { targetUnRegisteredCallbacks = new ArrayList<>(); @@ -93,7 +100,7 @@ class PackageMonitorCallbackHelper { public void notifyPackageAddedForNewUsers(String packageName, @AppIdInt int appId, @NonNull int[] userIds, @NonNull int[] instantUserIds, - int dataLoaderType) { + int dataLoaderType, SparseArray broadcastAllowList) { Bundle extras = new Bundle(2); // Set to UID of the first user, EXTRA_UID is automatically updated in sendPackageBroadcast final int uid = UserHandle.getUid( @@ -101,7 +108,7 @@ class PackageMonitorCallbackHelper { extras.putInt(Intent.EXTRA_UID, uid); extras.putInt(PackageInstaller.EXTRA_DATA_LOADER_TYPE, dataLoaderType); notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, packageName, extras , - userIds /* userIds */, instantUserIds); + userIds /* userIds */, instantUserIds, broadcastAllowList); } public void notifyResourcesChanged(boolean mediaStatus, boolean replacing, @@ -115,12 +122,12 @@ class PackageMonitorCallbackHelper { String action = mediaStatus ? Intent.ACTION_EXTERNAL_APPLICATIONS_AVAILABLE : Intent.ACTION_EXTERNAL_APPLICATIONS_UNAVAILABLE; notifyPackageMonitor(action, null /* pkg */, extras, null /* userIds */, - null /* instantUserIds */); + null /* instantUserIds */, null /* broadcastAllowList */); } public void notifyPackageChanged(String packageName, boolean dontKillApp, ArrayList componentNames, int packageUid, String reason, int[] userIds, - int[] instantUserIds) { + int[] instantUserIds, SparseArray broadcastAllowList) { Bundle extras = new Bundle(4); extras.putString(Intent.EXTRA_CHANGED_COMPONENT_NAME, componentNames.get(0)); String[] nameList = new String[componentNames.size()]; @@ -132,11 +139,11 @@ class PackageMonitorCallbackHelper { extras.putString(Intent.EXTRA_REASON, reason); } notifyPackageMonitor(Intent.ACTION_PACKAGE_CHANGED, packageName, extras, userIds, - instantUserIds); + instantUserIds, broadcastAllowList); } public void notifyPackageMonitor(String action, String pkg, Bundle extras, - int[] userIds, int[] instantUserIds) { + int[] userIds, int[] instantUserIds, SparseArray broadcastAllowList) { if (!isAllowedCallbackAction(action)) { return; } @@ -150,9 +157,9 @@ class PackageMonitorCallbackHelper { } if (ArrayUtils.isEmpty(instantUserIds)) { - doNotifyCallbacks(action, pkg, extras, resolvedUserIds); + doNotifyCallbacks(action, pkg, extras, resolvedUserIds, broadcastAllowList); } else { - doNotifyCallbacks(action, pkg, extras, instantUserIds); + doNotifyCallbacks(action, pkg, extras, instantUserIds, broadcastAllowList); } } catch (RemoteException e) { // do nothing @@ -170,7 +177,8 @@ class PackageMonitorCallbackHelper { || TextUtils.equals(action, Intent.ACTION_EXTERNAL_APPLICATIONS_UNAVAILABLE); } - private void doNotifyCallbacks(String action, String pkg, Bundle extras, int[] userIds) { + private void doNotifyCallbacks(String action, String pkg, Bundle extras, int[] userIds, + SparseArray broadcastAllowList) { RemoteCallbackList callbacks; synchronized (mLock) { callbacks = mCallbacks; @@ -188,9 +196,23 @@ class PackageMonitorCallbackHelper { } intent.putExtra(Intent.EXTRA_USER_HANDLE, userId); + final int[] allowUids = + broadcastAllowList != null ? broadcastAllowList.get(userId) : new int[]{}; + mHandler.post(() -> callbacks.broadcast((callback, user) -> { - int registerUserId = (int) user; - if ((registerUserId != UserHandle.USER_ALL) && (registerUserId != userId)) { + RegisterUser registerUser = (RegisterUser) user; + if ((registerUser.getUserId() != UserHandle.USER_ALL) && (registerUser.getUserId() + != userId)) { + return; + } + int registerUid = registerUser.getUid(); + if (broadcastAllowList != null && registerUid != Process.SYSTEM_UID + && !ArrayUtils.contains(allowUids, registerUid)) { + if (DEBUG) { + Slog.w("PackageMonitorCallbackHelper", + "Skip invoke PackageMonitorCallback for " + action + ", uid " + + registerUid); + } return; } invokeCallback(callback, intent); @@ -208,4 +230,22 @@ class PackageMonitorCallbackHelper { // do nothing } } + + private final class RegisterUser { + int mUserId; + int mUid; + + RegisterUser(int userId, int uid) { + mUid = uid; + mUserId = userId; + } + + public int getUid() { + return mUid; + } + + public int getUserId() { + return mUserId; + } + } } diff --git a/services/core/java/com/android/server/pm/SuspendPackageHelper.java b/services/core/java/com/android/server/pm/SuspendPackageHelper.java index 893bc11adf5c0..94e09f1d0bf0c 100644 --- a/services/core/java/com/android/server/pm/SuspendPackageHelper.java +++ b/services/core/java/com/android/server/pm/SuspendPackageHelper.java @@ -634,7 +634,7 @@ public final class SuspendPackageHelper { mPm.snapshotComputer(), callingUid, intentExtras), options)); mPm.notifyPackageMonitor(intent, null /* pkg */, extras, new int[]{userId}, - null /* instantUserIds */); + null /* instantUserIds */, null /* broadcastAllowList */); } /** diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/PackageMonitorCallbackHelperTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/PackageMonitorCallbackHelperTest.java index 4c633a1d3086c..3ac59e9f0efea 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/PackageMonitorCallbackHelperTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/PackageMonitorCallbackHelperTest.java @@ -28,6 +28,7 @@ import static org.mockito.Mockito.when; import android.content.Intent; import android.content.pm.PackageInstaller; import android.content.pm.PackageManager; +import android.os.Binder; import android.os.Bundle; import android.os.Handler; import android.os.IRemoteCallback; @@ -78,7 +79,7 @@ public class PackageMonitorCallbackHelperTest { mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */, - null /* instantUserIds */); + null /* instantUserIds */, null /* broadcastAllowList */); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).never()).sendResult(any()); } @@ -87,9 +88,11 @@ public class PackageMonitorCallbackHelperTest { public void testUnregisterPackageMonitorCallback_callbackShouldNotCalled() throws Exception { IRemoteCallback callback = createMockPackageMonitorCallback(); - mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */); + mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */, + Binder.getCallingUid()); mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, - FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0}, null /* instantUserIds */); + FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0}, null /* instantUserIds */, + null /* broadcastAllowList */); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).times(1)).sendResult(any()); @@ -97,7 +100,7 @@ public class PackageMonitorCallbackHelperTest { mPackageMonitorCallbackHelper.unregisterPackageMonitorCallback(callback); mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */, - null /* instantUserIds */); + null /* instantUserIds */, null /* broadcastAllowList */); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).never()).sendResult(any()); } @@ -106,10 +109,11 @@ public class PackageMonitorCallbackHelperTest { public void testRegisterPackageMonitorCallback_callbackCalled() throws Exception { IRemoteCallback callback = createMockPackageMonitorCallback(); - mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */); + mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */, + Binder.getCallingUid()); mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{0} /* userIds */, - null /* instantUserIds */); + null /* instantUserIds */, null /* broadcastAllowList */); ArgumentCaptor bundleCaptor = ArgumentCaptor.forClass(Bundle.class); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).times(1)).sendResult( @@ -128,11 +132,12 @@ public class PackageMonitorCallbackHelperTest { IRemoteCallback callback = createMockPackageMonitorCallback(); // Register for user 0 - mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */); + mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */, + Binder.getCallingUid()); // Notify for user 10 mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{10} /* userIds */, - null /* instantUserIds */); + null /* instantUserIds */, null /* broadcastAllowList */); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).never()).sendResult(any()); } @@ -144,10 +149,12 @@ public class PackageMonitorCallbackHelperTest { ArrayList components = new ArrayList<>(); String component1 = FAKE_PACKAGE_NAME + "/.Component1"; components.add(component1); - mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */); + mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */, + Binder.getCallingUid()); mPackageMonitorCallbackHelper.notifyPackageChanged(FAKE_PACKAGE_NAME, false /* dontKillApp */, components, FAKE_PACKAGE_UID, null /* reason */, - new int[]{0} /* userIds */, null /* instantUserIds */); + new int[]{0} /* userIds */, null /* instantUserIds */, + null /* broadcastAllowList */); ArgumentCaptor bundleCaptor = ArgumentCaptor.forClass(Bundle.class); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).times(1)).sendResult( @@ -171,10 +178,11 @@ public class PackageMonitorCallbackHelperTest { public void testNotifyPackageAddedForNewUsers_callbackCalled() throws Exception { IRemoteCallback callback = createMockPackageMonitorCallback(); - mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */); + mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */, + Binder.getCallingUid()); mPackageMonitorCallbackHelper.notifyPackageAddedForNewUsers(FAKE_PACKAGE_NAME, FAKE_PACKAGE_UID, new int[]{0} /* userIds */, new int[0], - PackageInstaller.DATA_LOADER_TYPE_STREAMING); + PackageInstaller.DATA_LOADER_TYPE_STREAMING, null /* broadcastAllowList */); ArgumentCaptor bundleCaptor = ArgumentCaptor.forClass(Bundle.class); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).times(1)).sendResult( @@ -194,7 +202,8 @@ public class PackageMonitorCallbackHelperTest { public void testNotifyResourcesChanged_callbackCalled() throws Exception { IRemoteCallback callback = createMockPackageMonitorCallback(); - mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */); + mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 0 /* userId */, + Binder.getCallingUid()); mPackageMonitorCallbackHelper.notifyResourcesChanged(true /* mediaStatus */, true /* replacing */, new String[]{FAKE_PACKAGE_NAME}, new int[]{FAKE_PACKAGE_UID} /* uids */); @@ -224,12 +233,13 @@ public class PackageMonitorCallbackHelperTest { @Test public void testPackageMonitorCallback_onUserRemoved_callbackNotCalled() throws Exception { IRemoteCallback callback = createMockPackageMonitorCallback(); - mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 10 /* userId */); + mPackageMonitorCallbackHelper.registerPackageMonitorCallback(callback, 10 /* userId */, + Binder.getCallingUid()); mPackageMonitorCallbackHelper.onUserRemoved(10); mPackageMonitorCallbackHelper.notifyPackageMonitor(Intent.ACTION_PACKAGE_ADDED, FAKE_PACKAGE_NAME, createFakeBundle(), new int[]{10} /* userIds */, - null /* instantUserIds */); + null /* instantUserIds */, null /* broadcastAllowList */); verify(callback, after(WAIT_CALLBACK_CALLED_IN_MS).never()).sendResult(any()); }