diff options
author | Kevin Krakauer <krakauer@google.com> | 2021-03-16 14:53:42 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-03-16 14:55:29 -0700 |
commit | 607a1e481c276c8ab0c3e194ed04b38bc07b71b6 (patch) | |
tree | e760228af2d1b5fc7766a284fb6a8bb9b2b6ba28 | |
parent | 05193de1ccaf487a175dead4121c62b99e02d0f5 (diff) |
setgid directory support in overlayfs
PiperOrigin-RevId: 363276495
-rw-r--r-- | pkg/sentry/fsimpl/overlay/filesystem.go | 21 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/overlay/overlay.go | 21 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/overlay/regular_file.go | 43 | ||||
-rw-r--r-- | test/syscalls/linux/BUILD | 1 | ||||
-rw-r--r-- | test/syscalls/linux/setgid.cc | 80 |
5 files changed, 129 insertions, 37 deletions
diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go index 84e37f793..46c500427 100644 --- a/pkg/sentry/fsimpl/overlay/filesystem.go +++ b/pkg/sentry/fsimpl/overlay/filesystem.go @@ -689,13 +689,9 @@ func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v } return err } - creds := rp.Credentials() + if err := vfsObj.SetStatAt(ctx, fs.creds, &pop, &vfs.SetStatOptions{ - Stat: linux.Statx{ - Mask: linux.STATX_UID | linux.STATX_GID, - UID: uint32(creds.EffectiveKUID), - GID: uint32(creds.EffectiveKGID), - }, + Stat: parent.newChildOwnerStat(opts.Mode, rp.Credentials()), }); err != nil { if cleanupErr := vfsObj.RmdirAt(ctx, fs.creds, &pop); cleanupErr != nil { panic(fmt.Sprintf("unrecoverable overlayfs inconsistency: failed to delete upper layer directory after MkdirAt metadata update failure: %v", cleanupErr)) @@ -750,11 +746,7 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v } creds := rp.Credentials() if err := vfsObj.SetStatAt(ctx, fs.creds, &pop, &vfs.SetStatOptions{ - Stat: linux.Statx{ - Mask: linux.STATX_UID | linux.STATX_GID, - UID: uint32(creds.EffectiveKUID), - GID: uint32(creds.EffectiveKGID), - }, + Stat: parent.newChildOwnerStat(opts.Mode, creds), }); err != nil { if cleanupErr := vfsObj.UnlinkAt(ctx, fs.creds, &pop); cleanupErr != nil { panic(fmt.Sprintf("unrecoverable overlayfs inconsistency: failed to delete upper layer file after MknodAt metadata update failure: %v", cleanupErr)) @@ -963,14 +955,11 @@ func (fs *filesystem) createAndOpenLocked(ctx context.Context, rp *vfs.Resolving } return nil, err } + // Change the file's owner to the caller. We can't use upperFD.SetStat() // because it will pick up creds from ctx. if err := vfsObj.SetStatAt(ctx, fs.creds, &pop, &vfs.SetStatOptions{ - Stat: linux.Statx{ - Mask: linux.STATX_UID | linux.STATX_GID, - UID: uint32(creds.EffectiveKUID), - GID: uint32(creds.EffectiveKGID), - }, + Stat: parent.newChildOwnerStat(opts.Mode, creds), }); err != nil { if cleanupErr := vfsObj.UnlinkAt(ctx, fs.creds, &pop); cleanupErr != nil { panic(fmt.Sprintf("unrecoverable overlayfs inconsistency: failed to delete upper layer file after OpenAt(O_CREAT) metadata update failure: %v", cleanupErr)) diff --git a/pkg/sentry/fsimpl/overlay/overlay.go b/pkg/sentry/fsimpl/overlay/overlay.go index 58680bc80..454c20d4f 100644 --- a/pkg/sentry/fsimpl/overlay/overlay.go +++ b/pkg/sentry/fsimpl/overlay/overlay.go @@ -749,6 +749,27 @@ func (d *dentry) mayDelete(creds *auth.Credentials, child *dentry) error { ) } +// newChildOwnerStat returns a Statx for configuring the UID, GID, and mode of +// children. +func (d *dentry) newChildOwnerStat(mode linux.FileMode, creds *auth.Credentials) linux.Statx { + stat := linux.Statx{ + Mask: uint32(linux.STATX_UID | linux.STATX_GID), + UID: uint32(creds.EffectiveKUID), + GID: uint32(creds.EffectiveKGID), + } + // Set GID and possibly the SGID bit if the parent is an SGID directory. + d.copyMu.RLock() + defer d.copyMu.RUnlock() + if atomic.LoadUint32(&d.mode)&linux.ModeSetGID == linux.ModeSetGID { + stat.GID = atomic.LoadUint32(&d.gid) + if stat.Mode&linux.ModeDirectory == linux.ModeDirectory { + stat.Mode = uint16(mode) | linux.ModeSetGID + stat.Mask |= linux.STATX_MODE + } + } + return stat +} + // fileDescription is embedded by overlay implementations of // vfs.FileDescriptionImpl. // diff --git a/pkg/sentry/fsimpl/overlay/regular_file.go b/pkg/sentry/fsimpl/overlay/regular_file.go index 25c785fd4..d791c06db 100644 --- a/pkg/sentry/fsimpl/overlay/regular_file.go +++ b/pkg/sentry/fsimpl/overlay/regular_file.go @@ -205,6 +205,20 @@ func (fd *regularFileFD) SetStat(ctx context.Context, opts vfs.SetStatOptions) e if err := wrappedFD.SetStat(ctx, opts); err != nil { return err } + + // Changing owners may clear one or both of the setuid and setgid bits, + // so we may have to update opts before setting d.mode. + if opts.Stat.Mask&(linux.STATX_UID|linux.STATX_GID) != 0 { + stat, err := wrappedFD.Stat(ctx, vfs.StatOptions{ + Mask: linux.STATX_MODE, + }) + if err != nil { + return err + } + opts.Stat.Mode = stat.Mode + opts.Stat.Mask |= linux.STATX_MODE + } + d.updateAfterSetStatLocked(&opts) if ev := vfs.InotifyEventFromStatMask(opts.Stat.Mask); ev != 0 { d.InotifyWithParent(ctx, ev, 0, vfs.InodeEvent) @@ -295,7 +309,11 @@ func (fd *regularFileFD) PWrite(ctx context.Context, src usermem.IOSequence, off return 0, err } defer wrappedFD.DecRef(ctx) - return wrappedFD.PWrite(ctx, src, offset, opts) + n, err := wrappedFD.PWrite(ctx, src, offset, opts) + if err != nil { + return n, err + } + return fd.updateSetUserGroupIDs(ctx, wrappedFD, n) } // Write implements vfs.FileDescriptionImpl.Write. @@ -307,7 +325,28 @@ func (fd *regularFileFD) Write(ctx context.Context, src usermem.IOSequence, opts if err != nil { return 0, err } - return wrappedFD.Write(ctx, src, opts) + n, err := wrappedFD.Write(ctx, src, opts) + if err != nil { + return n, err + } + return fd.updateSetUserGroupIDs(ctx, wrappedFD, n) +} + +func (fd *regularFileFD) updateSetUserGroupIDs(ctx context.Context, wrappedFD *vfs.FileDescription, written int64) (int64, error) { + // Writing can clear the setuid and/or setgid bits. We only have to + // check this if something was written and one of those bits was set. + dentry := fd.dentry() + if written == 0 || atomic.LoadUint32(&dentry.mode)&(linux.S_ISUID|linux.S_ISGID) == 0 { + return written, nil + } + stat, err := wrappedFD.Stat(ctx, vfs.StatOptions{Mask: linux.STATX_MODE}) + if err != nil { + return written, err + } + dentry.copyMu.Lock() + defer dentry.copyMu.Unlock() + atomic.StoreUint32(&dentry.mode, uint32(stat.Mode)) + return written, nil } // Seek implements vfs.FileDescriptionImpl.Seek. diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 4509b5e55..037181f7e 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -2162,6 +2162,7 @@ cc_binary( "//test/util:temp_path", "//test/util:test_main", "//test/util:test_util", + "@com_google_absl//absl/flags:flag", "@com_google_absl//absl/strings", gtest, ], diff --git a/test/syscalls/linux/setgid.cc b/test/syscalls/linux/setgid.cc index cd030b094..163242ace 100644 --- a/test/syscalls/linux/setgid.cc +++ b/test/syscalls/linux/setgid.cc @@ -17,6 +17,7 @@ #include <unistd.h> #include "gtest/gtest.h" +#include "absl/flags/flag.h" #include "test/util/capability_util.h" #include "test/util/cleanup.h" #include "test/util/fs_util.h" @@ -24,6 +25,11 @@ #include "test/util/temp_path.h" #include "test/util/test_util.h" +ABSL_FLAG(std::vector<std::string>, groups, std::vector<std::string>({}), + "groups the test can use"); + +constexpr gid_t kNobody = 65534; + namespace gvisor { namespace testing { @@ -46,6 +52,18 @@ PosixErrorOr<Cleanup> Setegid(gid_t egid) { // Returns a pair of groups that the user is a member of. PosixErrorOr<std::pair<gid_t, gid_t>> Groups() { + // Were we explicitly passed GIDs? + std::vector<std::string> flagged_groups = absl::GetFlag(FLAGS_groups); + if (flagged_groups.size() >= 2) { + int group1; + int group2; + if (!absl::SimpleAtoi(flagged_groups[0], &group1) || + !absl::SimpleAtoi(flagged_groups[1], &group2)) { + return PosixError(EINVAL, "failed converting group flags to ints"); + } + return std::pair<gid_t, gid_t>(group1, group2); + } + // See whether the user is a member of at least 2 groups. std::vector<gid_t> groups(64); for (; groups.size() <= NGROUPS_MAX; groups.resize(groups.size() * 2)) { @@ -58,26 +76,47 @@ PosixErrorOr<std::pair<gid_t, gid_t>> Groups() { return PosixError(errno, absl::StrFormat("getgroups(%d, %p)", groups.size(), groups.data())); } - if (ngroups >= 2) { - return std::pair<gid_t, gid_t>(groups[0], groups[1]); + + if (ngroups < 2) { + // There aren't enough groups. + break; + } + + // TODO(b/181878080): Read /proc/sys/fs/overflowgid once it is supported in + // gVisor. + if (groups[0] == kNobody || groups[1] == kNobody) { + // These groups aren't mapped into our user namespace, so we can't use + // them. + break; } - // There aren't enough groups. - break; + return std::pair<gid_t, gid_t>(groups[0], groups[1]); } - // If we're root in the root user namespace, we can set our GID to whatever we - // want. Try that before giving up. - constexpr gid_t kGID1 = 1111; - constexpr gid_t kGID2 = 2222; - auto cleanup1 = Setegid(kGID1); + // If we're running in gVisor and are root in the root user namespace, we can + // set our GID to whatever we want. Try that before giving up. + // + // This won't work in native tests, as despite having CAP_SETGID, the gofer + // process will be sandboxed and unable to change file GIDs. + if (!IsRunningOnGvisor()) { + return PosixError(EPERM, "no valid groups for native testing"); + } + PosixErrorOr<bool> capable = HaveCapability(CAP_SETGID); + if (!capable.ok()) { + return capable.error(); + } + if (!capable.ValueOrDie()) { + return PosixError(EPERM, "missing CAP_SETGID"); + } + gid_t gid = getegid(); + auto cleanup1 = Setegid(gid); if (!cleanup1.ok()) { return cleanup1.error(); } - auto cleanup2 = Setegid(kGID2); + auto cleanup2 = Setegid(kNobody); if (!cleanup2.ok()) { return cleanup2.error(); } - return std::pair<gid_t, gid_t>(kGID1, kGID2); + return std::pair<gid_t, gid_t>(gid, kNobody); } class SetgidDirTest : public ::testing::Test { @@ -85,17 +124,20 @@ class SetgidDirTest : public ::testing::Test { void SetUp() override { original_gid_ = getegid(); - // TODO(b/175325250): Enable when setgid directories are supported. SKIP_IF(IsRunningWithVFS1()); - SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETGID))); temp_dir_ = ASSERT_NO_ERRNO_AND_VALUE( TempPath::CreateDirWith(GetAbsoluteTestTmpdir(), 0777 /* mode */)); - groups_ = ASSERT_NO_ERRNO_AND_VALUE(Groups()); + + // If we can't find two usable groups, we're in an unsupporting environment. + // Skip the test. + PosixErrorOr<std::pair<gid_t, gid_t>> groups = Groups(); + SKIP_IF(!groups.ok()); + groups_ = groups.ValueOrDie(); } void TearDown() override { - ASSERT_THAT(setegid(original_gid_), SyscallSucceeds()); + EXPECT_THAT(setegid(original_gid_), SyscallSucceeds()); } void MkdirAsGid(gid_t gid, const std::string& path, mode_t mode) { @@ -131,7 +173,7 @@ TEST_F(SetgidDirTest, Control) { ASSERT_NO_FATAL_FAILURE(MkdirAsGid(groups_.first, g1owned, 0777)); // Set group to G2, create a file in g1owned, and confirm that G2 owns it. - ASSERT_THAT(setegid(groups_.second), SyscallSucceeds()); + auto cleanup = ASSERT_NO_ERRNO_AND_VALUE(Setegid(groups_.second)); FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE( Open(JoinPath(g1owned, "g2owned").c_str(), O_CREAT | O_RDWR, 0777)); struct stat stats = ASSERT_NO_ERRNO_AND_VALUE(Stat(fd)); @@ -146,7 +188,7 @@ TEST_F(SetgidDirTest, CreateFile) { ASSERT_THAT(chmod(g1owned.c_str(), kDirmodeSgid), SyscallSucceeds()); // Set group to G2, create a file, and confirm that G1 owns it. - ASSERT_THAT(setegid(groups_.second), SyscallSucceeds()); + auto cleanup = ASSERT_NO_ERRNO_AND_VALUE(Setegid(groups_.second)); FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE( Open(JoinPath(g1owned, "g2created").c_str(), O_CREAT | O_RDWR, 0666)); struct stat stats = ASSERT_NO_ERRNO_AND_VALUE(Stat(fd)); @@ -194,7 +236,7 @@ TEST_F(SetgidDirTest, OldFile) { ASSERT_THAT(chmod(g1owned.c_str(), kDirmodeNoSgid), SyscallSucceeds()); // Set group to G2, create a file, confirm that G2 owns it. - ASSERT_THAT(setegid(groups_.second), SyscallSucceeds()); + auto cleanup = ASSERT_NO_ERRNO_AND_VALUE(Setegid(groups_.second)); FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE( Open(JoinPath(g1owned, "g2created").c_str(), O_CREAT | O_RDWR, 0666)); struct stat stats = ASSERT_NO_ERRNO_AND_VALUE(Stat(fd)); @@ -217,7 +259,7 @@ TEST_F(SetgidDirTest, OldDir) { ASSERT_THAT(chmod(g1owned.c_str(), kDirmodeNoSgid), SyscallSucceeds()); // Set group to G2, create a directory, confirm that G2 owns it. - ASSERT_THAT(setegid(groups_.second), SyscallSucceeds()); + auto cleanup = ASSERT_NO_ERRNO_AND_VALUE(Setegid(groups_.second)); auto g2created = JoinPath(g1owned, "g2created"); ASSERT_NO_FATAL_FAILURE(MkdirAsGid(groups_.second, g2created, 0666)); struct stat stats = ASSERT_NO_ERRNO_AND_VALUE(Stat(g2created)); |