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/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index f77eeb2..d549346 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -100,7 +100,7 @@
const ui::Dataspace reqDataspace,
const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
uint32_t reqWidth, uint32_t reqHeight, bool useIdentityTransform,
- ISurfaceComposer::Rotation rotation) {
+ ISurfaceComposer::Rotation rotation, bool captureSecureLayers) {
Parcel data, reply;
data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
data.writeStrongBinder(display);
@@ -111,6 +111,7 @@
data.writeUint32(reqHeight);
data.writeInt32(static_cast<int32_t>(useIdentityTransform));
data.writeInt32(static_cast<int32_t>(rotation));
+ data.writeInt32(static_cast<int32_t>(captureSecureLayers));
status_t result = remote()->transact(BnSurfaceComposer::CAPTURE_SCREEN, data, &reply);
if (result != NO_ERROR) {
ALOGE("captureScreen failed to transact: %d", result);
@@ -884,10 +885,11 @@
uint32_t reqHeight = data.readUint32();
bool useIdentityTransform = static_cast<bool>(data.readInt32());
int32_t rotation = data.readInt32();
+ bool captureSecureLayers = static_cast<bool>(data.readInt32());
status_t res = captureScreen(display, &outBuffer, reqDataspace, reqPixelFormat,
sourceCrop, reqWidth, reqHeight, useIdentityTransform,
- static_cast<ISurfaceComposer::Rotation>(rotation));
+ static_cast<ISurfaceComposer::Rotation>(rotation), captureSecureLayers);
reply->writeInt32(res);
if (res == NO_ERROR) {
reply->write(*outBuffer);
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index bf2a03d..ffb981c 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1464,18 +1464,27 @@
status_t ScreenshotClient::capture(const sp<IBinder>& display, const ui::Dataspace reqDataSpace,
const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
uint32_t reqWidth, uint32_t reqHeight, bool useIdentityTransform,
- uint32_t rotation, sp<GraphicBuffer>* outBuffer) {
+ uint32_t rotation, bool captureSecureLayers, sp<GraphicBuffer>* outBuffer) {
sp<ISurfaceComposer> s(ComposerService::getComposerService());
if (s == nullptr) return NO_INIT;
status_t ret = s->captureScreen(display, outBuffer, reqDataSpace, reqPixelFormat, sourceCrop,
- reqWidth, reqHeight, useIdentityTransform,
- static_cast<ISurfaceComposer::Rotation>(rotation));
+ reqWidth, reqHeight, useIdentityTransform,
+ static_cast<ISurfaceComposer::Rotation>(rotation),
+ captureSecureLayers);
if (ret != NO_ERROR) {
return ret;
}
return ret;
}
+status_t ScreenshotClient::capture(const sp<IBinder>& display, const ui::Dataspace reqDataSpace,
+ const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
+ uint32_t reqWidth, uint32_t reqHeight, bool useIdentityTransform,
+ uint32_t rotation, sp<GraphicBuffer>* outBuffer) {
+ return capture(display, reqDataSpace, reqPixelFormat, sourceCrop, reqWidth,
+ reqHeight, useIdentityTransform, rotation, false, outBuffer);
+}
+
status_t ScreenshotClient::captureLayers(const sp<IBinder>& layerHandle,
const ui::Dataspace reqDataSpace,
const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index 1a0b6bb..6409a12 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -209,7 +209,8 @@
const ui::Dataspace reqDataspace,
const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
uint32_t reqWidth, uint32_t reqHeight, bool useIdentityTransform,
- Rotation rotation = eRotateNone) = 0;
+ Rotation rotation = eRotateNone,
+ bool captureSecureLayers = false) = 0;
/**
* Capture the specified screen. This requires READ_FRAME_BUFFER
* permission. This function will fail if there is a secure window on
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index aa99bd2..0a611c2 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -453,6 +453,10 @@
static status_t capture(const sp<IBinder>& display, const ui::Dataspace reqDataSpace,
const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
uint32_t reqWidth, uint32_t reqHeight, bool useIdentityTransform,
+ uint32_t rotation, bool captureSecureLayers, sp<GraphicBuffer>* outBuffer);
+ static status_t capture(const sp<IBinder>& display, const ui::Dataspace reqDataSpace,
+ const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
+ uint32_t reqWidth, uint32_t reqHeight, bool useIdentityTransform,
uint32_t rotation, sp<GraphicBuffer>* outBuffer);
static status_t captureLayers(const sp<IBinder>& layerHandle, const ui::Dataspace reqDataSpace,
const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index f127853..e14c698 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -615,7 +615,8 @@
const ui::Dataspace /*reqDataspace*/,
const ui::PixelFormat /*reqPixelFormat*/, Rect /*sourceCrop*/,
uint32_t /*reqWidth*/, uint32_t /*reqHeight*/,
- bool /*useIdentityTransform*/, Rotation /*rotation*/) override {
+ bool /*useIdentityTransform*/, Rotation /*rotation*/,
+ bool /*captureSecureLayers*/) override {
return NO_ERROR;
}
virtual status_t captureLayers(const sp<IBinder>& /*parentHandle*/,