From 2d6dabd23ef1b542163dda85762e48cae001efec Mon Sep 17 00:00:00 2001 From: Chris Li Date: Mon, 14 Nov 2022 16:44:58 +0800 Subject: [PATCH] Enforce onActivityReparentedToTask 1. Only allow when the activity is trusted embedded, because sharing ComponentName of untrusted embedded is not safe. 2. Trim the activity Intent to those that are needed and safe for Activity Filter. Bug: 255701223 Test: atest WmTests:TaskFragmentOrganizerControllerTest Change-Id: I0c03b95a2b7a9c91315d09a77dc4b855e464a2f5 --- .../wm/TaskFragmentOrganizerController.java | 19 ++++- .../TaskFragmentOrganizerControllerTest.java | 76 ++++++++++++++++++- 2 files changed, 90 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java b/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java index 509b1e6f41ca6..2e716ae23f208 100644 --- a/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java +++ b/services/core/java/com/android/server/wm/TaskFragmentOrganizerController.java @@ -34,6 +34,7 @@ import static java.util.Objects.requireNonNull; import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; +import android.content.Intent; import android.content.res.Configuration; import android.graphics.Rect; import android.os.Binder; @@ -322,9 +323,10 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr + " is not in a task belong to the organizer app."); return null; } - if (task.isAllowedToEmbedActivity(activity, mOrganizerUid) != EMBEDDING_ALLOWED) { + if (task.isAllowedToEmbedActivity(activity, mOrganizerUid) != EMBEDDING_ALLOWED + || !task.isAllowedToEmbedActivityInTrustedMode(activity, mOrganizerUid)) { Slog.d(TAG, "Reparent activity=" + activity.token - + " is not allowed to be embedded."); + + " is not allowed to be embedded in trusted mode."); return null; } @@ -350,7 +352,7 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr activity.token, task.mTaskId); return new TaskFragmentTransaction.Change(TYPE_ACTIVITY_REPARENTED_TO_TASK) .setTaskId(task.mTaskId) - .setActivityIntent(activity.intent) + .setActivityIntent(trimIntent(activity.intent)) .setActivityToken(activityToken); } @@ -1095,4 +1097,15 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr return false; } } + + /** + * Trims the given Intent to only those that are needed to for embedding rules. This helps to + * make it safer for cross-uid embedding even if we only send the Intent for trusted embedding. + */ + private static Intent trimIntent(@NonNull Intent intent) { + return new Intent() + .setComponent(intent.getComponent()) + .setPackage(intent.getPackage()) + .setAction(intent.getAction()); + } } diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java index 4202f46c188c6..c53518210af9b 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java @@ -62,10 +62,12 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import android.annotation.NonNull; +import android.content.ComponentName; import android.content.Intent; import android.content.pm.ActivityInfo; import android.content.res.Configuration; import android.graphics.Rect; +import android.net.Uri; import android.os.Binder; import android.os.Bundle; import android.os.IBinder; @@ -403,7 +405,7 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { final TaskFragmentTransaction.Change change = changes.get(0); assertEquals(TYPE_ACTIVITY_REPARENTED_TO_TASK, change.getType()); assertEquals(task.mTaskId, change.getTaskId()); - assertEquals(activity.intent, change.getActivityIntent()); + assertIntentsEqualForOrganizer(activity.intent, change.getActivityIntent()); assertNotEquals(activity.token, change.getActivityToken()); mTransaction.reparentActivityToTaskFragment(mFragmentToken, change.getActivityToken()); assertApplyTransactionAllowed(mTransaction); @@ -414,6 +416,62 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { change.getActivityToken())); } + @Test + public void testOnActivityReparentedToTask_untrustedEmbed_notReported() { + final int pid = Binder.getCallingPid(); + final int uid = Binder.getCallingUid(); + mTaskFragment.setTaskFragmentOrganizer(mOrganizer.getOrganizerToken(), uid, + DEFAULT_TASK_FRAGMENT_ORGANIZER_PROCESS_NAME); + mWindowOrganizerController.mLaunchTaskFragments.put(mFragmentToken, mTaskFragment); + final Task task = createTask(mDisplayContent); + task.addChild(mTaskFragment, POSITION_TOP); + final ActivityRecord activity = createActivityRecord(task); + + // Make sure the activity is embedded in untrusted mode. + activity.info.applicationInfo.uid = uid + 1; + doReturn(pid + 1).when(activity).getPid(); + task.effectiveUid = uid; + doReturn(EMBEDDING_ALLOWED).when(task).isAllowedToEmbedActivity(activity, uid); + doReturn(false).when(task).isAllowedToEmbedActivityInTrustedMode(activity, uid); + doReturn(true).when(task).isAllowedToEmbedActivityInUntrustedMode(activity); + + // Notify organizer if it was embedded before entered Pip. + // Create a temporary token since the activity doesn't belong to the same process. + clearInvocations(mOrganizer); + activity.mLastTaskFragmentOrganizerBeforePip = mIOrganizer; + mController.onActivityReparentedToTask(activity); + mController.dispatchPendingEvents(); + + // Disallow organizer to reparent activity that is untrusted embedded. + verify(mOrganizer, never()).onTransactionReady(mTransactionCaptor.capture()); + } + + @Test + public void testOnActivityReparentedToTask_trimReportedIntent() { + // Make sure the activity pid/uid is the same as the organizer caller. + final int pid = Binder.getCallingPid(); + final int uid = Binder.getCallingUid(); + final ActivityRecord activity = createActivityRecord(mDisplayContent); + final Task task = activity.getTask(); + activity.info.applicationInfo.uid = uid; + doReturn(pid).when(activity).getPid(); + task.effectiveUid = uid; + activity.mLastTaskFragmentOrganizerBeforePip = mIOrganizer; + + // Test the Intent trim in #assertIntentTrimmed + activity.intent.setComponent(new ComponentName("TestPackage", "TestClass")) + .setPackage("TestPackage") + .setAction("TestAction") + .setData(mock(Uri.class)) + .putExtra("Test", 123) + .setFlags(10); + + mController.onActivityReparentedToTask(activity); + mController.dispatchPendingEvents(); + + assertActivityReparentedToTaskTransaction(task.mTaskId, activity.intent, activity.token); + } + @Test public void testRegisterRemoteAnimations() { mController.registerRemoteAnimations(mIOrganizer, TASK_ID, mDefinition); @@ -1425,7 +1483,8 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { final TaskFragmentTransaction.Change change = changes.remove(0); assertEquals(TYPE_ACTIVITY_REPARENTED_TO_TASK, change.getType()); assertEquals(taskId, change.getTaskId()); - assertEquals(intent, change.getActivityIntent()); + assertIntentsEqualForOrganizer(intent, change.getActivityIntent()); + assertIntentTrimmed(change.getActivityIntent()); assertEquals(activityToken, change.getActivityToken()); } @@ -1452,4 +1511,17 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase { mockParent.lastActiveTime = 100; doReturn(true).when(mockParent).shouldBeVisible(any()); } + + private static void assertIntentsEqualForOrganizer(@NonNull Intent expected, + @NonNull Intent actual) { + assertEquals(expected.getComponent(), actual.getComponent()); + assertEquals(expected.getPackage(), actual.getPackage()); + assertEquals(expected.getAction(), actual.getAction()); + } + + private static void assertIntentTrimmed(@NonNull Intent intent) { + assertNull(intent.getData()); + assertNull(intent.getExtras()); + assertEquals(0, intent.getFlags()); + } }