Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 725ae979 authored by Vladimir Komsiyski's avatar Vladimir Komsiyski
Browse files

Require unique ComputerControlSession names 

And fail early instead of bringing down system server

Bug: 443074681
Test: presubmit
Flag: android.companion.virtualdevice.flags.computer_control_access
Change-Id: Ia820f4c9e53028dac480792a3dfddedc1b1b4875
parent ff09c42b

services/companion/java/com/android/server/companion/virtual/computercontrol/ComputerControlSessionImpl.java

+18 −5
Original line number Diff line number Diff line
@@ -87,6 +87,7 @@ final class ComputerControlSessionImpl extends IComputerControlSession.Stub 


    private final IBinder mAppToken;

    private final ComputerControlSessionParams mParams;

    private final String mOwnerPackageName;

    private final OnClosedListener mOnClosedListener;

    private final IVirtualDevice mVirtualDevice;

    private final int mVirtualDisplayId;
@@ -110,6 +111,7 @@ final class ComputerControlSessionImpl extends IComputerControlSession.Stub 
            OnClosedListener onClosedListener, Injector injector) {

        mAppToken = appToken;

        mParams = params;

        mOwnerPackageName = attributionSource.getPackageName();

        mOnClosedListener = onClosedListener;

        mInjector = injector;
@@ -174,7 +176,10 @@ final class ComputerControlSessionImpl extends IComputerControlSession.Stub 
            mVirtualDevice.setDisplayImePolicy(

                    mVirtualDisplayId, WindowManager.DISPLAY_IME_POLICY_HIDE);



            final String dpadName = mParams.getName() + "-dpad";

            final String inputDeviceNamePrefix =

                    attributionSource.getPackageName() + ":" + mParams.getName();



            final String dpadName = inputDeviceNamePrefix + "-dpad";

            final VirtualDpadConfig virtualDpadConfig =

                    new VirtualDpadConfig.Builder()

                            .setAssociatedDisplayId(mVirtualDisplayId)
@@ -183,7 +188,7 @@ final class ComputerControlSessionImpl extends IComputerControlSession.Stub 
            mVirtualDpad = mVirtualDevice.createVirtualDpad(

                    virtualDpadConfig, new Binder(dpadName));



            final String keyboardName = mParams.getName()  + "-keyboard";

            final String keyboardName = inputDeviceNamePrefix + "-keyboard";

            final VirtualKeyboardConfig virtualKeyboardConfig =

                    new VirtualKeyboardConfig.Builder()

                            .setAssociatedDisplayId(mVirtualDisplayId)
@@ -192,7 +197,7 @@ final class ComputerControlSessionImpl extends IComputerControlSession.Stub 
            mVirtualKeyboard = mVirtualDevice.createVirtualKeyboard(

                    virtualKeyboardConfig, new Binder(keyboardName));



            final String touchscreenName = mParams.getName() + "-touchscreen";

            final String touchscreenName = inputDeviceNamePrefix + "-touchscreen";

            final VirtualTouchscreenConfig virtualTouchscreenConfig =

                    new VirtualTouchscreenConfig.Builder(mDisplayWidth, mDisplayHeight)

                            .setAssociatedDisplayId(mVirtualDisplayId)
@@ -242,6 +247,14 @@ final class ComputerControlSessionImpl extends IComputerControlSession.Stub 
        return mVirtualDisplayToken;

    }



    String getName() {

        return mParams.getName();

    }



    String getOwnerPackageName() {

        return mOwnerPackageName;

    }



    public void launchApplication(@NonNull String packageName) {

        if (!mParams.getTargetPackageNames().contains(Objects.requireNonNull(packageName))) {

            throw new IllegalArgumentException(
@@ -309,7 +322,7 @@ final class ComputerControlSessionImpl extends IComputerControlSession.Stub 
    public void close() throws RemoteException {

        mVirtualDevice.close();

        mAppToken.unlinkToDeath(this, 0);

        mOnClosedListener.onClosed(asBinder());

        mOnClosedListener.onClosed(this);

    }



    @Override
@@ -383,7 +396,7 @@ final class ComputerControlSessionImpl extends IComputerControlSession.Stub 


    /** Interface for listening for closing of sessions. */

    interface OnClosedListener {

        void onClosed(IBinder token);

        void onClosed(ComputerControlSessionImpl session);

    }



    @VisibleForTesting

services/companion/java/com/android/server/companion/virtual/computercontrol/ComputerControlSessionProcessor.java

+20 −6
Original line number Diff line number Diff line
@@ -54,6 +54,8 @@ import com.android.internal.annotations.GuardedBy; 
import com.android.internal.annotations.VisibleForTesting;

import com.android.server.ServiceThread;



import java.util.Objects;



/**

 * Handles creation and lifecycle of {@link ComputerControlSession}s.

 *
@@ -76,7 +78,7 @@ public class ComputerControlSessionProcessor { 
    private final PendingIntentFactory mPendingIntentFactory;



    /** The binders of all currently active sessions. */

    private final ArraySet<IBinder> mSessions = new ArraySet<>();

    private final ArraySet<ComputerControlSessionImpl> mSessions = new ArraySet<>();



    private final Object mHandlerThreadLock = new Object();

    @GuardedBy("mHandlerThreadLock")
@@ -111,7 +113,7 @@ public class ComputerControlSessionProcessor { 
            @NonNull AttributionSource attributionSource,

            @NonNull ComputerControlSessionParams params,

            @NonNull IComputerControlSessionCallback callback) {

        validateParams(params);

        validateParams(attributionSource, params);

        startHandlerThreadIfNeeded();



        final boolean canCreateWithoutConsent;
@@ -150,7 +152,19 @@ public class ComputerControlSessionProcessor { 
        }

    }



    private void validateParams(ComputerControlSessionParams params) {

    private void validateParams(AttributionSource attributionSource,

            ComputerControlSessionParams params) {

        synchronized (mSessions) {

            for (int i = 0; i < mSessions.size(); i++) {

                ComputerControlSessionImpl session = mSessions.valueAt(i);

                if (Objects.equals(attributionSource.getPackageName(),

                        session.getOwnerPackageName())

                        && Objects.equals(params.getName(), session.getName())) {

                    throw new IllegalArgumentException("Session name must be unique");

                }

            }

        }



        if (!Flags.computerControlActivityPolicyStrict()) {

            return;

        }
@@ -207,7 +221,7 @@ public class ComputerControlSessionProcessor { 
                    callback.asBinder(), params, attributionSource, mVirtualDeviceFactory,

                    new OnSessionClosedListener(params.getName(), callback),

                    new ComputerControlSessionImpl.Injector(mContext));

            mSessions.add(session.asBinder());

            mSessions.add(session);

        }



        // If the client provided a surface, disable the screenshot API.
@@ -321,9 +335,9 @@ public class ComputerControlSessionProcessor { 
        }



        @Override

        public void onClosed(IBinder token) {

        public void onClosed(ComputerControlSessionImpl session) {

            synchronized (mSessions) {

                if (!mSessions.remove(token)) {

                if (!mSessions.remove(session)) {

                    // The session was already removed, which can happen if close() is called

                    // multiple times.

                    return;

services/tests/servicestests/src/com/android/server/companion/virtual/computercontrol/ComputerControlSessionProcessorTest.java

+25 −3
Original line number Diff line number Diff line
@@ -157,14 +157,14 @@ public class ComputerControlSessionProcessorTest { 
        try {

            for (int i = 0; i < MAXIMUM_CONCURRENT_SESSIONS; ++i) {

                mProcessor.processNewSessionRequest(AttributionSource.myAttributionSource(),

                        mParams, mComputerControlSessionCallback);

                        generateUniqueParams(i), mComputerControlSessionCallback);

            }

            verify(mComputerControlSessionCallback,

                    timeout(CALLBACK_TIMEOUT_MS).times(MAXIMUM_CONCURRENT_SESSIONS))

                    .onSessionCreated(anyInt(), any(), mSessionArgumentCaptor.capture());



            mProcessor.processNewSessionRequest(AttributionSource.myAttributionSource(),

                    mParams, mComputerControlSessionCallback);

                    generateUniqueParams(-1), mComputerControlSessionCallback);

            verify(mComputerControlSessionCallback, timeout(CALLBACK_TIMEOUT_MS))

                    .onSessionCreationFailed(ComputerControlSession.ERROR_SESSION_LIMIT_REACHED);
@@ -175,7 +175,7 @@ public class ComputerControlSessionProcessorTest { 
            verify(mComputerControlSessionCallback, times(1)).onSessionClosed();



            mProcessor.processNewSessionRequest(AttributionSource.myAttributionSource(),

                    mParams, mComputerControlSessionCallback);

                    generateUniqueParams(-1), mComputerControlSessionCallback);

            verify(mComputerControlSessionCallback,

                    timeout(CALLBACK_TIMEOUT_MS).times(MAXIMUM_CONCURRENT_SESSIONS + 1))

                    .onSessionCreated(anyInt(), any(), mSessionArgumentCaptor.capture());
@@ -224,6 +224,17 @@ public class ComputerControlSessionProcessorTest { 
                .onSessionCreationFailed(ComputerControlSession.ERROR_PERMISSION_DENIED);

    }



    @Test

    public void validateParams_sessionNameMustBeUnique() throws Exception {

        mProcessor.processNewSessionRequest(AttributionSource.myAttributionSource(),

                mParams, mComputerControlSessionCallback);

        verify(mComputerControlSessionCallback, timeout(CALLBACK_TIMEOUT_MS))

                .onSessionCreated(anyInt(), any(), any());

        assertThrows(IllegalArgumentException.class,

                () -> mProcessor.processNewSessionRequest(AttributionSource.myAttributionSource(),

                mParams, mComputerControlSessionCallback));

    }



    @EnableFlags(Flags.FLAG_COMPUTER_CONTROL_ACTIVITY_POLICY_STRICT)

    @Test

    public void validateParams_packageNamesAreValid() throws Exception {
@@ -276,4 +287,15 @@ public class ComputerControlSessionProcessorTest { 
                    params, mComputerControlSessionCallback);

        });

    }



    private ComputerControlSessionParams generateUniqueParams(int index) {

        return new ComputerControlSessionParams.Builder()

                .setName(mParams.getName() + index)

                .setDisplayDpi(mParams.getDisplayDpi())

                .setDisplayHeightPx(mParams.getDisplayHeightPx())

                .setDisplayWidthPx(mParams.getDisplayWidthPx())

                .setDisplaySurface(mParams.getDisplaySurface())

                .setDisplayAlwaysUnlocked(mParams.isDisplayAlwaysUnlocked())

                .build();

    }

}

services/tests/servicestests/src/com/android/server/companion/virtual/computercontrol/ComputerControlSessionTest.java

+1 −1
Original line number Diff line number Diff line
@@ -225,7 +225,7 @@ public class ComputerControlSessionTest { 
        createComputerControlSession(mDefaultParams);

        mSession.close();

        verify(mVirtualDevice).close();

        verify(mOnClosedListener).onClosed(mSession.asBinder());

        verify(mOnClosedListener).onClosed(mSession);

    }



    @Test