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

Commit 869525b8 authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

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

parents 6abe8e2f e70e129f
Loading
Loading
Loading
Loading
+3 −3
Original line number Diff line number Diff line
@@ -220,12 +220,12 @@ public:

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 @@ public:
        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,
+42 −65
Original line number Diff line number Diff line
@@ -1221,6 +1221,15 @@ 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);
@@ -3367,6 +3376,7 @@ 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;
@@ -4554,63 +4564,39 @@ void SurfaceFlinger::updateColorMatrixLocked() {
}

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;
            }
            return OK;
        }
        case GET_LAYER_DEBUG_INFO: {
            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;
            }
            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.
        /*
         * 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:
        // Creating a scoped connection is safe, as per discussion in ISurfaceComposer.h
        case CREATE_SCOPED_CONNECTION: {
        case CREATE_SCOPED_CONNECTION:
        {
            return OK;
        }
        case CAPTURE_LAYERS:
        case CAPTURE_SCREEN: {
        case CAPTURE_SCREEN:
        {
            // codes that require permission check
            IPCThreadState* ipc = IPCThreadState::self();
            const int pid = ipc->getCallingPid();
@@ -4620,35 +4606,26 @@ status_t SurfaceFlinger::CheckTransactCodeCredentials(uint32_t code) {
                ALOGE("Permission Denial: can't read framebuffer pid=%d, uid=%d", pid, uid);
                return PERMISSION_DENIED;
            }
            return OK;
            break;
        }
        // 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);
        case CAPTURE_LAYERS: {
            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);
                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
}

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;
+0 −1
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@ cc_test {
    defaults: ["surfaceflinger_defaults"],
    test_suites: ["device-tests"],
    srcs: [
        "Credentials_test.cpp",
        "Stress_test.cpp",
        "SurfaceInterceptor_test.cpp",
        "Transaction_test.cpp",
+0 −300
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/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
+1 −1
Original line number Diff line number Diff line
{
        "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"
        }
}