diff options
author | Kevin Krakauer <krakauer@google.com> | 2021-03-23 15:40:17 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-03-23 15:42:12 -0700 |
commit | 92374e51976c8a47e4705943f73cecbc6a27073b (patch) | |
tree | 5f267314a82b8dbdc4638c8eb1e2c5b062890ca0 /pkg | |
parent | acb4c62885629d6d3ee977b93c27282abed0b33f (diff) |
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
Diffstat (limited to 'pkg')
-rw-r--r-- | pkg/p9/p9.go | 11 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/gofer/filesystem.go | 20 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/gofer/gofer.go | 20 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/gofer/regular_file.go | 14 |
4 files changed, 59 insertions, 6 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 } |