Make a layer secure if its parent is secure.
Dialogs and context menus can be captured in screenshots even if their
parent layer/window is marked secure. Therefore we should let the secure
flag be inherited from their parents as well.
Bug: 169851503
Test: Screen capture can't capture dialogs in a window marked secure.
Test: atest libsurfaceflinger_unittest
Test: atest SurfaceFlinger_test:ScreenCaptureTest
Change-Id: I0f32a03aeb733682df787105b051c691d1da7bc6
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index a79cbe4..d9faec4 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -778,7 +778,12 @@
bool Layer::isSecure() const {
const State& s(mDrawingState);
- return (s.flags & layer_state_t::eLayerSecure);
+ if (s.flags & layer_state_t::eLayerSecure) {
+ return true;
+ }
+
+ const auto p = mDrawingParent.promote();
+ return (p != nullptr) ? p->isSecure() : false;
}
// ----------------------------------------------------------------------------
diff --git a/services/surfaceflinger/tests/ScreenCapture_test.cpp b/services/surfaceflinger/tests/ScreenCapture_test.cpp
index e173996..7df3711 100644
--- a/services/surfaceflinger/tests/ScreenCapture_test.cpp
+++ b/services/surfaceflinger/tests/ScreenCapture_test.cpp
@@ -109,6 +109,41 @@
sc.expectColor(Rect(0, 0, 32, 32), Color::RED);
}
+TEST_F(ScreenCaptureTest, CaptureChildSetParentFlagsSecureEUidSystem) {
+ sp<SurfaceControl> parentLayer;
+ ASSERT_NO_FATAL_FAILURE(
+ parentLayer = createLayer("parent-test", 32, 32,
+ ISurfaceComposerClient::eSecure |
+ ISurfaceComposerClient::eFXSurfaceBufferQueue));
+ ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(parentLayer, Color::RED, 32, 32));
+
+ sp<SurfaceControl> childLayer;
+ ASSERT_NO_FATAL_FAILURE(childLayer = createLayer("child-test", 10, 10,
+ ISurfaceComposerClient::eFXSurfaceBufferQueue,
+ parentLayer.get()));
+ ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(childLayer, Color::BLUE, 10, 10));
+
+ Transaction().show(parentLayer).setLayer(parentLayer, INT32_MAX).show(childLayer).apply(true);
+
+ UIDFaker f(AID_SYSTEM);
+
+ {
+ SCOPED_TRACE("as system");
+ auto shot = screenshot();
+ shot->expectColor(Rect(0, 0, 10, 10), 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.
+ DisplayCaptureArgs args;
+ args.displayToken = mDisplay;
+ args.captureSecureLayers = true;
+ ASSERT_EQ(NO_ERROR, ScreenCapture::captureDisplay(args, mCaptureResults));
+ ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
+ ScreenCapture sc(mCaptureResults.buffer);
+ sc.expectColor(Rect(0, 0, 10, 10), Color::BLUE);
+}
+
TEST_F(ScreenCaptureTest, CaptureSingleLayer) {
LayerCaptureArgs captureArgs;
captureArgs.layerHandle = mBGSurfaceControl->getHandle();
@@ -801,4 +836,4 @@
verify([&] { screenshot()->expectChildColor(80, 80); });
}
-} // namespace android
\ No newline at end of file
+} // namespace android
diff --git a/services/surfaceflinger/tests/unittests/CompositionTest.cpp b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
index 8a0b551..a4f7449 100644
--- a/services/surfaceflinger/tests/unittests/CompositionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/CompositionTest.cpp
@@ -35,6 +35,7 @@
#include <utils/String8.h>
#include "BufferQueueLayer.h"
+#include "ContainerLayer.h"
#include "DisplayRenderArea.h"
#include "EffectLayer.h"
#include "Layer.h"
@@ -188,6 +189,7 @@
sp<compositionengine::mock::DisplaySurface> mDisplaySurface =
new compositionengine::mock::DisplaySurface();
mock::NativeWindow* mNativeWindow = new mock::NativeWindow();
+ std::vector<sp<Layer>> mAuxiliaryLayers;
sp<GraphicBuffer> mBuffer = new GraphicBuffer();
ANativeWindowBuffer* mNativeWindowBuffer = mBuffer->getNativeBuffer();
@@ -762,10 +764,9 @@
static void setupREBufferCompositionCommonCallExpectations(CompositionTest* /*test*/) {}
};
-struct SecureLayerProperties : public BaseLayerProperties<SecureLayerProperties> {
- using Base = BaseLayerProperties<SecureLayerProperties>;
-
- static constexpr uint32_t LAYER_FLAGS = ISurfaceComposerClient::eSecure;
+template <typename LayerProperties>
+struct CommonSecureLayerProperties : public BaseLayerProperties<LayerProperties> {
+ using Base = BaseLayerProperties<LayerProperties>;
static void setupInsecureREBufferCompositionCommonCallExpectations(CompositionTest* test) {
EXPECT_CALL(*test->mRenderEngine, drawLayers)
@@ -806,6 +807,13 @@
}
};
+struct ParentSecureLayerProperties
+ : public CommonSecureLayerProperties<ParentSecureLayerProperties> {};
+
+struct SecureLayerProperties : public CommonSecureLayerProperties<SecureLayerProperties> {
+ static constexpr uint32_t LAYER_FLAGS = ISurfaceComposerClient::eSecure;
+};
+
struct CursorLayerProperties : public BaseLayerProperties<CursorLayerProperties> {
using Base = BaseLayerProperties<CursorLayerProperties>;
@@ -841,6 +849,13 @@
Mock::VerifyAndClear(test->mRenderEngine);
Mock::VerifyAndClearExpectations(test->mMessageQueue);
+ initLayerDrawingStateAndComputeBounds(test, layer);
+
+ return layer;
+ }
+
+ template <typename L>
+ static void initLayerDrawingStateAndComputeBounds(CompositionTest* test, sp<L> layer) {
auto& layerDrawingState = test->mFlinger.mutableLayerDrawingState(layer);
layerDrawingState.layerStack = DEFAULT_LAYER_STACK;
layerDrawingState.active.w = 100;
@@ -848,8 +863,6 @@
layerDrawingState.color = half4(LayerProperties::COLOR[0], LayerProperties::COLOR[1],
LayerProperties::COLOR[2], LayerProperties::COLOR[3]);
layer->computeBounds(FloatRect(0, 0, 100, 100), ui::Transform(), 0.f /* shadowRadius */);
-
- return layer;
}
static void injectLayer(CompositionTest* test, sp<Layer> layer) {
@@ -970,6 +983,49 @@
}
};
+template <typename LayerProperties>
+struct ContainerLayerVariant : public BaseLayerVariant<LayerProperties> {
+ using Base = BaseLayerVariant<LayerProperties>;
+ using FlingerLayerType = sp<ContainerLayer>;
+
+ static FlingerLayerType createLayer(CompositionTest* test) {
+ LayerCreationArgs args(test->mFlinger.flinger(), sp<Client>(), "test-container-layer",
+ LayerProperties::WIDTH, LayerProperties::HEIGHT,
+ LayerProperties::LAYER_FLAGS, LayerMetadata());
+ FlingerLayerType layer = new ContainerLayer(args);
+ Base::template initLayerDrawingStateAndComputeBounds(test, layer);
+ return layer;
+ }
+};
+
+template <typename LayerVariant, typename ParentLayerVariant>
+struct ChildLayerVariant : public LayerVariant {
+ using Base = LayerVariant;
+ using FlingerLayerType = typename LayerVariant::FlingerLayerType;
+ using ParentBase = ParentLayerVariant;
+
+ static FlingerLayerType createLayer(CompositionTest* test) {
+ // Need to create child layer first. Otherwise layer history size will be 2.
+ FlingerLayerType layer = Base::createLayer(test);
+
+ typename ParentBase::FlingerLayerType parentLayer = ParentBase::createLayer(test);
+ parentLayer->addChild(layer);
+ test->mFlinger.setLayerDrawingParent(layer, parentLayer);
+
+ test->mAuxiliaryLayers.push_back(parentLayer);
+
+ return layer;
+ }
+
+ static void cleanupInjectedLayers(CompositionTest* test) {
+ // Clear auxiliary layers first so that child layer can be successfully destroyed in the
+ // following call.
+ test->mAuxiliaryLayers.clear();
+
+ Base::cleanupInjectedLayers(test);
+ }
+};
+
/* ------------------------------------------------------------------------
* Variants to control how the composition type is changed
*/
@@ -1359,6 +1415,38 @@
}
/* ------------------------------------------------------------------------
+ * Layers with a parent layer with ISurfaceComposerClient::eSecure, on a non-secure display
+ */
+
+TEST_F(CompositionTest,
+ HWCComposedBufferLayerWithSecureParentLayerOnInsecureDisplayWithDirtyGeometry) {
+ displayRefreshCompositionDirtyGeometry<
+ CompositionCase<InsecureDisplaySetupVariant,
+ ChildLayerVariant<BufferLayerVariant<ParentSecureLayerProperties>,
+ ContainerLayerVariant<SecureLayerProperties>>,
+ KeepCompositionTypeVariant<IComposerClient::Composition::CLIENT>,
+ ForcedClientCompositionResultVariant>>();
+}
+
+TEST_F(CompositionTest,
+ HWCComposedBufferLayerWithSecureParentLayerOnInsecureDisplayWithDirtyFrame) {
+ displayRefreshCompositionDirtyFrame<
+ CompositionCase<InsecureDisplaySetupVariant,
+ ChildLayerVariant<BufferLayerVariant<ParentSecureLayerProperties>,
+ ContainerLayerVariant<SecureLayerProperties>>,
+ KeepCompositionTypeVariant<IComposerClient::Composition::CLIENT>,
+ ForcedClientCompositionResultVariant>>();
+}
+
+TEST_F(CompositionTest, captureScreenBufferLayerWithSecureParentLayerOnInsecureDisplay) {
+ captureScreenComposition<
+ CompositionCase<InsecureDisplaySetupVariant,
+ ChildLayerVariant<BufferLayerVariant<ParentSecureLayerProperties>,
+ ContainerLayerVariant<SecureLayerProperties>>,
+ NoCompositionTypeVariant, REScreenshotResultVariant>>();
+}
+
+/* ------------------------------------------------------------------------
* Cursor layers
*/
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 9ff8f4e..6ce738a 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -277,6 +277,10 @@
layer->mPotentialCursor = potentialCursor;
}
+ static void setLayerDrawingParent(const sp<Layer>& layer, const sp<Layer>& drawingParent) {
+ layer->mDrawingParent = drawingParent;
+ }
+
/* ------------------------------------------------------------------------
* Forwarding for functions being tested
*/