Only enforce system user check on builds prior to Q.
For Q+, enforce use of BIND_VMS_CLIENT permission in manifest.
Bug: 133167532
Test: Added CarServiceTest:VmsPublisherClientPermissionTest
Test: Updated CarServiceUnitTest:VmsClientManagerTest
Test: atest AndroidCarApiTest CarServiceTest CarServiceUnitTest
Change-Id: I92157304b01006bc4445839059f505a2bc78fea7
diff --git a/car-lib/src/android/car/Car.java b/car-lib/src/android/car/Car.java
index c3bd6f9..e3c3b6e 100644
--- a/car-lib/src/android/car/Car.java
+++ b/car-lib/src/android/car/Car.java
@@ -459,6 +459,14 @@
"android.car.permission.CAR_DRIVING_STATE";
/**
+ * Permission necessary to access VMS client service.
+ *
+ * @hide
+ */
+ public static final String PERMISSION_BIND_VMS_CLIENT =
+ "android.car.permission.BIND_VMS_CLIENT";
+
+ /**
* Permissions necessary to access VMS publisher APIs.
*
* @hide
diff --git a/car-lib/src/android/car/vms/VmsPublisherClientService.java b/car-lib/src/android/car/vms/VmsPublisherClientService.java
index 72e0f59..5847b19 100644
--- a/car-lib/src/android/car/vms/VmsPublisherClientService.java
+++ b/car-lib/src/android/car/vms/VmsPublisherClientService.java
@@ -22,6 +22,7 @@
import android.app.Service;
import android.content.Intent;
import android.os.Binder;
+import android.os.Build;
import android.os.Handler;
import android.os.IBinder;
import android.os.Looper;
@@ -261,6 +262,11 @@
}
private void assertSystemOrSelf() {
+ if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
+ if (DBG) Log.d(TAG, "Skipping system user check");
+ return;
+ }
+
if (!(Binder.getCallingUid() == Process.SYSTEM_UID
|| Binder.getCallingPid() == Process.myPid())) {
throw new SecurityException("Caller must be system user or same process");
diff --git a/service/src/com/android/car/vms/VmsClientManager.java b/service/src/com/android/car/vms/VmsClientManager.java
index d3c39ff..675c196 100644
--- a/service/src/com/android/car/vms/VmsClientManager.java
+++ b/service/src/com/android/car/vms/VmsClientManager.java
@@ -16,6 +16,7 @@
package com.android.car.vms;
+import android.car.Car;
import android.car.userlib.CarUserManagerHelper;
import android.content.BroadcastReceiver;
import android.content.ComponentName;
@@ -23,6 +24,8 @@
import android.content.Intent;
import android.content.IntentFilter;
import android.content.ServiceConnection;
+import android.content.pm.PackageManager;
+import android.content.pm.ServiceInfo;
import android.os.Handler;
import android.os.IBinder;
import android.os.Looper;
@@ -278,11 +281,21 @@
return;
}
- if (!mContext.getPackageManager().isPackageAvailable(name.getPackageName())) {
+ ServiceInfo serviceInfo;
+ try {
+ serviceInfo = mContext.getPackageManager().getServiceInfo(name,
+ PackageManager.MATCH_DIRECT_BOOT_AUTO);
+ } catch (PackageManager.NameNotFoundException e) {
Log.w(TAG, "Client not installed: " + clientName);
return;
}
+ if (!Car.PERMISSION_BIND_VMS_CLIENT.equals(serviceInfo.permission)) {
+ Log.w(TAG, "Client service: " + clientName
+ + " does not require " + Car.PERMISSION_BIND_VMS_CLIENT + " permission");
+ return;
+ }
+
ClientConnection connection = new ClientConnection(name, userHandle);
if (connection.bind()) {
Log.i(TAG, "Client bound: " + connection);
diff --git a/tests/carservice_test/AndroidManifest.xml b/tests/carservice_test/AndroidManifest.xml
index e12e6cb..297754d 100644
--- a/tests/carservice_test/AndroidManifest.xml
+++ b/tests/carservice_test/AndroidManifest.xml
@@ -43,7 +43,17 @@
</intent-filter>
</service>
<service android:name="com.android.car.MockedVmsTestBase$MockPublisherClient"
- android:exported="true" />
+ android:exported="true"
+ android:permission="android.car.permission.BIND_VMS_CLIENT"/>
+
+ <service android:name="com.android.car.VmsPublisherClientPermissionTest$PublisherClientExpectedPermission"
+ android:exported="true"
+ android:permission="android.car.permission.BIND_VMS_CLIENT"/>
+ <service android:name="com.android.car.VmsPublisherClientPermissionTest$PublisherClientWrongPermission"
+ android:exported="true"
+ android:permission="android.car.permission.VMS_PUBLISHER"/>
+ <service android:name="com.android.car.VmsPublisherClientPermissionTest$PublisherClientMissingPermission"
+ android:exported="true"/>
<activity android:name="com.android.car.SystemActivityMonitoringServiceTest$ActivityA"/>
<activity android:name="com.android.car.SystemActivityMonitoringServiceTest$ActivityB"/>
diff --git a/tests/carservice_test/src/com/android/car/MockedVmsTestBase.java b/tests/carservice_test/src/com/android/car/MockedVmsTestBase.java
index 7214f8b..450b9e9 100644
--- a/tests/carservice_test/src/com/android/car/MockedVmsTestBase.java
+++ b/tests/carservice_test/src/com/android/car/MockedVmsTestBase.java
@@ -51,6 +51,8 @@
import java.util.concurrent.TimeUnit;
public class MockedVmsTestBase extends MockedCarTestBase {
+ public static final long PUBLISHER_BIND_TIMEOUT_SECS = 2L;
+
private static final String TAG = "MockedVmsTestBase";
private static CountDownLatch sPublisherIsReady = new CountDownLatch(1);
private static MockPublisherClient sPublisherClient;
@@ -127,7 +129,9 @@
MockPublisherClient getMockPublisherClient() {
try {
- sPublisherIsReady.await(2L, TimeUnit.SECONDS);
+ assertTrue(
+ "Timeout while waiting for publisher client to be ready",
+ sPublisherIsReady.await(PUBLISHER_BIND_TIMEOUT_SECS, TimeUnit.SECONDS));
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
diff --git a/tests/carservice_test/src/com/android/car/VmsPublisherClientPermissionTest.java b/tests/carservice_test/src/com/android/car/VmsPublisherClientPermissionTest.java
new file mode 100644
index 0000000..0e3adde
--- /dev/null
+++ b/tests/carservice_test/src/com/android/car/VmsPublisherClientPermissionTest.java
@@ -0,0 +1,124 @@
+/*
+ * Copyright (C) 2019 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 com.android.car;
+
+import static com.android.car.MockedVmsTestBase.PUBLISHER_BIND_TIMEOUT_SECS;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import android.car.vms.VmsPublisherClientService;
+import android.car.vms.VmsSubscriptionState;
+import android.content.Intent;
+import android.os.UserHandle;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+public class VmsPublisherClientPermissionTest extends MockedCarTestBase {
+ private static CountDownLatch sPublisherExpectedPermission = new CountDownLatch(1);
+ private static CountDownLatch sPublisherWrongPermission = new CountDownLatch(1);
+ private static CountDownLatch sPublisherMissingPermission = new CountDownLatch(1);
+
+ private static class MockPublisherClient extends VmsPublisherClientService {
+ private CountDownLatch mReadyLatch;
+ MockPublisherClient(CountDownLatch readyLatch) {
+ mReadyLatch = readyLatch;
+ }
+ @Override
+ protected void onVmsPublisherServiceReady() {
+ mReadyLatch.countDown();
+ }
+
+ @Override
+ public void onVmsSubscriptionChange(VmsSubscriptionState subscriptionState) {}
+ }
+
+
+ // AndroidManifest.xml:
+ // <service android:name="com.android.car.VmsPublisherClientPermissionTest$PublisherClientExpectedPermission"
+ // android:exported="true"
+ // android:permission="android.car.permission.BIND_VMS_CLIENT"/>
+ public static class PublisherClientExpectedPermission extends MockPublisherClient {
+ public PublisherClientExpectedPermission() {
+ super(sPublisherExpectedPermission);
+ }
+ }
+
+ // AndroidManifest.xml:
+ // <service android:name="com.android.car.VmsPublisherClientPermissionTest$PublisherClientWrongPermission"
+ // android:exported="true"
+ // android:permission="android.car.permission.VMS_PUBLISHER"/>
+ public static class PublisherClientWrongPermission extends MockPublisherClient {
+ public PublisherClientWrongPermission() {
+ super(sPublisherWrongPermission);
+ }
+ }
+
+ // AndroidManifest.xml:
+ // <service android:name="com.android.car.VmsPublisherClientPermissionTest$PublisherClientMissingPermission"
+ // android:exported="true"/>
+ public static class PublisherClientMissingPermission extends MockPublisherClient {
+ public PublisherClientMissingPermission() {
+ super(sPublisherMissingPermission);
+ }
+ }
+
+ @Override
+ protected synchronized void configureResourceOverrides(MockResources resources) {
+ super.configureResourceOverrides(resources);
+ resources.overrideResource(com.android.car.R.array.vmsPublisherSystemClients, new String[]{
+ getFlattenComponent(PublisherClientExpectedPermission.class),
+ getFlattenComponent(PublisherClientWrongPermission.class),
+ getFlattenComponent(PublisherClientMissingPermission.class)
+ });
+ resources.overrideResource(com.android.car.R.array.vmsPublisherUserClients, new String[]{
+ getFlattenComponent(PublisherClientExpectedPermission.class),
+ getFlattenComponent(PublisherClientWrongPermission.class),
+ getFlattenComponent(PublisherClientMissingPermission.class)
+ });
+ }
+
+ @Before
+ public void triggerClientBinding() {
+ getContext().sendBroadcastAsUser(new Intent(Intent.ACTION_USER_UNLOCKED), UserHandle.ALL);
+ }
+
+ @Test
+ public void testExpectedPermission() throws Exception {
+ assertTrue(
+ "Timeout while waiting for publisher client to be ready",
+ sPublisherExpectedPermission.await(PUBLISHER_BIND_TIMEOUT_SECS, TimeUnit.SECONDS));
+ }
+
+ @Test
+ public void testWrongPermission() throws Exception {
+ assertFalse(
+ "Publisher with wrong android:permission was bound unexpectedly",
+ sPublisherWrongPermission.await(PUBLISHER_BIND_TIMEOUT_SECS, TimeUnit.SECONDS));
+ }
+
+ @Test
+ public void testMissingPermission() throws Exception {
+ assertFalse(
+ "Publisher with missing android:permission was bound unexpectedly",
+ sPublisherMissingPermission.await(PUBLISHER_BIND_TIMEOUT_SECS, TimeUnit.SECONDS));
+ }
+}
diff --git a/tests/carservice_unit_test/src/com/android/car/vms/VmsClientManagerTest.java b/tests/carservice_unit_test/src/com/android/car/vms/VmsClientManagerTest.java
index 772abc7..ac53dd6 100644
--- a/tests/carservice_unit_test/src/com/android/car/vms/VmsClientManagerTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/vms/VmsClientManagerTest.java
@@ -31,6 +31,7 @@
import static org.mockito.Mockito.verifyZeroInteractions;
import static org.mockito.Mockito.when;
+import android.car.Car;
import android.car.userlib.CarUserManagerHelper;
import android.content.ComponentName;
import android.content.Context;
@@ -38,6 +39,7 @@
import android.content.IntentFilter;
import android.content.ServiceConnection;
import android.content.pm.PackageManager;
+import android.content.pm.ServiceInfo;
import android.content.res.Resources;
import android.os.Binder;
import android.os.Handler;
@@ -65,8 +67,15 @@
@SmallTest
public class VmsClientManagerTest {
private static final String HAL_CLIENT_NAME = "VmsHalClient";
+ private static final String SYSTEM_CLIENT = "com.google.android.apps.vms.test/.VmsSystemClient";
+ private static final ComponentName SYSTEM_CLIENT_COMPONENT =
+ ComponentName.unflattenFromString(SYSTEM_CLIENT);
private static final String SYSTEM_CLIENT_NAME =
"com.google.android.apps.vms.test/com.google.android.apps.vms.test.VmsSystemClient U=0";
+
+ private static final String USER_CLIENT = "com.google.android.apps.vms.test/.VmsUserClient";
+ private static final ComponentName USER_CLIENT_COMPONENT =
+ ComponentName.unflattenFromString(USER_CLIENT);
private static final String USER_CLIENT_NAME =
"com.google.android.apps.vms.test/com.google.android.apps.vms.test.VmsUserClient U=10";
@Rule
@@ -97,23 +106,21 @@
private ArgumentCaptor<ServiceConnection> mConnectionCaptor;
@Before
- public void setUp() {
+ public void setUp() throws Exception {
resetContext();
- when(mPackageManager.isPackageAvailable(any())).thenReturn(true);
+ ServiceInfo serviceInfo = new ServiceInfo();
+ serviceInfo.permission = Car.PERMISSION_BIND_VMS_CLIENT;
+ when(mPackageManager.getServiceInfo(any(), anyInt())).thenReturn(serviceInfo);
when(mResources.getInteger(
com.android.car.R.integer.millisecondsBeforeRebindToVmsPublisher)).thenReturn(
5);
when(mResources.getStringArray(
com.android.car.R.array.vmsPublisherSystemClients)).thenReturn(
- new String[]{
- "com.google.android.apps.vms.test/.VmsSystemClient"
- });
+ new String[]{ SYSTEM_CLIENT });
when(mResources.getStringArray(
com.android.car.R.array.vmsPublisherUserClients)).thenReturn(
- new String[]{
- "com.google.android.apps.vms.test/.VmsUserClient"
- });
+ new String[]{ USER_CLIENT });
mUserId = 10;
when(mUserManager.getCurrentForegroundUserId()).thenAnswer((invocation) -> mUserId);
@@ -207,6 +214,28 @@
}
@Test
+ public void testSystemUserUnlocked_ClientNotFound() throws Exception {
+ when(mPackageManager.getServiceInfo(eq(SYSTEM_CLIENT_COMPONENT), anyInt()))
+ .thenThrow(new PackageManager.NameNotFoundException());
+ notifySystemUserUnlocked();
+
+ // Process will not be bound
+ verifySystemBind(0);
+ }
+
+ @Test
+ public void testSystemUserUnlocked_WrongPermission() throws Exception {
+ ServiceInfo serviceInfo = new ServiceInfo();
+ serviceInfo.permission = Car.PERMISSION_VMS_PUBLISHER;
+ when(mPackageManager.getServiceInfo(eq(SYSTEM_CLIENT_COMPONENT), anyInt()))
+ .thenReturn(serviceInfo);
+ notifySystemUserUnlocked();
+
+ // Process will not be bound
+ verifySystemBind(0);
+ }
+
+ @Test
public void testSystemUserUnlocked_BindFailed() {
when(mContext.bindServiceAsUser(any(), any(), anyInt(), any(), any())).thenReturn(false);
notifySystemUserUnlocked();
@@ -228,11 +257,6 @@
}
@Test
- public void testUserSwitched() {
- notifyUserSwitched();
- }
-
- @Test
public void testUserUnlocked() {
notifyUserUnlocked();
notifyUserUnlocked();
@@ -242,6 +266,28 @@
}
@Test
+ public void testUserUnlocked_ClientNotFound() throws Exception {
+ when(mPackageManager.getServiceInfo(eq(USER_CLIENT_COMPONENT), anyInt()))
+ .thenThrow(new PackageManager.NameNotFoundException());
+ notifyUserUnlocked();
+
+ // Process will not be bound
+ verifyUserBind(0);
+ }
+
+ @Test
+ public void testUserUnlocked_WrongPermission() throws Exception {
+ ServiceInfo serviceInfo = new ServiceInfo();
+ serviceInfo.permission = Car.PERMISSION_VMS_PUBLISHER;
+ when(mPackageManager.getServiceInfo(eq(USER_CLIENT_COMPONENT), anyInt()))
+ .thenReturn(serviceInfo);
+ notifyUserUnlocked();
+
+ // Process will not be bound
+ verifyUserBind(0);
+ }
+
+ @Test
public void testUserUnlocked_BindFailed() {
when(mContext.bindServiceAsUser(any(), any(), anyInt(), any(), any()))
.thenReturn(false);
@@ -324,6 +370,14 @@
}
@Test
+ public void testUserSwitched() {
+ notifyUserSwitched();
+
+ // Clients are not bound on user switch alone
+ verifyUserBind(0);
+ }
+
+ @Test
public void testUserSwitchedAndUnlocked() {
notifyUserSwitched();
notifyUserUnlocked();
@@ -593,20 +647,16 @@
}
private void verifySystemBind(int times) {
- verifyBind(times, "com.google.android.apps.vms.test/.VmsSystemClient",
- UserHandle.SYSTEM);
+ verifyBind(times, SYSTEM_CLIENT_COMPONENT, UserHandle.SYSTEM);
}
private void verifyUserBind(int times) {
- verifyBind(times, "com.google.android.apps.vms.test/.VmsUserClient",
- UserHandle.of(mUserId));
+ verifyBind(times, USER_CLIENT_COMPONENT, UserHandle.of(mUserId));
}
- private void verifyBind(int times, String componentName,
- UserHandle user) {
- ArgumentCaptor<Intent> intentCaptor = ArgumentCaptor.forClass(Intent.class);
+ private void verifyBind(int times, ComponentName componentName, UserHandle user) {
Intent expectedService = new Intent();
- expectedService.setComponent(ComponentName.unflattenFromString(componentName));
+ expectedService.setComponent(componentName);
verify(mContext, times(times)).bindServiceAsUser(
argThat((service) -> service.filterEquals(expectedService)),
mConnectionCaptor.capture(),