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

Commit 96d83385 authored by Treehugger Robot's avatar Treehugger Robot Committed by Automerger Merge Worker
Browse files

Merge "Only open a path that is a regular file" into main am: eee1cf5f am: e556c187 

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2911497



Change-Id: I25e6f44e67e73788d14bb395c81fcb3e5b2575fe
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 3651aae0 e556c187

cmds/installd/InstalldNativeService.cpp

+19 −6
Original line number Diff line number Diff line
@@ -4017,24 +4017,37 @@ binder::Status InstalldNativeService::enableFsverity(const sp<IFsveritySetupAuth 
        return exception(binder::Status::EX_ILLEGAL_ARGUMENT, "Received a null auth token");

    }



    // Authenticate to check the targeting file is the same inode as the authFd.

    // Authenticate to check the targeting file is the same inode as the authFd. With O_PATH, we

    // prevent a malicious client from blocking installd by providing a path to FIFO. After the

    // authentication, the actual open is safe.

    sp<IBinder> authTokenBinder = IInterface::asBinder(authToken)->localBinder();

    if (authTokenBinder == nullptr) {

        return exception(binder::Status::EX_SECURITY, "Received a non-local auth token");

    }

    auto authTokenInstance = sp<FsveritySetupAuthToken>::cast(authTokenBinder);

    unique_fd rfd(open(filePath.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW));

    unique_fd pathFd(open(filePath.c_str(), O_RDONLY | O_CLOEXEC | O_NOFOLLOW | O_PATH));

    // Returns a constant errno to avoid one app probing file existence of the others, before the

    // authentication is done.

    const int kFixedErrno = EPERM;

    if (pathFd.get() < 0) {

        PLOG(DEBUG) << "Failed to open the path";

        *_aidl_return = kFixedErrno;

        return ok();

    }

    std::string procFdPath(StringPrintf("/proc/self/fd/%d", pathFd.get()));

    struct stat stFromPath;

    if (fstat(rfd.get(), &stFromPath) < 0) {

        *_aidl_return = errno;

    if (stat(procFdPath.c_str(), &stFromPath) < 0) {

        PLOG(DEBUG) << "Failed to stat proc fd " << pathFd.get() << " -> " << filePath;

        *_aidl_return = kFixedErrno;

        return ok();

    }

    auto authTokenInstance = sp<FsveritySetupAuthToken>::cast(authTokenBinder);

    if (!authTokenInstance->isSameStat(stFromPath)) {

        LOG(DEBUG) << "FD authentication failed";

        *_aidl_return = EPERM;

        *_aidl_return = kFixedErrno;

        return ok();

    }



    unique_fd rfd(open(procFdPath.c_str(), O_RDONLY | O_CLOEXEC));

    fsverity_enable_arg arg = {};

    arg.version = 1;

    arg.hash_algorithm = FS_VERITY_HASH_ALG_SHA256;

cmds/installd/tests/installd_service_test.cpp

+31 −6
Original line number Diff line number Diff line
@@ -194,6 +194,12 @@ static bool exists_renamed_deleted_dir(const std::string& rootDirectory) { 
    });

}



static void unlink_path(const std::string& path) {

    if (unlink(path.c_str()) < 0) {

        PLOG(DEBUG) << "Failed to unlink " + path;

    }

}



class ServiceTest : public testing::Test {

protected:

    InstalldNativeService* service;
@@ -555,7 +561,7 @@ protected: 
TEST_F(FsverityTest, enableFsverity) {

    const std::string path = kTestPath + "/foo";

    create_with_content(path, kTestAppUid, kTestAppUid, 0600, "content");

    UniqueFile raii(/*fd=*/-1, path, [](const std::string& path) { unlink(path.c_str()); });

    UniqueFile raii(/*fd=*/-1, path, &unlink_path);



    // Expect to fs-verity setup to succeed

    sp<IFsveritySetupAuthToken> authToken;
@@ -573,7 +579,7 @@ TEST_F(FsverityTest, enableFsverity) { 
TEST_F(FsverityTest, enableFsverity_nullAuthToken) {

    const std::string path = kTestPath + "/foo";

    create_with_content(path, kTestAppUid, kTestAppUid, 0600, "content");

    UniqueFile raii(/*fd=*/-1, path, [](const std::string& path) { unlink(path.c_str()); });

    UniqueFile raii(/*fd=*/-1, path, &unlink_path);



    // Verity null auth token fails

    sp<IFsveritySetupAuthToken> authToken;
@@ -586,7 +592,7 @@ TEST_F(FsverityTest, enableFsverity_nullAuthToken) { 
TEST_F(FsverityTest, enableFsverity_differentFile) {

    const std::string path = kTestPath + "/foo";

    create_with_content(path, kTestAppUid, kTestAppUid, 0600, "content");

    UniqueFile raii(/*fd=*/-1, path, [](const std::string& path) { unlink(path.c_str()); });

    UniqueFile raii(/*fd=*/-1, path, &unlink_path);



    // Expect to fs-verity setup to succeed

    sp<IFsveritySetupAuthToken> authToken;
@@ -597,17 +603,36 @@ TEST_F(FsverityTest, enableFsverity_differentFile) { 
    // Verity auth token does not work for a different file

    const std::string anotherPath = kTestPath + "/bar";

    ASSERT_TRUE(android::base::WriteStringToFile("content", anotherPath));

    UniqueFile raii2(/*fd=*/-1, anotherPath, [](const std::string& path) { unlink(path.c_str()); });

    UniqueFile raii2(/*fd=*/-1, anotherPath, &unlink_path);

    int32_t errno_local;

    status = service->enableFsverity(authToken, anotherPath, "fake.package.name", &errno_local);

    EXPECT_TRUE(status.isOk());

    EXPECT_NE(errno_local, 0);

}



TEST_F(FsverityTest, enableFsverity_errnoBeforeAuthenticated) {

    const std::string path = kTestPath + "/foo";

    create_with_content(path, kTestAppUid, kTestAppUid, 0600, "content");

    UniqueFile raii(/*fd=*/-1, path, &unlink_path);



    // Expect to fs-verity setup to succeed

    sp<IFsveritySetupAuthToken> authToken;

    binder::Status status = createFsveritySetupAuthToken(path, O_RDWR, &authToken);

    EXPECT_TRUE(status.isOk());

    EXPECT_TRUE(authToken != nullptr);



    // Verity errno before the fd authentication is constant (EPERM)

    int32_t errno_local;

    status = service->enableFsverity(authToken, path + "-non-exist", "fake.package.name",

                                     &errno_local);

    EXPECT_TRUE(status.isOk());

    EXPECT_EQ(errno_local, EPERM);

}



TEST_F(FsverityTest, createFsveritySetupAuthToken_ReadonlyFdDoesNotAuthenticate) {

    const std::string path = kTestPath + "/foo";

    create_with_content(path, kTestAppUid, kTestAppUid, 0600, "content");

    UniqueFile raii(/*fd=*/-1, path, [](const std::string& path) { unlink(path.c_str()); });

    UniqueFile raii(/*fd=*/-1, path, &unlink_path);



    // Expect the fs-verity setup to fail

    sp<IFsveritySetupAuthToken> authToken;
@@ -619,7 +644,7 @@ TEST_F(FsverityTest, createFsveritySetupAuthToken_UnownedFile) { 
    const std::string path = kTestPath + "/foo";

    // Simulate world-writable file owned by another app

    create_with_content(path, kTestAppUid + 1, kTestAppUid + 1, 0666, "content");

    UniqueFile raii(/*fd=*/-1, path, [](const std::string& path) { unlink(path.c_str()); });

    UniqueFile raii(/*fd=*/-1, path, &unlink_path);



    // Expect the fs-verity setup to fail

    sp<IFsveritySetupAuthToken> authToken;