DO NOT MERGE 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
(cherry picked from commit 7aa55ffca6)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:3706488d7f0e63feb560ef180fe6e83a52e80834)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:635e2feeb8ee6f84786e4c405c1e8fa39e6743cb)
Merged-In: I30caed1481dff5af2223a8ff589846597cee8229
Change-Id: I30caed1481dff5af2223a8ff589846597cee8229

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

@@ -1134,6 +1134,7 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable,
         return (!mIsTransactionalCall ? mConnectionService : mTransactionalService);
     }
 
+    @VisibleForTesting
     public int getState() {
         return mState;
     }

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

@@ -45,6 +45,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,12 +74,14 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.Objects;
 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;
 
 /**
  * Wrapper for {@link IConnectionService}s, handles binding to {@link IConnectionService} and keeps
@@ -95,6 +98,11 @@ public class ConnectionServiceWrapper extends ServiceBinder implements
     private @Nullable CancellationSignal mOngoingQueryLocationRequest = null;
     private final ExecutorService mQueryLocationExecutor = Executors.newSingleThreadExecutor();
 
+    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 {
 
         @Override
@@ -107,6 +115,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 +158,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);
 
@@ -1597,6 +1617,26 @@ public class ConnectionServiceWrapper extends ServiceBinder implements
                         .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);
                 if (mServiceInterface != null) {
                     try {
                         mServiceInterface.createConference(
@@ -1705,6 +1745,26 @@ public class ConnectionServiceWrapper extends ServiceBinder implements
                         .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);
                 if (mServiceInterface != null) {
                     try {
                         mServiceInterface.createConnection(
@@ -2181,7 +2241,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);
         }
@@ -2658,4 +2719,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

@@ -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

@@ -995,6 +995,7 @@ public class BasicCallTests extends TelecomSystemTest {
         call.setTargetPhoneAccount(mPhoneAccountA1.getAccountHandle());
         assert(call.isVideoCallingSupportedByPhoneAccount());
         assertEquals(VideoProfile.STATE_BIDIRECTIONAL, call.getVideoState());
+        call.setIsCreateConnectionComplete(true);
     }
 
     /**
@@ -1018,6 +1019,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

@@ -43,6 +43,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.Manifest;
 import android.content.ComponentName;
@@ -54,6 +55,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;
@@ -80,6 +82,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.AnomalyReporterAdapter;
 import com.android.server.telecom.AsyncRingtonePlayer;
 import com.android.server.telecom.Call;
@@ -97,6 +100,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;
@@ -277,6 +281,7 @@ public class CallsManagerTest extends TelecomTestCase {
     @Mock private BlockedNumbersAdapter mBlockedNumbersAdapter;
     @Mock private PhoneCapability mPhoneCapability;
     @Mock private CallStreamingNotification mCallStreamingNotification;
+    @Mock private IConnectionService mIConnectionService;
 
     private CallsManager mCallsManager;
 
@@ -362,11 +367,19 @@ public class CallsManagerTest extends TelecomTestCase {
                 eq(WORK_HANDLE), any())).thenReturn(WORK_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(new ComponentName(mContext.getPackageName(),
+                mContext.getPackageName().getClass().getName()), mIConnectionService);
     }
 
     @Override
     @After
     public void tearDown() throws Exception {
+        mComponentContextFixture.removeConnectionService(
+                new ComponentName(mContext.getPackageName(),
+                        mContext.getPackageName().getClass().getName()),
+                mock(IConnectionService.class));
         super.tearDown();
     }
 
@@ -3252,6 +3265,31 @@ public class CallsManagerTest extends TelecomTestCase {
         assertTrue(mCallsManager.isInSelfManagedCall(TEST_PACKAGE_NAME, TEST_USER_HANDLE));
     }
 
+    @Test
+    public void testConnectionServiceCreateConnectionTimeout() throws Exception {
+        ConnectionServiceWrapper service = new ConnectionServiceWrapper(new ComponentName(
+                mContext.getPackageName(), mContext.getPackageName().getClass().getName()), 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");
+    }
 
     private Call addSpyCall() {
         return addSpyCall(SIM_2_HANDLE, CallState.ACTIVE);
@@ -3355,4 +3393,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();
+    }
 }

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

@@ -735,6 +735,14 @@ public class ComponentContextFixture implements TestFixture<Context> {
         mServiceInfoByComponentName.put(componentName, serviceInfo);
     }
 
+    public void removeConnectionService(
+            ComponentName componentName,
+            IConnectionService service)
+            throws Exception {
+        removeService(ConnectionService.SERVICE_INTERFACE, componentName, service);
+        mServiceInfoByComponentName.remove(componentName);
+    }
+
     public void addInCallService(
             ComponentName componentName,
             IInCallService service,
@@ -828,6 +836,12 @@ public class ComponentContextFixture implements TestFixture<Context> {
         mComponentNameByService.put(service, name);
     }
 
+    private void removeService(String action, ComponentName name, IInterface service) {
+        mComponentNamesByAction.remove(action, name);
+        mServiceByComponentName.remove(name);
+        mComponentNameByService.remove(service);
+    }
+
     private List<ResolveInfo> doQueryIntentServices(Intent intent, int flags) {
         List<ResolveInfo> result = new ArrayList<>();
         for (ComponentName componentName : mComponentNamesByAction.get(intent.getAction())) {