Exclude secure layers from most screenshots taken by the system server.
In pre-P versions of Android, it was allowed to screenshot secure layers if the
buffer queue producer which was the target of the screenshot was owned by
the system (in this case SurfaceFlinger). This really was a synonym for:
The screen rotation animation was allowed to capture secure layers, but the other
code paths weren't. In O we mistakenly changed this check to always allow the system server
to capture secure layers via the captureScreen path (the captureLayers path used for
TaskSnapshots was unaffected). This can result in data leakage in cases where the
system server takes screenshots on behalf of other parts of the system (e.g. for
the assistant). To mitigate this we provide an explicit switch for the system server
to specify whether it wishes to capture Secure layers. While this is dangerous, I think
it is less dangerous than the previous implicit switch of capturing secure layers based on which
type of BufferQueue was passed in. The flag defaults to not capturing secure layers
and we set it to true in the one place we need it (for the screen rotation animation).
Non privileged clients can still not capture secure layers at all directly.
Test: SetFlagsSecureEUidSystem
Bug: 120610669
Change-Id: I288ad3bbb0444306e90fe3bb15e51b447539dea5
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index a827c47..c80925e 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -255,17 +255,18 @@
device->getCompositionDataSpace(), rotation) {}
DisplayRenderArea(const sp<const DisplayDevice> device, Rect sourceCrop, uint32_t reqWidth,
uint32_t reqHeight, ui::Dataspace reqDataSpace,
- ui::Transform::orientation_flags rotation)
+ ui::Transform::orientation_flags rotation, bool allowSecureLayers = true)
: RenderArea(reqWidth, reqHeight, CaptureFill::OPAQUE, reqDataSpace,
getDisplayRotation(rotation, device->getInstallOrientation())),
mDevice(device),
- mSourceCrop(sourceCrop) {}
+ mSourceCrop(sourceCrop),
+ mAllowSecureLayers(allowSecureLayers) {}
const ui::Transform& getTransform() const override { return mDevice->getTransform(); }
Rect getBounds() const override { return mDevice->getBounds(); }
int getHeight() const override { return mDevice->getHeight(); }
int getWidth() const override { return mDevice->getWidth(); }
- bool isSecure() const override { return mDevice->isSecure(); }
+ bool isSecure() const override { return mAllowSecureLayers && mDevice->isSecure(); }
const sp<const DisplayDevice> getDisplayDevice() const override { return mDevice; }
bool needsFiltering() const override {
@@ -356,6 +357,7 @@
const sp<const DisplayDevice> mDevice;
const Rect mSourceCrop;
+ const bool mAllowSecureLayers;
};
}; // namespace android
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 3abd6a7..9d58805 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -5484,7 +5484,8 @@
const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
uint32_t reqWidth, uint32_t reqHeight,
bool useIdentityTransform,
- ISurfaceComposer::Rotation rotation) {
+ ISurfaceComposer::Rotation rotation,
+ bool captureSecureLayers) {
ATRACE_CALL();
if (!displayToken) return BAD_VALUE;
@@ -5507,7 +5508,7 @@
}
DisplayRenderArea renderArea(display, sourceCrop, reqWidth, reqHeight, reqDataspace,
- renderAreaRotation);
+ renderAreaRotation, captureSecureLayers);
auto traverseLayers = std::bind(&SurfaceFlinger::traverseLayersInDisplay, this, display,
std::placeholders::_1);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 728e8d3..57911d5 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -431,7 +431,8 @@
status_t captureScreen(const sp<IBinder>& displayToken, sp<GraphicBuffer>* outBuffer,
const ui::Dataspace reqDataspace, const ui::PixelFormat reqPixelFormat,
Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight,
- bool useIdentityTransform, ISurfaceComposer::Rotation rotation) override;
+ bool useIdentityTransform, ISurfaceComposer::Rotation rotation,
+ bool captureSecureLayers) override;
status_t captureLayers(const sp<IBinder>& parentHandle, sp<GraphicBuffer>* outBuffer,
const ui::Dataspace reqDataspace, const ui::PixelFormat reqPixelFormat,
const Rect& sourceCrop, float frameScale, bool childrenOnly) override;
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index 7b21dbc..917453e 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -33,6 +33,7 @@
#include <gui/Surface.h>
#include <gui/SurfaceComposerClient.h>
#include <private/gui/ComposerService.h>
+#include <private/android_filesystem_config.h>
#include <ui/ColorSpace.h>
#include <ui/DisplayInfo.h>
@@ -41,6 +42,8 @@
#include <math.h>
#include <math/vec3.h>
+#include <sys/types.h>
+#include <unistd.h>
#include "BufferGenerator.h"
@@ -1201,6 +1204,56 @@
composer->captureScreen(mDisplay, &outBuffer, Rect(), 0, 0, false));
}
+/** RAII Wrapper around get/seteuid */
+class UIDFaker {
+ uid_t oldId;
+public:
+ UIDFaker(uid_t uid) {
+ oldId = geteuid();
+ seteuid(uid);
+ }
+ ~UIDFaker() {
+ seteuid(oldId);
+ }
+};
+
+TEST_F(LayerTransactionTest, SetFlagsSecureEUidSystem) {
+ sp<SurfaceControl> layer;
+ ASSERT_NO_FATAL_FAILURE(layer = createLayer("test", 32, 32));
+ ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(layer, Color::RED, 32, 32));
+
+ sp<ISurfaceComposer> composer = ComposerService::getComposerService();
+ sp<GraphicBuffer> outBuffer;
+ Transaction()
+ .setFlags(layer, layer_state_t::eLayerSecure, layer_state_t::eLayerSecure)
+ .apply(true);
+ ASSERT_EQ(PERMISSION_DENIED,
+ composer->captureScreen(mDisplay, &outBuffer, Rect(), 0, 0, false));
+
+ UIDFaker f(AID_SYSTEM);
+
+ // By default the system can capture screenshots with secure layers but they
+ // will be blacked out
+ ASSERT_EQ(NO_ERROR,
+ composer->captureScreen(mDisplay, &outBuffer, Rect(), 0, 0, false));
+
+ {
+ SCOPED_TRACE("as system");
+ auto shot = screenshot();
+ shot->expectColor(Rect(0, 0, 32, 32), Color::BLACK);
+ }
+
+ // Here we pass captureSecureLayers = true and since we are AID_SYSTEM we should be able
+ // to receive them...we are expected to take care with the results.
+ ASSERT_EQ(NO_ERROR,
+ composer->captureScreen(mDisplay, &outBuffer,
+ ui::Dataspace::V0_SRGB, ui::PixelFormat::RGBA_8888,
+ Rect(), 0, 0, false,
+ ISurfaceComposer::eRotateNone, true));
+ ScreenCapture sc(outBuffer);
+ sc.expectColor(Rect(0, 0, 32, 32), Color::RED);
+}
+
TEST_P(LayerRenderTypeTransactionTest, SetTransparentRegionHintBasic_BufferQueue) {
const Rect top(0, 0, 32, 16);
const Rect bottom(0, 16, 32, 32);