From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Grant Menke Date: Thu, 25 Apr 2024 10:43:43 -0700 Subject: [PATCH] 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 7aa55ffca65d6166145fd9660e0f7340c07053bf) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:286781dfcb78d8b5c1a77f2390f5251f01943add) Merged-In: I30caed1481dff5af2223a8ff589846597cee8229 Change-Id: I30caed1481dff5af2223a8ff589846597cee8229 --- src/com/android/server/telecom/Call.java | 26 ++ .../telecom/ConnectionServiceWrapper.java | 49 ++- src/com/android/server/telecom/LogUtils.java | 1 + .../server/telecom/tests/BasicCallTests.java | 2 + .../tests/ComponentContextFixture.java | 14 + .../tests/TestScheduledExecutorService.java | 283 ++++++++++++++++++ 6 files changed, 373 insertions(+), 2 deletions(-) create mode 100644 tests/src/com/android/server/telecom/tests/TestScheduledExecutorService.java diff --git a/src/com/android/server/telecom/Call.java b/src/com/android/server/telecom/Call.java index 90c1fe81a..02604b5a2 100644 --- a/src/com/android/server/telecom/Call.java +++ b/src/com/android/server/telecom/Call.java @@ -294,6 +294,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; @@ -736,6 +747,20 @@ public class Call implements CreateConnectionResponse, EventManager.Loggable { return sb.toString(); } + /** + * @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; @@ -1498,6 +1523,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( diff --git a/src/com/android/server/telecom/ConnectionServiceWrapper.java b/src/com/android/server/telecom/ConnectionServiceWrapper.java index 0335b230b..e825878b5 100644 --- a/src/com/android/server/telecom/ConnectionServiceWrapper.java +++ b/src/com/android/server/telecom/ConnectionServiceWrapper.java @@ -34,6 +34,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; @@ -56,6 +57,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 @@ -66,6 +72,12 @@ import java.util.concurrent.ConcurrentHashMap; @VisibleForTesting public class ConnectionServiceWrapper extends ServiceBinder { + 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> mScheduledFutureMap = new ConcurrentHashMap<>(2); + private final class Adapter extends IConnectionServiceAdapter.Stub { @Override @@ -77,6 +89,12 @@ public class ConnectionServiceWrapper extends ServiceBinder { 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(); @@ -884,7 +902,8 @@ public class ConnectionServiceWrapper extends ServiceBinder { * @param context The context. * @param userHandle The {@link UserHandle} to use when binding. */ - ConnectionServiceWrapper( + @VisibleForTesting + public ConnectionServiceWrapper( ComponentName componentName, ConnectionServiceRepository connectionServiceRepository, PhoneAccountRegistrar phoneAccountRegistrar, @@ -986,6 +1005,26 @@ public class ConnectionServiceWrapper extends ServiceBinder { .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(), @@ -1195,7 +1234,8 @@ public class ConnectionServiceWrapper extends ServiceBinder { } } - void addCall(Call call) { + @VisibleForTesting + public void addCall(Call call) { if (mCallIdMapper.getCallId(call) == null) { mCallIdMapper.addCall(call); } @@ -1500,4 +1540,9 @@ public class ConnectionServiceWrapper extends ServiceBinder { private void noRemoteServices(RemoteServiceCallback callback) { setRemoteServices(callback, Collections.EMPTY_LIST, Collections.EMPTY_LIST); } + + @VisibleForTesting + public void setScheduledExecutorService(ScheduledExecutorService service) { + mScheduledExecutor = service; + } } diff --git a/src/com/android/server/telecom/LogUtils.java b/src/com/android/server/telecom/LogUtils.java index 0411355e7..1b59f93a0 100644 --- a/src/com/android/server/telecom/LogUtils.java +++ b/src/com/android/server/telecom/LogUtils.java @@ -84,6 +84,7 @@ 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 BIND_CS = "BIND_CS"; public static final String CS_BOUND = "CS_BOUND"; public static final String CONFERENCE_WITH = "CONF_WITH"; diff --git a/tests/src/com/android/server/telecom/tests/BasicCallTests.java b/tests/src/com/android/server/telecom/tests/BasicCallTests.java index 18bfc41e6..94b621eae 100644 --- a/tests/src/com/android/server/telecom/tests/BasicCallTests.java +++ b/tests/src/com/android/server/telecom/tests/BasicCallTests.java @@ -904,6 +904,7 @@ public class BasicCallTests extends TelecomSystemTest { call.setTargetPhoneAccount(mPhoneAccountA1.getAccountHandle()); assert(call.isVideoCallingSupported()); assertEquals(VideoProfile.STATE_BIDIRECTIONAL, call.getVideoState()); + call.setIsCreateConnectionComplete(true); } /** @@ -926,6 +927,7 @@ public class BasicCallTests extends TelecomSystemTest { call.setTargetPhoneAccount(mPhoneAccountA2.getAccountHandle()); assert(!call.isVideoCallingSupported()); assertEquals(VideoProfile.STATE_AUDIO_ONLY, call.getVideoState()); + call.setIsCreateConnectionComplete(true); } /** diff --git a/tests/src/com/android/server/telecom/tests/ComponentContextFixture.java b/tests/src/com/android/server/telecom/tests/ComponentContextFixture.java index 1c74bfacb..8a0c7c2c1 100644 --- a/tests/src/com/android/server/telecom/tests/ComponentContextFixture.java +++ b/tests/src/com/android/server/telecom/tests/ComponentContextFixture.java @@ -495,6 +495,14 @@ public class ComponentContextFixture implements TestFixture { 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) @@ -533,6 +541,12 @@ public class ComponentContextFixture implements TestFixture { 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 doQueryIntentServices(Intent intent, int flags) { List result = new ArrayList<>(); for (ComponentName componentName : mComponentNamesByAction.get(intent.getAction())) { diff --git a/tests/src/com/android/server/telecom/tests/TestScheduledExecutorService.java b/tests/src/com/android/server/telecom/tests/TestScheduledExecutorService.java new file mode 100644 index 000000000..8ddf42b9b --- /dev/null +++ b/tests/src/com/android/server/telecom/tests/TestScheduledExecutorService.java @@ -0,0 +1,283 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.telecom.tests; + +import android.util.Log; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.Callable; +import java.util.concurrent.Delayed; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +/** + * A test implementation of a scheduled executor service. + */ +public class TestScheduledExecutorService implements ScheduledExecutorService { + private static final String TAG = "TestScheduledExecutorService"; + + private class CompletedFuture implements Future, ScheduledFuture { + + private final Callable mTask; + private final long mDelayMs; + private Runnable mRunnable; + + CompletedFuture(Callable task) { + mTask = task; + mDelayMs = 0; + } + + @SuppressWarnings("unused") + CompletedFuture(Callable task, long delayMs) { + mTask = task; + mDelayMs = delayMs; + } + + CompletedFuture(Runnable task, long delayMs) { + mRunnable = task; + mTask = (Callable) Executors.callable(task); + mDelayMs = delayMs; + } + + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + cancelRunnable(mRunnable); + return true; + } + + @Override + public boolean isCancelled() { + return false; + } + + @Override + public boolean isDone() { + return true; + } + + @Override + public T get() throws InterruptedException, ExecutionException { + try { + return mTask.call(); + } catch (Exception e) { + throw new ExecutionException(e); + } + } + + @Override + public T get(long timeout, TimeUnit unit) + throws InterruptedException, ExecutionException, TimeoutException { + try { + return mTask.call(); + } catch (Exception e) { + throw new ExecutionException(e); + } + } + + @Override + public long getDelay(TimeUnit unit) { + if (unit == TimeUnit.MILLISECONDS) { + return mDelayMs; + } else { + // not implemented + return 0; + } + } + + @Override + public int compareTo(Delayed o) { + if (o == null) return 1; + if (o.getDelay(TimeUnit.MILLISECONDS) > mDelayMs) return -1; + if (o.getDelay(TimeUnit.MILLISECONDS) < mDelayMs) return 1; + return 0; + } + } + + private long mClock = 0; + private Map mScheduledRunnables = new HashMap<>(); + private Map mRepeatDuration = new HashMap<>(); + + @Override + public void shutdown() { + } + + @Override + public List shutdownNow() { + return null; + } + + @Override + public boolean isShutdown() { + return false; + } + + @Override + public boolean isTerminated() { + return false; + } + + @Override + public boolean awaitTermination(long timeout, TimeUnit unit) { + return false; + } + + @Override + public Future submit(Callable task) { + return new TestScheduledExecutorService.CompletedFuture<>(task); + } + + @Override + public Future submit(Runnable task, T result) { + throw new UnsupportedOperationException("Not implemented"); + } + + @Override + public Future submit(Runnable task) { + task.run(); + return new TestScheduledExecutorService.CompletedFuture<>(() -> null); + } + + @Override + public List> invokeAll(Collection> tasks) { + throw new UnsupportedOperationException("Not implemented"); + } + + @Override + public List> invokeAll(Collection> tasks, long timeout, + TimeUnit unit) { + throw new UnsupportedOperationException("Not implemented"); + } + + @Override + public T invokeAny(Collection> tasks) { + throw new UnsupportedOperationException("Not implemented"); + } + + @Override + public T invokeAny(Collection> tasks, long timeout, TimeUnit unit) { + throw new UnsupportedOperationException("Not implemented"); + } + + @Override + public ScheduledFuture schedule(Runnable command, long delay, TimeUnit unit) { + // Schedule the runnable for execution at the specified time. + long scheduledTime = getNextExecutionTime(delay, unit); + mScheduledRunnables.put(scheduledTime, command); + + Log.i(TAG, "schedule: runnable=" + System.identityHashCode(command) + ", time=" + + scheduledTime); + + return new TestScheduledExecutorService.CompletedFuture(command, delay); + } + + @Override + public ScheduledFuture schedule(Callable callable, long delay, TimeUnit unit) { + throw new UnsupportedOperationException("Not implemented"); + } + + @Override + public ScheduledFuture scheduleAtFixedRate(Runnable command, long initialDelay, long period, + TimeUnit unit) { + return scheduleWithFixedDelay(command, initialDelay, period, unit); + } + + @Override + public ScheduledFuture scheduleWithFixedDelay(Runnable command, long initialDelay, + long delay, TimeUnit unit) { + // Schedule the runnable for execution at the specified time. + long nextScheduledTime = getNextExecutionTime(delay, unit); + mScheduledRunnables.put(nextScheduledTime, command); + mRepeatDuration.put(command, unit.toMillis(delay)); + + return new TestScheduledExecutorService.CompletedFuture(command, delay); + } + + private long getNextExecutionTime(long delay, TimeUnit unit) { + long delayMillis = unit.toMillis(delay); + return mClock + delayMillis; + } + + @Override + public void execute(Runnable command) { + command.run(); + } + + /** + * Used in unit tests, used to add a delta to the "clock" so that we can fire off scheduled + * items and reschedule the repeats. + * @param duration The duration (millis) to add to the clock. + */ + public void advanceTime(long duration) { + Map nextRepeats = new HashMap<>(); + List toRun = new ArrayList<>(); + mClock += duration; + Iterator> iterator = mScheduledRunnables.entrySet().iterator(); + while (iterator.hasNext()) { + Map.Entry entry = iterator.next(); + if (mClock >= entry.getKey()) { + toRun.add(entry.getValue()); + + Runnable r = entry.getValue(); + Log.i(TAG, "advanceTime: runningRunnable=" + System.identityHashCode(r)); + // If this is a repeating scheduled item, schedule the repeat. + if (mRepeatDuration.containsKey(r)) { + // schedule next execution + nextRepeats.put(mClock + mRepeatDuration.get(r), entry.getValue()); + } + iterator.remove(); + } + } + + // Update things at the end to avoid concurrent access. + mScheduledRunnables.putAll(nextRepeats); + toRun.forEach(r -> r.run()); + } + + /** + * Used from a {@link CompletedFuture} as defined above to cancel a scheduled task. + * @param r The runnable to cancel. + */ + private void cancelRunnable(Runnable r) { + Optional> found = mScheduledRunnables.entrySet().stream() + .filter(e -> e.getValue() == r) + .findFirst(); + if (found.isPresent()) { + mScheduledRunnables.remove(found.get().getKey()); + } + mRepeatDuration.remove(r); + Log.i(TAG, "cancelRunnable: runnable=" + System.identityHashCode(r)); + } + + public int getNumberOfScheduledRunnables() { + return mScheduledRunnables.size(); + } + + public boolean isRunnableScheduledAtTime(long time) { + return mScheduledRunnables.containsKey(time); + } +} \ No newline at end of file