SurfaceFlinger: Indicate whether we have captured secure layers.
For purposes of the screen rotation animation the system server is allowed
to capture secure (not protected) layers and trusted not to persist screenshots
which may contain secure layers. However when displaying the screen rotation animation,
the layer the screenshot is placed on will itself not be secure, so if we record
the animation the recording will contain persisted versions of the secure content. Here
we forward whether the screenshot contains secure content so that system server can do the right thing.
Bug: 69703445
Test: Transaction_test#SetFlagsSecureEuidSystem
Change-Id: I828cfe3faee3a0c84525f90b9df8b66e392bc240
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index 8d7baf3..a3165dd 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -109,7 +109,7 @@
}
virtual status_t captureScreen(const sp<IBinder>& display, sp<GraphicBuffer>* outBuffer,
- const ui::Dataspace reqDataspace,
+ bool& outCapturedSecureLayers, const ui::Dataspace reqDataspace,
const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
uint32_t reqWidth, uint32_t reqHeight, bool useIdentityTransform,
ISurfaceComposer::Rotation rotation, bool captureSecureLayers) {
@@ -137,6 +137,7 @@
*outBuffer = new GraphicBuffer();
reply.read(**outBuffer);
+ outCapturedSecureLayers = reply.readBool();
return result;
}
@@ -1027,12 +1028,17 @@
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), captureSecureLayers);
+ bool capturedSecureLayers = false;
+ status_t res = captureScreen(display, &outBuffer, capturedSecureLayers, reqDataspace,
+ reqPixelFormat, sourceCrop, reqWidth, reqHeight,
+ useIdentityTransform,
+ static_cast<ISurfaceComposer::Rotation>(rotation),
+ captureSecureLayers);
+
reply->writeInt32(res);
if (res == NO_ERROR) {
reply->write(*outBuffer);
+ reply->writeBool(capturedSecureLayers);
}
return NO_ERROR;
}
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 0e61702..611da89 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -1542,13 +1542,15 @@
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, bool captureSecureLayers, sp<GraphicBuffer>* outBuffer) {
+ uint32_t rotation, bool captureSecureLayers,
+ sp<GraphicBuffer>* outBuffer, bool& outCapturedSecureLayers) {
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),
- captureSecureLayers);
+ status_t ret =
+ s->captureScreen(display, outBuffer, outCapturedSecureLayers, reqDataSpace,
+ reqPixelFormat, sourceCrop, reqWidth, reqHeight, useIdentityTransform,
+ static_cast<ISurfaceComposer::Rotation>(rotation),
+ captureSecureLayers);
if (ret != NO_ERROR) {
return ret;
}
@@ -1559,8 +1561,9 @@
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);
+ bool ignored;
+ return capture(display, reqDataSpace, reqPixelFormat, sourceCrop, reqWidth, reqHeight,
+ useIdentityTransform, rotation, false, outBuffer, ignored);
}
status_t ScreenshotClient::captureLayers(const sp<IBinder>& layerHandle,
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index 14d92bf..415b2d5 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -212,7 +212,7 @@
* it) around its center.
*/
virtual status_t captureScreen(const sp<IBinder>& display, sp<GraphicBuffer>* outBuffer,
- const ui::Dataspace reqDataspace,
+ bool& outCapturedSecureLayers, const ui::Dataspace reqDataspace,
const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
uint32_t reqWidth, uint32_t reqHeight, bool useIdentityTransform,
Rotation rotation = eRotateNone,
@@ -241,8 +241,10 @@
virtual status_t captureScreen(const sp<IBinder>& display, sp<GraphicBuffer>* outBuffer,
Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight,
bool useIdentityTransform, Rotation rotation = eRotateNone) {
- return captureScreen(display, outBuffer, ui::Dataspace::V0_SRGB, ui::PixelFormat::RGBA_8888,
- sourceCrop, reqWidth, reqHeight, useIdentityTransform, rotation);
+ bool outIgnored;
+ return captureScreen(display, outBuffer, outIgnored, ui::Dataspace::V0_SRGB,
+ ui::PixelFormat::RGBA_8888, sourceCrop, reqWidth, reqHeight,
+ useIdentityTransform, rotation);
}
template <class AA>
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index f64fb61..9d34468 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -509,7 +509,8 @@
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);
+ uint32_t rotation, bool captureSecureLayers,
+ sp<GraphicBuffer>* outBuffer, bool& outCapturedSecureLayers);
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,
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index fa2e97f..d5bdd74 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -126,7 +126,7 @@
}
// This test probably doesn't belong here.
-TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersSucceed) {
+TEST_F(SurfaceTest, ScreenshotsOfProtectedBuffersDontSucceed) {
sp<ANativeWindow> anw(mSurface);
// Verify the screenshot works with no protected buffers.
@@ -136,8 +136,9 @@
ASSERT_FALSE(display == nullptr);
sp<GraphicBuffer> outBuffer;
+ bool ignored;
ASSERT_EQ(NO_ERROR,
- sf->captureScreen(display, &outBuffer, ui::Dataspace::V0_SRGB,
+ sf->captureScreen(display, &outBuffer, ignored, ui::Dataspace::V0_SRGB,
ui::PixelFormat::RGBA_8888, Rect(), 64, 64, false));
ASSERT_EQ(NO_ERROR, native_window_api_connect(anw.get(),
@@ -169,7 +170,7 @@
ASSERT_EQ(NO_ERROR, anw->queueBuffer(anw.get(), buf, -1));
}
ASSERT_EQ(NO_ERROR,
- sf->captureScreen(display, &outBuffer, ui::Dataspace::V0_SRGB,
+ sf->captureScreen(display, &outBuffer, ignored, ui::Dataspace::V0_SRGB,
ui::PixelFormat::RGBA_8888, Rect(), 64, 64, false));
}
@@ -615,6 +616,7 @@
status_t setActiveColorMode(const sp<IBinder>& /*display*/,
ColorMode /*colorMode*/) override { return NO_ERROR; }
status_t captureScreen(const sp<IBinder>& /*display*/, sp<GraphicBuffer>* /*outBuffer*/,
+ bool& /* outCapturedSecureLayers */,
const ui::Dataspace /*reqDataspace*/,
const ui::PixelFormat /*reqPixelFormat*/, Rect /*sourceCrop*/,
uint32_t /*reqWidth*/, uint32_t /*reqHeight*/,
diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp
index 35f11fc..252ff0d 100644
--- a/services/surfaceflinger/RegionSamplingThread.cpp
+++ b/services/surfaceflinger/RegionSamplingThread.cpp
@@ -392,7 +392,8 @@
//
// To avoid this, we drop the mutex while we call into SF.
mMutex.unlock();
- mFlinger.captureScreenCommon(renderArea, traverseLayers, buffer, false);
+ bool ignored;
+ mFlinger.captureScreenCommon(renderArea, traverseLayers, buffer, false, ignored);
mMutex.lock();
std::vector<Descriptor> activeDescriptors;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index fccd910..e2a134b 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -5369,7 +5369,8 @@
};
status_t SurfaceFlinger::captureScreen(const sp<IBinder>& displayToken,
- sp<GraphicBuffer>* outBuffer, const Dataspace reqDataspace,
+ sp<GraphicBuffer>* outBuffer, bool& outCapturedSecureLayers,
+ const Dataspace reqDataspace,
const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
uint32_t reqWidth, uint32_t reqHeight,
bool useIdentityTransform,
@@ -5402,7 +5403,7 @@
auto traverseLayers = std::bind(&SurfaceFlinger::traverseLayersInDisplay, this, display,
std::placeholders::_1);
return captureScreenCommon(renderArea, traverseLayers, outBuffer, reqPixelFormat,
- useIdentityTransform);
+ useIdentityTransform, outCapturedSecureLayers);
}
status_t SurfaceFlinger::captureLayers(
@@ -5564,14 +5565,18 @@
visitor(layer);
});
};
- return captureScreenCommon(renderArea, traverseLayers, outBuffer, reqPixelFormat, false);
+
+ bool outCapturedSecureLayers = false;
+ return captureScreenCommon(renderArea, traverseLayers, outBuffer, reqPixelFormat, false,
+ outCapturedSecureLayers);
}
status_t SurfaceFlinger::captureScreenCommon(RenderArea& renderArea,
TraverseLayersFunction traverseLayers,
sp<GraphicBuffer>* outBuffer,
const ui::PixelFormat reqPixelFormat,
- bool useIdentityTransform) {
+ bool useIdentityTransform,
+ bool& outCapturedSecureLayers) {
ATRACE_CALL();
// TODO(b/116112787) Make buffer usage a parameter.
@@ -5582,13 +5587,15 @@
static_cast<android_pixel_format>(reqPixelFormat), 1,
usage, "screenshot");
- return captureScreenCommon(renderArea, traverseLayers, *outBuffer, useIdentityTransform);
+ return captureScreenCommon(renderArea, traverseLayers, *outBuffer, useIdentityTransform,
+ outCapturedSecureLayers);
}
status_t SurfaceFlinger::captureScreenCommon(RenderArea& renderArea,
TraverseLayersFunction traverseLayers,
const sp<GraphicBuffer>& buffer,
- bool useIdentityTransform) {
+ bool useIdentityTransform,
+ bool& outCapturedSecureLayers) {
// This mutex protects syncFd and captureResult for communication of the return values from the
// main thread back to this Binder thread
std::mutex captureMutex;
@@ -5617,7 +5624,8 @@
Mutex::Autolock _l(mStateLock);
renderArea.render([&] {
result = captureScreenImplLocked(renderArea, traverseLayers, buffer.get(),
- useIdentityTransform, forSystem, &fd);
+ useIdentityTransform, forSystem, &fd,
+ outCapturedSecureLayers);
});
}
@@ -5757,21 +5765,19 @@
status_t SurfaceFlinger::captureScreenImplLocked(const RenderArea& renderArea,
TraverseLayersFunction traverseLayers,
ANativeWindowBuffer* buffer,
- bool useIdentityTransform,
- bool forSystem,
- int* outSyncFd) {
+ bool useIdentityTransform, bool forSystem,
+ int* outSyncFd, bool& outCapturedSecureLayers) {
ATRACE_CALL();
- bool secureLayerIsVisible = false;
-
traverseLayers([&](Layer* layer) {
- secureLayerIsVisible = secureLayerIsVisible || (layer->isVisible() && layer->isSecure());
+ outCapturedSecureLayers =
+ outCapturedSecureLayers || (layer->isVisible() && layer->isSecure());
});
// We allow the system server to take screenshots of secure layers for
// use in situations like the Screen-rotation animation and place
// the impetus on WindowManager to not persist them.
- if (secureLayerIsVisible && !forSystem) {
+ if (outCapturedSecureLayers && !forSystem) {
ALOGW("FB is protected: PERMISSION_DENIED");
return PERMISSION_DENIED;
}
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 7e8e836..7914afc 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -395,16 +395,17 @@
sp<IDisplayEventConnection> createDisplayEventConnection(
ISurfaceComposer::VsyncSource vsyncSource = eVsyncSourceApp) override;
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,
- bool captureSecureLayers) override;
+ bool& outCapturedSecureLayers, const ui::Dataspace reqDataspace,
+ const ui::PixelFormat reqPixelFormat, Rect sourceCrop,
+ uint32_t reqWidth, uint32_t reqHeight,
+ 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,
const std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& exclude,
float frameScale, bool childrenOnly) override;
+
status_t getDisplayStats(const sp<IBinder>& displayToken, DisplayStatInfo* stats) override;
status_t getDisplayConfigs(const sp<IBinder>& displayToken,
Vector<DisplayInfo>* configs) override {
@@ -640,13 +641,14 @@
int* outSyncFd);
status_t captureScreenCommon(RenderArea& renderArea, TraverseLayersFunction traverseLayers,
sp<GraphicBuffer>* outBuffer, const ui::PixelFormat reqPixelFormat,
- bool useIdentityTransform);
+ bool useIdentityTransform, bool& outCapturedSecureLayers);
status_t captureScreenCommon(RenderArea& renderArea, TraverseLayersFunction traverseLayers,
- const sp<GraphicBuffer>& buffer, bool useIdentityTransform);
+ const sp<GraphicBuffer>& buffer, bool useIdentityTransform,
+ bool& outCapturedSecureLayers);
status_t captureScreenImplLocked(const RenderArea& renderArea,
TraverseLayersFunction traverseLayers,
ANativeWindowBuffer* buffer, bool useIdentityTransform,
- bool forSystem, int* outSyncFd);
+ bool forSystem, int* outSyncFd, bool& outCapturedSecureLayers);
void traverseLayersInDisplay(const sp<const DisplayDevice>& display,
const LayerVector::Visitor& visitor);
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index ba854e3..d03c25d 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -1269,11 +1269,12 @@
// 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.
+ bool outCapturedSecureLayers;
ASSERT_EQ(NO_ERROR,
- composer->captureScreen(mDisplay, &outBuffer,
- ui::Dataspace::V0_SRGB, ui::PixelFormat::RGBA_8888,
- Rect(), 0, 0, false,
- ISurfaceComposer::eRotateNone, true));
+ composer->captureScreen(mDisplay, &outBuffer, outCapturedSecureLayers,
+ ui::Dataspace::V0_SRGB, ui::PixelFormat::RGBA_8888, Rect(), 0,
+ 0, false, ISurfaceComposer::eRotateNone, true));
+ ASSERT_EQ(true, outCapturedSecureLayers);
ScreenCapture sc(outBuffer);
sc.expectColor(Rect(0, 0, 32, 32), Color::RED);
}
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 81235ba..6bbcae3 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -276,12 +276,13 @@
auto onMessageReceived(int32_t what) { return mFlinger->onMessageReceived(what); }
- auto captureScreenImplLocked(const RenderArea& renderArea,
- SurfaceFlinger::TraverseLayersFunction traverseLayers,
- ANativeWindowBuffer* buffer, bool useIdentityTransform,
- bool forSystem, int* outSyncFd) {
+ auto captureScreenImplLocked(
+ const RenderArea& renderArea, SurfaceFlinger::TraverseLayersFunction traverseLayers,
+ ANativeWindowBuffer* buffer, bool useIdentityTransform, bool forSystem, int* outSyncFd) {
+ bool ignored;
return mFlinger->captureScreenImplLocked(renderArea, traverseLayers, buffer,
- useIdentityTransform, forSystem, outSyncFd);
+ useIdentityTransform, forSystem, outSyncFd,
+ ignored);
}
auto traverseLayersInDisplay(const sp<const DisplayDevice>& display,