[RESTRICT AUTOMERGE]: 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
(cherry picked from commit 5c99901e77437e403ba643c9dc839ee9659acada)

9 files changed, 135 insertions, 12 deletions

diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index d2d27e8239..e230b3affb 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -105,7 +105,7 @@ public:
     virtual status_t captureScreen(const sp<IBinder>& display, sp<GraphicBuffer>* outBuffer,
                                    Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight,
                                    int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform,
-                                   ISurfaceComposer::Rotation rotation) {
+                                   ISurfaceComposer::Rotation rotation, bool captureSecureLayers) {
         Parcel data, reply;
         data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
         data.writeStrongBinder(display);
@@ -116,6 +116,7 @@ public:
         data.writeInt32(maxLayerZ);
         data.writeInt32(static_cast<int32_t>(useIdentityTransform));
         data.writeInt32(static_cast<int32_t>(rotation));
+        data.writeInt32(static_cast<int32_t>(captureSecureLayers));
         status_t err = remote()->transact(BnSurfaceComposer::CAPTURE_SCREEN, data, &reply);
 
         if (err != NO_ERROR) {
@@ -643,10 +644,11 @@ status_t BnSurfaceComposer::onTransact(
             int32_t maxLayerZ = data.readInt32();
             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, sourceCrop, reqWidth, reqHeight,
                                          minLayerZ, maxLayerZ, 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 f3c6fd2f87..263c7ef9e0 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -768,18 +768,27 @@ status_t SurfaceComposerClient::getHdrCapabilities(const sp<IBinder>& display,
 status_t ScreenshotClient::capture(const sp<IBinder>& display, Rect sourceCrop, uint32_t reqWidth,
                                    uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ,
                                    bool useIdentityTransform, uint32_t rotation,
-                                   sp<GraphicBuffer>* outBuffer) {
+                                   bool captureSecureLayers, sp<GraphicBuffer>* outBuffer) {
     sp<ISurfaceComposer> s(ComposerService::getComposerService());
     if (s == NULL) return NO_INIT;
     status_t ret = s->captureScreen(display, outBuffer, sourceCrop, reqWidth, reqHeight, minLayerZ,
                                     maxLayerZ, useIdentityTransform,
-                                    static_cast<ISurfaceComposer::Rotation>(rotation));
+                                    static_cast<ISurfaceComposer::Rotation>(rotation),
+                                    captureSecureLayers);
     if (ret != NO_ERROR) {
         return ret;
     }
     return ret;
 }
 
+status_t ScreenshotClient::capture(const sp<IBinder>& display, Rect sourceCrop, uint32_t reqWidth,
+                                   uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ,
+                                   bool useIdentityTransform, uint32_t rotation,
+                                   sp<GraphicBuffer>* outBuffer) {
+    return capture(display, sourceCrop, reqWidth, reqHeight,
+            minLayerZ, maxLayerZ, useIdentityTransform, rotation, false, outBuffer);
+}
+
 status_t ScreenshotClient::captureLayers(const sp<IBinder>& layerHandle, Rect sourceCrop,
                                          float frameScale, sp<GraphicBuffer>* outBuffer) {
     sp<ISurfaceComposer> s(ComposerService::getComposerService());
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index 99a3a75502..fc7579ff11 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -180,7 +180,8 @@ public:
     virtual status_t captureScreen(const sp<IBinder>& display, sp<GraphicBuffer>* outBuffer,
                                    Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight,
                                    int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform,
-                                   Rotation rotation = eRotateNone) = 0;
+                                   Rotation rotation = eRotateNone,
+                                   bool captureSecureLayers = false) = 0;
 
     /**
      * Capture a subtree of the layer hierarchy, potentially ignoring the root node.
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index ad8a8b09d0..7d72661717 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -315,6 +315,10 @@ public:
     static status_t capture(const sp<IBinder>& display, Rect sourceCrop, uint32_t reqWidth,
                             uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ,
                             bool useIdentityTransform, uint32_t rotation,
+                            bool captureSecureLayers, sp<GraphicBuffer>* outBuffer);
+    static status_t capture(const sp<IBinder>& display, Rect sourceCrop, uint32_t reqWidth,
+                            uint32_t reqHeight, int32_t minLayerZ, int32_t maxLayerZ,
+                            bool useIdentityTransform, uint32_t rotation,
                             sp<GraphicBuffer>* outBuffer);
     static status_t captureLayers(const sp<IBinder>& layerHandle, Rect sourceCrop, float frameScale,
                                   sp<GraphicBuffer>* outBuffer);
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 6e196bfac8..959d782506 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -604,7 +604,8 @@ public:
             Rect /*sourceCrop*/, uint32_t /*reqWidth*/, uint32_t /*reqHeight*/,
             int32_t /*minLayerZ*/, int32_t /*maxLayerZ*/,
             bool /*useIdentityTransform*/,
-            Rotation /*rotation*/) override { return NO_ERROR; }
+            Rotation /*rotation*/,
+            bool /*captureSecureLayers*/) override { return NO_ERROR; }
     virtual status_t captureLayers(const sp<IBinder>& /*parentHandle*/,
                                    sp<GraphicBuffer>* /*outBuffer*/, const Rect& /*sourceCrop*/,
                                    float /*frameScale*/, bool /*childrenOnly*/) override {
diff --git a/services/surfaceflinger/DisplayDevice.h b/services/surfaceflinger/DisplayDevice.h
index 6c3bd91793..1262209773 100644
--- a/services/surfaceflinger/DisplayDevice.h
+++ b/services/surfaceflinger/DisplayDevice.h
@@ -341,21 +341,24 @@ public:
           : DisplayRenderArea(device, device->getBounds(), device->getHeight(), device->getWidth(),
                               rotation) {}
     DisplayRenderArea(const sp<const DisplayDevice> device, Rect sourceCrop, uint32_t reqHeight,
-                      uint32_t reqWidth, ISurfaceComposer::Rotation rotation)
+                      uint32_t reqWidth, ISurfaceComposer::Rotation rotation,
+                      bool allowSecureLayers = true)
           : RenderArea(reqHeight, reqWidth, CaptureFill::OPAQUE, rotation), mDevice(device),
-                              mSourceCrop(sourceCrop) {}
+                              mSourceCrop(sourceCrop),
+                              mAllowSecureLayers(allowSecureLayers) {}
 
     const 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(); }
     bool needsFiltering() const override { return mDevice->needsFiltering(); }
     Rect getSourceCrop() const override { return mSourceCrop; }
 
 private:
     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 11e7ff0f68..c1dc65b11a 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4836,7 +4836,8 @@ status_t SurfaceFlinger::captureScreen(const sp<IBinder>& display, sp<GraphicBuf
                                        Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight,
                                        int32_t minLayerZ, int32_t maxLayerZ,
                                        bool useIdentityTransform,
-                                       ISurfaceComposer::Rotation rotation) {
+                                       ISurfaceComposer::Rotation rotation,
+                                       bool captureSecureLayers) {
     ATRACE_CALL();
 
     if (CC_UNLIKELY(display == 0)) return BAD_VALUE;
@@ -4854,7 +4855,8 @@ status_t SurfaceFlinger::captureScreen(const sp<IBinder>& display, sp<GraphicBuf
         }
     }
 
-    DisplayRenderArea renderArea(device, sourceCrop, reqHeight, reqWidth, rotation);
+    DisplayRenderArea renderArea(device, sourceCrop, reqHeight, reqWidth, rotation,
+                                 captureSecureLayers);
 
     auto traverseLayers = std::bind(std::mem_fn(&SurfaceFlinger::traverseLayersInDisplay), this,
                                     device, minLayerZ, maxLayerZ, std::placeholders::_1);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 0148ab6754..2577f179cb 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -424,7 +424,7 @@ private:
     virtual status_t captureScreen(const sp<IBinder>& display, sp<GraphicBuffer>* outBuffer,
                                    Rect sourceCrop, uint32_t reqWidth, uint32_t reqHeight,
                                    int32_t minLayerZ, int32_t maxLayerZ, bool useIdentityTransform,
-                                   ISurfaceComposer::Rotation rotation);
+                                   ISurfaceComposer::Rotation rotation, bool captureSecureLayers);
     virtual status_t captureLayers(const sp<IBinder>& parentHandle, sp<GraphicBuffer>* outBuffer,
                                    const Rect& sourceCrop, float frameScale, bool childrenOnly);
     virtual status_t getDisplayStats(const sp<IBinder>& display,
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index 5108279043..deca17799c 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -29,6 +29,7 @@
 #include <gui/Surface.h>
 #include <gui/SurfaceComposerClient.h>
 #include <private/gui/ComposerService.h>
+#include <private/android_filesystem_config.h>
 
 #include <ui/DisplayInfo.h>
 #include <ui/Rect.h>
@@ -36,6 +37,8 @@
 
 #include <math.h>
 #include <math/vec3.h>
+#include <sys/types.h>
+#include <unistd.h>
 
 namespace android {
 
@@ -68,6 +71,30 @@ std::ostream& operator<<(std::ostream& os, const Color& color) {
 }
 
 // Fill a region with the specified color.
+void fillANativeWindowBufferColor(const ANativeWindow_Buffer& buffer, const Rect& rect,
+                                  const Color& color) {
+    Rect r(0, 0, buffer.width, buffer.height);
+    if (!r.intersect(rect, &r)) {
+        return;
+    }
+
+    int32_t width = r.right - r.left;
+    int32_t height = r.bottom - r.top;
+
+    for (int32_t row = 0; row < height; row++) {
+        uint8_t* dst =
+                static_cast<uint8_t*>(buffer.bits) + (buffer.stride * (r.top + row) + r.left) * 4;
+        for (int32_t column = 0; column < width; column++) {
+            dst[0] = color.r;
+            dst[1] = color.g;
+            dst[2] = color.b;
+            dst[3] = color.a;
+            dst += 4;
+        }
+    }
+}
+
+// Fill a region with the specified color.
 void fillBufferColor(const ANativeWindow_Buffer& buffer, const Rect& rect, const Color& color) {
     int32_t x = rect.left;
     int32_t y = rect.top;
@@ -319,6 +346,16 @@ protected:
         return layer;
     }
 
+    ANativeWindow_Buffer getBufferQueueLayerBuffer(const sp<SurfaceControl>& layer) {
+        // wait for previous transactions (such as setSize) to complete
+        Transaction().apply(true);
+
+        ANativeWindow_Buffer buffer = {};
+        EXPECT_EQ(NO_ERROR, layer->getSurface()->lock(&buffer, nullptr));
+
+        return buffer;
+    }
+
     ANativeWindow_Buffer getLayerBuffer(const sp<SurfaceControl>& layer) {
         // wait for previous transactions (such as setSize) to complete
         Transaction().apply(true);
@@ -336,6 +373,21 @@ protected:
         waitForLayerBuffers();
     }
 
+    void postBufferQueueLayerBuffer(const sp<SurfaceControl>& layer) {
+        ASSERT_EQ(NO_ERROR, layer->getSurface()->unlockAndPost());
+
+        // wait for the newly posted buffer to be latched
+        waitForLayerBuffers();
+    }
+
+    virtual void fillBufferQueueLayerColor(const sp<SurfaceControl>& layer, const Color& color,
+                                           int32_t bufferWidth, int32_t bufferHeight) {
+        ANativeWindow_Buffer buffer;
+        ASSERT_NO_FATAL_FAILURE(buffer = getBufferQueueLayerBuffer(layer));
+        fillANativeWindowBufferColor(buffer, Rect(0, 0, bufferWidth, bufferHeight), color);
+        postBufferQueueLayerBuffer(layer);
+    }
+
     void fillLayerColor(const sp<SurfaceControl>& layer, const Color& color) {
         ANativeWindow_Buffer buffer;
         ASSERT_NO_FATAL_FAILURE(buffer = getLayerBuffer(layer));
@@ -847,6 +899,55 @@ TEST_F(LayerTransactionTest, SetFlagsSecure) {
                                       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, 0, INT_MAX, 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, 0, INT_MAX, 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,
+                      Rect(), 0, 0, 0, INT_MAX, false, ISurfaceComposer::eRotateNone, true));
+    ScreenCapture sc(outBuffer);
+    sc.expectColor(Rect(0, 0, 32, 32), Color::RED);
+}
+
 TEST_F(LayerTransactionTest, SetTransparentRegionHintBasic) {
     const Rect top(0, 0, 32, 16);
     const Rect bottom(0, 16, 32, 32);