From 97b59a72fad2acd3fe66e6ed304e8d48855404f3 Mon Sep 17 00:00:00 2001 From: Andrii Kulian Date: Tue, 8 Feb 2022 12:45:14 -0800 Subject: [PATCH] Verify cross-uid activity embedding By default activities cannot be embedded by non-privileged processes. Apps can either declare their activities to be available for untrusted embedding, or limit it to certain trusted hosts. This CL verifies that an application cannot embed activities that didn't opt in, and verifies that in untrusted mode the host cannot request SurfaceControl transactions or position the embedded container outside of the task bounds. Bug: 197364677 Test: ActivityEmbeddingCrossUidTests, TaskFragmentOrganizerTest Change-Id: Ief54c9acae1a9a4152af710f67e7810afdcff9ba --- .../extensions/embedding/SplitController.java | 1 + .../android/server/wm/ActivityStarter.java | 7 +- .../com/android/server/wm/TaskFragment.java | 53 ++++++++++ .../wm/TaskFragmentOrganizerController.java | 18 +++- .../server/wm/WindowOrganizerController.java | 61 ++++++++++- .../server/wm/ActivityStarterTests.java | 100 +++++++++++++++++- 6 files changed, 231 insertions(+), 9 deletions(-) diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java index 8f368c2bee226..d35ecbde529b3 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java @@ -924,6 +924,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen * Checks if an activity is embedded and its presentation is customized by a * {@link android.window.TaskFragmentOrganizer} to only occupy a portion of Task bounds. */ + @Override public boolean isActivityEmbedded(@NonNull Activity activity) { return mPresenter.isActivityEmbedded(activity.getActivityToken()); } diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java index 5164bf0531483..fb3d17ab3daa8 100644 --- a/services/core/java/com/android/server/wm/ActivityStarter.java +++ b/services/core/java/com/android/server/wm/ActivityStarter.java @@ -2003,8 +2003,8 @@ class ActivityStarter { * @param targetTask the target task for launching activity, which could be different from * the one who hosting the embedding. */ - private boolean canEmbedActivity(@NonNull TaskFragment taskFragment, ActivityRecord starting, - boolean newTask, Task targetTask) { + private boolean canEmbedActivity(@NonNull TaskFragment taskFragment, + @NonNull ActivityRecord starting, boolean newTask, Task targetTask) { final Task hostTask = taskFragment.getTask(); if (hostTask == null) { return false; @@ -2016,8 +2016,7 @@ class ActivityStarter { return true; } - // Not allowed embedding an activity of another app. - if (hostUid != starting.getUid()) { + if (!taskFragment.isAllowedToEmbedActivity(starting)) { return false; } diff --git a/services/core/java/com/android/server/wm/TaskFragment.java b/services/core/java/com/android/server/wm/TaskFragment.java index a59d7b6e72198..c880aba03423a 100644 --- a/services/core/java/com/android/server/wm/TaskFragment.java +++ b/services/core/java/com/android/server/wm/TaskFragment.java @@ -25,6 +25,7 @@ import static android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN; import static android.app.WindowConfiguration.WINDOWING_MODE_MULTI_WINDOW; import static android.app.WindowConfiguration.WINDOWING_MODE_PINNED; import static android.app.WindowConfiguration.WINDOWING_MODE_UNDEFINED; +import static android.content.pm.ActivityInfo.FLAG_ALLOW_UNTRUSTED_ACTIVITY_EMBEDDING; import static android.content.pm.ActivityInfo.FLAG_RESUME_WHILE_PAUSING; import static android.content.res.Configuration.ORIENTATION_LANDSCAPE; import static android.content.res.Configuration.ORIENTATION_PORTRAIT; @@ -95,6 +96,7 @@ import com.android.internal.annotations.VisibleForTesting; import com.android.internal.protolog.common.ProtoLog; import com.android.internal.util.function.pooled.PooledLambda; import com.android.internal.util.function.pooled.PooledPredicate; +import com.android.server.pm.parsing.pkg.AndroidPackage; import java.io.FileDescriptor; import java.io.PrintWriter; @@ -102,6 +104,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.function.Consumer; import java.util.function.Predicate; @@ -504,6 +507,56 @@ class TaskFragment extends WindowContainer { return false; } + /** + * Checks if the organized task fragment is allowed to have the specified activity, which is + * allowed if an activity allows embedding in untrusted mode, or if the trusted mode can be + * enabled. + * @see #isAllowedToEmbedActivityInTrustedMode(ActivityRecord) + */ + boolean isAllowedToEmbedActivity(@NonNull ActivityRecord a) { + if ((a.info.flags & FLAG_ALLOW_UNTRUSTED_ACTIVITY_EMBEDDING) + == FLAG_ALLOW_UNTRUSTED_ACTIVITY_EMBEDDING) { + return true; + } + + return isAllowedToEmbedActivityInTrustedMode(a); + } + + /** + * Checks if the organized task fragment is allowed to embed activity in fully trusted mode, + * which means that all transactions are allowed. This is supported in the following cases: + *
  • the activity belongs to the same app as the organizer host;
    • + *
  • the activity has declared the organizer host as trusted explicitly via known + * certificate.
    • + */ + private boolean isAllowedToEmbedActivityInTrustedMode(@NonNull ActivityRecord a) { + if (mTaskFragmentOrganizerUid == a.getUid()) { + // Activities from the same UID can be embedded freely by the host. + return true; + } + + Set knownActivityEmbeddingCerts = a.info.getKnownActivityEmbeddingCerts(); + if (knownActivityEmbeddingCerts.isEmpty()) { + // An application must either declare that it allows untrusted embedding, or specify + // a set of app certificates that are allowed to embed it in trusted mode. + return false; + } + + AndroidPackage hostPackage = mAtmService.getPackageManagerInternalLocked() + .getPackage(mTaskFragmentOrganizerUid); + + return hostPackage != null && hostPackage.getSigningDetails().hasAncestorOrSelfWithDigest( + knownActivityEmbeddingCerts); + } + + /** + * Checks if all activities in the task fragment are allowed to be embedded in trusted mode. + * @see #isAllowedToEmbedActivityInTrustedMode(ActivityRecord) + */ + boolean isAllowedToBeEmbeddedInTrustedMode() { + return forAllActivities(this::isAllowedToEmbedActivityInTrustedMode); + } + /** * Returns the TaskFragment that is being organized, which could be this or the ascendant * TaskFragment. diff --git a/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java b/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java index 123ca889c73e9..19f921deab4c2 100644 --- a/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java +++ b/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java @@ -294,14 +294,28 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr } } - /** Gets the {@link RemoteAnimationDefinition} set on the given organizer if exists. */ + /** + * Gets the {@link RemoteAnimationDefinition} set on the given organizer if exists. Returns + * {@code null} if it doesn't, or if the organizer has activity(ies) embedded in untrusted mode. + */ @Nullable public RemoteAnimationDefinition getRemoteAnimationDefinition( ITaskFragmentOrganizer organizer) { synchronized (mGlobalLock) { final TaskFragmentOrganizerState organizerState = mTaskFragmentOrganizerState.get(organizer.asBinder()); - return organizerState != null ? organizerState.mRemoteAnimationDefinition : null; + if (organizerState == null) { + return null; + } + for (TaskFragment tf : organizerState.mOrganizedTaskFragments) { + if (!tf.isAllowedToBeEmbeddedInTrustedMode()) { + // Disable client-driven animations for organizer if at least one of the + // embedded task fragments is not embedding in trusted mode. + // TODO(b/197364677): replace with a stub or Shell-driven one instead of skip? + return null; + } + } + return organizerState.mRemoteAnimationDefinition; } } diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java index 1d5c1841fde14..4c7891b9fc519 100644 --- a/services/core/java/com/android/server/wm/WindowOrganizerController.java +++ b/services/core/java/com/android/server/wm/WindowOrganizerController.java @@ -19,6 +19,8 @@ package com.android.server.wm; import static android.Manifest.permission.START_TASKS_FROM_RECENTS; import static android.app.ActivityManager.isStartResultSuccessful; import static android.view.Display.DEFAULT_DISPLAY; +import static android.window.WindowContainerTransaction.Change.CHANGE_BOUNDS_TRANSACTION; +import static android.window.WindowContainerTransaction.Change.CHANGE_BOUNDS_TRANSACTION_RECT; import static android.window.WindowContainerTransaction.HierarchyOp.HIERARCHY_OP_TYPE_CHILDREN_TASKS_REPARENT; import static android.window.WindowContainerTransaction.HierarchyOp.HIERARCHY_OP_TYPE_CREATE_TASK_FRAGMENT; import static android.window.WindowContainerTransaction.HierarchyOp.HIERARCHY_OP_TYPE_DELETE_TASK_FRAGMENT; @@ -1224,10 +1226,14 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub while (entries.hasNext()) { final Map.Entry entry = entries.next(); // Only allow to apply changes to TaskFragment that is created by this organizer. - enforceTaskFragmentOrganized(func, WindowContainer.fromBinder(entry.getKey()), - organizer); + WindowContainer wc = WindowContainer.fromBinder(entry.getKey()); + enforceTaskFragmentOrganized(func, wc, organizer); + enforceTaskFragmentConfigChangeAllowed(func, wc, entry.getValue(), organizer); } + // TODO(b/197364677): Enforce safety of hierarchy operations in untrusted mode. E.g. one + // could first change a trusted TF, and then start/reparent untrusted activity there. + // Hierarchy changes final List hops = t.getHierarchyOps(); for (int i = hops.size() - 1; i >= 0; i--) { @@ -1297,6 +1303,57 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub } } + /** + * Makes sure that SurfaceControl transactions and the ability to set bounds outside of the + * parent bounds are not allowed for embedding without full trust between the host and the + * target. + * TODO(b/197364677): Allow SC transactions when the client-driven animations are protected from + * tapjacking. + */ + private void enforceTaskFragmentConfigChangeAllowed(String func, @Nullable WindowContainer wc, + WindowContainerTransaction.Change change, ITaskFragmentOrganizer organizer) { + if (wc == null) { + Slog.e(TAG, "Attempt to operate on task fragment that no longer exists"); + return; + } + // Check if TaskFragment is embedded in fully trusted mode + if (wc.asTaskFragment().isAllowedToBeEmbeddedInTrustedMode()) { + // Fully trusted, no need to check further + return; + } + + if (change == null) { + return; + } + final int changeMask = change.getChangeMask(); + if ((changeMask & (CHANGE_BOUNDS_TRANSACTION | CHANGE_BOUNDS_TRANSACTION_RECT)) != 0) { + String msg = "Permission Denial: " + func + " from pid=" + + Binder.getCallingPid() + ", uid=" + Binder.getCallingUid() + + " trying to apply SurfaceControl changes to TaskFragment in non-trusted " + + "embedding mode, TaskFragmentOrganizer=" + organizer; + Slog.w(TAG, msg); + throw new SecurityException(msg); + } + if (change.getWindowSetMask() == 0) { + // Nothing else to check. + return; + } + WindowConfiguration requestedWindowConfig = change.getConfiguration().windowConfiguration; + WindowContainer wcParent = wc.getParent(); + if (wcParent == null) { + Slog.e(TAG, "Attempt to set bounds on task fragment that has no parent"); + return; + } + if (!wcParent.getBounds().contains(requestedWindowConfig.getBounds())) { + String msg = "Permission Denial: " + func + " from pid=" + + Binder.getCallingPid() + ", uid=" + Binder.getCallingUid() + + " trying to apply bounds outside of parent for non-trusted host," + + " TaskFragmentOrganizer=" + organizer; + Slog.w(TAG, msg); + throw new SecurityException(msg); + } + } + void createTaskFragment(@NonNull TaskFragmentCreationParams creationParams, @Nullable IBinder errorCallbackToken, @NonNull CallerInfo caller) { final ActivityRecord ownerActivity = diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java index c58bf3bf1d564..c8e48a48d3fb0 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityStarterTests.java @@ -75,6 +75,7 @@ import android.content.pm.ActivityInfo.WindowLayout; import android.content.pm.ApplicationInfo; import android.content.pm.IPackageManager; import android.content.pm.PackageManagerInternal; +import android.content.pm.SigningDetails; import android.graphics.Rect; import android.os.Binder; import android.os.IBinder; @@ -84,9 +85,11 @@ import android.platform.test.annotations.Presubmit; import android.service.voice.IVoiceInteractionSession; import android.util.Pair; import android.view.Gravity; +import android.window.TaskFragmentOrganizerToken; import androidx.test.filters.SmallTest; +import com.android.server.pm.parsing.pkg.AndroidPackage; import com.android.server.wm.LaunchParamsController.LaunchParamsModifier; import com.android.server.wm.utils.MockTracker; @@ -94,6 +97,10 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + /** * Tests for the {@link ActivityStarter} class. * @@ -1109,7 +1116,7 @@ public class ActivityStarterTests extends WindowTestsBase { } @Test - public void testStartActivityInner_inTaskFragment() { + public void testStartActivityInner_inTaskFragment_failsByDefault() { final ActivityStarter starter = prepareStarter(0, false); final ActivityRecord targetRecord = new ActivityBuilder(mAtm).build(); final ActivityRecord sourceRecord = new ActivityBuilder(mAtm).setCreateTask(true).build(); @@ -1117,6 +1124,97 @@ public class ActivityStarterTests extends WindowTestsBase { true /* createdByOrganizer */); sourceRecord.getTask().addChild(taskFragment, POSITION_TOP); + starter.startActivityInner( + /* r */targetRecord, + /* sourceRecord */ sourceRecord, + /* voiceSession */null, + /* voiceInteractor */ null, + /* startFlags */ 0, + /* doResume */true, + /* options */null, + /* inTask */null, + /* inTaskFragment */ taskFragment, + /* restrictedBgActivity */false, + /* intentGrants */null); + + assertFalse(taskFragment.hasChild()); + } + + @Test + public void testStartActivityInner_inTaskFragment_allowedForSameUid() { + final ActivityStarter starter = prepareStarter(0, false); + final ActivityRecord targetRecord = new ActivityBuilder(mAtm).build(); + final ActivityRecord sourceRecord = new ActivityBuilder(mAtm).setCreateTask(true).build(); + final TaskFragment taskFragment = new TaskFragment(mAtm, sourceRecord.token, + true /* createdByOrganizer */); + sourceRecord.getTask().addChild(taskFragment, POSITION_TOP); + + taskFragment.setTaskFragmentOrganizer(mock(TaskFragmentOrganizerToken.class), + targetRecord.getUid(), "test_process_name"); + + starter.startActivityInner( + /* r */targetRecord, + /* sourceRecord */ sourceRecord, + /* voiceSession */null, + /* voiceInteractor */ null, + /* startFlags */ 0, + /* doResume */true, + /* options */null, + /* inTask */null, + /* inTaskFragment */ taskFragment, + /* restrictedBgActivity */false, + /* intentGrants */null); + + assertTrue(taskFragment.hasChild()); + } + + @Test + public void testStartActivityInner_inTaskFragment_allowedTrustedCertUid() { + final ActivityStarter starter = prepareStarter(0, false); + final ActivityRecord targetRecord = new ActivityBuilder(mAtm).build(); + final ActivityRecord sourceRecord = new ActivityBuilder(mAtm).setCreateTask(true).build(); + final TaskFragment taskFragment = new TaskFragment(mAtm, sourceRecord.token, + true /* createdByOrganizer */); + sourceRecord.getTask().addChild(taskFragment, POSITION_TOP); + + taskFragment.setTaskFragmentOrganizer(mock(TaskFragmentOrganizerToken.class), + 12345, "test_process_name"); + AndroidPackage androidPackage = mock(AndroidPackage.class); + doReturn(androidPackage).when(mMockPackageManager).getPackage(eq(12345)); + + Set certs = new HashSet(Arrays.asList("test_cert1", "test_cert1")); + targetRecord.info.setKnownActivityEmbeddingCerts(certs); + SigningDetails signingDetails = mock(SigningDetails.class); + doReturn(true).when(signingDetails).hasAncestorOrSelfWithDigest(any()); + doReturn(signingDetails).when(androidPackage).getSigningDetails(); + + starter.startActivityInner( + /* r */targetRecord, + /* sourceRecord */ sourceRecord, + /* voiceSession */null, + /* voiceInteractor */ null, + /* startFlags */ 0, + /* doResume */true, + /* options */null, + /* inTask */null, + /* inTaskFragment */ taskFragment, + /* restrictedBgActivity */false, + /* intentGrants */null); + + assertTrue(taskFragment.hasChild()); + } + + @Test + public void testStartActivityInner_inTaskFragment_allowedForUntrustedEmbedding() { + final ActivityStarter starter = prepareStarter(0, false); + final ActivityRecord targetRecord = new ActivityBuilder(mAtm).build(); + final ActivityRecord sourceRecord = new ActivityBuilder(mAtm).setCreateTask(true).build(); + final TaskFragment taskFragment = new TaskFragment(mAtm, sourceRecord.token, + true /* createdByOrganizer */); + sourceRecord.getTask().addChild(taskFragment, POSITION_TOP); + + targetRecord.info.flags |= ActivityInfo.FLAG_ALLOW_UNTRUSTED_ACTIVITY_EMBEDDING; + starter.startActivityInner( /* r */targetRecord, /* sourceRecord */ sourceRecord,