setgid directory support in goferfs

Also adds support for clearing the setuid bit when appropriate (writing,
truncating, changing size, changing UID, or changing GID).

VFS2 only.

PiperOrigin-RevId: 364661835

6 files changed, 81 insertions, 20 deletions

diff --git a/pkg/p9/p9.go b/pkg/p9/p9.go
index 2235f8968..648cf4b49 100644
--- a/pkg/p9/p9.go
+++ b/pkg/p9/p9.go
@@ -151,9 +151,16 @@ const (
 	// Sticky is a mode bit indicating sticky directories.
 	Sticky FileMode = 01000
 
+	// SetGID is the set group ID bit.
+	SetGID FileMode = 02000
+
+	// SetUID is the set user ID bit.
+	SetUID FileMode = 04000
+
 	// permissionsMask is the mask to apply to FileModes for permissions. It
-	// includes rwx bits for user, group and others, and sticky bit.
-	permissionsMask FileMode = 01777
+	// includes rwx bits for user, group, and others, as well as the sticky
+	// bit, setuid bit, and setgid bit.
+	permissionsMask FileMode = 07777
 )
 
 // QIDType is the most significant byte of the FileMode word, to be used as the
diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go
index c34451269..43c3c5a2d 100644
--- a/pkg/sentry/fsimpl/gofer/filesystem.go
+++ b/pkg/sentry/fsimpl/gofer/filesystem.go
@@ -783,7 +783,15 @@ func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs.
 func (fs *filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.MkdirOptions) error {
 	creds := rp.Credentials()
 	return fs.doCreateAt(ctx, rp, true /* dir */, func(parent *dentry, name string, _ **[]*dentry) error {
-		if _, err := parent.file.mkdir(ctx, name, (p9.FileMode)(opts.Mode), (p9.UID)(creds.EffectiveKUID), (p9.GID)(creds.EffectiveKGID)); err != nil {
+		// If the parent is a setgid directory, use the parent's GID
+		// rather than the caller's and enable setgid.
+		kgid := creds.EffectiveKGID
+		mode := opts.Mode
+		if atomic.LoadUint32(&parent.mode)&linux.S_ISGID != 0 {
+			kgid = auth.KGID(atomic.LoadUint32(&parent.gid))
+			mode |= linux.S_ISGID
+		}
+		if _, err := parent.file.mkdir(ctx, name, p9.FileMode(mode), (p9.UID)(creds.EffectiveKUID), p9.GID(kgid)); err != nil {
 			if !opts.ForSyntheticMountpoint || err == syserror.EEXIST {
 				return err
 			}
@@ -1145,7 +1153,15 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving
 	name := rp.Component()
 	// We only want the access mode for creating the file.
 	createFlags := p9.OpenFlags(opts.Flags) & p9.OpenFlagsModeMask
-	fdobj, openFile, createQID, _, err := dirfile.create(ctx, name, createFlags, (p9.FileMode)(opts.Mode), (p9.UID)(creds.EffectiveKUID), (p9.GID)(creds.EffectiveKGID))
+
+	// If the parent is a setgid directory, use the parent's GID rather
+	// than the caller's.
+	kgid := creds.EffectiveKGID
+	if atomic.LoadUint32(&d.mode)&linux.S_ISGID != 0 {
+		kgid = auth.KGID(atomic.LoadUint32(&d.gid))
+	}
+
+	fdobj, openFile, createQID, _, err := dirfile.create(ctx, name, createFlags, p9.FileMode(opts.Mode), (p9.UID)(creds.EffectiveKUID), p9.GID(kgid))
 	if err != nil {
 		dirfile.close(ctx)
 		return nil, err
diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go
index 71569dc65..692da02c1 100644
--- a/pkg/sentry/fsimpl/gofer/gofer.go
+++ b/pkg/sentry/fsimpl/gofer/gofer.go
@@ -1102,10 +1102,26 @@ func (d *dentry) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs
 
 	d.metadataMu.Lock()
 	defer d.metadataMu.Unlock()
+
+	// As with Linux, if the UID, GID, or file size is changing, we have to
+	// clear permission bits. Note that when set, clearSGID causes
+	// permissions to be updated, but does not modify stat.Mask, as
+	// modification would cause an extra inotify flag to be set.
+	clearSGID := stat.Mask&linux.STATX_UID != 0 && stat.UID != atomic.LoadUint32(&d.uid) ||
+		stat.Mask&linux.STATX_GID != 0 && stat.GID != atomic.LoadUint32(&d.gid) ||
+		stat.Mask&linux.STATX_SIZE != 0
+	if clearSGID {
+		if stat.Mask&linux.STATX_MODE != 0 {
+			stat.Mode = uint16(vfs.ClearSUIDAndSGID(uint32(stat.Mode)))
+		} else {
+			stat.Mode = uint16(vfs.ClearSUIDAndSGID(atomic.LoadUint32(&d.mode)))
+		}
+	}
+
 	if !d.isSynthetic() {
 		if stat.Mask != 0 {
 			if err := d.file.setAttr(ctx, p9.SetAttrMask{
-				Permissions:        stat.Mask&linux.STATX_MODE != 0,
+				Permissions:        stat.Mask&linux.STATX_MODE != 0 || clearSGID,
 				UID:                stat.Mask&linux.STATX_UID != 0,
 				GID:                stat.Mask&linux.STATX_GID != 0,
 				Size:               stat.Mask&linux.STATX_SIZE != 0,
@@ -1140,7 +1156,7 @@ func (d *dentry) setStat(ctx context.Context, creds *auth.Credentials, opts *vfs
 			return nil
 		}
 	}
-	if stat.Mask&linux.STATX_MODE != 0 {
+	if stat.Mask&linux.STATX_MODE != 0 || clearSGID {
 		atomic.StoreUint32(&d.mode, d.fileType()|uint32(stat.Mode))
 	}
 	if stat.Mask&linux.STATX_UID != 0 {
diff --git a/pkg/sentry/fsimpl/gofer/regular_file.go b/pkg/sentry/fsimpl/gofer/regular_file.go
index 283b220bb..4f1ad0c88 100644
--- a/pkg/sentry/fsimpl/gofer/regular_file.go
+++ b/pkg/sentry/fsimpl/gofer/regular_file.go
@@ -266,6 +266,20 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off
 			return 0, offset, err
 		}
 	}
+
+	// As with Linux, writing clears the setuid and setgid bits.
+	if n > 0 {
+		oldMode := atomic.LoadUint32(&d.mode)
+		// If setuid or setgid were set, update d.mode and propagate
+		// changes to the host.
+		if newMode := vfs.ClearSUIDAndSGID(oldMode); newMode != oldMode {
+			atomic.StoreUint32(&d.mode, newMode)
+			if err := d.file.setAttr(ctx, p9.SetAttrMask{Permissions: true}, p9.SetAttr{Permissions: p9.FileMode(newMode)}); err != nil {
+				return 0, offset, err
+			}
+		}
+	}
+
 	return n, offset + n, nil
 }
 
diff --git a/runsc/cmd/do.go b/runsc/cmd/do.go
index 22c1dfeb8..455c57692 100644
--- a/runsc/cmd/do.go
+++ b/runsc/cmd/do.go
@@ -42,10 +42,11 @@ var errNoDefaultInterface = errors.New("no default interface found")
 // Do implements subcommands.Command for the "do" command. It sets up a simple
 // sandbox and executes the command inside it. See Usage() for more details.
 type Do struct {
-	root  string
-	cwd   string
-	ip    string
-	quiet bool
+	root    string
+	cwd     string
+	ip      string
+	quiet   bool
+	overlay bool
 }
 
 // Name implements subcommands.Command.Name.
@@ -76,6 +77,7 @@ func (c *Do) SetFlags(f *flag.FlagSet) {
 	f.StringVar(&c.cwd, "cwd", ".", "path to the current directory, defaults to the current directory")
 	f.StringVar(&c.ip, "ip", "192.168.10.2", "IPv4 address for the sandbox")
 	f.BoolVar(&c.quiet, "quiet", false, "suppress runsc messages to stdout. Application output is still sent to stdout and stderr")
+	f.BoolVar(&c.overlay, "force-overlay", true, "use an overlay. WARNING: disabling gives the command write access to the host")
 }
 
 // Execute implements subcommands.Command.Execute.
@@ -100,9 +102,8 @@ func (c *Do) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) su
 		return Errorf("Error to retrieve hostname: %v", err)
 	}
 
-	// Map the entire host file system, but make it readonly with a writable
-	// overlay on top (ignore --overlay option).
-	conf.Overlay = true
+	// Map the entire host file system, optionally using an overlay.
+	conf.Overlay = c.overlay
 	absRoot, err := resolvePath(c.root)
 	if err != nil {
 		return Errorf("Error resolving root: %v", err)
diff --git a/test/syscalls/linux/setgid.cc b/test/syscalls/linux/setgid.cc
index 163242ace..98f8f3dfe 100644
--- a/test/syscalls/linux/setgid.cc
+++ b/test/syscalls/linux/setgid.cc
@@ -126,14 +126,15 @@ class SetgidDirTest : public ::testing::Test {
 
     SKIP_IF(IsRunningWithVFS1());
 
-    temp_dir_ = ASSERT_NO_ERRNO_AND_VALUE(
-        TempPath::CreateDirWith(GetAbsoluteTestTmpdir(), 0777 /* mode */));
-
     // 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();
+
+    auto cleanup = Setegid(groups_.first);
+    temp_dir_ = ASSERT_NO_ERRNO_AND_VALUE(
+        TempPath::CreateDirWith(GetAbsoluteTestTmpdir(), 0777 /* mode */));
   }
 
   void TearDown() override {
@@ -348,6 +349,10 @@ class FileModeTest : public ::testing::TestWithParam<FileModeTestcase> {};
 
 TEST_P(FileModeTest, WriteToFile) {
   SKIP_IF(IsRunningWithVFS1());
+  PosixErrorOr<std::pair<gid_t, gid_t>> groups = Groups();
+  SKIP_IF(!groups.ok());
+
+  auto cleanup = Setegid(groups.ValueOrDie().first);
   auto temp_dir = ASSERT_NO_ERRNO_AND_VALUE(
       TempPath::CreateDirWith(GetAbsoluteTestTmpdir(), 0777 /* mode */));
   auto path = JoinPath(temp_dir.path(), GetParam().name);
@@ -371,26 +376,28 @@ TEST_P(FileModeTest, WriteToFile) {
 
 TEST_P(FileModeTest, TruncateFile) {
   SKIP_IF(IsRunningWithVFS1());
+  PosixErrorOr<std::pair<gid_t, gid_t>> groups = Groups();
+  SKIP_IF(!groups.ok());
+
+  auto cleanup = Setegid(groups.ValueOrDie().first);
   auto temp_dir = ASSERT_NO_ERRNO_AND_VALUE(
       TempPath::CreateDirWith(GetAbsoluteTestTmpdir(), 0777 /* mode */));
   auto path = JoinPath(temp_dir.path(), GetParam().name);
   FileDescriptor fd =
       ASSERT_NO_ERRNO_AND_VALUE(Open(path.c_str(), O_CREAT | O_RDWR, 0666));
-  ASSERT_THAT(fchmod(fd.get(), GetParam().mode), SyscallSucceeds());
-  struct stat stats;
-  ASSERT_THAT(fstat(fd.get(), &stats), SyscallSucceeds());
-  EXPECT_EQ(stats.st_mode & kDirmodeMask, GetParam().mode);
 
   // Write something to the file, as truncating an empty file is a no-op.
   constexpr char c = 'M';
   ASSERT_THAT(write(fd.get(), &c, sizeof(c)),
               SyscallSucceedsWithValue(sizeof(c)));
+  ASSERT_THAT(fchmod(fd.get(), GetParam().mode), SyscallSucceeds());
 
   // For security reasons, truncating the file clears the SUID bit, and clears
   // the SGID bit when the group executable bit is unset (which is not a true
   // SGID binary).
   ASSERT_THAT(ftruncate(fd.get(), 0), SyscallSucceeds());
 
+  struct stat stats;
   ASSERT_THAT(fstat(fd.get(), &stats), SyscallSucceeds());
   EXPECT_EQ(stats.st_mode & kDirmodeMask, GetParam().result_mode);
 }