Revert "Verify hierarchy change for cross-UID ActivityEmbedding"

Revert "Verify hierarchy change for cross-UID ActivityEmbedding"

Revert submission 17157035-cross-process-ae-hierarchy

Reason for revert: broken test b/225331448
Reverted Changes:
Ie102621bd:Verify hierarchy change for cross-UID ActivityEmbe...
I0c3c1d837:Verify hierarchy change for cross-UID ActivityEmbe...

Change-Id: If69102b2f47b470a0987173f3e0e39332a0f6bbd
This commit is contained in:
Santiago Aboy Solanes
2022-03-18 12:45:12 +00:00
parent 0f2ead8d49
commit c9deac5afb
4 changed files with 85 additions and 185 deletions

View File

@@ -520,16 +520,7 @@ class TaskFragment extends WindowContainer<WindowContainer> {
|| isAllowedToEmbedActivityInTrustedMode(a);
}
/**
* Checks if the organized task fragment is allowed to embed activity in untrusted mode.
*/
boolean isAllowedToEmbedActivityInUntrustedMode(@NonNull ActivityRecord a) {
final WindowContainer parent = getParent();
if (parent == null || !parent.getBounds().contains(getBounds())) {
// Without full trust between the host and the embedded activity, we don't allow
// TaskFragment to have bounds outside of the parent bounds.
return false;
}
return (a.info.flags & FLAG_ALLOW_UNTRUSTED_ACTIVITY_EMBEDDING)
== FLAG_ALLOW_UNTRUSTED_ACTIVITY_EMBEDDING;
}

View File

@@ -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_ADD_RECT_INSETS_PROVIDER;
import static android.window.WindowContainerTransaction.HierarchyOp.HIERARCHY_OP_TYPE_CHILDREN_TASKS_REPARENT;
import static android.window.WindowContainerTransaction.HierarchyOp.HIERARCHY_OP_TYPE_CREATE_TASK_FRAGMENT;
@@ -678,7 +680,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
break;
}
}
effects |= deleteTaskFragment(taskFragment, organizer, errorCallbackToken);
effects |= deleteTaskFragment(taskFragment, errorCallbackToken);
break;
}
case HIERARCHY_OP_TYPE_START_ACTIVITY_IN_TASK_FRAGMENT: {
@@ -696,7 +698,8 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
.startActivityInTaskFragment(tf, activityIntent, activityOptions,
hop.getCallingActivity(), caller.mUid, caller.mPid);
if (!isStartResultSuccessful(result)) {
sendTaskFragmentOperationFailure(organizer, errorCallbackToken,
sendTaskFragmentOperationFailure(tf.getTaskFragmentOrganizer(),
errorCallbackToken,
convertStartFailureToThrowable(result, activityIntent));
} else {
effects |= TRANSACT_EFFECTS_LIFECYCLE;
@@ -706,20 +709,13 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
case HIERARCHY_OP_TYPE_REPARENT_ACTIVITY_TO_TASK_FRAGMENT: {
final IBinder fragmentToken = hop.getNewParent();
final ActivityRecord activity = ActivityRecord.forTokenLocked(hop.getContainer());
final TaskFragment parent = mLaunchTaskFragments.get(fragmentToken);
if (parent == null || activity == null) {
if (!mLaunchTaskFragments.containsKey(fragmentToken) || activity == null) {
final Throwable exception = new IllegalArgumentException(
"Not allowed to operate with invalid fragment token or activity.");
sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);
break;
}
if (!parent.isAllowedToEmbedActivity(activity)) {
final Throwable exception = new SecurityException(
"The task fragment is not trusted to embed the given activity.");
sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);
break;
}
activity.reparent(parent, POSITION_TOP);
activity.reparent(mLaunchTaskFragments.get(fragmentToken), POSITION_TOP);
effects |= TRANSACT_EFFECTS_LIFECYCLE;
break;
}
@@ -881,14 +877,12 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
final WindowContainer newParent = hop.getNewParent() != null
? WindowContainer.fromBinder(hop.getNewParent())
: null;
if (oldParent == null || oldParent.asTaskFragment() == null
|| !oldParent.isAttached()) {
if (oldParent == null || !oldParent.isAttached()) {
Slog.e(TAG, "Attempt to operate on unknown or detached container: "
+ oldParent);
break;
}
reparentTaskFragment(oldParent.asTaskFragment(), newParent, organizer,
errorCallbackToken);
reparentTaskFragment(oldParent, newParent, errorCallbackToken);
effects |= TRANSACT_EFFECTS_LIFECYCLE;
break;
}
@@ -1244,7 +1238,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
mService.enforceTaskPermission(func);
}
private void enforceTaskPermission(String func, @Nullable WindowContainerTransaction t) {
private void enforceTaskPermission(String func, WindowContainerTransaction t) {
if (t == null || t.getTaskFragmentOrganizer() == null) {
enforceTaskPermission(func);
return;
@@ -1269,11 +1263,14 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
while (entries.hasNext()) {
final Map.Entry<IBinder, WindowContainerTransaction.Change> entry = entries.next();
// Only allow to apply changes to TaskFragment that is created by this organizer.
final WindowContainer wc = WindowContainer.fromBinder(entry.getKey());
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<WindowContainerTransaction.HierarchyOp> hops = t.getHierarchyOps();
for (int i = hops.size() - 1; i >= 0; i--) {
@@ -1347,6 +1344,8 @@ 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) {
@@ -1354,48 +1353,35 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
Slog.e(TAG, "Attempt to operate on task fragment that no longer exists");
return;
}
if (change == null) {
return;
}
final int changeMask = change.getChangeMask();
if (changeMask != 0) {
// None of the change should be requested from a TaskFragment organizer.
String msg = "Permission Denial: " + func + " from pid="
+ Binder.getCallingPid() + ", uid=" + Binder.getCallingUid()
+ " trying to apply changes of " + changeMask + " to TaskFragment"
+ " TaskFragmentOrganizer=" + organizer;
Slog.w(TAG, msg);
throw new SecurityException(msg);
}
// Check if TaskFragment is embedded in fully trusted mode.
// Check if TaskFragment is embedded in fully trusted mode
if (wc.asTaskFragment().isAllowedToBeEmbeddedInTrustedMode()) {
// Fully trusted, no need to check further
return;
}
final WindowContainer wcParent = wc.getParent();
if (wcParent == null) {
Slog.e(TAG, "Attempt to apply config change on task fragment that has no parent");
if (change == null) {
return;
}
final Configuration requestedConfig = change.getConfiguration();
final Configuration parentConfig = wcParent.getConfiguration();
if (parentConfig.screenWidthDp < requestedConfig.screenWidthDp
|| parentConfig.screenHeightDp < requestedConfig.screenHeightDp
|| parentConfig.smallestScreenWidthDp < requestedConfig.smallestScreenWidthDp) {
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 screen width/height greater than parent's for non-trusted"
+ " host, TaskFragmentOrganizer=" + organizer;
+ " 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) {
// No bounds change.
// Nothing else to check.
return;
}
final WindowConfiguration requestedWindowConfig = requestedConfig.windowConfiguration;
final WindowConfiguration parentWindowConfig = parentConfig.windowConfiguration;
if (!parentWindowConfig.getBounds().contains(requestedWindowConfig.getBounds())) {
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,"
@@ -1403,17 +1389,6 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
Slog.w(TAG, msg);
throw new SecurityException(msg);
}
if (requestedWindowConfig.getAppBounds() != null
&& parentWindowConfig.getAppBounds() != null
&& !parentWindowConfig.getAppBounds().contains(
requestedWindowConfig.getAppBounds())) {
String msg = "Permission Denial: " + func + " from pid="
+ Binder.getCallingPid() + ", uid=" + Binder.getCallingUid()
+ " trying to apply app bounds outside of parent for non-trusted host,"
+ " TaskFragmentOrganizer=" + organizer;
Slog.w(TAG, msg);
throw new SecurityException(msg);
}
}
void createTaskFragment(@NonNull TaskFragmentCreationParams creationParams,
@@ -1439,7 +1414,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
if (ownerActivity.getTask().effectiveUid != ownerActivity.getUid()
|| ownerActivity.getTask().effectiveUid != caller.mUid) {
final Throwable exception =
new SecurityException("Not allowed to operate with the ownerToken while "
new IllegalArgumentException("Not allowed to operate with the ownerToken while "
+ "the root activity of the target task belong to the different app");
sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);
return;
@@ -1456,46 +1431,33 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
mLaunchTaskFragments.put(creationParams.getFragmentToken(), taskFragment);
}
void reparentTaskFragment(@NonNull TaskFragment oldParent, @Nullable WindowContainer newParent,
@Nullable ITaskFragmentOrganizer organizer, @Nullable IBinder errorCallbackToken) {
final TaskFragment newParentTF;
if (newParent == null) {
// Use the old parent's parent if the caller doesn't specify the new parent.
newParentTF = oldParent.getTask();
} else {
newParentTF = newParent.asTaskFragment();
void reparentTaskFragment(@NonNull WindowContainer oldParent,
@Nullable WindowContainer newParent, @Nullable IBinder errorCallbackToken) {
WindowContainer parent = newParent;
if (parent == null && oldParent.asTaskFragment() != null) {
parent = oldParent.asTaskFragment().getTask();
}
if (newParentTF == null) {
if (parent == null) {
final Throwable exception =
new IllegalArgumentException("Not allowed to operate with invalid container");
sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);
sendTaskFragmentOperationFailure(oldParent.asTaskFragment().getTaskFragmentOrganizer(),
errorCallbackToken, exception);
return;
}
if (newParentTF.getTaskFragmentOrganizer() != null) {
// We are reparenting activities to a new embedded TaskFragment, this operation is only
// allowed if the new parent is trusted by all reparent activities.
final boolean isEmbeddingDisallowed = oldParent.forAllActivities(activity ->
!newParentTF.isAllowedToEmbedActivity(activity));
if (isEmbeddingDisallowed) {
final Throwable exception = new SecurityException(
"The new parent is not trusted to embed the activities.");
sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);
return;
}
}
while (oldParent.hasChild()) {
oldParent.getChildAt(0).reparent(newParentTF, POSITION_TOP);
oldParent.getChildAt(0).reparent(parent, POSITION_TOP);
}
}
private int deleteTaskFragment(@NonNull TaskFragment taskFragment,
@Nullable ITaskFragmentOrganizer organizer, @Nullable IBinder errorCallbackToken) {
@Nullable IBinder errorCallbackToken) {
final int index = mLaunchTaskFragments.indexOfValue(taskFragment);
if (index < 0) {
final Throwable exception =
new IllegalArgumentException("Not allowed to operate with invalid "
+ "taskFragment");
sendTaskFragmentOperationFailure(organizer, errorCallbackToken, exception);
sendTaskFragmentOperationFailure(taskFragment.getTaskFragmentOrganizer(),
errorCallbackToken, exception);
return 0;
}
mLaunchTaskFragments.removeAt(index);

View File

@@ -18,7 +18,6 @@ package com.android.server.wm;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn;
import static com.android.server.wm.WindowContainer.POSITION_TOP;
import static com.android.server.wm.testing.Assert.assertThrows;
import static org.junit.Assert.assertEquals;
@@ -250,13 +249,19 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
// the organizer.
mTransaction.setBounds(mFragmentWindowToken, new Rect(0, 0, 100, 100));
assertApplyTransactionDisallowed(mTransaction);
assertThrows(SecurityException.class, () -> {
try {
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
} catch (RemoteException e) {
fail();
}
});
// Allow transaction to change a TaskFragment created by the organizer.
mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
"Test:TaskFragmentOrganizer" /* processName */);
assertApplyTransactionAllowed(mTransaction);
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
}
@Test
@@ -267,13 +272,19 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
// the organizer.
mTransaction.reorder(mFragmentWindowToken, true /* onTop */);
assertApplyTransactionDisallowed(mTransaction);
assertThrows(SecurityException.class, () -> {
try {
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
} catch (RemoteException e) {
fail();
}
});
// Allow transaction to change a TaskFragment created by the organizer.
mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
"Test:TaskFragmentOrganizer" /* processName */);
assertApplyTransactionAllowed(mTransaction);
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
}
@Test
@@ -287,21 +298,27 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
// the organizer.
mTransaction.deleteTaskFragment(mFragmentWindowToken);
assertApplyTransactionDisallowed(mTransaction);
assertThrows(SecurityException.class, () -> {
try {
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
} catch (RemoteException e) {
fail();
}
});
// Allow transaction to change a TaskFragment created by the organizer.
mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
"Test:TaskFragmentOrganizer" /* processName */);
clearInvocations(mAtm.mRootWindowContainer);
assertApplyTransactionAllowed(mTransaction);
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
// No lifecycle update when the TaskFragment is not recorded.
verify(mAtm.mRootWindowContainer, never()).resumeFocusedTasksTopActivities();
mAtm.mWindowOrganizerController.mLaunchTaskFragments
.put(mFragmentToken, mTaskFragment);
assertApplyTransactionAllowed(mTransaction);
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
verify(mAtm.mRootWindowContainer).resumeFocusedTasksTopActivities();
}
@@ -318,7 +335,13 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
// the organizer.
mTransaction.setAdjacentRoots(mFragmentWindowToken, token2, false /* moveTogether */);
assertApplyTransactionDisallowed(mTransaction);
assertThrows(SecurityException.class, () -> {
try {
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
} catch (RemoteException e) {
fail();
}
});
// Allow transaction to change a TaskFragment created by the organizer.
mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
@@ -327,7 +350,7 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
"Test:TaskFragmentOrganizer" /* processName */);
clearInvocations(mAtm.mRootWindowContainer);
assertApplyTransactionAllowed(mTransaction);
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
verify(mAtm.mRootWindowContainer).resumeFocusedTasksTopActivities();
}
@@ -400,14 +423,20 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
// the organizer.
mTransaction.reparentChildren(mFragmentWindowToken, null /* newParent */);
assertApplyTransactionDisallowed(mTransaction);
assertThrows(SecurityException.class, () -> {
try {
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
} catch (RemoteException e) {
fail();
}
});
// Allow transaction to change a TaskFragment created by the organizer.
mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
"Test:TaskFragmentOrganizer" /* processName */);
clearInvocations(mAtm.mRootWindowContainer);
assertApplyTransactionAllowed(mTransaction);
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
verify(mAtm.mRootWindowContainer).resumeFocusedTasksTopActivities();
}
@@ -425,7 +454,6 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
mAtm.mWindowOrganizerController.mLaunchTaskFragments
.put(mFragmentToken, mTaskFragment);
mTransaction.reparentActivityToTaskFragment(mFragmentToken, activity.token);
doReturn(true).when(mTaskFragment).isAllowedToEmbedActivity(activity);
clearInvocations(mAtm.mRootWindowContainer);
mAtm.getWindowOrganizerController().applyTransaction(mTransaction);
@@ -512,66 +540,6 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
assertNull(mAtm.mWindowOrganizerController.getTaskFragment(fragmentToken));
}
/**
* For config change to untrusted embedded TaskFragment, we only allow bounds change within
* its parent bounds.
*/
@Test
public void testUntrustedEmbedding_configChange() throws RemoteException {
mController.registerOrganizer(mIOrganizer);
mOrganizer.applyTransaction(mTransaction);
mTaskFragment.setTaskFragmentOrganizer(mOrganizerToken, 10 /* uid */,
"Test:TaskFragmentOrganizer" /* processName */);
doReturn(false).when(mTaskFragment).isAllowedToBeEmbeddedInTrustedMode();
final Task task = createTask(mDisplayContent);
final Rect taskBounds = new Rect(task.getBounds());
final Rect taskAppBounds = new Rect(task.getWindowConfiguration().getAppBounds());
final int taskScreenWidthDp = task.getConfiguration().screenWidthDp;
final int taskScreenHeightDp = task.getConfiguration().screenHeightDp;
final int taskSmallestScreenWidthDp = task.getConfiguration().smallestScreenWidthDp;
task.addChild(mTaskFragment, POSITION_TOP);
// Throw exception if the transaction is trying to change bounds of an untrusted outside of
// its parent's.
// setBounds
final Rect tfBounds = new Rect(taskBounds);
tfBounds.right++;
mTransaction.setBounds(mFragmentWindowToken, tfBounds);
assertApplyTransactionDisallowed(mTransaction);
mTransaction.setBounds(mFragmentWindowToken, taskBounds);
assertApplyTransactionAllowed(mTransaction);
// setAppBounds
final Rect tfAppBounds = new Rect(taskAppBounds);
tfAppBounds.right++;
mTransaction.setAppBounds(mFragmentWindowToken, tfAppBounds);
assertApplyTransactionDisallowed(mTransaction);
mTransaction.setAppBounds(mFragmentWindowToken, taskAppBounds);
assertApplyTransactionAllowed(mTransaction);
// setScreenSizeDp
mTransaction.setScreenSizeDp(mFragmentWindowToken, taskScreenWidthDp + 1,
taskScreenHeightDp + 1);
assertApplyTransactionDisallowed(mTransaction);
mTransaction.setScreenSizeDp(mFragmentWindowToken, taskScreenWidthDp, taskScreenHeightDp);
assertApplyTransactionAllowed(mTransaction);
// setSmallestScreenWidthDp
mTransaction.setSmallestScreenWidthDp(mFragmentWindowToken, taskSmallestScreenWidthDp + 1);
assertApplyTransactionDisallowed(mTransaction);
mTransaction.setSmallestScreenWidthDp(mFragmentWindowToken, taskSmallestScreenWidthDp);
assertApplyTransactionAllowed(mTransaction);
// Any of the change mask is not allowed.
mTransaction.setFocusable(mFragmentWindowToken, false);
assertApplyTransactionDisallowed(mTransaction);
}
/**
* Creates a {@link TaskFragment} with the {@link WindowContainerTransaction}. Calls
* {@link WindowOrganizerController#applyTransaction} to apply the transaction,
@@ -588,24 +556,4 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
// Allow organizer to create TaskFragment and start/reparent activity to TaskFragment.
wct.createTaskFragment(params);
}
/** Asserts that applying the given transaction will throw a {@link SecurityException}. */
private void assertApplyTransactionDisallowed(WindowContainerTransaction t) {
assertThrows(SecurityException.class, () -> {
try {
mAtm.getWindowOrganizerController().applyTransaction(t);
} catch (RemoteException e) {
fail();
}
});
}
/** Asserts that applying the given transaction will not throw any exception. */
private void assertApplyTransactionAllowed(WindowContainerTransaction t) {
try {
mAtm.getWindowOrganizerController().applyTransaction(t);
} catch (RemoteException e) {
fail();
}
}
}

View File

@@ -1264,7 +1264,6 @@ class WindowTestsBase extends SystemServiceTestsBase {
mOrganizer.getOrganizerToken(), DEFAULT_TASK_FRAGMENT_ORGANIZER_UID,
DEFAULT_TASK_FRAGMENT_ORGANIZER_PROCESS_NAME);
}
spyOn(taskFragment);
return taskFragment;
}
}