[vfs] Make sticky bit check consistent with Linux.

Our implementation of vfs.CheckDeleteSticky was not consistent with Linux,
specifically not consistent with fs/linux.h:check_sticky().

One of the biggest differences was that the vfs implementation did not
allow the owner of the sticky directory to delete files inside it that belonged
to other users.

This change makes our implementation consistent with Linux.

Also adds an integration test to check for this. This bug is also present in
VFS1.

Updates #3027

PiperOrigin-RevId: 355557425

7 files changed, 141 insertions, 7 deletions

diff --git a/images/basic/integrationtest/test_sticky.c b/images/basic/integrationtest/test_sticky.c
new file mode 100644
index 000000000..58dcf91d3
--- /dev/null
+++ b/images/basic/integrationtest/test_sticky.c
@@ -0,0 +1,96 @@
+#include <err.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+void createFile(const char* path) {
+  int fd = open(path, O_WRONLY | O_CREAT, 0777);
+  if (fd < 0) {
+    err(1, "open(%s)", path);
+    exit(1);
+  } else {
+    close(fd);
+  }
+}
+
+void waitAndCheckStatus(pid_t child) {
+  int status;
+  if (waitpid(child, &status, 0) == -1) {
+    err(1, "waitpid() failed");
+    exit(1);
+  }
+
+  if (WIFEXITED(status)) {
+    int es = WEXITSTATUS(status);
+    if (es) {
+      err(1, "child exit status %d", es);
+      exit(1);
+    }
+  } else {
+    err(1, "child did not exit normally");
+    exit(1);
+  }
+}
+
+void deleteFile(uid_t user, const char* path) {
+  pid_t child = fork();
+  if (child == 0) {
+    if (setuid(user)) {
+      err(1, "setuid(%d)", user);
+      exit(1);
+    }
+
+    if (unlink(path)) {
+      err(1, "unlink(%s)", path);
+      exit(1);
+    }
+    exit(0);
+  }
+  waitAndCheckStatus(child);
+}
+
+int main(int argc, char** argv) {
+  const char kUser1Dir[] = "/user1dir";
+  const char kUser2File[] = "/user1dir/user2file";
+  const char kUser2File2[] = "/user1dir/user2file2";
+
+  const uid_t user1 = 6666;
+  const uid_t user2 = 6667;
+
+  if (mkdir(kUser1Dir, 0755) != 0) {
+    err(1, "mkdir(%s)", kUser1Dir);
+    exit(1);
+  }
+  // Enable sticky bit for user1dir.
+  if (chmod(kUser1Dir, 01777) != 0) {
+    err(1, "chmod(%s)", kUser1Dir);
+    exit(1);
+  }
+  createFile(kUser2File);
+  createFile(kUser2File2);
+
+  if (chown(kUser1Dir, user1, getegid())) {
+    err(1, "chown(%s)", kUser1Dir);
+    exit(1);
+  }
+  if (chown(kUser2File, user2, getegid())) {
+    err(1, "chown(%s)", kUser2File);
+    exit(1);
+  }
+  if (chown(kUser2File2, user2, getegid())) {
+    err(1, "chown(%s)", kUser2File2);
+    exit(1);
+  }
+
+  // User1 should be able to delete any file inside user1dir, even files of
+  // other users due to the sticky bit.
+  deleteFile(user1, kUser2File);
+
+  // User2 should naturally be able to delete its own file even if the file is
+  // inside a sticky dir owned by someone else.
+  deleteFile(user2, kUser2File2);
+}
diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go
index 98f7bc52f..094d993a8 100644
--- a/pkg/sentry/fsimpl/gofer/gofer.go
+++ b/pkg/sentry/fsimpl/gofer/gofer.go
@@ -1216,7 +1216,13 @@ func (d *dentry) checkXattrPermissions(creds *auth.Credentials, name string, ats
 }
 
 func (d *dentry) mayDelete(creds *auth.Credentials, child *dentry) error {
-	return vfs.CheckDeleteSticky(creds, linux.FileMode(atomic.LoadUint32(&d.mode)), auth.KUID(atomic.LoadUint32(&child.uid)))
+	return vfs.CheckDeleteSticky(
+		creds,
+		linux.FileMode(atomic.LoadUint32(&d.mode)),
+		auth.KUID(atomic.LoadUint32(&d.uid)),
+		auth.KUID(atomic.LoadUint32(&child.uid)),
+		auth.KGID(atomic.LoadUint32(&child.gid)),
+	)
 }
 
 func dentryUIDFromP9UID(uid p9.UID) uint32 {
diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go
index e46f593c7..b36031291 100644
--- a/pkg/sentry/fsimpl/overlay/filesystem.go
+++ b/pkg/sentry/fsimpl/overlay/filesystem.go
@@ -1068,7 +1068,7 @@ func (fs *filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa
 	if err != nil {
 		return err
 	}
-	if err := vfs.CheckDeleteSticky(creds, linux.FileMode(atomic.LoadUint32(&oldParent.mode)), auth.KUID(atomic.LoadUint32(&renamed.uid))); err != nil {
+	if err := oldParent.mayDelete(creds, renamed); err != nil {
 		return err
 	}
 	if renamed.isDir() {
@@ -1317,7 +1317,7 @@ func (fs *filesystem) RmdirAt(ctx context.Context, rp *vfs.ResolvingPath) error
 	if !child.isDir() {
 		return syserror.ENOTDIR
 	}
-	if err := vfs.CheckDeleteSticky(rp.Credentials(), linux.FileMode(atomic.LoadUint32(&parent.mode)), auth.KUID(atomic.LoadUint32(&child.uid))); err != nil {
+	if err := parent.mayDelete(rp.Credentials(), child); err != nil {
 		return err
 	}
 	child.dirMu.Lock()
@@ -1584,7 +1584,7 @@ func (fs *filesystem) UnlinkAt(ctx context.Context, rp *vfs.ResolvingPath) error
 		if child.isDir() {
 			return syserror.EISDIR
 		}
-		if err := vfs.CheckDeleteSticky(rp.Credentials(), linux.FileMode(parentMode), auth.KUID(atomic.LoadUint32(&child.uid))); err != nil {
+		if err := parent.mayDelete(rp.Credentials(), child); err != nil {
 			return err
 		}
 		if err := vfsObj.PrepareDeleteDentry(mntns, &child.vfsd); err != nil {
diff --git a/pkg/sentry/fsimpl/overlay/overlay.go b/pkg/sentry/fsimpl/overlay/overlay.go
index 082fa6504..acd3684c6 100644
--- a/pkg/sentry/fsimpl/overlay/overlay.go
+++ b/pkg/sentry/fsimpl/overlay/overlay.go
@@ -760,6 +760,16 @@ func (d *dentry) updateAfterSetStatLocked(opts *vfs.SetStatOptions) {
 	}
 }
 
+func (d *dentry) mayDelete(creds *auth.Credentials, child *dentry) error {
+	return vfs.CheckDeleteSticky(
+		creds,
+		linux.FileMode(atomic.LoadUint32(&d.mode)),
+		auth.KUID(atomic.LoadUint32(&d.uid)),
+		auth.KUID(atomic.LoadUint32(&child.uid)),
+		auth.KGID(atomic.LoadUint32(&child.gid)),
+	)
+}
+
 // fileDescription is embedded by overlay implementations of
 // vfs.FileDescriptionImpl.
 //
diff --git a/pkg/sentry/fsimpl/tmpfs/directory.go b/pkg/sentry/fsimpl/tmpfs/directory.go
index e90669cf0..417ac2eff 100644
--- a/pkg/sentry/fsimpl/tmpfs/directory.go
+++ b/pkg/sentry/fsimpl/tmpfs/directory.go
@@ -84,7 +84,13 @@ func (dir *directory) removeChildLocked(child *dentry) {
 }
 
 func (dir *directory) mayDelete(creds *auth.Credentials, child *dentry) error {
-	return vfs.CheckDeleteSticky(creds, linux.FileMode(atomic.LoadUint32(&dir.inode.mode)), auth.KUID(atomic.LoadUint32(&child.inode.uid)))
+	return vfs.CheckDeleteSticky(
+		creds,
+		linux.FileMode(atomic.LoadUint32(&dir.inode.mode)),
+		auth.KUID(atomic.LoadUint32(&dir.inode.uid)),
+		auth.KUID(atomic.LoadUint32(&child.inode.uid)),
+		auth.KGID(atomic.LoadUint32(&child.inode.gid)),
+	)
 }
 
 // +stateify savable
diff --git a/pkg/sentry/vfs/permissions.go b/pkg/sentry/vfs/permissions.go
index d48520d58..db6146fd2 100644
--- a/pkg/sentry/vfs/permissions.go
+++ b/pkg/sentry/vfs/permissions.go
@@ -243,11 +243,13 @@ func CheckSetStat(ctx context.Context, creds *auth.Credentials, opts *SetStatOpt
 // the given file mode, and if so, checks whether creds has permission to
 // remove a file owned by childKUID from a directory with the given mode.
 // CheckDeleteSticky is consistent with fs/linux.h:check_sticky().
-func CheckDeleteSticky(creds *auth.Credentials, parentMode linux.FileMode, childKUID auth.KUID) error {
+func CheckDeleteSticky(creds *auth.Credentials, parentMode linux.FileMode, parentKUID auth.KUID, childKUID auth.KUID, childKGID auth.KGID) error {
 	if parentMode&linux.ModeSticky == 0 {
 		return nil
 	}
-	if CanActAsOwner(creds, childKUID) {
+	if creds.EffectiveKUID == childKUID ||
+		creds.EffectiveKUID == parentKUID ||
+		HasCapabilityOnFile(creds, linux.CAP_FOWNER, childKUID, childKGID) {
 		return nil
 	}
 	return syserror.EPERM
diff --git a/test/e2e/integration_test.go b/test/e2e/integration_test.go
index aaffabfd0..a180f5ac5 100644
--- a/test/e2e/integration_test.go
+++ b/test/e2e/integration_test.go
@@ -487,6 +487,20 @@ func TestPing6Loopback(t *testing.T) {
 	runIntegrationTest(t, []string{"NET_ADMIN"}, "./ping6.sh")
 }
 
+// This test checks that the owner of the sticky directory can delete files
+// inside it belonging to other users. It also checks that the owner of a file
+// can always delete its file when the file is inside a sticky directory owned
+// by another user.
+func TestStickyDir(t *testing.T) {
+	if vfs2Used, err := dockerutil.UsingVFS2(); err != nil {
+		t.Fatalf("failed to read config for runtime %s: %v", dockerutil.Runtime(), err)
+	} else if !vfs2Used {
+		t.Skip("sticky bit test fails on VFS1.")
+	}
+
+	runIntegrationTest(t, nil, "sh", "-c", "gcc -O2 -o test_sticky test_sticky.c && ./test_sticky")
+}
+
 func runIntegrationTest(t *testing.T, capAdd []string, args ...string) {
 	ctx := context.Background()
 	d := dockerutil.MakeContainer(ctx, t)