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

Commit b976a8b4 authored by Grant Menke's avatar Grant Menke
Browse files

Unbind CS if connection is not created within 15 seconds. 

This CL adds a check to ensure that connection creation occurs within 15 seconds after binding to that ConnectionService. If the connection/conference is not created in that timespan, this CL adds logic to manually unbind the ConnectionService at that point in time. This prevents malicious apps from keeping a declared permission in forever even in the background. This updated CL includes a null check to avoid a NPE that occurred in the first iteration of this change.

Bug: 293458004
Test: manually using the provided apk + atest CallsManagerTest
Flag: EXEMPT Security High/Critical Severity CVE
Change-Id: If46cfa26278f09854c10266af6eaa73382f20296
(cherry picked from commit 48d6b0df)
Merged-In: If46cfa26278f09854c10266af6eaa73382f20296
parent ccf52418

src/com/android/server/telecom/Call.java

+26 −0
Original line number Diff line number Diff line
@@ -353,6 +353,17 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable, 
    /** The state of the call. */

    private int mState;



    /**

     * Determines whether the {@link ConnectionService} has responded to the initial request to

     * create the connection.

     *

     * {@code false} indicates the {@link Call} has been added to Telecom, but the

     * {@link Connection} has not yet been returned by the associated {@link ConnectionService}.

     * {@code true} indicates the {@link Call} has an associated {@link Connection} reported by the

     * {@link ConnectionService}.

     */

    private boolean mIsCreateConnectionComplete = false;



    /** The handle with which to establish this call. */

    private Uri mHandle;
@@ -1038,6 +1049,19 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable, 
        return mConnectionService;

    }



    /**

     * @return {@code true} if the connection has been created by the underlying

     * {@link ConnectionService}, {@code false} otherwise.

     */

    public boolean isCreateConnectionComplete() {

        return mIsCreateConnectionComplete;

    }



    @VisibleForTesting

    public void setIsCreateConnectionComplete(boolean isCreateConnectionComplete) {

        mIsCreateConnectionComplete = isCreateConnectionComplete;

    }



    @VisibleForTesting

    public int getState() {

        return mState;
@@ -2189,6 +2213,7 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable, 
            CallIdMapper idMapper,

            ParcelableConference conference) {

        Log.v(this, "handleCreateConferenceSuccessful %s", conference);

        mIsCreateConnectionComplete = true;

        setTargetPhoneAccount(conference.getPhoneAccount());

        setHandle(conference.getHandle(), conference.getHandlePresentation());
@@ -2222,6 +2247,7 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable, 
            CallIdMapper idMapper,

            ParcelableConnection connection) {

        Log.v(this, "handleCreateConnectionSuccessful %s", connection);

        mIsCreateConnectionComplete = true;

        setTargetPhoneAccount(connection.getPhoneAccount());

        setHandle(connection.getHandle(), connection.getHandlePresentation());

        setCallerDisplayName(

src/com/android/server/telecom/ConnectionServiceWrapper.java

100644 → 100755
+84 −4
Original line number Diff line number Diff line
@@ -40,6 +40,7 @@ import android.telecom.DisconnectCause; 
import android.telecom.GatewayInfo;

import android.telecom.Log;

import android.telecom.Logging.Session;

import android.telecom.Logging.Runnable;

import android.telecom.ParcelableConference;

import android.telecom.ParcelableConnection;

import android.telecom.PhoneAccountHandle;
@@ -61,6 +62,11 @@ import java.util.List; 
import java.util.Map;

import java.util.Set;

import java.util.concurrent.ConcurrentHashMap;

import java.util.concurrent.ExecutorService;

import java.util.concurrent.Executors;

import java.util.concurrent.ScheduledExecutorService;

import java.util.concurrent.ScheduledFuture;

import java.util.concurrent.TimeUnit;



/**

 * Wrapper for {@link IConnectionService}s, handles binding to {@link IConnectionService} and keeps
@@ -73,6 +79,11 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
        ConnectionServiceFocusManager.ConnectionServiceFocus {



    private static final String TELECOM_ABBREVIATION = "cast";

    private static final long SERVICE_BINDING_TIMEOUT = 15000L;

    private ScheduledExecutorService mScheduledExecutor =

            Executors.newSingleThreadScheduledExecutor();

    // Pre-allocate space for 2 calls; realistically thats all we should ever need (tm)

    private final Map<Call, ScheduledFuture<?>> mScheduledFutureMap = new ConcurrentHashMap<>(2);



    private final class Adapter extends IConnectionServiceAdapter.Stub {
@@ -86,6 +97,12 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
            try {

                synchronized (mLock) {

                    logIncoming("handleCreateConnectionComplete %s", callId);

                    Call call = mCallIdMapper.getCall(callId);

                    if (call != null && mScheduledFutureMap.containsKey(call)) {

                        ScheduledFuture<?> existingTimeout = mScheduledFutureMap.get(call);

                        existingTimeout.cancel(false /* cancelIfRunning */);

                        mScheduledFutureMap.remove(call);

                    }

                    // Check status hints image for cross user access

                    if (connection.getStatusHints() != null) {

                        Icon icon = connection.getStatusHints().getIcon();
@@ -124,6 +141,12 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
            try {

                synchronized (mLock) {

                    logIncoming("handleCreateConferenceComplete %s", callId);

                    Call call = mCallIdMapper.getCall(callId);

                    if (call != null && mScheduledFutureMap.containsKey(call)) {

                        ScheduledFuture<?> existingTimeout = mScheduledFutureMap.get(call);

                        existingTimeout.cancel(false /* cancelIfRunning */);

                        mScheduledFutureMap.remove(call);

                    }

                    // Check status hints image for cross user access

                    if (conference.getStatusHints() != null) {

                        Icon icon = conference.getStatusHints().getIcon();
@@ -1232,7 +1255,8 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
     * @param context The context.

     * @param userHandle The {@link UserHandle} to use when binding.

     */

    ConnectionServiceWrapper(

    @VisibleForTesting

    public ConnectionServiceWrapper(

            ComponentName componentName,

            ConnectionServiceRepository connectionServiceRepository,

            PhoneAccountRegistrar phoneAccountRegistrar,
@@ -1310,7 +1334,26 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
                        .setParticipants(call.getParticipants())

                        .setIsAdhocConferenceCall(call.isAdhocConferenceCall())

                        .build();



                Runnable r = new Runnable("CSW.cC", mLock) {

                    @Override

                    public void loggedRun() {

                        if (!call.isCreateConnectionComplete()) {

                            Log.e(this, new Exception(),

                                    "Conference %s creation timeout",

                                    getComponentName());

                            Log.addEvent(call, LogUtils.Events.CREATE_CONFERENCE_TIMEOUT,

                                    Log.piiHandle(call.getHandle()) + " via:" +

                                            getComponentName().getPackageName());

                            response.handleCreateConferenceFailure(

                                    new DisconnectCause(DisconnectCause.ERROR));

                        }

                    }

                };

                // Post cleanup to the executor service and cache the future, so we can cancel it if

                // needed.

                ScheduledFuture<?> future = mScheduledExecutor.schedule(r.getRunnableToCancel(),

                        SERVICE_BINDING_TIMEOUT, TimeUnit.MILLISECONDS);

                mScheduledFutureMap.put(call, future);

                try {

                    mServiceInterface.createConference(

                            call.getConnectionManagerPhoneAccount(),
@@ -1407,7 +1450,26 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
                        .setRttPipeFromInCall(call.getInCallToCsRttPipeForCs())

                        .setRttPipeToInCall(call.getCsToInCallRttPipeForCs())

                        .build();



                Runnable r = new Runnable("CSW.cC", mLock) {

                    @Override

                    public void loggedRun() {

                        if (!call.isCreateConnectionComplete()) {

                            Log.e(this, new Exception(),

                                    "Connection %s creation timeout",

                                    getComponentName());

                            Log.addEvent(call, LogUtils.Events.CREATE_CONNECTION_TIMEOUT,

                                    Log.piiHandle(call.getHandle()) + " via:" +

                                            getComponentName().getPackageName());

                            response.handleCreateConnectionFailure(

                                    new DisconnectCause(DisconnectCause.ERROR));

                        }

                    }

                };

                // Post cleanup to the executor service and cache the future, so we can cancel it if

                // needed.

                ScheduledFuture<?> future = mScheduledExecutor.schedule(r.getRunnableToCancel(),

                        SERVICE_BINDING_TIMEOUT, TimeUnit.MILLISECONDS);

                mScheduledFutureMap.put(call, future);

                try {

                    mServiceInterface.createConnection(

                            call.getConnectionManagerPhoneAccount(),
@@ -1817,7 +1879,8 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
        }

    }



    void addCall(Call call) {

    @VisibleForTesting

    public void addCall(Call call) {

        if (mCallIdMapper.getCallId(call) == null) {

            mCallIdMapper.addCall(call);

        }
@@ -2033,6 +2096,13 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
    @Override

    protected void removeServiceInterface() {

        Log.v(this, "Removing Connection Service Adapter.");

        if (mServiceInterface == null) {

            // In some cases, we may receive multiple calls to

            // remoteServiceInterface, such as when the remote process crashes

            // (onBinderDied & onServiceDisconnected)

            Log.w(this, "removeServiceInterface: mServiceInterface is null");

            return;

        }

        removeConnectionServiceAdapter(mAdapter);

        // We have lost our service connection. Notify the world that this service is done.

        // We must notify the adapter before CallsManager. The adapter will force any pending
@@ -2041,6 +2111,10 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
        handleConnectionServiceDeath();

        mCallsManager.handleConnectionServiceDeath(this);

        mServiceInterface = null;

        if (mScheduledExecutor != null) {

            mScheduledExecutor.shutdown();

            mScheduledExecutor = null;

        }

    }



    @Override
@@ -2159,6 +2233,7 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
            }

        }

        mCallIdMapper.clear();

        mScheduledFutureMap.clear();



        if (mConnSvrFocusListener != null) {

            mConnSvrFocusListener.onConnectionServiceDeath(this);
@@ -2284,4 +2359,9 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
        sb.append("]");

        return sb.toString();

    }



    @VisibleForTesting

    public void setScheduledExecutorService(ScheduledExecutorService service) {

        mScheduledExecutor = service;

    }

}

src/com/android/server/telecom/LogUtils.java

+2 −0
Original line number Diff line number Diff line
@@ -126,8 +126,10 @@ public class LogUtils { 
        public static final String STOP_CALL_WAITING_TONE = "STOP_CALL_WAITING_TONE";

        public static final String START_CONNECTION = "START_CONNECTION";

        public static final String CREATE_CONNECTION_FAILED = "CREATE_CONNECTION_FAILED";

        public static final String CREATE_CONNECTION_TIMEOUT = "CREATE_CONNECTION_TIMEOUT";

        public static final String START_CONFERENCE = "START_CONFERENCE";

        public static final String CREATE_CONFERENCE_FAILED = "CREATE_CONFERENCE_FAILED";

        public static final String CREATE_CONFERENCE_TIMEOUT = "CREATE_CONFERENCE_TIMEOUT";

        public static final String BIND_CS = "BIND_CS";

        public static final String CS_BOUND = "CS_BOUND";

        public static final String CONFERENCE_WITH = "CONF_WITH";

tests/src/com/android/server/telecom/tests/BasicCallTests.java

+2 −0
Original line number Diff line number Diff line
@@ -978,6 +978,7 @@ public class BasicCallTests extends TelecomSystemTest { 
        call.setTargetPhoneAccount(mPhoneAccountA1.getAccountHandle());

        assert(call.isVideoCallingSupportedByPhoneAccount());

        assertEquals(VideoProfile.STATE_BIDIRECTIONAL, call.getVideoState());

        call.setIsCreateConnectionComplete(true);

    }



    /**
@@ -1001,6 +1002,7 @@ public class BasicCallTests extends TelecomSystemTest { 
        call.setTargetPhoneAccount(mPhoneAccountA2.getAccountHandle());

        assert(!call.isVideoCallingSupportedByPhoneAccount());

        assertEquals(VideoProfile.STATE_AUDIO_ONLY, call.getVideoState());

        call.setIsCreateConnectionComplete(true);

    }



    /**

tests/src/com/android/server/telecom/tests/CallsManagerTest.java

+55 −0
Original line number Diff line number Diff line
@@ -42,6 +42,7 @@ import static org.mockito.Mockito.timeout; 
import static org.mockito.Mockito.times;

import static org.mockito.Mockito.verify;

import static org.mockito.Mockito.when;

import static java.lang.Thread.sleep;



import android.content.ComponentName;

import android.content.ContentResolver;
@@ -50,6 +51,7 @@ import android.content.Intent; 
import android.net.Uri;

import android.os.Bundle;

import android.os.Handler;

import android.os.IBinder;

import android.os.Looper;

import android.os.Process;

import android.os.SystemClock;
@@ -71,6 +73,7 @@ import android.test.suitebuilder.annotation.SmallTest; 
import android.util.Pair;

import android.widget.Toast;



import com.android.internal.telecom.IConnectionService;

import com.android.server.telecom.AsyncRingtonePlayer;

import com.android.server.telecom.Call;

import com.android.server.telecom.CallAudioManager;
@@ -85,6 +88,7 @@ import com.android.server.telecom.ClockProxy; 
import com.android.server.telecom.ConnectionServiceFocusManager;

import com.android.server.telecom.ConnectionServiceFocusManager.ConnectionServiceFocusManagerFactory;

import com.android.server.telecom.ConnectionServiceWrapper;

import com.android.server.telecom.CreateConnectionResponse;

import com.android.server.telecom.DefaultDialerCache;

import com.android.server.telecom.EmergencyCallHelper;

import com.android.server.telecom.HeadsetMediaButton;
@@ -216,6 +220,7 @@ public class CallsManagerTest extends TelecomTestCase { 
    @Mock private RoleManagerAdapter mRoleManagerAdapter;

    @Mock private ToastFactory mToastFactory;

    @Mock private Toast mToast;

    @Mock private IConnectionService mIConnectionService;



    private CallsManager mCallsManager;
@@ -283,11 +288,17 @@ public class CallsManagerTest extends TelecomTestCase { 
                eq(SIM_2_HANDLE), any())).thenReturn(SIM_2_ACCOUNT);

        when(mToastFactory.makeText(any(), anyInt(), anyInt())).thenReturn(mToast);

        when(mToastFactory.makeText(any(), any(), anyInt())).thenReturn(mToast);

        when(mIConnectionService.asBinder()).thenReturn(mock(IBinder.class));



        mComponentContextFixture.addConnectionService(

                SIM_1_ACCOUNT.getAccountHandle().getComponentName(), mIConnectionService);

    }



    @Override

    @After

    public void tearDown() throws Exception {

        mComponentContextFixture.removeConnectionService(

                SIM_1_ACCOUNT.getAccountHandle().getComponentName(), mIConnectionService);

        super.tearDown();

    }
@@ -1649,6 +1660,35 @@ public class CallsManagerTest extends TelecomTestCase { 
        assertTrue(argumentCaptor.getValue().contains("Unavailable phoneAccountHandle"));

    }



    @Test

    public void testConnectionServiceCreateConnectionTimeout() throws Exception {

        ConnectionServiceWrapper service = new ConnectionServiceWrapper(

                SIM_1_ACCOUNT.getAccountHandle().getComponentName(), null,

                mPhoneAccountRegistrar, mCallsManager, mContext, mLock, null);

        TestScheduledExecutorService scheduledExecutorService = new TestScheduledExecutorService();

        service.setScheduledExecutorService(scheduledExecutorService);

        Call call = addSpyCall();

        service.addCall(call);

        when(call.isCreateConnectionComplete()).thenReturn(false);

        CreateConnectionResponse response = mock(CreateConnectionResponse.class);



        service.createConnection(call, response);

        waitUntilConditionIsTrueOrTimeout(new Condition() {

            @Override

            public Object expected() {

                return true;

            }



            @Override

            public Object actual() {

                return scheduledExecutorService.isRunnableScheduledAtTime(15000L);

            }

        }, 5000L, "Expected job failed to schedule");

        scheduledExecutorService.advanceTime(15000L);

        verify(response).handleCreateConnectionFailure(

                eq(new DisconnectCause(DisconnectCause.ERROR)));

    }



    private Call addSpyCall() {

        return addSpyCall(SIM_2_HANDLE, CallState.ACTIVE);

    }
@@ -1741,4 +1781,19 @@ public class CallsManagerTest extends TelecomTestCase { 
        when(mPhoneAccountRegistrar.getSimPhoneAccountsOfCurrentUser()).thenReturn(

                new ArrayList<>(Arrays.asList(SIM_1_HANDLE, SIM_2_HANDLE)));

    }



    private void waitUntilConditionIsTrueOrTimeout(Condition condition, long timeout,

            String description) throws InterruptedException {

        final long start = System.currentTimeMillis();

        while (!condition.expected().equals(condition.actual())

                && System.currentTimeMillis() - start < timeout) {

            sleep(50);

        }

        assertEquals(description, condition.expected(), condition.actual());

    }



    protected interface Condition {

        Object expected();

        Object actual();

    }

}