Revert "SF: Updating permissions checking in Surface Flinger."

This reverts commit e243771083d10fab0a7df5cc785f9a231dcc193b.

Reason for revert: Breaks eglCreateWindowSurface, see b/112339582

BUG: 112339582
Test: Build, flash, can't reproduce bug
Change-Id: I393f39efd7742fd3be7b877341d55bd4fc2942cc
(cherry picked from commit e70e129fcb2635d48582ecf4ffda8d54b76cc146)
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index 98ec338..e401572 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -220,12 +220,12 @@
 
 class BnSurfaceComposer: public BnInterface<ISurfaceComposer> {
 public:
-    enum ISurfaceComposerTag {
+    enum {
         // Note: BOOT_FINISHED must remain this value, it is called from
         // Java by ActivityManagerService.
         BOOT_FINISHED = IBinder::FIRST_CALL_TRANSACTION,
         CREATE_CONNECTION,
-        CREATE_GRAPHIC_BUFFER_ALLOC_UNUSED, // unused, fails permissions check
+        UNUSED, // formerly CREATE_GRAPHIC_BUFFER_ALLOC
         CREATE_DISPLAY_EVENT_CONNECTION,
         CREATE_DISPLAY,
         DESTROY_DISPLAY,
@@ -236,7 +236,7 @@
         GET_DISPLAY_CONFIGS,
         GET_ACTIVE_CONFIG,
         SET_ACTIVE_CONFIG,
-        CONNECT_DISPLAY_UNUSED, // unused, fails permissions check
+        CONNECT_DISPLAY,
         CAPTURE_SCREEN,
         CAPTURE_LAYERS,
         CLEAR_ANIMATION_FRAME_STATS,
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 8310455..1c12bd5 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -1221,6 +1221,15 @@
 
 status_t SurfaceFlinger::getLayerDebugInfo(std::vector<LayerDebugInfo>* outLayers) const
         NO_THREAD_SAFETY_ANALYSIS {
+    IPCThreadState* ipc = IPCThreadState::self();
+    const int pid = ipc->getCallingPid();
+    const int uid = ipc->getCallingUid();
+    if ((uid != AID_SHELL) &&
+            !PermissionCache::checkPermission(sDump, pid, uid)) {
+        ALOGE("Layer debug info permission denied for pid=%d, uid=%d", pid, uid);
+        return PERMISSION_DENIED;
+    }
+
     // Try to acquire a lock for 1s, fail gracefully
     const status_t err = mStateLock.timedLock(s2ns(1));
     const bool locked = (err == NO_ERROR);
@@ -3367,8 +3376,9 @@
     IPCThreadState* ipc = IPCThreadState::self();
     const int pid = ipc->getCallingPid();
     const int uid = ipc->getCallingUid();
+
     if ((uid != AID_GRAPHICS && uid != AID_SYSTEM) &&
-        !PermissionCache::checkPermission(sAccessSurfaceFlinger, pid, uid)) {
+            !PermissionCache::checkPermission(sAccessSurfaceFlinger, pid, uid)) {
         return false;
     }
     return true;
@@ -4554,64 +4564,51 @@
 }
 
 status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) {
-#pragma clang diagnostic push
-#pragma clang diagnostic error "-Wswitch-enum"
-    switch (static_cast<ISurfaceComposerTag>(code)) {
-        // These methods should at minimum make sure that the client requested
-        // access to SF.
-        case AUTHENTICATE_SURFACE:
-        case BOOT_FINISHED:
-        case CLEAR_ANIMATION_FRAME_STATS:
+    switch (code) {
         case CREATE_CONNECTION:
         case CREATE_DISPLAY:
-        case DESTROY_DISPLAY:
-        case ENABLE_VSYNC_INJECTIONS:
-        case GET_ACTIVE_COLOR_MODE:
+        case BOOT_FINISHED:
+        case CLEAR_ANIMATION_FRAME_STATS:
         case GET_ANIMATION_FRAME_STATS:
+        case SET_POWER_MODE:
         case GET_HDR_CAPABILITIES:
-        case GET_DISPLAY_COLOR_MODES:
-        case SET_ACTIVE_CONFIG:
-        case SET_ACTIVE_COLOR_MODE:
+        case ENABLE_VSYNC_INJECTIONS:
         case INJECT_VSYNC:
-        case SET_POWER_MODE: {
+        {
+            // codes that require permission check
             if (!callingThreadHasUnscopedSurfaceFlingerAccess()) {
                 IPCThreadState* ipc = IPCThreadState::self();
                 ALOGE("Permission Denial: can't access SurfaceFlinger pid=%d, uid=%d",
                         ipc->getCallingPid(), ipc->getCallingUid());
                 return PERMISSION_DENIED;
             }
+            break;
+        }
+        /*
+         * Calling setTransactionState is safe, because you need to have been
+         * granted a reference to Client* and Handle* to do anything with it.
+         *
+         * Creating a scoped connection is safe, as per discussion in ISurfaceComposer.h
+         */
+        case SET_TRANSACTION_STATE:
+        case CREATE_SCOPED_CONNECTION:
+        {
             return OK;
         }
-        case GET_LAYER_DEBUG_INFO: {
+        case CAPTURE_SCREEN:
+        {
+            // codes that require permission check
             IPCThreadState* ipc = IPCThreadState::self();
             const int pid = ipc->getCallingPid();
             const int uid = ipc->getCallingUid();
-            if ((uid != AID_SHELL) && !PermissionCache::checkPermission(sDump, pid, uid)) {
-                ALOGE("Layer debug info permission denied for pid=%d, uid=%d", pid, uid);
+            if ((uid != AID_GRAPHICS) &&
+                    !PermissionCache::checkPermission(sReadFramebuffer, pid, uid)) {
+                ALOGE("Permission Denial: can't read framebuffer pid=%d, uid=%d", pid, uid);
                 return PERMISSION_DENIED;
             }
-            return OK;
+            break;
         }
-        // Used by apps to hook Choreographer to SurfaceFlinger.
-        case CREATE_DISPLAY_EVENT_CONNECTION:
-        // The following calls are currently used by clients that do not
-        // request necessary permissions. However, they do not expose any secret
-        // information, so it is OK to pass them.
-        case GET_ACTIVE_CONFIG:
-        case GET_BUILT_IN_DISPLAY:
-        case GET_DISPLAY_CONFIGS:
-        case GET_DISPLAY_STATS:
-        case GET_SUPPORTED_FRAME_TIMESTAMPS:
-        // Calling setTransactionState is safe, because you need to have been
-        // granted a reference to Client* and Handle* to do anything with it.
-        case SET_TRANSACTION_STATE:
-        // Creating a scoped connection is safe, as per discussion in ISurfaceComposer.h
-        case CREATE_SCOPED_CONNECTION: {
-            return OK;
-        }
-        case CAPTURE_LAYERS:
-        case CAPTURE_SCREEN: {
-            // codes that require permission check
+        case CAPTURE_LAYERS: {
             IPCThreadState* ipc = IPCThreadState::self();
             const int pid = ipc->getCallingPid();
             const int uid = ipc->getCallingUid();
@@ -4620,35 +4617,15 @@
                 ALOGE("Permission Denial: can't read framebuffer pid=%d, uid=%d", pid, uid);
                 return PERMISSION_DENIED;
             }
-            return OK;
-        }
-        // The following codes are deprecated and should never be allowed to access SF.
-        case CONNECT_DISPLAY_UNUSED:
-        case CREATE_GRAPHIC_BUFFER_ALLOC_UNUSED: {
-            ALOGE("Attempting to access SurfaceFlinger with unused code: %u", code);
-            return PERMISSION_DENIED;
+            break;
         }
     }
-
-    // These codes are used for the IBinder protocol to either interrogate the recipient
-    // side of the transaction for its canonical interface descriptor or to dump its state.
-    // We let them pass by default.
-    if (code == IBinder::INTERFACE_TRANSACTION || code == IBinder::DUMP_TRANSACTION) {
-        return OK;
-    }
-    // Numbers from 1000 to 1029 are currently use for backdoors. The code
-    // in onTransact verifies that the user is root, and has access to use SF.
-    if (code >= 1000 && code <= 1029) {
-        ALOGV("Accessing SurfaceFlinger through backdoor code: %u", code);
-        return OK;
-    }
-    ALOGE("Permission Denial: SurfaceFlinger did not recognize request code: %u", code);
-    return PERMISSION_DENIED;
-#pragma clang diagnostic pop
+    return OK;
 }
 
-status_t SurfaceFlinger::onTransact(uint32_t code, const Parcel& data, Parcel* reply,
-                                    uint32_t flags) {
+status_t SurfaceFlinger::onTransact(
+    uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags)
+{
     status_t credentialCheck = CheckTransactCodeCredentials(code);
     if (credentialCheck != OK) {
         return credentialCheck;
diff --git a/services/surfaceflinger/tests/Android.bp b/services/surfaceflinger/tests/Android.bp
index 604aa7d..c511c5e 100644
--- a/services/surfaceflinger/tests/Android.bp
+++ b/services/surfaceflinger/tests/Android.bp
@@ -17,7 +17,6 @@
     defaults: ["surfaceflinger_defaults"],
     test_suites: ["device-tests"],
     srcs: [
-        "Credentials_test.cpp",
         "Stress_test.cpp",
         "SurfaceInterceptor_test.cpp",
         "Transaction_test.cpp",
diff --git a/services/surfaceflinger/tests/Credentials_test.cpp b/services/surfaceflinger/tests/Credentials_test.cpp
deleted file mode 100644
index 9ccada5..0000000
--- a/services/surfaceflinger/tests/Credentials_test.cpp
+++ /dev/null
@@ -1,300 +0,0 @@
-#include <algorithm>
-#include <functional>
-#include <limits>
-#include <ostream>
-
-#include <gtest/gtest.h>
-
-#include <gui/ISurfaceComposer.h>
-#include <gui/Surface.h>
-#include <gui/SurfaceComposerClient.h>
-
-#include <private/android_filesystem_config.h>
-#include <private/gui/ComposerService.h>
-
-#include <ui/DisplayInfo.h>
-#include <utils/String8.h>
-
-namespace android {
-
-using Transaction = SurfaceComposerClient::Transaction;
-
-namespace {
-const String8 DISPLAY_NAME("Credentials Display Test");
-const String8 SURFACE_NAME("Test Surface Name");
-const int32_t MIN_LAYER_Z = 0;
-const int32_t MAX_LAYER_Z = std::numeric_limits<int32_t>::max();
-const uint32_t ROTATION = 0;
-const float FRAME_SCALE = 1.0f;
-} // namespace
-
-/**
- * This class tests the CheckCredentials method in SurfaceFlinger.
- * Methods like EnableVsyncInjections and InjectVsync are not tested since they do not
- * return anything meaningful.
- */
-class CredentialsTest : public ::testing::Test {
-protected:
-    void SetUp() override {
-        // Start the tests as root.
-        seteuid(AID_ROOT);
-
-        ASSERT_NO_FATAL_FAILURE(initClient());
-    }
-
-    void TearDown() override {
-        mComposerClient->dispose();
-        mBGSurfaceControl.clear();
-        mComposerClient.clear();
-        // Finish the tests as root.
-        seteuid(AID_ROOT);
-    }
-
-    sp<IBinder> mDisplay;
-    sp<IBinder> mVirtualDisplay;
-    sp<SurfaceComposerClient> mComposerClient;
-    sp<SurfaceControl> mBGSurfaceControl;
-    sp<SurfaceControl> mVirtualSurfaceControl;
-
-    void initClient() {
-        mComposerClient = new SurfaceComposerClient;
-        ASSERT_EQ(NO_ERROR, mComposerClient->initCheck());
-    }
-
-    void setupBackgroundSurface() {
-        mDisplay = SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdMain);
-        DisplayInfo info;
-        SurfaceComposerClient::getDisplayInfo(mDisplay, &info);
-        const ssize_t displayWidth = info.w;
-        const ssize_t displayHeight = info.h;
-
-        // Background surface
-        mBGSurfaceControl =
-                mComposerClient->createSurface(SURFACE_NAME, displayWidth, displayHeight,
-                                               PIXEL_FORMAT_RGBA_8888, 0);
-        ASSERT_TRUE(mBGSurfaceControl != nullptr);
-        ASSERT_TRUE(mBGSurfaceControl->isValid());
-
-        Transaction t;
-        t.setDisplayLayerStack(mDisplay, 0);
-        ASSERT_EQ(NO_ERROR,
-                  t.setLayer(mBGSurfaceControl, INT_MAX - 3).show(mBGSurfaceControl).apply());
-    }
-
-    void setupVirtualDisplay() {
-        mVirtualDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, true);
-        const ssize_t displayWidth = 100;
-        const ssize_t displayHeight = 100;
-
-        // Background surface
-        mVirtualSurfaceControl =
-                mComposerClient->createSurface(SURFACE_NAME, displayWidth, displayHeight,
-                                               PIXEL_FORMAT_RGBA_8888, 0);
-        ASSERT_TRUE(mVirtualSurfaceControl != nullptr);
-        ASSERT_TRUE(mVirtualSurfaceControl->isValid());
-
-        Transaction t;
-        t.setDisplayLayerStack(mVirtualDisplay, 0);
-        ASSERT_EQ(NO_ERROR,
-                  t.setLayer(mVirtualSurfaceControl, INT_MAX - 3)
-                          .show(mVirtualSurfaceControl)
-                          .apply());
-    }
-
-    /**
-     * Sets UID to imitate Graphic's process.
-     */
-    void setGraphicsUID() {
-        seteuid(AID_ROOT);
-        seteuid(AID_GRAPHICS);
-    }
-
-    /**
-     * Sets UID to imitate System's process.
-     */
-    void setSystemUID() {
-        seteuid(AID_ROOT);
-        seteuid(AID_SYSTEM);
-    }
-
-    /**
-     * Sets UID to imitate a process that doesn't have any special privileges in
-     * our code.
-     */
-    void setBinUID() {
-        seteuid(AID_ROOT);
-        seteuid(AID_BIN);
-    }
-
-    /**
-     * Template function the check a condition for different types of users: root
-     * graphics, system, and non-supported user. Root, graphics, and system should
-     * always equal privilegedValue, and non-supported user should equal unprivilegedValue.
-     */
-    template <typename T>
-    void checkWithPrivileges(std::function<T()> condition, T privilegedValue, T unprivilegedValue) {
-        // Check with root.
-        seteuid(AID_ROOT);
-        ASSERT_EQ(privilegedValue, condition());
-
-        // Check as a Graphics user.
-        setGraphicsUID();
-        ASSERT_EQ(privilegedValue, condition());
-
-        // Check as a system user.
-        setSystemUID();
-        ASSERT_EQ(privilegedValue, condition());
-
-        // Check as a non-supported user.
-        setBinUID();
-        ASSERT_EQ(unprivilegedValue, condition());
-    }
-};
-
-TEST_F(CredentialsTest, ClientInitTest) {
-    // Root can init can init the client.
-    ASSERT_NO_FATAL_FAILURE(initClient());
-
-    // Graphics can init the client.
-    setGraphicsUID();
-    ASSERT_NO_FATAL_FAILURE(initClient());
-
-    // System can init the client.
-    setSystemUID();
-    ASSERT_NO_FATAL_FAILURE(initClient());
-
-    // No one else can init the client.
-    setBinUID();
-    mComposerClient = new SurfaceComposerClient;
-    ASSERT_EQ(NO_INIT, mComposerClient->initCheck());
-}
-
-TEST_F(CredentialsTest, GetBuiltInDisplayAccessTest) {
-    std::function<bool()> condition = [=]() {
-        sp<IBinder> display(
-                SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdMain));
-        return (display != nullptr);
-    };
-    // Anyone can access display information.
-    ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, true));
-}
-
-TEST_F(CredentialsTest, AllowedGetterMethodsTest) {
-    // The following methods are tested with a UID that is not root, graphics,
-    // or system, to show that anyone can access them.
-    setBinUID();
-    sp<IBinder> display(SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdMain));
-    ASSERT_TRUE(display != nullptr);
-
-    DisplayInfo info;
-    ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getDisplayInfo(display, &info));
-
-    Vector<DisplayInfo> configs;
-    ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getDisplayConfigs(display, &configs));
-
-    ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getActiveConfig(display));
-
-    ASSERT_NE(static_cast<ui::ColorMode>(BAD_VALUE),
-              SurfaceComposerClient::getActiveColorMode(display));
-}
-
-TEST_F(CredentialsTest, GetDisplayColorModesTest) {
-    sp<IBinder> display(SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdMain));
-    std::function<status_t()> condition = [=]() {
-        Vector<ui::ColorMode> outColorModes;
-        return SurfaceComposerClient::getDisplayColorModes(display, &outColorModes);
-    };
-    ASSERT_NO_FATAL_FAILURE(checkWithPrivileges<status_t>(condition, NO_ERROR, PERMISSION_DENIED));
-}
-
-TEST_F(CredentialsTest, SetActiveConfigTest) {
-    sp<IBinder> display(SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdMain));
-    std::function<status_t()> condition = [=]() {
-        return SurfaceComposerClient::setActiveConfig(display, 0);
-    };
-    ASSERT_NO_FATAL_FAILURE(checkWithPrivileges<status_t>(condition, NO_ERROR, PERMISSION_DENIED));
-}
-
-TEST_F(CredentialsTest, SetActiveColorModeTest) {
-    sp<IBinder> display(SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdMain));
-    std::function<status_t()> condition = [=]() {
-        return SurfaceComposerClient::setActiveColorMode(display, ui::ColorMode::NATIVE);
-    };
-    ASSERT_NO_FATAL_FAILURE(checkWithPrivileges<status_t>(condition, NO_ERROR, PERMISSION_DENIED));
-}
-
-TEST_F(CredentialsTest, CreateSurfaceTest) {
-    sp<IBinder> display(SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdMain));
-    DisplayInfo info;
-    SurfaceComposerClient::getDisplayInfo(display, &info);
-    const ssize_t displayWidth = info.w;
-    const ssize_t displayHeight = info.h;
-
-    std::function<bool()> condition = [=]() {
-        mBGSurfaceControl =
-                mComposerClient->createSurface(SURFACE_NAME, displayWidth, displayHeight,
-                                               PIXEL_FORMAT_RGBA_8888, 0);
-        return mBGSurfaceControl != nullptr && mBGSurfaceControl->isValid();
-    };
-    ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, false));
-}
-
-TEST_F(CredentialsTest, CreateDisplayTest) {
-    std::function<bool()> condition = [=]() {
-        sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, true);
-        return testDisplay.get() != nullptr;
-    };
-    ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, false));
-
-    condition = [=]() {
-        sp<IBinder> testDisplay = SurfaceComposerClient::createDisplay(DISPLAY_NAME, false);
-        return testDisplay.get() != nullptr;
-    };
-    ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, false));
-}
-
-TEST_F(CredentialsTest, DISABLED_DestroyDisplayTest) {
-    setupVirtualDisplay();
-
-    DisplayInfo info;
-    ASSERT_EQ(NO_ERROR, SurfaceComposerClient::getDisplayInfo(mVirtualDisplay, &info));
-    SurfaceComposerClient::destroyDisplay(mVirtualDisplay);
-    // This test currently fails. TODO(b/112002626): Find a way to properly create
-    // a display in the test environment, so that destroy display can remove it.
-    ASSERT_EQ(NAME_NOT_FOUND, SurfaceComposerClient::getDisplayInfo(mVirtualDisplay, &info));
-}
-
-TEST_F(CredentialsTest, CaptureTest) {
-    sp<IBinder> display(SurfaceComposerClient::getBuiltInDisplay(ISurfaceComposer::eDisplayIdMain));
-    std::function<status_t()> condition = [=]() {
-        sp<GraphicBuffer> outBuffer;
-        return ScreenshotClient::capture(display, Rect(), 0 /*reqWidth*/, 0 /*reqHeight*/,
-                                         MIN_LAYER_Z, MAX_LAYER_Z, false, ROTATION, &outBuffer);
-    };
-    ASSERT_NO_FATAL_FAILURE(checkWithPrivileges<status_t>(condition, NO_ERROR, PERMISSION_DENIED));
-}
-
-TEST_F(CredentialsTest, CaptureLayersTest) {
-    setupBackgroundSurface();
-    sp<GraphicBuffer> outBuffer;
-    std::function<status_t()> condition = [=]() {
-        sp<GraphicBuffer> outBuffer;
-        return ScreenshotClient::captureLayers(mBGSurfaceControl->getHandle(), Rect(), FRAME_SCALE,
-                                               &outBuffer);
-    };
-    ASSERT_NO_FATAL_FAILURE(checkWithPrivileges<status_t>(condition, NO_ERROR, PERMISSION_DENIED));
-}
-
-/**
- * Test for methods accessible directly through SurfaceFlinger:
- */
-TEST_F(CredentialsTest, AuthenticateSurfaceTextureTest) {
-    setupBackgroundSurface();
-    sp<IGraphicBufferProducer> producer =
-            mBGSurfaceControl->getSurface()->getIGraphicBufferProducer();
-    sp<ISurfaceComposer> sf(ComposerService::getComposerService());
-
-    std::function<bool()> condition = [=]() { return sf->authenticateSurfaceTexture(producer); };
-    ASSERT_NO_FATAL_FAILURE(checkWithPrivileges(condition, true, false));
-}
-} // namespace android
diff --git a/services/surfaceflinger/tests/SurfaceFlinger_test.filter b/services/surfaceflinger/tests/SurfaceFlinger_test.filter
index 1319e12..731e628 100644
--- a/services/surfaceflinger/tests/SurfaceFlinger_test.filter
+++ b/services/surfaceflinger/tests/SurfaceFlinger_test.filter
@@ -1,5 +1,5 @@
 {
         "presubmit": {
-            "filter": "CredentialsTest.*:LayerTransactionTest.*:LayerUpdateTest.*:ChildLayerTest.*:SurfaceFlingerStress.*:CropLatchingTest.*:GeometryLatchingTest.*:ScreenCaptureTest.*:DereferenceSurfaceControlTest.*:SurfaceInterceptorTest.*:-CropLatchingTest.FinalCropLatchingBufferOldSize"
+            "filter": "LayerTransactionTest.*:LayerUpdateTest.*:ChildLayerTest.*:SurfaceFlingerStress.*:CropLatchingTest.*:GeometryLatchingTest.*:ScreenCaptureTest.*:DereferenceSurfaceControlTest.*:SurfaceInterceptorTest.*:-CropLatchingTest.FinalCropLatchingBufferOldSize"
         }
 }