From e4b2087602a9217098559347d82d316a8cef8140 Mon Sep 17 00:00:00 2001 From: Dean Deng Date: Wed, 1 Jul 2020 15:17:39 -0700 Subject: Use directory fds in sticky test to avoid permission issues. After we change credentials, it is possible that we no longer have access to the sticky directory where we are trying to delete files. Use an open fd so this is not an issue. PiperOrigin-RevId: 319306255 --- test/syscalls/linux/sticky.cc | 77 ++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/test/syscalls/linux/sticky.cc b/test/syscalls/linux/sticky.cc index 39f4fb801..4afed6d08 100644 --- a/test/syscalls/linux/sticky.cc +++ b/test/syscalls/linux/sticky.cc @@ -42,12 +42,15 @@ TEST(StickyTest, StickyBitPermDenied) { const TempPath parent = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); EXPECT_THAT(chmod(parent.path().c_str(), 0777 | S_ISVTX), SyscallSucceeds()); - const TempPath file = ASSERT_NO_ERRNO_AND_VALUE( - TempPath::CreateFileWith(parent.path(), "some content", 0755)); - const TempPath dir = - ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirWith(parent.path(), 0755)); - const TempPath link = ASSERT_NO_ERRNO_AND_VALUE( - TempPath::CreateSymlinkTo(parent.path(), file.path())); + + // After changing credentials below, we need to use an open fd to make + // modifications in the parent dir, because there is no guarantee that we will + // still have the ability to open it. + const FileDescriptor parent_fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(parent.path(), O_DIRECTORY)); + ASSERT_THAT(openat(parent_fd.get(), "file", O_CREAT), SyscallSucceeds()); + ASSERT_THAT(mkdirat(parent_fd.get(), "dir", 0777), SyscallSucceeds()); + ASSERT_THAT(symlinkat("xyz", parent_fd.get(), "link"), SyscallSucceeds()); // Drop privileges and change IDs only in child thread, or else this parent // thread won't be able to open some log files after the test ends. @@ -65,12 +68,14 @@ TEST(StickyTest, StickyBitPermDenied) { syscall(SYS_setresuid, -1, absl::GetFlag(FLAGS_scratch_uid), -1), SyscallSucceeds()); - std::string new_path = NewTempAbsPath(); - EXPECT_THAT(rename(file.path().c_str(), new_path.c_str()), + EXPECT_THAT(renameat(parent_fd.get(), "file", parent_fd.get(), "file2"), + SyscallFailsWithErrno(EPERM)); + EXPECT_THAT(unlinkat(parent_fd.get(), "file", 0), + SyscallFailsWithErrno(EPERM)); + EXPECT_THAT(unlinkat(parent_fd.get(), "dir", AT_REMOVEDIR), + SyscallFailsWithErrno(EPERM)); + EXPECT_THAT(unlinkat(parent_fd.get(), "link", 0), SyscallFailsWithErrno(EPERM)); - EXPECT_THAT(unlink(file.path().c_str()), SyscallFailsWithErrno(EPERM)); - EXPECT_THAT(rmdir(dir.path().c_str()), SyscallFailsWithErrno(EPERM)); - EXPECT_THAT(unlink(link.path().c_str()), SyscallFailsWithErrno(EPERM)); }); } @@ -79,12 +84,15 @@ TEST(StickyTest, StickyBitSameUID) { const TempPath parent = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); EXPECT_THAT(chmod(parent.path().c_str(), 0777 | S_ISVTX), SyscallSucceeds()); - const TempPath file = ASSERT_NO_ERRNO_AND_VALUE( - TempPath::CreateFileWith(parent.path(), "some content", 0755)); - const TempPath dir = - ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirWith(parent.path(), 0755)); - const TempPath link = ASSERT_NO_ERRNO_AND_VALUE( - TempPath::CreateSymlinkTo(parent.path(), file.path())); + + // After changing credentials below, we need to use an open fd to make + // modifications in the parent dir, because there is no guarantee that we will + // still have the ability to open it. + const FileDescriptor parent_fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(parent.path(), O_DIRECTORY)); + ASSERT_THAT(openat(parent_fd.get(), "file", O_CREAT), SyscallSucceeds()); + ASSERT_THAT(mkdirat(parent_fd.get(), "dir", 0777), SyscallSucceeds()); + ASSERT_THAT(symlinkat("xyz", parent_fd.get(), "link"), SyscallSucceeds()); // Drop privileges and change IDs only in child thread, or else this parent // thread won't be able to open some log files after the test ends. @@ -100,12 +108,12 @@ TEST(StickyTest, StickyBitSameUID) { SyscallSucceeds()); // We still have the same EUID. - std::string new_path = NewTempAbsPath(); - EXPECT_THAT(rename(file.path().c_str(), new_path.c_str()), + EXPECT_THAT(renameat(parent_fd.get(), "file", parent_fd.get(), "file2"), + SyscallSucceeds()); + EXPECT_THAT(unlinkat(parent_fd.get(), "file2", 0), SyscallSucceeds()); + EXPECT_THAT(unlinkat(parent_fd.get(), "dir", AT_REMOVEDIR), SyscallSucceeds()); - EXPECT_THAT(unlink(new_path.c_str()), SyscallSucceeds()); - EXPECT_THAT(rmdir(dir.path().c_str()), SyscallSucceeds()); - EXPECT_THAT(unlink(link.path().c_str()), SyscallSucceeds()); + EXPECT_THAT(unlinkat(parent_fd.get(), "link", 0), SyscallSucceeds()); }); } @@ -114,12 +122,15 @@ TEST(StickyTest, StickyBitCapFOWNER) { const TempPath parent = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir()); EXPECT_THAT(chmod(parent.path().c_str(), 0777 | S_ISVTX), SyscallSucceeds()); - const TempPath file = ASSERT_NO_ERRNO_AND_VALUE( - TempPath::CreateFileWith(parent.path(), "some content", 0755)); - const TempPath dir = - ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirWith(parent.path(), 0755)); - const TempPath link = ASSERT_NO_ERRNO_AND_VALUE( - TempPath::CreateSymlinkTo(parent.path(), file.path())); + + // After changing credentials below, we need to use an open fd to make + // modifications in the parent dir, because there is no guarantee that we will + // still have the ability to open it. + const FileDescriptor parent_fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(parent.path(), O_DIRECTORY)); + ASSERT_THAT(openat(parent_fd.get(), "file", O_CREAT), SyscallSucceeds()); + ASSERT_THAT(mkdirat(parent_fd.get(), "dir", 0777), SyscallSucceeds()); + ASSERT_THAT(symlinkat("xyz", parent_fd.get(), "link"), SyscallSucceeds()); // Drop privileges and change IDs only in child thread, or else this parent // thread won't be able to open some log files after the test ends. @@ -136,12 +147,12 @@ TEST(StickyTest, StickyBitCapFOWNER) { SyscallSucceeds()); EXPECT_NO_ERRNO(SetCapability(CAP_FOWNER, true)); - std::string new_path = NewTempAbsPath(); - EXPECT_THAT(rename(file.path().c_str(), new_path.c_str()), + EXPECT_THAT(renameat(parent_fd.get(), "file", parent_fd.get(), "file2"), + SyscallSucceeds()); + EXPECT_THAT(unlinkat(parent_fd.get(), "file2", 0), SyscallSucceeds()); + EXPECT_THAT(unlinkat(parent_fd.get(), "dir", AT_REMOVEDIR), SyscallSucceeds()); - EXPECT_THAT(unlink(new_path.c_str()), SyscallSucceeds()); - EXPECT_THAT(rmdir(dir.path().c_str()), SyscallSucceeds()); - EXPECT_THAT(unlink(link.path().c_str()), SyscallSucceeds()); + EXPECT_THAT(unlinkat(parent_fd.get(), "link", 0), SyscallSucceeds()); }); } } // namespace -- cgit v1.2.3