Ensure sub-hierarchy screenshots inherit secure from parent.

If a sub-hierarchy is screenshot that doesn't include the parent layer
that had the secure flag, the screenshot request would not respect the
secure flag. This is incorrect because we want to ensure secure flag
remains on all children regardless of where in the hierarchy the
screenshot is taken.

Test: CaptureChildRespectsParentSecureFlag
Test: CaptureOffscreenChildRespectsParentSecureFlag
Test: LayerSnapshotTest#setSecureRootSnapshot
Bug: 308662081
Change-Id: I13f19a7fa4b9e51da0aa097314f015fe1340fc54

7 files changed, 172 insertions, 11 deletions

diff --git a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
index a826ec18a1..0983e7c1ac 100644
--- a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
@@ -433,4 +433,15 @@ void LayerLifecycleManager::updateDisplayMirrorLayers(RequestedLayerState& rootL
     }
 }
 
+bool LayerLifecycleManager::isLayerSecure(uint32_t layerId) const {
+    if (layerId == UNASSIGNED_LAYER_ID) {
+        return false;
+    }
+
+    if (getLayerFromId(layerId)->flags & layer_state_t::eLayerSecure) {
+        return true;
+    }
+    return isLayerSecure(getLayerFromId(layerId)->parentId);
+}
+
 } // namespace android::surfaceflinger::frontend
diff --git a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h
index 9aff78e463..330da9a3d3 100644
--- a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h
+++ b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.h
@@ -80,6 +80,7 @@ public:
     const std::vector<RequestedLayerState*>& getChangedLayers() const;
     const ftl::Flags<RequestedLayerState::Changes> getGlobalChanges() const;
     const RequestedLayerState* getLayerFromId(uint32_t) const;
+    bool isLayerSecure(uint32_t) const;
 
 private:
     friend class LayerLifecycleManagerTest;
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
index 9476ff4932..743cbf3cb1 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
@@ -365,7 +365,7 @@ LayerSnapshot LayerSnapshotBuilder::getRootSnapshot() {
     return snapshot;
 }
 
-LayerSnapshotBuilder::LayerSnapshotBuilder() : mRootSnapshot(getRootSnapshot()) {}
+LayerSnapshotBuilder::LayerSnapshotBuilder() {}
 
 LayerSnapshotBuilder::LayerSnapshotBuilder(Args args) : LayerSnapshotBuilder() {
     args.forceUpdate = ForceUpdateFlags::ALL;
@@ -417,19 +417,20 @@ bool LayerSnapshotBuilder::tryFastUpdate(const Args& args) {
 
 void LayerSnapshotBuilder::updateSnapshots(const Args& args) {
     ATRACE_NAME("UpdateSnapshots");
+    LayerSnapshot rootSnapshot = args.rootSnapshot;
     if (args.parentCrop) {
-        mRootSnapshot.geomLayerBounds = *args.parentCrop;
+        rootSnapshot.geomLayerBounds = *args.parentCrop;
     } else if (args.forceUpdate == ForceUpdateFlags::ALL || args.displayChanges) {
-        mRootSnapshot.geomLayerBounds = getMaxDisplayBounds(args.displays);
+        rootSnapshot.geomLayerBounds = getMaxDisplayBounds(args.displays);
     }
     if (args.displayChanges) {
-        mRootSnapshot.changes = RequestedLayerState::Changes::AffectsChildren |
+        rootSnapshot.changes = RequestedLayerState::Changes::AffectsChildren |
                 RequestedLayerState::Changes::Geometry;
     }
     if (args.forceUpdate == ForceUpdateFlags::HIERARCHY) {
-        mRootSnapshot.changes |=
+        rootSnapshot.changes |=
                 RequestedLayerState::Changes::Hierarchy | RequestedLayerState::Changes::Visibility;
-        mRootSnapshot.clientChanges |= layer_state_t::eReparent;
+        rootSnapshot.clientChanges |= layer_state_t::eReparent;
     }
 
     for (auto& snapshot : mSnapshots) {
@@ -444,13 +445,13 @@ void LayerSnapshotBuilder::updateSnapshots(const Args& args) {
         // multiple children.
         LayerHierarchy::ScopedAddToTraversalPath addChildToPath(root, args.root.getLayer()->id,
                                                                 LayerHierarchy::Variant::Attached);
-        updateSnapshotsInHierarchy(args, args.root, root, mRootSnapshot, /*depth=*/0);
+        updateSnapshotsInHierarchy(args, args.root, root, rootSnapshot, /*depth=*/0);
     } else {
         for (auto& [childHierarchy, variant] : args.root.mChildren) {
             LayerHierarchy::ScopedAddToTraversalPath addChildToPath(root,
                                                                     childHierarchy->getLayer()->id,
                                                                     variant);
-            updateSnapshotsInHierarchy(args, *childHierarchy, root, mRootSnapshot, /*depth=*/0);
+            updateSnapshotsInHierarchy(args, *childHierarchy, root, rootSnapshot, /*depth=*/0);
         }
     }
 
@@ -459,7 +460,6 @@ void LayerSnapshotBuilder::updateSnapshots(const Args& args) {
     updateTouchableRegionCrop(args);
 
     const bool hasUnreachableSnapshots = sortSnapshotsByZ(args);
-    clearChanges(mRootSnapshot);
 
     // Destroy unreachable snapshots for clone layers. And destroy snapshots for non-clone
     // layers if the layer have been destroyed.
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
index 1506913e15..1cec0183b9 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
@@ -33,6 +33,9 @@ namespace android::surfaceflinger::frontend {
 // The builder also uses a fast path to update
 // snapshots when there are only buffer updates.
 class LayerSnapshotBuilder {
+private:
+    static LayerSnapshot getRootSnapshot();
+
 public:
     enum class ForceUpdateFlags {
         NONE,
@@ -55,6 +58,7 @@ public:
         const std::unordered_map<std::string, bool>& supportedLayerGenericMetadata;
         const std::unordered_map<std::string, uint32_t>& genericLayerMetadataKeyMap;
         bool skipRoundCornersWhenProtected = false;
+        LayerSnapshot rootSnapshot = getRootSnapshot();
     };
     LayerSnapshotBuilder();
 
@@ -87,7 +91,6 @@ public:
 
 private:
     friend class LayerSnapshotTest;
-    static LayerSnapshot getRootSnapshot();
 
     // return true if we were able to successfully update the snapshots via
     // the fast path.
@@ -130,7 +133,6 @@ private:
     std::unordered_set<LayerHierarchy::TraversalPath, LayerHierarchy::TraversalPathHash>
             mNeedsTouchableRegionCrop;
     std::vector<std::unique_ptr<LayerSnapshot>> mSnapshots;
-    LayerSnapshot mRootSnapshot;
     bool mResortSnapshots = false;
     int mNumInterestingSnapshots = 0;
 };
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 644b6ef30e..da2a6e0cc3 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -8958,6 +8958,7 @@ SurfaceFlinger::getLayerSnapshotsForScreenshots(uint32_t rootLayerId, uint32_t u
                      .genericLayerMetadataKeyMap = getGenericLayerMetadataKeyMap(),
                      .skipRoundCornersWhenProtected =
                              !getRenderEngine().supportsProtectedContent()};
+        args.rootSnapshot.isSecure = mLayerLifecycleManager.isLayerSecure(rootLayerId);
         mLayerSnapshotBuilder.update(args);
 
         auto getLayerSnapshotsFn =
diff --git a/services/surfaceflinger/tests/ScreenCapture_test.cpp b/services/surfaceflinger/tests/ScreenCapture_test.cpp
index 79864e05af..061e121950 100644
--- a/services/surfaceflinger/tests/ScreenCapture_test.cpp
+++ b/services/surfaceflinger/tests/ScreenCapture_test.cpp
@@ -154,6 +154,133 @@ TEST_F(ScreenCaptureTest, CaptureChildSetParentFlagsSecureEUidSystem) {
     sc.expectColor(Rect(0, 0, 10, 10), Color::BLUE);
 }
 
+/**
+ * If a parent layer sets the secure flag, but the screenshot requests is for the child hierarchy,
+ * we need to ensure the secure flag is respected from the parent even though the parent isn't
+ * in the captured sub-hierarchy
+ */
+TEST_F(ScreenCaptureTest, CaptureChildRespectsParentSecureFlag) {
+    Rect size(0, 0, 100, 100);
+    Transaction().hide(mBGSurfaceControl).hide(mFGSurfaceControl).apply();
+    sp<SurfaceControl> parentLayer;
+    ASSERT_NO_FATAL_FAILURE(parentLayer = createLayer("parent-test", 0, 0,
+                                                      ISurfaceComposerClient::eHidden,
+                                                      mRootSurfaceControl.get()));
+
+    sp<SurfaceControl> childLayer;
+    ASSERT_NO_FATAL_FAILURE(childLayer = createLayer("child-test", 0, 0,
+                                                     ISurfaceComposerClient::eFXSurfaceBufferState,
+                                                     parentLayer.get()));
+    ASSERT_NO_FATAL_FAILURE(
+            fillBufferLayerColor(childLayer, Color::GREEN, size.width(), size.height()));
+
+    // hide the parent layer to ensure secure flag is passed down to child when screenshotting
+    Transaction().setLayer(parentLayer, INT32_MAX).show(childLayer).apply(true);
+    Transaction()
+            .setFlags(parentLayer, layer_state_t::eLayerSecure, layer_state_t::eLayerSecure)
+            .apply();
+    LayerCaptureArgs captureArgs;
+    captureArgs.layerHandle = childLayer->getHandle();
+    captureArgs.sourceCrop = size;
+    captureArgs.captureSecureLayers = false;
+    {
+        SCOPED_TRACE("parent hidden");
+        ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults));
+        ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
+        ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers);
+        sc.expectColor(size, Color::BLACK);
+    }
+
+    captureArgs.captureSecureLayers = true;
+    {
+        SCOPED_TRACE("capture secure parent not visible");
+        ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults));
+        ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
+        ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers);
+        sc.expectColor(size, Color::GREEN);
+    }
+
+    Transaction().show(parentLayer).apply();
+    captureArgs.captureSecureLayers = false;
+    {
+        SCOPED_TRACE("parent visible");
+        ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults));
+        ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
+        ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers);
+        sc.expectColor(size, Color::BLACK);
+    }
+
+    captureArgs.captureSecureLayers = true;
+    {
+        SCOPED_TRACE("capture secure parent visible");
+        ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults));
+        ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
+        ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers);
+        sc.expectColor(size, Color::GREEN);
+    }
+}
+
+TEST_F(ScreenCaptureTest, CaptureOffscreenChildRespectsParentSecureFlag) {
+    Rect size(0, 0, 100, 100);
+    Transaction().hide(mBGSurfaceControl).hide(mFGSurfaceControl).apply();
+    // Parent layer should be offscreen.
+    sp<SurfaceControl> parentLayer;
+    ASSERT_NO_FATAL_FAILURE(
+            parentLayer = createLayer("parent-test", 0, 0, ISurfaceComposerClient::eHidden));
+
+    sp<SurfaceControl> childLayer;
+    ASSERT_NO_FATAL_FAILURE(childLayer = createLayer("child-test", 0, 0,
+                                                     ISurfaceComposerClient::eFXSurfaceBufferState,
+                                                     parentLayer.get()));
+    ASSERT_NO_FATAL_FAILURE(
+            fillBufferLayerColor(childLayer, Color::GREEN, size.width(), size.height()));
+
+    // hide the parent layer to ensure secure flag is passed down to child when screenshotting
+    Transaction().setLayer(parentLayer, INT32_MAX).show(childLayer).apply(true);
+    Transaction()
+            .setFlags(parentLayer, layer_state_t::eLayerSecure, layer_state_t::eLayerSecure)
+            .apply();
+    LayerCaptureArgs captureArgs;
+    captureArgs.layerHandle = childLayer->getHandle();
+    captureArgs.sourceCrop = size;
+    captureArgs.captureSecureLayers = false;
+    {
+        SCOPED_TRACE("parent hidden");
+        ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults));
+        ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
+        ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers);
+        sc.expectColor(size, Color::BLACK);
+    }
+
+    captureArgs.captureSecureLayers = true;
+    {
+        SCOPED_TRACE("capture secure parent not visible");
+        ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults));
+        ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
+        ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers);
+        sc.expectColor(size, Color::GREEN);
+    }
+
+    Transaction().show(parentLayer).apply();
+    captureArgs.captureSecureLayers = false;
+    {
+        SCOPED_TRACE("parent visible");
+        ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults));
+        ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
+        ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers);
+        sc.expectColor(size, Color::BLACK);
+    }
+
+    captureArgs.captureSecureLayers = true;
+    {
+        SCOPED_TRACE("capture secure parent visible");
+        ASSERT_EQ(NO_ERROR, ScreenCapture::captureLayers(captureArgs, mCaptureResults));
+        ASSERT_TRUE(mCaptureResults.capturedSecureLayers);
+        ScreenCapture sc(mCaptureResults.buffer, mCaptureResults.capturedHdrLayers);
+        sc.expectColor(size, Color::GREEN);
+    }
+}
+
 TEST_F(ScreenCaptureTest, CaptureSingleLayer) {
     LayerCaptureArgs captureArgs;
     captureArgs.layerHandle = mBGSurfaceControl->getHandle();
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
index 2cc6dc7721..16143e3a7f 100644
--- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -1055,4 +1055,23 @@ TEST_F(LayerSnapshotTest, isFrontBuffered) {
     EXPECT_FALSE(getSnapshot(1)->isFrontBuffered());
 }
 
+TEST_F(LayerSnapshotTest, setSecureRootSnapshot) {
+    setFlags(1, layer_state_t::eLayerSecure, layer_state_t::eLayerSecure);
+    LayerSnapshotBuilder::Args args{.root = mHierarchyBuilder.getHierarchy(),
+                                    .layerLifecycleManager = mLifecycleManager,
+                                    .includeMetadata = false,
+                                    .displays = mFrontEndDisplayInfos,
+                                    .displayChanges = false,
+                                    .globalShadowSettings = globalShadowSettings,
+                                    .supportsBlur = true,
+                                    .supportedLayerGenericMetadata = {},
+                                    .genericLayerMetadataKeyMap = {}};
+    args.rootSnapshot.isSecure = true;
+    update(mSnapshotBuilder, args);
+
+    EXPECT_TRUE(getSnapshot(1)->isSecure);
+    // Ensure child is also marked as secure
+    EXPECT_TRUE(getSnapshot(11)->isSecure);
+}
+
 } // namespace android::surfaceflinger::frontend