SurfaceFlinger: Add Transaction#sanitize
Various elements of the Transaction interface require
a permission in order to apply. In particular the setTrustedOverlay
and setInputWindowInfo fields. These permission checks are
implemented by checking the PID and the UID of the process which
sent the transaction. Unfortunately widespread use of transaction
merging makes this inadequate. At the moment
IWindowSession#finishDrawing seems to be the only boundary on which
transactions move from client to system processes, and so we expose
a sanitize method and use it from there to resolve the situation
in an easily backportable way.
Moving forward it likely make sense to move security sensitive
interfaces off of Transaction. Most of the things behind permissions
currently are not truly security sensitive, more of just a request
not to use them.
It was also considered to sanitize transactions at all process
boundaries through writeToParcel, however this could be disruptive
as previously permissioned processes (WM and SysUI) could freely
exchange transactions. As the change needs to be backportable the
lowest risk option was chosen.
Bug: 213644870
Test: Existing tests pass
Change-Id: I424f45bc30ea8e56e4c4493203ee0749eabf239c
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 27d86bb..eec4a87 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -340,6 +340,79 @@
}
}
+void layer_state_t::sanitize(int32_t permissions) {
+ // TODO: b/109894387
+ //
+ // SurfaceFlinger's renderer is not prepared to handle cropping in the face of arbitrary
+ // rotation. To see the problem observe that if we have a square parent, and a child
+ // of the same size, then we rotate the child 45 degrees around its center, the child
+ // must now be cropped to a non rectangular 8 sided region.
+ //
+ // Of course we can fix this in the future. For now, we are lucky, SurfaceControl is
+ // private API, and arbitrary rotation is used in limited use cases, for instance:
+ // - WindowManager only uses rotation in one case, which is on a top level layer in which
+ // cropping is not an issue.
+ // - Launcher, as a privileged app, uses this to transition an application to PiP
+ // (picture-in-picture) mode.
+ //
+ // However given that abuse of rotation matrices could lead to surfaces extending outside
+ // of cropped areas, we need to prevent non-root clients without permission
+ // ACCESS_SURFACE_FLINGER nor ROTATE_SURFACE_FLINGER
+ // (a.k.a. everyone except WindowManager / tests / Launcher) from setting non rectangle
+ // preserving transformations.
+ if (what & eMatrixChanged) {
+ if (!(permissions & Permission::ROTATE_SURFACE_FLINGER)) {
+ ui::Transform t;
+ t.set(matrix.dsdx, matrix.dtdy, matrix.dtdx, matrix.dsdy);
+ if (!t.preserveRects()) {
+ what &= ~eMatrixChanged;
+ ALOGE("Stripped non rect preserving matrix in sanitize");
+ }
+ }
+ }
+
+ if (what & eFlagsChanged) {
+ if ((flags & eLayerIsDisplayDecoration) &&
+ !(permissions & Permission::INTERNAL_SYSTEM_WINDOW)) {
+ flags &= ~eLayerIsDisplayDecoration;
+ ALOGE("Stripped attempt to set LayerIsDisplayDecoration in sanitize");
+ }
+ }
+
+ if (what & layer_state_t::eInputInfoChanged) {
+ if (!(permissions & Permission::ACCESS_SURFACE_FLINGER)) {
+ what &= ~eInputInfoChanged;
+ ALOGE("Stripped attempt to set eInputInfoChanged in sanitize");
+ }
+ }
+ if (what & layer_state_t::eTrustedOverlayChanged) {
+ if (!(permissions & Permission::ACCESS_SURFACE_FLINGER)) {
+ what &= ~eTrustedOverlayChanged;
+ ALOGE("Stripped attempt to set eTrustedOverlay in sanitize");
+ }
+ }
+ if (what & layer_state_t::eDropInputModeChanged) {
+ if (!(permissions & Permission::ACCESS_SURFACE_FLINGER)) {
+ what &= ~eDropInputModeChanged;
+ ALOGE("Stripped attempt to set eDropInputModeChanged in sanitize");
+ }
+ }
+ if (what & layer_state_t::eFrameRateSelectionPriority) {
+ if (!(permissions & Permission::ACCESS_SURFACE_FLINGER)) {
+ what &= ~eFrameRateSelectionPriority;
+ ALOGE("Stripped attempt to set eFrameRateSelectionPriority in sanitize");
+ }
+ }
+ if (what & layer_state_t::eFrameRateChanged) {
+ if (!ValidateFrameRate(frameRate, frameRateCompatibility,
+ changeFrameRateStrategy,
+ "layer_state_t::sanitize",
+ permissions & Permission::ACCESS_SURFACE_FLINGER)) {
+ what &= ~eFrameRateChanged; // logged in ValidateFrameRate
+ }
+ }
+}
+
void layer_state_t::merge(const layer_state_t& other) {
if (other.what & ePositionChanged) {
what |= ePositionChanged;