Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit fbe2c001 authored by Ana Krulec's avatar Ana Krulec
Browse files

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

This reverts commit e70e129f.

Reason for revert: Try 2. Updating GetDisplayColorModes to generally accessible.

Test: SF tests pass. Updating GetDisplayColorModeTest.

Change-Id: I4b4aea7b4b2e51caf217a254778821e28a6450c9
parent f4c8f386

libs/gui/include/gui/ISurfaceComposer.h

+3 −3
Original line number Diff line number Diff line
@@ -220,12 +220,12 @@ public: 


class BnSurfaceComposer: public BnInterface<ISurfaceComposer> {

public:

    enum {

    enum ISurfaceComposerTag {

        // Note: BOOT_FINISHED must remain this value, it is called from

        // Java by ActivityManagerService.

        BOOT_FINISHED = IBinder::FIRST_CALL_TRANSACTION,

        CREATE_CONNECTION,

        UNUSED, // formerly CREATE_GRAPHIC_BUFFER_ALLOC

        CREATE_GRAPHIC_BUFFER_ALLOC_UNUSED, // unused, fails permissions check

        CREATE_DISPLAY_EVENT_CONNECTION,

        CREATE_DISPLAY,

        DESTROY_DISPLAY,
@@ -236,7 +236,7 @@ public: 
        GET_DISPLAY_CONFIGS,

        GET_ACTIVE_CONFIG,

        SET_ACTIVE_CONFIG,

        CONNECT_DISPLAY,

        CONNECT_DISPLAY_UNUSED, // unused, fails permissions check

        CAPTURE_SCREEN,

        CAPTURE_LAYERS,

        CLEAR_ANIMATION_FRAME_STATS,

services/surfaceflinger/SurfaceFlinger.cpp

+67 −42
Original line number Diff line number Diff line
@@ -1220,15 +1220,6 @@ status_t SurfaceFlinger::injectVSync(nsecs_t when) { 


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);
@@ -3375,7 +3366,6 @@ bool callingThreadHasUnscopedSurfaceFlingerAccess() { 
    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)) {

        return false;
@@ -4563,51 +4553,64 @@ void SurfaceFlinger::updateColorMatrixLocked() { 
}



status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) {

    switch (code) {

        case CREATE_CONNECTION:

        case CREATE_DISPLAY:

#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:

        case CREATE_CONNECTION:

        case CREATE_DISPLAY:

        case DESTROY_DISPLAY:

        case ENABLE_VSYNC_INJECTIONS:

        case GET_ACTIVE_COLOR_MODE:

        case GET_ANIMATION_FRAME_STATS:

        case SET_POWER_MODE:

        case GET_HDR_CAPABILITIES:

        case ENABLE_VSYNC_INJECTIONS:

        case SET_ACTIVE_CONFIG:

        case SET_ACTIVE_COLOR_MODE:

        case INJECT_VSYNC:

        {

            // codes that require permission check

        case SET_POWER_MODE: {

            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 CAPTURE_SCREEN:

        {

            // codes that require permission check

        case GET_LAYER_DEBUG_INFO: {

            IPCThreadState* ipc = IPCThreadState::self();

            const int pid = ipc->getCallingPid();

            const int uid = ipc->getCallingUid();

            if ((uid != AID_GRAPHICS) &&

                    !PermissionCache::checkPermission(sReadFramebuffer, pid, uid)) {

                ALOGE("Permission Denial: can't read framebuffer pid=%d, uid=%d", pid, uid);

            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;

            }

            break;

            return OK;

        }

        // 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_COLOR_MODES:

        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_LAYERS:

        case CAPTURE_SCREEN: {

            // codes that require permission check

            IPCThreadState* ipc = IPCThreadState::self();

            const int pid = ipc->getCallingPid();

            const int uid = ipc->getCallingUid();
@@ -4616,15 +4619,37 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) { 
                ALOGE("Permission Denial: can't read framebuffer pid=%d, uid=%d", pid, uid);

                return PERMISSION_DENIED;

            }

            break;

            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;

        }

    }



    // 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 ||

        code == IBinder::PING_TRANSACTION || code == IBinder::SHELL_COMMAND_TRANSACTION ||

        code == IBinder::SYSPROPS_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

}



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;

services/surfaceflinger/tests/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ cc_test { 
    defaults: ["surfaceflinger_defaults"],

    test_suites: ["device-tests"],

    srcs: [

        "Credentials_test.cpp",

        "Stress_test.cpp",

        "SurfaceInterceptor_test.cpp",

        "Transaction_test.cpp",

services/surfaceflinger/tests/Credentials_test.cpp

0 → 100644
+322 −0
Original line number Diff line number Diff line 
#include <algorithm>

#include <functional>

#include <limits>

#include <ostream>



#include <gtest/gtest.h>



#include <gui/ISurfaceComposer.h>

#include <gui/LayerDebugInfo.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, NO_ERROR));

}



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));

}



TEST_F(CredentialsTest, GetLayerDebugInfo) {

    setupBackgroundSurface();

    sp<ISurfaceComposer> sf(ComposerService::getComposerService());



    // Historically, only root and shell can access the getLayerDebugInfo which

    // is called when we call dumpsys. I don't see a reason why we should change this.

    std::vector<LayerDebugInfo> outLayers;

    // Check with root.

    seteuid(AID_ROOT);

    ASSERT_EQ(NO_ERROR, sf->getLayerDebugInfo(&outLayers));



    // Check as a shell.

    seteuid(AID_SHELL);

    ASSERT_EQ(NO_ERROR, sf->getLayerDebugInfo(&outLayers));



    // Check as anyone else.

    seteuid(AID_ROOT);

    seteuid(AID_BIN);

    ASSERT_EQ(PERMISSION_DENIED, sf->getLayerDebugInfo(&outLayers));

}

} // namespace android

services/surfaceflinger/tests/SurfaceFlinger_test.filter

+1 −1
Original line number Diff line number Diff line 
{

        "presubmit": {

            "filter": "LayerTransactionTest.*:LayerUpdateTest.*:ChildLayerTest.*:SurfaceFlingerStress.*:CropLatchingTest.*:GeometryLatchingTest.*:ScreenCaptureTest.*:DereferenceSurfaceControlTest.*:SurfaceInterceptorTest.*:-CropLatchingTest.FinalCropLatchingBufferOldSize"

            "filter": "CredentialsTest.*:LayerTransactionTest.*:LayerUpdateTest.*:ChildLayerTest.*:SurfaceFlingerStress.*:CropLatchingTest.*:GeometryLatchingTest.*:ScreenCaptureTest.*:DereferenceSurfaceControlTest.*:SurfaceInterceptorTest.*:-CropLatchingTest.FinalCropLatchingBufferOldSize"

        }

}