Merge "Enforce TaskFragmentOrganizer can only manipulate organized TF" into tm-qpr-dev

services/core/java/com/android/server/wm/WindowOrganizerController.java

@@ -99,6 +99,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.function.IntSupplier;
 
 /**
@@ -1524,7 +1525,6 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
             final int type = hop.getType();
             // Check for each type of the operations that are allowed for TaskFragmentOrganizer.
             switch (type) {
-                case HIERARCHY_OP_TYPE_REORDER:
                 case HIERARCHY_OP_TYPE_DELETE_TASK_FRAGMENT:
                     enforceTaskFragmentOrganized(func,
                             WindowContainer.fromBinder(hop.getContainer()), organizer);
@@ -1540,14 +1540,19 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                     // We are allowing organizer to create TaskFragment. We will check the
                     // ownerToken in #createTaskFragment, and trigger error callback if that is not
                     // valid.
+                    break;
                 case HIERARCHY_OP_TYPE_START_ACTIVITY_IN_TASK_FRAGMENT:
-                case HIERARCHY_OP_TYPE_REPARENT_ACTIVITY_TO_TASK_FRAGMENT:
-                case HIERARCHY_OP_TYPE_SET_ADJACENT_TASK_FRAGMENTS:
                 case HIERARCHY_OP_TYPE_REQUEST_FOCUS_ON_TASK_FRAGMENT:
-                    // We are allowing organizer to start/reparent activity to a TaskFragment it
+                    enforceTaskFragmentOrganized(func, hop.getContainer(), organizer);
-                    // created, or set two TaskFragments adjacent to each other. Nothing to check
+                    break;
-                    // here because the TaskFragment may not be created yet, but will be created in
+                case HIERARCHY_OP_TYPE_REPARENT_ACTIVITY_TO_TASK_FRAGMENT:
-                    // the same transaction.
+                    enforceTaskFragmentOrganized(func, hop.getNewParent(), organizer);
+                    break;
+                case HIERARCHY_OP_TYPE_SET_ADJACENT_TASK_FRAGMENTS:
+                    enforceTaskFragmentOrganized(func, hop.getContainer(), organizer);
+                    if (hop.getAdjacentRoot() != null) {
+                        enforceTaskFragmentOrganized(func, hop.getAdjacentRoot(), organizer);
+                    }
                     break;
                 case HIERARCHY_OP_TYPE_REPARENT_CHILDREN:
                     enforceTaskFragmentOrganized(func,
@@ -1570,8 +1575,12 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
         }
     }
 
-    private void enforceTaskFragmentOrganized(String func, @Nullable WindowContainer wc,
+    /**
-            ITaskFragmentOrganizer organizer) {
+     * Makes sure that the given {@link WindowContainer} is a {@link TaskFragment} organized by the
+     * given {@link ITaskFragmentOrganizer}.
+     */
+    private void enforceTaskFragmentOrganized(@NonNull String func, @Nullable WindowContainer wc,
+            @NonNull ITaskFragmentOrganizer organizer) {
         if (wc == null) {
             Slog.e(TAG, "Attempt to operate on window that no longer exists");
             return;
@@ -1587,6 +1596,26 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
         }
     }
 
+    /**
+     * Makes sure that the {@link TaskFragment} of the given fragment token is created and organized
+     * by the given {@link ITaskFragmentOrganizer}.
+     */
+    private void enforceTaskFragmentOrganized(@NonNull String func,
+            @NonNull IBinder fragmentToken, @NonNull ITaskFragmentOrganizer organizer) {
+        Objects.requireNonNull(fragmentToken);
+        final TaskFragment tf = mLaunchTaskFragments.get(fragmentToken);
+        // When the TaskFragment is {@code null}, it means that the TaskFragment will be created
+        // later in the same transaction, in which case it will always be organized by the given
+        // organizer.
+        if (tf != null && !tf.hasTaskFragmentOrganizer(organizer)) {
+            String msg = "Permission Denial: " + func + " from pid=" + Binder.getCallingPid()
+                    + ", uid=" + Binder.getCallingUid() + " trying to modify TaskFragment not"
+                    + " belonging to the TaskFragmentOrganizer=" + organizer;
+            Slog.w(TAG, msg);
+            throw new SecurityException(msg);
+        }
+    }
+
     /**
      * 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
@@ -1669,6 +1698,13 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
         final ITaskFragmentOrganizer organizer = ITaskFragmentOrganizer.Stub.asInterface(
                 creationParams.getOrganizer().asBinder());
 
+        if (mLaunchTaskFragments.containsKey(creationParams.getFragmentToken())) {
+            final Throwable exception =
+                    new IllegalArgumentException("TaskFragment token must be unique");
+            sendTaskFragmentOperationFailure(organizer, errorCallbackToken, null /* taskFragment */,
+                    HIERARCHY_OP_TYPE_CREATE_TASK_FRAGMENT, exception);
+            return;
+        }
         if (ownerActivity == null || ownerActivity.getTask() == null) {
             final Throwable exception =
                     new IllegalArgumentException("Not allowed to operate with invalid ownerToken");

services/tests/wmtests/src/com/android/server/wm/TaskFragmentOrganizerControllerTest.java

@@ -434,22 +434,6 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
         assertApplyTransactionAllowed(mTransaction);
     }
 
-    @Test
-    public void testApplyTransaction_enforceHierarchyChange_reorder() throws RemoteException {
-        mOrganizer.applyTransaction(mTransaction);
-
-        // Throw exception if the transaction is trying to change a window that is not organized by
-        // the organizer.
-        mTransaction.reorder(mFragmentWindowToken, true /* onTop */);
-
-        assertApplyTransactionDisallowed(mTransaction);
-
-        // Allow transaction to change a TaskFragment created by the organizer.
-        mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
-                "Test:TaskFragmentOrganizer" /* processName */);
-
-        assertApplyTransactionAllowed(mTransaction);
-    }
 
     @Test
     public void testApplyTransaction_enforceHierarchyChange_deleteTaskFragment()
@@ -530,6 +514,112 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
         assertEquals(ownerActivity.getTask(), taskFragment.getTask());
     }
 
+    @Test
+    public void testApplyTransaction_enforceTaskFragmentOrganized_startActivityInTaskFragment() {
+        final Task task = createTask(mDisplayContent);
+        final ActivityRecord ownerActivity = createActivityRecord(task);
+        mController.registerOrganizer(mIOrganizer);
+        mTaskFragment = new TaskFragmentBuilder(mAtm)
+                .setParentTask(task)
+                .setFragmentToken(mFragmentToken)
+                .build();
+        mWindowOrganizerController.mLaunchTaskFragments.put(mFragmentToken, mTaskFragment);
+        mTransaction.startActivityInTaskFragment(
+                mFragmentToken, ownerActivity.token, new Intent(), null /* activityOptions */);
+        mOrganizer.applyTransaction(mTransaction);
+
+        // Not allowed because TaskFragment is not organized by the caller organizer.
+        assertApplyTransactionDisallowed(mTransaction);
+
+        mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
+                "Test:TaskFragmentOrganizer" /* processName */);
+
+        assertApplyTransactionAllowed(mTransaction);
+    }
+
+    @Test
+    public void testApplyTransaction_enforceTaskFragmentOrganized_reparentActivityInTaskFragment() {
+        final Task task = createTask(mDisplayContent);
+        final ActivityRecord activity = createActivityRecord(task);
+        mController.registerOrganizer(mIOrganizer);
+        mTaskFragment = new TaskFragmentBuilder(mAtm)
+                .setParentTask(task)
+                .setFragmentToken(mFragmentToken)
+                .build();
+        mWindowOrganizerController.mLaunchTaskFragments.put(mFragmentToken, mTaskFragment);
+        mTransaction.reparentActivityToTaskFragment(mFragmentToken, activity.token);
+        mOrganizer.applyTransaction(mTransaction);
+
+        // Not allowed because TaskFragment is not organized by the caller organizer.
+        assertApplyTransactionDisallowed(mTransaction);
+
+        mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
+                "Test:TaskFragmentOrganizer" /* processName */);
+
+        assertApplyTransactionAllowed(mTransaction);
+    }
+
+    @Test
+    public void testApplyTransaction_enforceTaskFragmentOrganized_setAdjacentTaskFragments() {
+        final Task task = createTask(mDisplayContent);
+        mController.registerOrganizer(mIOrganizer);
+        mTaskFragment = new TaskFragmentBuilder(mAtm)
+                .setParentTask(task)
+                .setFragmentToken(mFragmentToken)
+                .build();
+        mWindowOrganizerController.mLaunchTaskFragments.put(mFragmentToken, mTaskFragment);
+        final IBinder fragmentToken2 = new Binder();
+        final TaskFragment taskFragment2 = new TaskFragmentBuilder(mAtm)
+                .setParentTask(task)
+                .setFragmentToken(fragmentToken2)
+                .build();
+        mWindowOrganizerController.mLaunchTaskFragments.put(fragmentToken2, taskFragment2);
+        mTransaction.setAdjacentTaskFragments(mFragmentToken, fragmentToken2, null /* params */);
+        mOrganizer.applyTransaction(mTransaction);
+
+        // Not allowed because TaskFragments are not organized by the caller organizer.
+        assertApplyTransactionDisallowed(mTransaction);
+
+        mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
+                "Test:TaskFragmentOrganizer" /* processName */);
+
+        // Not allowed because TaskFragment2 is not organized by the caller organizer.
+        assertApplyTransactionDisallowed(mTransaction);
+
+        mTaskFragment.onTaskFragmentOrganizerRemoved();
+        taskFragment2.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
+                "Test:TaskFragmentOrganizer" /* processName */);
+
+        // Not allowed because mTaskFragment is not organized by the caller organizer.
+        assertApplyTransactionDisallowed(mTransaction);
+
+        mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
+                "Test:TaskFragmentOrganizer" /* processName */);
+
+        assertApplyTransactionAllowed(mTransaction);
+    }
+
+    @Test
+    public void testApplyTransaction_enforceTaskFragmentOrganized_requestFocusOnTaskFragment() {
+        final Task task = createTask(mDisplayContent);
+        mController.registerOrganizer(mIOrganizer);
+        mTaskFragment = new TaskFragmentBuilder(mAtm)
+                .setParentTask(task)
+                .setFragmentToken(mFragmentToken)
+                .build();
+        mWindowOrganizerController.mLaunchTaskFragments.put(mFragmentToken, mTaskFragment);
+        mTransaction.requestFocusOnTaskFragment(mFragmentToken);
+        mOrganizer.applyTransaction(mTransaction);
+
+        // Not allowed because TaskFragment is not organized by the caller organizer.
+        assertApplyTransactionDisallowed(mTransaction);
+
+        mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
+                "Test:TaskFragmentOrganizer" /* processName */);
+
+        assertApplyTransactionAllowed(mTransaction);
+    }
+
     @Test
     public void testApplyTransaction_createTaskFragment_failForDifferentUid()
             throws RemoteException {
@@ -592,14 +682,16 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
             throws RemoteException {
         final Task task = createTask(mDisplayContent);
         final ActivityRecord activity = createActivityRecord(task);
+        // Skip manipulate the SurfaceControl.
+        doNothing().when(activity).setDropInputMode(anyInt());
         mOrganizer.applyTransaction(mTransaction);
         mController.registerOrganizer(mIOrganizer);
         mTaskFragment = new TaskFragmentBuilder(mAtm)
                 .setParentTask(task)
                 .setFragmentToken(mFragmentToken)
+                .setOrganizer(mOrganizer)
                 .build();
-        mWindowOrganizerController.mLaunchTaskFragments
+        mWindowOrganizerController.mLaunchTaskFragments.put(mFragmentToken, mTaskFragment);
-                .put(mFragmentToken, mTaskFragment);
         mTransaction.reparentActivityToTaskFragment(mFragmentToken, activity.token);
         doReturn(EMBEDDING_ALLOWED).when(mTaskFragment).isAllowedToEmbedActivity(activity);
         clearInvocations(mAtm.mRootWindowContainer);