Unbind ConnectionService if connection creation timed out.
Currently we won't check if the connection creation can complete in a
reasonable time after we bind to that ConnectionService. This may allow
malicious app keep its declared permission forever even if it is in
background. Solve this by manually unbind the ConnectionService if the
connection/conference creation can't complete in 15 seconds.
Bug: b/293458004
Test: Manually test using BugFinder app provided in the bug
Change-Id: I0d17564d1977b61770cf7e4a46a8ee353b8b42e8
diff --git a/flags/telecom_api_flags.aconfig b/flags/telecom_api_flags.aconfig
index 4dbc03b..cbad597 100644
--- a/flags/telecom_api_flags.aconfig
+++ b/flags/telecom_api_flags.aconfig
@@ -12,4 +12,11 @@
namespace: "telecom"
description: "When set, call details/extras id updates to Telecom APIs for Android V are active."
bug: "301713560"
-}
\ No newline at end of file
+}
+
+flag {
+ name: "unbind_timeout_connections"
+ namespace: "telecom"
+ description: "When set, Telecom will auto-unbind if a ConnectionService returns no connections after some time."
+ bug: "293458004"
+}
diff --git a/src/com/android/server/telecom/ConnectionServiceWrapper.java b/src/com/android/server/telecom/ConnectionServiceWrapper.java
index a43000a..c11ee6e 100644
--- a/src/com/android/server/telecom/ConnectionServiceWrapper.java
+++ b/src/com/android/server/telecom/ConnectionServiceWrapper.java
@@ -33,7 +33,6 @@
import android.os.CancellationSignal;
import android.os.IBinder;
import android.os.ParcelFileDescriptor;
-import android.os.Process;
import android.os.RemoteException;
import android.os.ResultReceiver;
import android.os.UserHandle;
@@ -65,6 +64,7 @@
import com.android.internal.telecom.IVideoProvider;
import com.android.internal.telecom.RemoteServiceCallback;
import com.android.internal.util.Preconditions;
+import com.android.server.telecom.flags.Flags;
import java.util.ArrayList;
import java.util.Collection;
@@ -72,13 +72,14 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
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.TimeUnit;
-import java.util.Objects;
/**
* Wrapper for {@link IConnectionService}s, handles binding to {@link IConnectionService} and keeps
@@ -91,9 +92,12 @@
ConnectionServiceFocusManager.ConnectionServiceFocus {
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();
private final class Adapter extends IConnectionServiceAdapter.Stub {
@@ -1600,7 +1604,22 @@
.setParticipants(call.getParticipants())
.setIsAdhocConferenceCall(call.isAdhocConferenceCall())
.build();
-
+ if (Flags.unbindTimeoutConnections()) {
+ android.telecom.Logging.Runnable r =
+ new android.telecom.Logging.Runnable("CSW.cC", mLock) {
+ @Override
+ public void loggedRun() {
+ if (!call.isCreateConnectionComplete()) {
+ Log.e(this, new Exception(), "Conference %s creation timeout",
+ getComponentName());
+ response.handleCreateConferenceFailure(
+ new DisconnectCause(DisconnectCause.ERROR));
+ }
+ }
+ };
+ mScheduledExecutor.schedule(r.getRunnableToCancel(), SERVICE_BINDING_TIMEOUT,
+ TimeUnit.MILLISECONDS);
+ }
try {
mServiceInterface.createConference(
call.getConnectionManagerPhoneAccount(),
@@ -1609,7 +1628,6 @@
call.shouldAttachToExistingConnection(),
call.isUnknown(),
Log.getExternalSession(TELECOM_ABBREVIATION));
-
} catch (RemoteException e) {
Log.e(this, e, "Failure to createConference -- %s", getComponentName());
mPendingResponses.remove(callId).handleCreateConferenceFailure(
@@ -1642,6 +1660,7 @@
Log.i(ConnectionServiceWrapper.this, "Call not present"
+ " in call id mapper, maybe it was aborted before the bind"
+ " completed successfully?");
+
response.handleCreateConnectionFailure(
new DisconnectCause(DisconnectCause.CANCELED));
return;
@@ -1703,6 +1722,23 @@
.setRttPipeToInCall(call.getCsToInCallRttPipeForCs())
.build();
+ if (Flags.unbindTimeoutConnections()) {
+ android.telecom.Logging.Runnable r =
+ new android.telecom.Logging.Runnable("CSW.cC", mLock) {
+ @Override
+ public void loggedRun() {
+ if (!call.isCreateConnectionComplete()) {
+ Log.e(this, new Exception(),
+ "Connection %s creation timeout",
+ getComponentName());
+ response.handleCreateConnectionFailure(
+ new DisconnectCause(DisconnectCause.ERROR));
+ }
+ }
+ };
+ mScheduledExecutor.schedule(r.getRunnableToCancel(), SERVICE_BINDING_TIMEOUT,
+ TimeUnit.MILLISECONDS);
+ }
try {
mServiceInterface.createConnection(
call.getConnectionManagerPhoneAccount(),
@@ -1711,7 +1747,6 @@
call.shouldAttachToExistingConnection(),
call.isUnknown(),
Log.getExternalSession(TELECOM_ABBREVIATION));
-
} catch (RemoteException e) {
Log.e(this, e, "Failure to createConnection -- %s", getComponentName());
mPendingResponses.remove(callId).handleCreateConnectionFailure(
@@ -2160,7 +2195,8 @@
}
}
- void addCall(Call call) {
+ @VisibleForTesting
+ public void addCall(Call call) {
if (mCallIdMapper.getCallId(call) == null) {
mCallIdMapper.addCall(call);
}
@@ -2629,4 +2665,9 @@
sb.append("]");
return sb.toString();
}
+
+ @VisibleForTesting
+ public void setScheduledExecutorService(ScheduledExecutorService service) {
+ mScheduledExecutor = service;
+ }
}
diff --git a/tests/src/com/android/server/telecom/tests/BasicCallTests.java b/tests/src/com/android/server/telecom/tests/BasicCallTests.java
index 5bef130..c3dbf87 100644
--- a/tests/src/com/android/server/telecom/tests/BasicCallTests.java
+++ b/tests/src/com/android/server/telecom/tests/BasicCallTests.java
@@ -997,6 +997,7 @@
call.setTargetPhoneAccount(mPhoneAccountA1.getAccountHandle());
assert(call.isVideoCallingSupportedByPhoneAccount());
assertEquals(VideoProfile.STATE_BIDIRECTIONAL, call.getVideoState());
+ call.setIsCreateConnectionComplete(true);
}
/**
@@ -1020,6 +1021,7 @@
call.setTargetPhoneAccount(mPhoneAccountA2.getAccountHandle());
assert(!call.isVideoCallingSupportedByPhoneAccount());
assertEquals(VideoProfile.STATE_AUDIO_ONLY, call.getVideoState());
+ call.setIsCreateConnectionComplete(true);
}
/**
diff --git a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
index bfc6dc5..23b1360 100644
--- a/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
+++ b/tests/src/com/android/server/telecom/tests/CallsManagerTest.java
@@ -17,10 +17,8 @@
package com.android.server.telecom.tests;
import static android.provider.CallLog.Calls.USER_MISSED_NOT_RUNNING;
-
import static junit.framework.Assert.assertNotNull;
import static junit.framework.TestCase.fail;
-
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
@@ -43,6 +41,7 @@
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 +53,7 @@
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;
@@ -61,6 +61,7 @@
import android.os.SystemClock;
import android.os.UserHandle;
import android.os.UserManager;
+import android.platform.test.flag.junit.SetFlagsRule;
import android.provider.BlockedNumberContract;
import android.telecom.CallException;
import android.telecom.CallScreeningService;
@@ -80,6 +81,7 @@
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;
@@ -98,6 +100,7 @@
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;
@@ -124,8 +127,9 @@
import com.android.server.telecom.bluetooth.BluetoothStateReceiver;
import com.android.server.telecom.callfiltering.BlockedNumbersAdapter;
import com.android.server.telecom.callfiltering.CallFilteringResult;
-import com.android.server.telecom.flags.FeatureFlags;
import com.android.server.telecom.callfiltering.IncomingCallFilterGraph;
+import com.android.server.telecom.flags.FeatureFlags;
+import com.android.server.telecom.flags.Flags;
import com.android.server.telecom.ui.AudioProcessingNotification;
import com.android.server.telecom.ui.CallStreamingNotification;
import com.android.server.telecom.ui.DisconnectedCallNotifier;
@@ -134,6 +138,7 @@
import org.junit.After;
import org.junit.Before;
+import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -284,7 +289,8 @@
@Mock private FeatureFlags mFeatureFlags;
@Mock private IncomingCallFilterGraph mIncomingCallFilterGraph;
-
+ @Mock private IConnectionService mIConnectionService;
+ @Rule public SetFlagsRule mSetRlagsRule = new SetFlagsRule();
private CallsManager mCallsManager;
@Override
@@ -372,11 +378,17 @@
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(
+ SIM_1_ACCOUNT.getAccountHandle().getComponentName(), mIConnectionService);
}
@Override
@After
public void tearDown() throws Exception {
+ mComponentContextFixture.removeConnectionService(
+ SIM_1_ACCOUNT.getAccountHandle().getComponentName(), mIConnectionService);
super.tearDown();
}
@@ -2822,6 +2834,36 @@
assertTrue(result.contains("onReceiveResult"));
}
+ @Test
+ public void testConnectionServiceCreateConnectionTimeout() throws Exception {
+ mSetRlagsRule.enableFlags(Flags.FLAG_UNBIND_TIMEOUT_CONNECTIONS);
+ 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)));
+ }
+
@SmallTest
@Test
public void testOnFailedOutgoingCallUnholdsCallAfterLocallyDisconnect() {
@@ -3417,4 +3459,19 @@
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();
+ }
}