Fix secondary user test for VirtualDevice

- Clear binder identity before calling getPackageUid - VirtualDeviceImpl uses the owner's user ID to create its Context - For virtual camera, create a different CameraAccessController for each user Fixes: 218790107 Test: (On secondary user) atest CtsVirtualDevicesTestCases Change-Id: I63027ab58854f11faa4e2b20da202886dd10d2f3
2022-02-11 22:14:43 +00:00
parent 7f9d05c6d2
commit 8e34e6352c
6 changed files with 80 additions and 27 deletions
--- a/services/companion/java/com/android/server/companion/virtual/CameraAccessController.java
+++ b/services/companion/java/com/android/server/companion/virtual/CameraAccessController.java
@@ -33,17 +33,19 @@ import com.android.internal.annotations.GuardedBy;
 /**
 * Handles blocking access to the camera for apps running on virtual devices.
 */
-class CameraAccessController extends CameraManager.AvailabilityCallback {
+class CameraAccessController extends CameraManager.AvailabilityCallback implements AutoCloseable {
    private static final String TAG = "CameraAccessController";

    private final Object mLock = new Object();

    private final Context mContext;
-    private VirtualDeviceManagerInternal mVirtualDeviceManagerInternal;
-    CameraAccessBlockedCallback mBlockedCallback;
-    private CameraManager mCameraManager;
-    private boolean mListeningForCameraEvents;
-    private PackageManager mPackageManager;
+    private final VirtualDeviceManagerInternal mVirtualDeviceManagerInternal;
+    private final CameraAccessBlockedCallback mBlockedCallback;
+    private final CameraManager mCameraManager;
+    private final PackageManager mPackageManager;
+
+    @GuardedBy("mLock")
+    private int mObserverCount = 0;

    @GuardedBy("mLock")
    private ArrayMap<String, InjectionSessionData> mPackageToSessionData = new ArrayMap<>();
@@ -77,23 +79,38 @@ class CameraAccessController extends CameraManager.AvailabilityCallback {
     */
    public void startObservingIfNeeded() {
        synchronized (mLock) {
-            if (!mListeningForCameraEvents) {
+            if (mObserverCount == 0) {
                mCameraManager.registerAvailabilityCallback(mContext.getMainExecutor(), this);
-                mListeningForCameraEvents = true;
            }
+            mObserverCount++;
        }
    }

    /**
     * Stop watching for camera access.
     */
-    public void stopObserving() {
+    public void stopObservingIfNeeded() {
        synchronized (mLock) {
-            mCameraManager.unregisterAvailabilityCallback(this);
-            mListeningForCameraEvents = false;
+            mObserverCount--;
+            if (mObserverCount <= 0) {
+                close();
+            }
        }
    }

+
+    @Override
+    public void close() {
+        synchronized (mLock) {
+            if (mObserverCount < 0) {
+                Slog.wtf(TAG, "Unexpected negative mObserverCount: " + mObserverCount);
+            } else if (mObserverCount > 0) {
+                Slog.w(TAG, "Unexpected close with observers remaining: " + mObserverCount);
+            }
+        }
+        mCameraManager.unregisterAvailabilityCallback(this);
+    }
+
    @Override
    public void onCameraOpened(@NonNull String cameraId, @NonNull String packageName) {
        synchronized (mLock) {
--- a/services/companion/java/com/android/server/companion/virtual/PermissionUtils.java
+++ b/services/companion/java/com/android/server/companion/virtual/PermissionUtils.java
@@ -18,6 +18,8 @@ package com.android.server.companion.virtual;

 import android.content.Context;
 import android.content.pm.PackageManager;
+import android.os.Binder;
+import android.os.UserHandle;
 import android.util.Slog;

 /**
@@ -32,13 +34,15 @@ class PermissionUtils {
     *
     * @param context the context
     * @param callingPackage the calling application package name
-     * @param callingUid the calling application uid
-     * @return {@code true} if the package name matches the calling app uid, {@code false} otherwise
+     * @return {@code true} if the package name matches {@link Binder#getCallingUid()}, or
+     *   {@code false} otherwise
     */
-    public static boolean validatePackageName(Context context, String callingPackage,
-            int callingUid) {
+    public static boolean validateCallingPackageName(Context context, String callingPackage) {
+        final int callingUid = Binder.getCallingUid();
+        final long token = Binder.clearCallingIdentity();
        try {
-            int packageUid = context.getPackageManager().getPackageUid(callingPackage, 0);
+            int packageUid = context.getPackageManager()
+                    .getPackageUidAsUser(callingPackage, UserHandle.getUserId(callingUid));
            if (packageUid != callingUid) {
                Slog.e(LOG_TAG, "validatePackageName: App with package name " + callingPackage
                        + " is UID " + packageUid + " but caller is " + callingUid);
@@ -48,6 +52,8 @@ class PermissionUtils {
            Slog.e(LOG_TAG, "validatePackageName: App with package name " + callingPackage
                    + " does not exist");
            return false;
+        } finally {
+            Binder.restoreCallingIdentity(token);
        }
        return true;
    }
--- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java
+++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java
@@ -140,7 +140,8 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub
            int ownerUid, InputController inputController, OnDeviceCloseListener listener,
            PendingTrampolineCallback pendingTrampolineCallback,
            IVirtualDeviceActivityListener activityListener, VirtualDeviceParams params) {
-        mContext = context;
+        UserHandle ownerUserHandle = UserHandle.getUserHandleForUid(ownerUid);
+        mContext = context.createContextAsUser(ownerUserHandle, 0);
        mAssociationInfo = associationInfo;
        mPendingTrampolineCallback = pendingTrampolineCallback;
        mActivityListener = activityListener;
--- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java
+++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java
@@ -36,6 +36,7 @@ import android.os.IBinder;
 import android.os.Looper;
 import android.os.Parcel;
 import android.os.RemoteException;
+import android.os.UserHandle;
 import android.util.ExceptionUtils;
 import android.util.Slog;
 import android.util.SparseArray;
@@ -66,7 +67,12 @@ public class VirtualDeviceManagerService extends SystemService {
    private VirtualDeviceManagerInternal mLocalService;
    private final Handler mHandler = new Handler(Looper.getMainLooper());
    private final PendingTrampolineMap mPendingTrampolines = new PendingTrampolineMap(mHandler);
-    private final CameraAccessController mCameraAccessController;
+    /**
+     * Mapping from user IDs to CameraAccessControllers.
+     */
+    @GuardedBy("mVirtualDeviceManagerLock")
+    private final SparseArray<CameraAccessController> mCameraAccessControllers =
+            new SparseArray<>();

    /**
     * Mapping from CDM association IDs to virtual devices. Only one virtual device is allowed for
@@ -94,8 +100,6 @@ public class VirtualDeviceManagerService extends SystemService {
        super(context);
        mImpl = new VirtualDeviceManagerImpl();
        mLocalService = new LocalService();
-        mCameraAccessController = new CameraAccessController(getContext(), mLocalService,
-                this::onCameraAccessBlocked);
    }

    private final ActivityInterceptorCallback mActivityInterceptorCallback =
@@ -144,16 +148,19 @@ public class VirtualDeviceManagerService extends SystemService {
    @Override
    public void onUserStarting(@NonNull TargetUser user) {
        super.onUserStarting(user);
+        Context userContext = getContext().createContextAsUser(user.getUserHandle(), 0);
        synchronized (mVirtualDeviceManagerLock) {
-            final CompanionDeviceManager cdm = getContext()
-                    .createContextAsUser(user.getUserHandle(), 0)
-                    .getSystemService(CompanionDeviceManager.class);
+            final CompanionDeviceManager cdm =
+                    userContext.getSystemService(CompanionDeviceManager.class);
            final int userId = user.getUserIdentifier();
            mAllAssociations.put(userId, cdm.getAllAssociations());
            OnAssociationsChangedListener listener =
                    associations -> mAllAssociations.put(userId, associations);
            mOnAssociationsChangedListeners.put(userId, listener);
            cdm.addOnAssociationsChangedListener(Runnable::run, listener);
+            CameraAccessController cameraAccessController = new CameraAccessController(
+                    userContext, mLocalService, this::onCameraAccessBlocked);
+            mCameraAccessControllers.put(user.getUserIdentifier(), cameraAccessController);
        }
    }

@@ -171,6 +178,14 @@ public class VirtualDeviceManagerService extends SystemService {
                cdm.removeOnAssociationsChangedListener(listener);
                mOnAssociationsChangedListeners.remove(userId);
            }
+            CameraAccessController cameraAccessController = mCameraAccessControllers.get(
+                    user.getUserIdentifier());
+            if (cameraAccessController != null) {
+                cameraAccessController.close();
+                mCameraAccessControllers.remove(user.getUserIdentifier());
+            } else {
+                Slog.w(TAG, "Cannot unregister cameraAccessController for user " + user);
+            }
        }
    }

@@ -198,7 +213,7 @@ public class VirtualDeviceManagerService extends SystemService {
                    android.Manifest.permission.CREATE_VIRTUAL_DEVICE,
                    "createVirtualDevice");
            final int callingUid = getCallingUid();
-            if (!PermissionUtils.validatePackageName(getContext(), packageName, callingUid)) {
+            if (!PermissionUtils.validateCallingPackageName(getContext(), packageName)) {
                throw new SecurityException(
                        "Package name " + packageName + " does not belong to calling uid "
                                + callingUid);
@@ -213,6 +228,9 @@ public class VirtualDeviceManagerService extends SystemService {
                            "Virtual device for association ID " + associationId
                                    + " already exists");
                }
+                final int userId = UserHandle.getUserId(callingUid);
+                final CameraAccessController cameraAccessController =
+                        mCameraAccessControllers.get(userId);
                VirtualDeviceImpl virtualDevice = new VirtualDeviceImpl(getContext(),
                        associationInfo, token, callingUid,
                        new VirtualDeviceImpl.OnDeviceCloseListener() {
@@ -220,14 +238,21 @@ public class VirtualDeviceManagerService extends SystemService {
                            public void onClose(int associationId) {
                                synchronized (mVirtualDeviceManagerLock) {
                                    mVirtualDevices.remove(associationId);
-                                    if (mVirtualDevices.size() == 0) {
-                                        mCameraAccessController.stopObserving();
+                                    if (cameraAccessController != null) {
+                                        cameraAccessController.stopObservingIfNeeded();
+                                    } else {
+                                        Slog.w(TAG, "cameraAccessController not found for user "
+                                                + userId);
                                    }
                                }
                            }
                        },
                        this, activityListener, params);
-                mCameraAccessController.startObservingIfNeeded();
+                if (cameraAccessController != null) {
+                    cameraAccessController.startObservingIfNeeded();
+                } else {
+                    Slog.w(TAG, "cameraAccessController not found for user " + userId);
+                }
                mVirtualDevices.put(associationInfo.getId(), virtualDevice);
                return virtualDevice;
            }
--- a/services/tests/servicestests/AndroidManifest.xml
+++ b/services/tests/servicestests/AndroidManifest.xml
@@ -98,6 +98,8 @@
    <uses-permission android:name="android.permission.MAINLINE_NETWORK_STACK"/>
    <uses-permission
        android:name="android.permission.OVERRIDE_COMPAT_CHANGE_CONFIG_ON_RELEASE_BUILD"/>
+    <uses-permission android:name="android.permission.ASSOCIATE_INPUT_DEVICE_TO_DISPLAY" />
+    <uses-permission android:name="android.permission.READ_NEARBY_STREAMING_POLICY" />

    <uses-permission android:name="android.permission.PACKAGE_VERIFICATION_AGENT" />

--- a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java
@@ -58,6 +58,7 @@ import android.os.IBinder;
 import android.os.IPowerManager;
 import android.os.IThermalService;
 import android.os.PowerManager;
+import android.os.Process;
 import android.os.RemoteException;
 import android.os.WorkSource;
 import android.platform.test.annotations.Presubmit;
@@ -136,6 +137,7 @@ public class VirtualDeviceManagerServiceTest {
        LocalServices.addService(DisplayManagerInternal.class, mDisplayManagerInternalMock);

        mContext = Mockito.spy(new ContextWrapper(InstrumentationRegistry.getTargetContext()));
+        doReturn(mContext).when(mContext).createContextAsUser(eq(Process.myUserHandle()), anyInt());
        doNothing().when(mContext).enforceCallingOrSelfPermission(
                eq(Manifest.permission.CREATE_VIRTUAL_DEVICE), anyString());
        when(mContext.getSystemService(Context.DEVICE_POLICY_SERVICE)).thenReturn(