From 5c52b139f648064aa7fe10392a9807006b9990f3 Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Fri, 15 Feb 2019 15:48:11 -0800 Subject: 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: TransactionTest.cpp#SetFlagsSecureEUidSystem Bug: 120610669 Change-Id: I9d32c5ac2b005059be9f464859a415167d9ddbd4 --- core/java/android/view/SurfaceControl.java | 27 ++++++++++++++++++++-- core/jni/android_view_SurfaceControl.cpp | 10 ++++---- .../server/display/DisplayManagerService.java | 11 ++++++++- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java index aaf85aff2e8f..ee91c8519ff4 100644 --- a/core/java/android/view/SurfaceControl.java +++ b/core/java/android/view/SurfaceControl.java @@ -88,7 +88,8 @@ public final class SurfaceControl implements Parcelable { private static native void nativeDisconnect(long nativeObject); private static native GraphicBuffer nativeScreenshot(IBinder displayToken, - Rect sourceCrop, int width, int height, boolean useIdentityTransform, int rotation); + Rect sourceCrop, int width, int height, boolean useIdentityTransform, int rotation, + boolean captureSecureLayers); private static native GraphicBuffer nativeCaptureLayers(IBinder layerHandleToken, Rect sourceCrop, float frameScale); @@ -1853,7 +1854,29 @@ public final class SurfaceControl implements Parcelable { throw new IllegalArgumentException("displayToken must not be null"); } - return nativeScreenshot(display, sourceCrop, width, height, useIdentityTransform, rotation); + return nativeScreenshot(display, sourceCrop, width, height, useIdentityTransform, rotation, + false /* captureSecureLayers */); + } + + /** + * Like screenshotToBuffer, but if the caller is AID_SYSTEM, allows + * for the capture of secure layers. This is used for the screen rotation + * animation where the system server takes screenshots but does + * not persist them or allow them to leave the server. However in other + * cases in the system server, we mostly want to omit secure layers + * like when we take a screenshot on behalf of the assistant. + * + * @hide + */ + public static GraphicBuffer screenshotToBufferWithSecureLayersUnsafe(IBinder display, + Rect sourceCrop, int width, int height, boolean useIdentityTransform, + int rotation) { + if (display == null) { + throw new IllegalArgumentException("displayToken must not be null"); + } + + return nativeScreenshot(display, sourceCrop, width, height, useIdentityTransform, rotation, + true /* captureSecureLayers */); } private static void rotateCropForSF(Rect crop, int rot) { diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp index 003ee37995af..4069e95d8c35 100644 --- a/core/jni/android_view_SurfaceControl.cpp +++ b/core/jni/android_view_SurfaceControl.cpp @@ -205,7 +205,7 @@ static Rect rectFromObj(JNIEnv* env, jobject rectObj) { static jobject nativeScreenshot(JNIEnv* env, jclass clazz, jobject displayTokenObj, jobject sourceCropObj, jint width, jint height, - bool useIdentityTransform, int rotation) { + bool useIdentityTransform, int rotation, bool captureSecureLayers) { sp displayToken = ibinderForJavaObject(env, displayTokenObj); if (displayToken == NULL) { return NULL; @@ -213,9 +213,9 @@ static jobject nativeScreenshot(JNIEnv* env, jclass clazz, Rect sourceCrop = rectFromObj(env, sourceCropObj); sp buffer; status_t res = ScreenshotClient::capture(displayToken, ui::Dataspace::V0_SRGB, - ui::PixelFormat::RGBA_8888, - sourceCrop, width, height, - useIdentityTransform, rotation, &buffer); + ui::PixelFormat::RGBA_8888, + sourceCrop, width, height, + useIdentityTransform, rotation, captureSecureLayers, &buffer); if (res != NO_ERROR) { return NULL; } @@ -1232,7 +1232,7 @@ static const JNINativeMethod sSurfaceControlMethods[] = { (void*)nativeSetOverrideScalingMode }, {"nativeGetHandle", "(J)Landroid/os/IBinder;", (void*)nativeGetHandle }, - {"nativeScreenshot", "(Landroid/os/IBinder;Landroid/graphics/Rect;IIZI)Landroid/graphics/GraphicBuffer;", + {"nativeScreenshot", "(Landroid/os/IBinder;Landroid/graphics/Rect;IIZIZ)Landroid/graphics/GraphicBuffer;", (void*)nativeScreenshot }, {"nativeCaptureLayers", "(Landroid/os/IBinder;Landroid/graphics/Rect;F)Landroid/graphics/GraphicBuffer;", (void*)nativeCaptureLayers }, diff --git a/services/core/java/com/android/server/display/DisplayManagerService.java b/services/core/java/com/android/server/display/DisplayManagerService.java index b0209973edbf..01d19c04d97e 100644 --- a/services/core/java/com/android/server/display/DisplayManagerService.java +++ b/services/core/java/com/android/server/display/DisplayManagerService.java @@ -37,7 +37,9 @@ import android.content.pm.ParceledListSlice; import android.content.res.Resources; import android.content.res.TypedArray; import android.graphics.ColorSpace; +import android.graphics.GraphicBuffer; import android.graphics.Point; +import android.graphics.Rect; import android.hardware.SensorManager; import android.hardware.display.AmbientBrightnessDayStats; import android.hardware.display.BrightnessChangeEvent; @@ -1277,7 +1279,14 @@ public final class DisplayManagerService extends SystemService { if (token == null) { return false; } - SurfaceControl.screenshot(token, outSurface); + final GraphicBuffer gb = SurfaceControl.screenshotToBufferWithSecureLayersUnsafe( + token, new Rect(), 0 /* width */, 0 /* height */, false /* useIdentityTransform */, + 0 /* rotation */); + try { + outSurface.attachAndQueueBuffer(gb); + } catch (RuntimeException e) { + Slog.w(TAG, "Failed to take screenshot - " + e.getMessage()); + } return true; } -- cgit v1.2.3-59-g8ed1b