summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorKevin Krakauer <krakauer@google.com>2021-03-16 14:53:42 -0700
committergVisor bot <gvisor-bot@google.com>2021-03-16 14:55:29 -0700
commit607a1e481c276c8ab0c3e194ed04b38bc07b71b6 (patch)
treee760228af2d1b5fc7766a284fb6a8bb9b2b6ba28
parent05193de1ccaf487a175dead4121c62b99e02d0f5 (diff)
setgid directory support in overlayfs
PiperOrigin-RevId: 363276495
-rw-r--r--pkg/sentry/fsimpl/overlay/filesystem.go21
-rw-r--r--pkg/sentry/fsimpl/overlay/overlay.go21
-rw-r--r--pkg/sentry/fsimpl/overlay/regular_file.go43
-rw-r--r--test/syscalls/linux/BUILD1
-rw-r--r--test/syscalls/linux/setgid.cc80
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));