From 95dd8a1942e3d639c451f8725d35e1115c1905c0 Mon Sep 17 00:00:00 2001 From: Xiang Wang Date: Tue, 16 Aug 2022 00:39:23 +0000 Subject: Check car target version before sending new user lifecycle events. This is to make sure only apps that are aware of the new user lifecycle event types will have the events sent to them and therefore preventing unaware apps from misbehaving upon unexpected events. Bug: 235524989 Test: atest CarUserServiceTest Change-Id: I2ad8991cbfaeaa1266e48d01a105996266331dab Merged-In: I2ad8991cbfaeaa1266e48d01a105996266331dab --- service/src/com/android/car/ICarImpl.java | 12 ++++---- .../src/com/android/car/user/CarUserService.java | 32 ++++++++++++++++++---- .../com/android/car/CarInputRotaryServiceTest.java | 4 ++- .../car/pm/VendorServiceControllerTest.java | 5 +++- .../car/user/BaseCarUserServiceTestCase.java | 5 +++- .../com/android/car/user/CarUserServiceTest.java | 32 +++++++++++++++++++++- 6 files changed, 74 insertions(+), 16 deletions(-) diff --git a/service/src/com/android/car/ICarImpl.java b/service/src/com/android/car/ICarImpl.java index 3ca5cde12f..61c63d0c5a 100644 --- a/service/src/com/android/car/ICarImpl.java +++ b/service/src/com/android/car/ICarImpl.java @@ -248,6 +248,11 @@ public class ICarImpl extends ICar.Stub { mCarUXRestrictionsService = constructWithTrace(t, CarUxRestrictionsManagerService.class, () -> new CarUxRestrictionsManagerService(serviceContext, mCarDrivingStateService, mCarPropertyService, mCarOccupantZoneService)); + mCarActivityService = constructWithTrace(t, CarActivityService.class, + () -> new CarActivityService(serviceContext)); + mCarPackageManagerService = constructWithTrace(t, CarPackageManagerService.class, + () -> new CarPackageManagerService(serviceContext, mCarUXRestrictionsService, + mCarActivityService, mCarOccupantZoneService)); if (carUserService != null) { mCarUserService = carUserService; CarLocalServices.addService(CarUserService.class, carUserService); @@ -256,7 +261,7 @@ public class ICarImpl extends ICar.Stub { int maxRunningUsers = UserManagerHelper.getMaxRunningUsers(serviceContext); mCarUserService = constructWithTrace(t, CarUserService.class, () -> new CarUserService(serviceContext, mHal.getUserHal(), userManager, - maxRunningUsers, mCarUXRestrictionsService)); + maxRunningUsers, mCarUXRestrictionsService, mCarPackageManagerService)); } if (mFeatureController.isFeatureEnabled(Car.EXPERIMENTAL_CAR_USER_SERVICE)) { mExperimentalCarUserService = constructWithTrace(t, ExperimentalCarUserService.class, @@ -284,11 +289,6 @@ public class ICarImpl extends ICar.Stub { } else { mOccupantAwarenessService = null; } - mCarActivityService = constructWithTrace(t, CarActivityService.class, - () -> new CarActivityService(serviceContext)); - mCarPackageManagerService = constructWithTrace(t, CarPackageManagerService.class, - () -> new CarPackageManagerService(serviceContext, mCarUXRestrictionsService, - mCarActivityService, mCarOccupantZoneService)); mPerUserCarServiceHelper = constructWithTrace( t, PerUserCarServiceHelper.class, () -> new PerUserCarServiceHelper(serviceContext, mCarUserService)); diff --git a/service/src/com/android/car/user/CarUserService.java b/service/src/com/android/car/user/CarUserService.java index 46c1e59337..8f96c0bab8 100644 --- a/service/src/com/android/car/user/CarUserService.java +++ b/service/src/com/android/car/user/CarUserService.java @@ -32,6 +32,7 @@ import android.annotation.Nullable; import android.annotation.UserIdInt; import android.app.ActivityManager; import android.app.admin.DevicePolicyManager; +import android.car.CarVersion; import android.car.ICarResultReceiver; import android.car.ICarUserService; import android.car.builtin.app.ActivityManagerHelper; @@ -110,6 +111,7 @@ import com.android.car.internal.util.ArrayUtils; import com.android.car.internal.util.DebugUtils; import com.android.car.internal.util.FunctionalUtils; import com.android.car.internal.util.IndentingPrintWriter; +import com.android.car.pm.CarPackageManagerService; import com.android.car.power.CarPowerManagementService; import com.android.car.user.InitialUserSetter.InitialUserInfo; import com.android.internal.annotations.GuardedBy; @@ -250,6 +252,8 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer private final CarUxRestrictionsManagerService mCarUxRestrictionService; + private final CarPackageManagerService mCarPackageManagerService; + private static final int PRE_CREATION_STAGE_BEFORE_SUSPEND = 1; private static final int PRE_CREATION_STAGE_ON_SYSTEM_START = 2; @@ -310,12 +314,13 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer public CarUserService(@NonNull Context context, @NonNull UserHalService hal, @NonNull UserManager userManager, int maxRunningUsers, - @NonNull CarUxRestrictionsManagerService uxRestrictionService) { + @NonNull CarUxRestrictionsManagerService uxRestrictionService, + @NonNull CarPackageManagerService carPackageManagerService) { this(context, hal, userManager, new UserHandleHelper(context, userManager), context.getSystemService(DevicePolicyManager.class), context.getSystemService(ActivityManager.class), maxRunningUsers, /* initialUserSetter= */ null, /* userPreCreator= */ null, uxRestrictionService, - null); + null, carPackageManagerService); } @VisibleForTesting @@ -328,7 +333,8 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer @Nullable InitialUserSetter initialUserSetter, @Nullable UserPreCreator userPreCreator, @NonNull CarUxRestrictionsManagerService uxRestrictionService, - @Nullable Handler handler) { + @Nullable Handler handler, + @NonNull CarPackageManagerService carPackageManagerService) { Slogf.d(TAG, "CarUserService(): DBG=%b, user=%s", DBG, context.getUser()); mContext = context; mHal = hal; @@ -347,6 +353,7 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer mSwitchGuestUserBeforeSleep = resources.getBoolean( R.bool.config_switchGuestUserBeforeGoingSleep); mCarUxRestrictionService = uxRestrictionService; + mCarPackageManagerService = carPackageManagerService; mPreCreationStage = resources.getInteger(R.integer.config_userPreCreationStage); int preCreationDelayMs = resources .getInteger(R.integer.config_userPreCreationDelay); @@ -2044,9 +2051,6 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer // POST_UNLOCKED event is meant only for internal service listeners. Skip sending it to // app listeners. if (eventType != CarUserManager.USER_LIFECYCLE_EVENT_TYPE_POST_UNLOCKED) { - // TODO(b/235524989): Do target version check inside - // handleNotifyAppUserLifecycleListeners and do not send the event if an app's - // target car version is lower than TIRAMISU_1. handleNotifyAppUserLifecycleListeners(event); } }); @@ -2087,6 +2091,22 @@ public final class CarUserService extends ICarUserService.Stub implements CarSer t.traceBegin("notify-app-listeners-user-" + userId + "-event-" + eventType); for (int i = 0; i < listenersSize; i++) { AppLifecycleListener listener = mAppLifecycleListeners.valueAt(i); + // Perform target car version check to ensure only apps expecting the new + // lifecycle event types will have the events sent to them. + // TODO(b/235524989): Cache the target car version for packages in + // CarPackageManagerService. + if (eventType == CarUserManager.USER_LIFECYCLE_EVENT_TYPE_CREATED + || eventType == CarUserManager.USER_LIFECYCLE_EVENT_TYPE_REMOVED) { + CarVersion targetCarVersion = mCarPackageManagerService.getTargetCarVersion( + listener.packageName); + if (!targetCarVersion.isAtLeast(CarVersion.VERSION_CODES.TIRAMISU_1)) { + if (DBG) { + Slogf.d(TAG, "Skipping app listener %s for event %s due to incompatible" + + " target car version %s.", listener, event, targetCarVersion); + } + continue; + } + } if (!listener.applyFilters(event)) { if (DBG) { Slogf.d(TAG, "Skipping app listener %s for event %s due to the filters" diff --git a/tests/carservice_unit_test/src/com/android/car/CarInputRotaryServiceTest.java b/tests/carservice_unit_test/src/com/android/car/CarInputRotaryServiceTest.java index 66a5d7361b..bb05ead67f 100644 --- a/tests/carservice_unit_test/src/com/android/car/CarInputRotaryServiceTest.java +++ b/tests/carservice_unit_test/src/com/android/car/CarInputRotaryServiceTest.java @@ -49,6 +49,7 @@ import com.android.car.bluetooth.CarBluetoothService; import com.android.car.hal.InputHalService; import com.android.car.hal.UserHalService; import com.android.car.internal.common.CommonConstants.UserLifecycleEventType; +import com.android.car.pm.CarPackageManagerService; import com.android.car.user.CarUserService; import com.android.internal.util.test.BroadcastInterceptingContext; import com.android.internal.util.test.FakeSettingsProvider; @@ -85,6 +86,7 @@ public class CarInputRotaryServiceTest { @Mock private CarOccupantZoneService mCarOccupantZoneService; @Mock private CarUxRestrictionsManagerService mUxRestrictionService; @Mock private CarBluetoothService mCarBluetoothService; + @Mock private CarPackageManagerService mCarPackageManagerService; @Spy private final Context mContext = ApplicationProvider.getApplicationContext(); @Spy private final Handler mHandler = new Handler(Looper.getMainLooper()); @@ -287,7 +289,7 @@ public class CarInputRotaryServiceTest { UserHalService userHal = mock(UserHalService.class); mCarUserService = new CarUserService(mMockContext, userHal, userManager, /* maxRunningUsers= */ 2, - mUxRestrictionService); + mUxRestrictionService, mCarPackageManagerService); mCarInputService = new CarInputService(mMockContext, mInputHalService, mCarUserService, mCarOccupantZoneService, mCarBluetoothService, mHandler, mTelecomManager, diff --git a/tests/carservice_unit_test/src/com/android/car/pm/VendorServiceControllerTest.java b/tests/carservice_unit_test/src/com/android/car/pm/VendorServiceControllerTest.java index 362c23ff83..58cb175c2f 100644 --- a/tests/carservice_unit_test/src/com/android/car/pm/VendorServiceControllerTest.java +++ b/tests/carservice_unit_test/src/com/android/car/pm/VendorServiceControllerTest.java @@ -105,6 +105,9 @@ public final class VendorServiceControllerTest extends AbstractExtendedMockitoTe @Mock private CarUxRestrictionsManagerService mUxRestrictionService; + @Mock + private CarPackageManagerService mCarPackageManagerService; + private ServiceLauncherContext mContext; private CarUserService mCarUserService; private VendorServiceController mController; @@ -123,7 +126,7 @@ public final class VendorServiceControllerTest extends AbstractExtendedMockitoTe mContext = new ServiceLauncherContext(ApplicationProvider.getApplicationContext()); mCarUserService = new CarUserService(mContext, mUserHal, mUserManager, - /* maxRunningUsers= */ 2, mUxRestrictionService); + /* maxRunningUsers= */ 2, mUxRestrictionService, mCarPackageManagerService); CarLocalServices.addService(CarUserService.class, mCarUserService); mController = new VendorServiceController(mContext, Looper.getMainLooper()); diff --git a/tests/carservice_unit_test/src/com/android/car/user/BaseCarUserServiceTestCase.java b/tests/carservice_unit_test/src/com/android/car/user/BaseCarUserServiceTestCase.java index 50dc55b17e..af4c0e2245 100644 --- a/tests/carservice_unit_test/src/com/android/car/user/BaseCarUserServiceTestCase.java +++ b/tests/carservice_unit_test/src/com/android/car/user/BaseCarUserServiceTestCase.java @@ -103,6 +103,7 @@ import com.android.car.internal.common.CommonConstants.UserLifecycleEventType; import com.android.car.internal.common.UserHelperLite; import com.android.car.internal.os.CarSystemProperties; import com.android.car.internal.user.UserHelper; +import com.android.car.pm.CarPackageManagerService; import com.android.internal.R; import com.android.internal.util.Preconditions; @@ -158,6 +159,7 @@ abstract class BaseCarUserServiceTestCase extends AbstractExtendedMockitoTestCas @Mock protected ICarServiceHelper mICarServiceHelper; @Mock protected Handler mMockedHandler; @Mock protected UserHandleHelper mMockedUserHandleHelper; + @Mock protected CarPackageManagerService mCarPackageManagerService; protected final BlockingUserLifecycleListener mUserLifecycleListener = BlockingUserLifecycleListener.forAnyEvent().build(); @@ -472,7 +474,8 @@ abstract class BaseCarUserServiceTestCase extends AbstractExtendedMockitoTestCas mInitialUserSetter, mUserPreCreator, mCarUxRestrictionService, - mHandler); + mHandler, + mCarPackageManagerService); } /** diff --git a/tests/carservice_unit_test/src/com/android/car/user/CarUserServiceTest.java b/tests/carservice_unit_test/src/com/android/car/user/CarUserServiceTest.java index fa0e50874a..6818a7a584 100644 --- a/tests/carservice_unit_test/src/com/android/car/user/CarUserServiceTest.java +++ b/tests/carservice_unit_test/src/com/android/car/user/CarUserServiceTest.java @@ -52,6 +52,7 @@ import static org.testng.Assert.expectThrows; import android.annotation.Nullable; import android.app.ActivityManager; +import android.car.CarVersion; import android.car.ICarResultReceiver; import android.car.builtin.app.ActivityManagerHelper; import android.car.builtin.os.UserManagerHelper; @@ -393,6 +394,34 @@ public final class CarUserServiceTest extends BaseCarUserServiceTestCase { verify(mAnotherLifecycleEventReceiver, times(2)).send(anyInt(), any()); } + @Test + public void testOnUserLifecycleEvent_notifyReceiver_targetVersionCheck() throws Exception { + // Arrange: add receivers. + mCarUserService.setLifecycleListenerForApp("package1", + new UserLifecycleEventFilter.Builder() + .addEventType(CarUserManager.USER_LIFECYCLE_EVENT_TYPE_CREATED).build(), + mLifecycleEventReceiver); + mCarUserService.setLifecycleListenerForApp("package2", + new UserLifecycleEventFilter.Builder() + .addEventType(CarUserManager.USER_LIFECYCLE_EVENT_TYPE_CREATED).build(), + mAnotherLifecycleEventReceiver); + + when(mCarPackageManagerService.getTargetCarVersion("package1")) + .thenReturn(CarVersion.VERSION_CODES.TIRAMISU_0); + when(mCarPackageManagerService.getTargetCarVersion("package2")) + .thenReturn(CarVersion.VERSION_CODES.TIRAMISU_1); + + // Act: User created event occurs. + sendUserLifecycleEvent(/* fromUser */ 0, mRegularUserId, + CarUserManager.USER_LIFECYCLE_EVENT_TYPE_CREATED); + waitForHandlerThreadToFinish(); + + // Verify: receivers are called or not depending on whether the target version meets + // requirement. + verify(mLifecycleEventReceiver, never()).send(anyInt(), any()); + verify(mAnotherLifecycleEventReceiver).send(anyInt(), any()); + } + @Test public void testOnUserLifecycleEvent_notifyReceiver_singleReceiverWithMultipleFilters() throws Exception { @@ -608,7 +637,8 @@ public final class CarUserServiceTest extends BaseCarUserServiceTestCase { mInitialUserSetter, mUserPreCreator, mCarUxRestrictionService, - mMockedHandler); + mMockedHandler, + mCarPackageManagerService); mockStopUserWithDelayedLockingThrows(userId, new IllegalStateException()); carUserServiceLocal.stopUser(userId, userStopResult); -- cgit v1.2.3