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

Commit 7aa55ffc 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.

Bug: 293458004
Test: manually using the provided apk + atest CallsManagerTest
Flag: EXEMPT Security High/Critical Severity CVE
Change-Id: I30caed1481dff5af2223a8ff589846597cee8229
parent 7dfbdf60

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

+92 −2
Original line number Diff line number Diff line
@@ -44,6 +44,7 @@ import android.telecom.ConnectionService; 
import android.telecom.DisconnectCause;

import android.telecom.GatewayInfo;

import android.telecom.Log;

import android.telecom.Logging.Runnable;

import android.telecom.Logging.Session;

import android.telecom.ParcelableConference;

import android.telecom.ParcelableConnection;
@@ -73,10 +74,13 @@ import java.util.HashMap; 
import java.util.List;

import java.util.Map;

import java.util.Set;

import java.util.UUID;

import java.util.concurrent.CompletableFuture;

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;

import java.util.Objects;
@@ -90,11 +94,28 @@ import java.util.Objects; 
public class ConnectionServiceWrapper extends ServiceBinder implements

        ConnectionServiceFocusManager.ConnectionServiceFocus, CallSourceService {



    /**

     * Anomaly Report UUIDs and corresponding error descriptions specific to CallsManager.

     */

    public static final UUID CREATE_CONNECTION_TIMEOUT_ERROR_UUID =

            UUID.fromString("54b7203d-a79f-4cbd-b639-85cd93a39cbb");

    public static final String CREATE_CONNECTION_TIMEOUT_ERROR_MSG =

            "Timeout expired before Telecom connection was created.";

    public static final UUID CREATE_CONFERENCE_TIMEOUT_ERROR_UUID =

            UUID.fromString("caafe5ea-2472-4c61-b2d8-acb9d47e13dd");

    public static final String CREATE_CONFERENCE_TIMEOUT_ERROR_MSG =

            "Timeout expired before Telecom conference was created.";



    private static final String TELECOM_ABBREVIATION = "cast";

    private static final long SERVICE_BINDING_TIMEOUT = 15000L;

    private CompletableFuture<Pair<Integer, Location>> mQueryLocationFuture = null;

    private @Nullable CancellationSignal mOngoingQueryLocationRequest = null;

    private final ExecutorService mQueryLocationExecutor = Executors.newSingleThreadExecutor();



    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 AnomalyReporterAdapter mAnomalyReporter = new AnomalyReporterAdapterImpl();

    private final class Adapter extends IConnectionServiceAdapter.Stub {



        @Override
@@ -107,6 +128,12 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
            try {

                synchronized (mLock) {

                    logIncoming("handleCreateConnectionComplete %s", callId);

                    Call call = mCallIdMapper.getCall(callId);

                    if (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();
@@ -144,6 +171,12 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
            try {

                synchronized (mLock) {

                    logIncoming("handleCreateConferenceComplete %s", callId);

                    Call call = mCallIdMapper.getCall(callId);

                    if (mScheduledFutureMap.containsKey(call)) {

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

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

                        mScheduledFutureMap.remove(call);

                    }

                    ConnectionServiceWrapper.this

                            .handleCreateConferenceComplete(callId, request, conference);
@@ -1599,6 +1632,29 @@ 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());

                                    mAnomalyReporter.reportAnomaly(

                                            CREATE_CONFERENCE_TIMEOUT_ERROR_UUID,

                                            CREATE_CONFERENCE_TIMEOUT_ERROR_MSG);

                                    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(),
@@ -1699,6 +1755,29 @@ 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());

                                    mAnomalyReporter.reportAnomaly(

                                            CREATE_CONNECTION_TIMEOUT_ERROR_UUID,

                                            CREATE_CONNECTION_TIMEOUT_ERROR_MSG);

                                    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(),
@@ -2160,7 +2239,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);

        }
@@ -2628,4 +2708,14 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
        sb.append("]");

        return sb.toString();

    }



    @VisibleForTesting

    public void setScheduledExecutorService(ScheduledExecutorService service) {

        mScheduledExecutor = service;

    }



    @VisibleForTesting

    public void setAnomalyReporterAdapter(AnomalyReporterAdapter mAnomalyReporterAdapter){

        mAnomalyReporter = mAnomalyReporterAdapter;

    }

}

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

+2 −0
Original line number Diff line number Diff line
@@ -139,8 +139,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
@@ -1036,6 +1036,7 @@ public class BasicCallTests extends TelecomSystemTest { 
        call.setTargetPhoneAccount(mPhoneAccountA1.getAccountHandle());

        assert(call.isVideoCallingSupportedByPhoneAccount());

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

        call.setIsCreateConnectionComplete(true);

    }



    /**
@@ -1059,6 +1060,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

+54 −0
Original line number Diff line number Diff line
@@ -58,6 +58,7 @@ import android.media.AudioManager; 
import android.net.Uri;

import android.os.Bundle;

import android.os.Handler;

import android.os.IBinder;

import android.os.Looper;

import android.os.OutcomeReceiver;

import android.os.Process;
@@ -86,6 +87,7 @@ import android.widget.Toast; 
import androidx.test.filters.MediumTest;

import androidx.test.filters.SmallTest;



import com.android.internal.telecom.IConnectionService;

import com.android.server.telecom.AnomalyReporterAdapter;

import com.android.server.telecom.AsyncRingtonePlayer;

import com.android.server.telecom.Call;
@@ -104,6 +106,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.EmergencyCallDiagnosticLogger;

import com.android.server.telecom.EmergencyCallHelper;
@@ -314,6 +317,7 @@ public class CallsManagerTest extends TelecomTestCase { 
    @Mock private IncomingCallFilterGraph mIncomingCallFilterGraph;

    @Mock private Context mMockCreateContextAsUser;

    @Mock private UserManager mMockCurrentUserManager;

    @Mock private IConnectionService mIConnectionService;

    private CallsManager mCallsManager;



    @Override
@@ -411,11 +415,17 @@ public class CallsManagerTest extends TelecomTestCase { 
                .thenReturn(mMockCreateContextAsUser);

        when(mMockCreateContextAsUser.getSystemService(UserManager.class))

                .thenReturn(mMockCurrentUserManager);

        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();

    }
@@ -3126,6 +3136,35 @@ public class CallsManagerTest extends TelecomTestCase { 
        assertTrue(result.contains("onReceiveResult"));

    }



    @Test

    public void testConnectionServiceCreateConnectionTimeout() throws Exception {

        ConnectionServiceWrapper service = new ConnectionServiceWrapper(

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

                mPhoneAccountRegistrar, mCallsManager, mContext, mLock, null, mFeatureFlags);

        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)));

    }



    @SmallTest

    @Test

    public void testOnFailedOutgoingCallUnholdsCallAfterLocallyDisconnect() {
@@ -3778,4 +3817,19 @@ public class CallsManagerTest extends TelecomTestCase { 
        when(mockTelephonyManager.getPhoneCapability()).thenReturn(mPhoneCapability);

        when(mPhoneCapability.getMaxActiveVoiceSubscriptions()).thenReturn(num);

    }



    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();

    }

}