From ad74f20bc02e7fcc21a791e0b81c9cf72ab32d4c Mon Sep 17 00:00:00 2001 From: Ryan El Kochta Date: Wed, 29 Jul 2026 16:13:27 -0700 Subject: [PATCH] overlayfs: implement POSIX ACLs Implements POSIX ACLs for gVisor's overlayfs. - Adds plumbing to keep access and default ACLs in sync with the underlying filesystems, simliar to mode - Moves permission/filetype checking into helpers in posix_acl.go, used by both tmpfs and overlayfs - Updates the POSIX ACL tests to be parametrized by filesystem. The existing acl test suite runs both on overlayfs and on tmpfs This fixes journald not working for systemd user services. --- pkg/sentry/fsimpl/overlay/BUILD | 5 + pkg/sentry/fsimpl/overlay/copy_up.go | 41 +++- pkg/sentry/fsimpl/overlay/filesystem.go | 105 ++++++++- pkg/sentry/fsimpl/overlay/overlay.go | 64 ++++-- pkg/sentry/fsimpl/overlay/regular_file.go | 2 +- pkg/sentry/fsimpl/overlay/save_restore.go | 21 ++ pkg/sentry/fsimpl/tmpfs/tmpfs.go | 94 +++----- pkg/sentry/vfs/posix_acl.go | 88 ++++++++ test/syscalls/linux/posix_acl.cc | 249 +++++++++++++++------- 9 files changed, 497 insertions(+), 172 deletions(-) diff --git a/pkg/sentry/fsimpl/overlay/BUILD b/pkg/sentry/fsimpl/overlay/BUILD index b7af8e92c3f..bdc5d9d285f 100644 --- a/pkg/sentry/fsimpl/overlay/BUILD +++ b/pkg/sentry/fsimpl/overlay/BUILD @@ -114,10 +114,15 @@ go_library( "req_file_fd_mutex.go", "save_restore.go", ], + imports = [ + "gvisor.dev/gvisor/pkg/sentry/checkpoint", + "gvisor.dev/gvisor/pkg/sentry/vfs", + ], visibility = ["//pkg/sentry:internal"], deps = [ "//pkg/abi/linux", "//pkg/atomicbitops", + "//pkg/cleanup", "//pkg/context", "//pkg/errors/linuxerr", "//pkg/fspath", diff --git a/pkg/sentry/fsimpl/overlay/copy_up.go b/pkg/sentry/fsimpl/overlay/copy_up.go index 5f6aa935bc7..e01e02cbb52 100644 --- a/pkg/sentry/fsimpl/overlay/copy_up.go +++ b/pkg/sentry/fsimpl/overlay/copy_up.go @@ -181,6 +181,13 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy cleanupUndoCopyUp() return err } + acl, newMode, err := newFD.SetPosixACL(ctx, vfs.AccessACL, d.accessACL.Load(), false /* clearSGID */) + if err != nil { + cleanupUndoCopyUp() + return err + } + d.accessACL.Store(acl) + d.mode.Store(uint32(newMode)) d.upperVD = newFD.VirtualDentry() d.upperVD.IncRef() @@ -207,6 +214,19 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy cleanupUndoCopyUp() return err } + acl, newMode, err := vfsObj.SetPosixACLAt(ctx, d.fs.creds, &newpop, vfs.AccessACL, d.accessACL.Load(), false /* clearSGID */) + if err != nil { + cleanupUndoCopyUp() + return err + } + d.accessACL.Store(acl) + d.mode.Store(uint32(newMode)) + defaultACL, _, err := vfsObj.SetPosixACLAt(ctx, d.fs.creds, &newpop, vfs.DefaultACL, d.defaultACL.Load(), false /* clearSGID */) + if err != nil { + cleanupUndoCopyUp() + return err + } + d.defaultACL.Store(defaultACL) upperVD, err := vfsObj.GetDentryAt(ctx, d.fs.creds, &newpop, &vfs.GetDentryOptions{}) if err != nil { cleanupUndoCopyUp() @@ -236,6 +256,13 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy cleanupUndoCopyUp() return err } + acl, newMode, err := vfsObj.SetPosixACLAt(ctx, d.fs.creds, &newpop, vfs.AccessACL, d.accessACL.Load(), false /* clearSGID */) + if err != nil { + cleanupUndoCopyUp() + return err + } + d.accessACL.Store(acl) + d.mode.Store(uint32(newMode)) upperVD, err := vfsObj.GetDentryAt(ctx, d.fs.creds, &newpop, &vfs.GetDentryOptions{}) if err != nil { cleanupUndoCopyUp() @@ -265,6 +292,13 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy cleanupUndoCopyUp() return err } + acl, newMode, err := vfsObj.SetPosixACLAt(ctx, d.fs.creds, &newpop, vfs.AccessACL, d.accessACL.Load(), false /* clearSGID */) + if err != nil { + cleanupUndoCopyUp() + return err + } + d.accessACL.Store(acl) + d.mode.Store(uint32(newMode)) upperVD, err := vfsObj.GetDentryAt(ctx, d.fs.creds, &newpop, &vfs.GetDentryOptions{}) if err != nil { cleanupUndoCopyUp() @@ -375,7 +409,7 @@ func (d *dentry) copyUpMaybeSyntheticMountpointLocked(ctx context.Context, forSy // abort the copy-up. Loosely analogous to Linux's // fs/overlayfs/util.c:ovl_must_copy_xattr(). Here are the differences: // - Linux includes "system.posix_acl_access" and "system.posix_acl_default". -// As of writing, these are not supported in gVisor and so are excluded. +// In gVisor, we handle these separately. // - Linux includes all "security.*" xattrs. gVisor only supports // "security.capability" and so only that is included here. func mustCopyXattr(name string) bool { @@ -407,6 +441,11 @@ func (d *dentry) copyXattrsLocked(ctx context.Context) error { continue } + // Skip POSIX ACLs, which are handled separately. + if name == linux.XATTR_NAME_POSIX_ACL_ACCESS || name == linux.XATTR_NAME_POSIX_ACL_DEFAULT { + continue + } + value, err := vfsObj.GetXattrAt(ctx, d.fs.creds, lowerPop, &vfs.GetXattrOptions{Name: name, Size: 0}) if err != nil { ctx.Infof("failed to copy up %q xattr because GetXattrAt failed: %v", name, err) diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go index 13aab59237b..d1d56c0fa9a 100644 --- a/pkg/sentry/fsimpl/overlay/filesystem.go +++ b/pkg/sentry/fsimpl/overlay/filesystem.go @@ -226,16 +226,27 @@ func (fs *filesystem) lookupLocked(ctx context.Context, parent *dentry, name str // the topmost layer on which the file exists. mask |= linux.STATX_MODE | linux.STATX_UID | linux.STATX_GID | linux.STATX_INO } - stat, err := vfsObj.StatAt(ctx, fs.creds, &vfs.PathOperation{ + childPop := &vfs.PathOperation{ Root: childVD, Start: childVD, - }, &vfs.StatOptions{ + } + stat, err := vfsObj.StatAt(ctx, fs.creds, childPop, &vfs.StatOptions{ Mask: mask, }) if err != nil { lookupErr = err return false } + acl, err := vfsObj.GetPosixACLAt(ctx, fs.creds, childPop, vfs.AccessACL) + if err != nil { + lookupErr = err + return false + } + defaultACL, err := vfsObj.GetPosixACLAt(ctx, fs.creds, childPop, vfs.DefaultACL) + if err != nil { + lookupErr = err + return false + } if stat.Mask&mask != mask { lookupErr = linuxerr.EREMOTE return false @@ -274,6 +285,8 @@ func (fs *filesystem) lookupLocked(ctx context.Context, parent *dentry, name str topLookupLayer = lookupLayerLower } child.mode = atomicbitops.FromUint32(uint32(stat.Mode)) + child.accessACL.Store(acl) + child.defaultACL.Store(defaultACL) child.uid = atomicbitops.FromUint32(stat.UID) child.gid = atomicbitops.FromUint32(stat.GID) child.devMajor = atomicbitops.FromUint32(stat.DevMajor) @@ -1500,7 +1513,7 @@ func (fs *filesystem) SetStatAt(ctx context.Context, rp *vfs.ResolvingPath, opts // Precondition: d.fs.renameMu must be held for reading. func (d *dentry) setStatLocked(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.SetStatOptions) error { mode := linux.FileMode(d.mode.Load()) - if err := vfs.CheckSetStat(ctx, rp.Credentials(), &opts, mode, nil, auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())); err != nil { + if err := vfs.CheckSetStat(ctx, rp.Credentials(), &opts, mode, d.accessACL.Load(), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())); err != nil { return err } mnt := rp.Mount() @@ -1758,6 +1771,13 @@ func (fs *filesystem) GetXattrAt(ctx context.Context, rp *vfs.ResolvingPath, opt } func (fs *filesystem) getXattr(ctx context.Context, d *dentry, creds *auth.Credentials, opts *vfs.GetXattrOptions) (string, error) { + // Handle POSIX access ACL xattr + if strings.HasPrefix(opts.Name, linux.XATTR_SYSTEM_PREFIX) { + // Handle POSIX ACL xattrs + xattr, err := vfs.ACLGetXattr(creds, opts, linux.FileMode(d.mode.Load()), d.accessACL.Load(), d.defaultACL.Load()) + return xattr, err + } + if err := d.checkXattrPermissions(creds, opts.Name, vfs.MayRead); err != nil { return "", err } @@ -1794,8 +1814,20 @@ func (fs *filesystem) SetXattrAt(ctx context.Context, rp *vfs.ResolvingPath, opt return nil } -// Precondition: fs.renameMu must be locked. +// Precondition: fs.renameMu must be locked, d.copyMu must be unlocked. func (fs *filesystem) setXattrLocked(ctx context.Context, d *dentry, mnt *vfs.Mount, creds *auth.Credentials, opts *vfs.SetXattrOptions) error { + // Handle POSIX ACLs separately + if strings.HasPrefix(opts.Name, linux.XATTR_SYSTEM_PREFIX) { + acl, aclType, err := vfs.ACLSetXattr(creds, opts, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load())) + if err != nil { + return err + } + + // Set the appropriate POSIX ACL + _, _, err = fs.setPosixACLLocked(ctx, d, creds, mnt, aclType, acl, true /* clearSGID */) + return err + } + if err := d.checkXattrPermissions(creds, opts.Name, vfs.MayWrite); err != nil { return err } @@ -1838,8 +1870,19 @@ func (fs *filesystem) RemoveXattrAt(ctx context.Context, rp *vfs.ResolvingPath, return nil } -// Precondition: fs.renameMu must be locked. +// Precondition: fs.renameMu must be locked, d.copyMu must be unlocked. func (fs *filesystem) removeXattrLocked(ctx context.Context, d *dentry, mnt *vfs.Mount, creds *auth.Credentials, name string) error { + if strings.HasPrefix(name, linux.XATTR_SYSTEM_PREFIX) { + aclType, err := vfs.ACLRemoveXattr(creds, name, linux.FileMode(d.mode.Load()), auth.KUID(d.uid.Load())) + if err != nil { + return err + } + + // Clear the appropriate POSIX ACL + _, _, err = fs.setPosixACLLocked(ctx, d, creds, mnt, aclType, nil, true /* clearSGID */) + return err + } + if err := d.checkXattrPermissions(creds, name, vfs.MayWrite); err != nil { return err } @@ -1867,9 +1910,17 @@ func (fs *filesystem) GetPosixACLAt(ctx context.Context, rp *vfs.ResolvingPath, var ds *[]*dentry fs.renameMu.RLock() defer fs.renameMuRUnlockAndCheckDrop(ctx, &ds) - // overlayfs does not currently support POSIX ACLs. - _, err := fs.resolveLocked(ctx, rp, &ds) - return nil, err + d, err := fs.resolveLocked(ctx, rp, &ds) + if err != nil { + return nil, err + } + switch t { + case vfs.AccessACL: + return d.accessACL.Load(), nil + case vfs.DefaultACL: + return d.defaultACL.Load(), nil + } + return nil, linuxerr.EINVAL } // SetPosixACLAt implements vfs.FilesystemImpl.SetPosixACLAt. @@ -1877,12 +1928,44 @@ func (fs *filesystem) SetPosixACLAt(ctx context.Context, rp *vfs.ResolvingPath, var ds *[]*dentry fs.renameMu.RLock() defer fs.renameMuRUnlockAndCheckDrop(ctx, &ds) - _, err := fs.resolveLocked(ctx, rp, &ds) + d, err := fs.resolveLocked(ctx, rp, &ds) + if err != nil { + return nil, 0, err + } + newACL, mode, err := fs.setPosixACLLocked(ctx, d, rp.Credentials(), rp.Mount(), t, acl, clearSGID) + return newACL, mode, err +} + +// Precondition: fs.renameMu must be locked, d.copyMu must be unlocked. +func (fs *filesystem) setPosixACLLocked(ctx context.Context, d *dentry, creds *auth.Credentials, mnt *vfs.Mount, t vfs.ACLType, acl *vfs.PosixACL, clearSGID bool) (*vfs.PosixACL, linux.FileMode, error) { + if err := mnt.CheckBeginWrite(); err != nil { + return nil, 0, err + } + defer mnt.EndWrite() + if err := d.copyUpLocked(ctx); err != nil { + return nil, 0, err + } + vfsObj := d.fs.vfsfs.VirtualFilesystem() + upperPop := &vfs.PathOperation{Root: d.upperVD, Start: d.upperVD} + + // First, set the ACL on the underlying filesystem + newACL, mode, err := vfsObj.SetPosixACLAt(ctx, creds, upperPop, t, acl, clearSGID) if err != nil { return nil, 0, err } - // overlayfs does not currently support POSIX ACLs. - return nil, 0, linuxerr.EOPNOTSUPP + + // Next, update the ACL in our dentry + d.copyMu.Lock() + defer d.copyMu.Unlock() + switch t { + case vfs.AccessACL: + d.accessACL.Store(newACL) + d.mode.Store(uint32(mode)) + case vfs.DefaultACL: + d.defaultACL.Store(newACL) + } + + return newACL, mode, nil } // PrependPath implements vfs.FilesystemImpl.PrependPath. diff --git a/pkg/sentry/fsimpl/overlay/overlay.go b/pkg/sentry/fsimpl/overlay/overlay.go index 6c96834bee8..86cd269a450 100644 --- a/pkg/sentry/fsimpl/overlay/overlay.go +++ b/pkg/sentry/fsimpl/overlay/overlay.go @@ -42,6 +42,7 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/atomicbitops" + "gvisor.dev/gvisor/pkg/cleanup" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" "gvisor.dev/gvisor/pkg/fspath" @@ -379,31 +380,41 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt root.lowerVDs = append(root.lowerVDs, lowerRoot) } rootTopVD := root.topLayer() + rootCleanup := cleanup.Make(func() { + root.destroyLocked(ctx) + fs.vfsfs.DecRef(ctx) + }) + defer rootCleanup.Clean() // Get metadata from the topmost layer. See fs.lookupLocked(). const rootStatMask = linux.STATX_TYPE | linux.STATX_MODE | linux.STATX_UID | linux.STATX_GID | linux.STATX_INO - rootStat, err := vfsObj.StatAt(ctx, creds, &vfs.PathOperation{ + rootPop := &vfs.PathOperation{ Root: rootTopVD, Start: rootTopVD, - }, &vfs.StatOptions{ + } + rootStat, err := vfsObj.StatAt(ctx, creds, rootPop, &vfs.StatOptions{ Mask: rootStatMask, }) if err != nil { - root.destroyLocked(ctx) - fs.vfsfs.DecRef(ctx) + return nil, nil, err + } + rootACL, err := vfsObj.GetPosixACLAt(ctx, creds, rootPop, vfs.AccessACL) + if err != nil { + return nil, nil, err + } + rootDefaultACL, err := vfsObj.GetPosixACLAt(ctx, creds, rootPop, vfs.DefaultACL) + if err != nil { return nil, nil, err } if rootStat.Mask&rootStatMask != rootStatMask { - root.destroyLocked(ctx) - fs.vfsfs.DecRef(ctx) return nil, nil, linuxerr.EREMOTE } if isWhiteout(&rootStat) { ctx.Infof("overlay.FilesystemType.GetFilesystem: filesystem root is a whiteout") - root.destroyLocked(ctx) - fs.vfsfs.DecRef(ctx) return nil, nil, linuxerr.EINVAL } root.mode = atomicbitops.FromUint32(uint32(rootStat.Mode)) + root.accessACL.Store(rootACL) + root.defaultACL.Store(rootDefaultACL) root.uid = atomicbitops.FromUint32(rootStat.UID) root.gid = atomicbitops.FromUint32(rootStat.GID) if rootStat.Mode&linux.S_IFMT == linux.S_IFDIR { @@ -422,8 +433,6 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt rootDevMinor, err := fs.getLowerDevMinor(rootStat.DevMajor, rootStat.DevMinor) if err != nil { ctx.Infof("overlay.FilesystemType.GetFilesystem: failed to get device number for root: %v", err) - root.destroyLocked(ctx) - fs.vfsfs.DecRef(ctx) return nil, nil, err } root.devMinor = atomicbitops.FromUint32(rootDevMinor) @@ -434,6 +443,7 @@ func (fstype FilesystemType) GetFilesystem(ctx context.Context, vfsObj *vfs.Virt root.ino.Store(rootStat.Ino) } + rootCleanup.Release() return &fs.vfsfs, &root.vfsd, nil } @@ -561,13 +571,15 @@ type dentry struct { // fs is the owning filesystem. fs is immutable. fs *filesystem - // mode, uid, and gid are the file mode, owner, and group of the file in - // the topmost layer (and therefore the overlay file as well), and are used - // for permission checks on this dentry. These fields are protected by - // copyMu. - mode atomicbitops.Uint32 - uid atomicbitops.Uint32 - gid atomicbitops.Uint32 + // mode, uid, gid, accessACL, and defaultACL are the file mode, owner, + // group, and POSIX ACLs of the file in the topmost layer (and + // therefore the overlay file as well), and are used for permission checks + // on this dentry. These fields are protected by copyMu. + mode atomicbitops.Uint32 + accessACL atomic.Pointer[vfs.PosixACL] `state:".(*vfs.PosixACL)"` + defaultACL atomic.Pointer[vfs.PosixACL] `state:".(*vfs.PosixACL)"` + uid atomicbitops.Uint32 + gid atomicbitops.Uint32 // copiedUp is 1 if this dentry has been copied-up (i.e. upperVD.Ok()) and // 0 otherwise. @@ -872,14 +884,14 @@ func (d *dentry) topLookupLayer() lookupLayer { } func (d *dentry) checkPermissions(creds *auth.Credentials, ats vfs.AccessTypes) error { - return vfs.GenericCheckPermissions(creds, ats, linux.FileMode(d.mode.Load()), nil, auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())) + return vfs.GenericCheckPermissions(creds, ats, linux.FileMode(d.mode.Load()), d.accessACL.Load(), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())) } func (d *dentry) checkXattrPermissions(creds *auth.Credentials, name string, ats vfs.AccessTypes) error { mode := linux.FileMode(d.mode.Load()) kuid := auth.KUID(d.uid.Load()) kgid := auth.KGID(d.gid.Load()) - if err := vfs.GenericCheckPermissions(creds, ats, mode, nil, kuid, kgid); err != nil { + if err := vfs.GenericCheckPermissions(creds, ats, mode, d.accessACL.Load(), kuid, kgid); err != nil { return err } return vfs.CheckXattrPermissions(creds, ats, mode, kuid, name) @@ -909,6 +921,20 @@ func (d *dentry) statInternalTo(ctx context.Context, opts *vfs.StatOptions, stat // Preconditions: d.copyMu must be locked for writing. func (d *dentry) updateAfterSetStatLocked(opts *vfs.SetStatOptions) { if opts.Stat.Mask&linux.STATX_MODE != 0 { + // If the mode was changed, the underlying file's ACL may have changed, + // so recompute the ACL too. + oldACL := d.accessACL.Load() + if oldACL != nil { + newACL := oldACL.Chmod(opts.Stat.Mode) + mode, equiv := newACL.Mode() + opts.Stat.Mode = (opts.Stat.Mode &^ linux.PermissionsMask) | (mode & linux.PermissionsMask) + if equiv { + d.accessACL.Store(nil) + } else { + d.accessACL.Store(&newACL) + } + } + d.mode.Store((d.mode.RacyLoad() & linux.S_IFMT) | uint32(opts.Stat.Mode&^linux.S_IFMT)) } if opts.Stat.Mask&linux.STATX_UID != 0 { diff --git a/pkg/sentry/fsimpl/overlay/regular_file.go b/pkg/sentry/fsimpl/overlay/regular_file.go index df5c0be7598..8a5fde4daa0 100644 --- a/pkg/sentry/fsimpl/overlay/regular_file.go +++ b/pkg/sentry/fsimpl/overlay/regular_file.go @@ -167,7 +167,7 @@ func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint func (fd *regularFileFD) SetStat(ctx context.Context, opts vfs.SetStatOptions) error { d := fd.dentry() mode := linux.FileMode(d.mode.Load()) - if err := vfs.CheckSetStat(ctx, auth.CredentialsFromContext(ctx), &opts, mode, nil, auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())); err != nil { + if err := vfs.CheckSetStat(ctx, auth.CredentialsFromContext(ctx), &opts, mode, d.accessACL.Load(), auth.KUID(d.uid.Load()), auth.KGID(d.gid.Load())); err != nil { return err } mnt := fd.vfsfd.Mount() diff --git a/pkg/sentry/fsimpl/overlay/save_restore.go b/pkg/sentry/fsimpl/overlay/save_restore.go index d3e2c5ada86..1ea261aac6b 100644 --- a/pkg/sentry/fsimpl/overlay/save_restore.go +++ b/pkg/sentry/fsimpl/overlay/save_restore.go @@ -18,6 +18,7 @@ import ( "context" "gvisor.dev/gvisor/pkg/refs" + "gvisor.dev/gvisor/pkg/sentry/vfs" ) func (d *dentry) afterLoad(context.Context) { @@ -35,3 +36,23 @@ func (d *dentry) saveParent() *dentry { func (d *dentry) loadParent(_ context.Context, parent *dentry) { d.parent.Store(parent) } + +// saveAccessACL is called by stateify. +func (d *dentry) saveAccessACL() *vfs.PosixACL { + return d.accessACL.Load() +} + +// loadAccessACL is called by stateify. +func (d *dentry) loadAccessACL(_ context.Context, accessACL *vfs.PosixACL) { + d.accessACL.Store(accessACL) +} + +// saveDefaultACL is called by stateify. +func (d *dentry) saveDefaultACL() *vfs.PosixACL { + return d.defaultACL.Load() +} + +// loadDefaultACL is called by stateify. +func (d *dentry) loadDefaultACL(_ context.Context, defaultACL *vfs.PosixACL) { + d.defaultACL.Store(defaultACL) +} diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index adbc23aecfd..e1e31442b98 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -1085,28 +1085,10 @@ func (i *inode) getXattr(creds *auth.Credentials, opts *vfs.GetXattrOptions) (st kuid := auth.KUID(i.uid.Load()) kgid := auth.KGID(i.gid.Load()) - // Handle POSIX ACL xattrs if strings.HasPrefix(opts.Name, linux.XATTR_SYSTEM_PREFIX) { - var acl *vfs.PosixACL - switch opts.Name { - case linux.XATTR_NAME_POSIX_ACL_ACCESS: - acl = i.accessACL.Load() - case linux.XATTR_NAME_POSIX_ACL_DEFAULT: - acl = i.defaultACL.Load() - default: - return "", linuxerr.EOPNOTSUPP - } - - if mode.FileType() == linux.ModeSymlink { - return "", linuxerr.EOPNOTSUPP - } - - if acl == nil { - return "", linuxerr.ENODATA - } - - // Serialize the access ACL for userspace - return string(acl.Serialize(creds.UserNamespace)), nil + // Handle POSIX ACL xattrs + xattr, err := vfs.ACLGetXattr(creds, opts, mode, i.accessACL.Load(), i.defaultACL.Load()) + return xattr, err } acl := i.accessACL.Load() @@ -1122,51 +1104,23 @@ func (i *inode) setXattr(creds *auth.Credentials, opts *vfs.SetXattrOptions) err return err } mode := linux.FileMode(i.mode.Load()) - acl := i.accessACL.Load() kuid := auth.KUID(i.uid.Load()) kgid := auth.KGID(i.gid.Load()) if strings.HasPrefix(opts.Name, linux.XATTR_SYSTEM_PREFIX) { // Handle POSIX ACLs - var aclType vfs.ACLType - switch opts.Name { - case linux.XATTR_NAME_POSIX_ACL_ACCESS: - aclType = vfs.AccessACL - case linux.XATTR_NAME_POSIX_ACL_DEFAULT: - aclType = vfs.DefaultACL - default: - return linuxerr.EOPNOTSUPP - } - - if mode.FileType() == linux.ModeSymlink { - // ACLs cannot be set on symlinks - return linuxerr.EOPNOTSUPP - } - - // Parse the ACL from userspace - acl, err := vfs.ParsePosixACL([]byte(opts.Value), creds.UserNamespace) + acl, aclType, err := vfs.ACLSetXattr(creds, opts, mode, kuid) if err != nil { return err } - if aclType == vfs.DefaultACL && !mode.IsDir() { - if acl != nil { - // Default ACL can only be set on directories - return linuxerr.EACCES - } - return nil - } - - if !vfs.CanActAsOwner(creds, kuid) { - return linuxerr.EPERM - } - // Set the ACL _, _, err = i.setPosixACL(creds, aclType, acl, true /* clearSGID */) return err } + acl := i.accessACL.Load() if err := vfs.GenericCheckPermissions(creds, vfs.MayWrite, mode, acl, kuid, kgid); err != nil { return err } @@ -1183,26 +1137,13 @@ func (i *inode) removeXattr(creds *auth.Credentials, name string) error { kgid := auth.KGID(i.gid.Load()) if strings.HasPrefix(name, linux.XATTR_SYSTEM_PREFIX) { - var aclType vfs.ACLType - switch name { - case linux.XATTR_NAME_POSIX_ACL_ACCESS: - aclType = vfs.AccessACL - case linux.XATTR_NAME_POSIX_ACL_DEFAULT: - aclType = vfs.DefaultACL - default: - return linuxerr.EOPNOTSUPP - } - - if mode.FileType() == linux.ModeSymlink { - return linuxerr.EOPNOTSUPP - } - - if !vfs.CanActAsOwner(creds, kuid) { - return linuxerr.EPERM + aclType, err := vfs.ACLRemoveXattr(creds, name, mode, kuid) + if err != nil { + return err } // Clear the ACL - _, _, err := i.setPosixACL(creds, aclType, nil /* acl */, true /* clearSGID */) + _, _, err = i.setPosixACL(creds, aclType, nil /* acl */, true /* clearSGID */) return err } @@ -1320,6 +1261,23 @@ func (fd *fileDescription) RemoveXattr(ctx context.Context, name string) error { return fd.dentry().inode.removeXattr(auth.CredentialsFromContext(ctx), name) } +// GetPosixACL implements vfs.FileDescriptionImpl.GetPosixACL. +func (fd *fileDescription) GetPosixACL(ctx context.Context, t vfs.ACLType) (*vfs.PosixACL, error) { + switch t { + case vfs.AccessACL: + return fd.dentry().inode.accessACL.Load(), nil + case vfs.DefaultACL: + return fd.dentry().inode.defaultACL.Load(), nil + default: + return nil, linuxerr.EOPNOTSUPP + } +} + +// SetPosixACL implements vfs.FileDescriptionImpl.SetPosixACL. +func (fd *fileDescription) SetPosixACL(ctx context.Context, t vfs.ACLType, acl *vfs.PosixACL, clearSGID bool) (*vfs.PosixACL, linux.FileMode, error) { + return fd.dentry().inode.setPosixACL(auth.CredentialsFromContext(ctx), t, acl, clearSGID) +} + // Sync implements vfs.FileDescriptionImpl.Sync. It does nothing because all // filesystem state is in-memory. func (*fileDescription) Sync(context.Context) error { diff --git a/pkg/sentry/vfs/posix_acl.go b/pkg/sentry/vfs/posix_acl.go index 9d4e5c10cac..5275e10f6a8 100644 --- a/pkg/sentry/vfs/posix_acl.go +++ b/pkg/sentry/vfs/posix_acl.go @@ -429,3 +429,91 @@ func (a PosixACL) String() string { return fmt.Sprintf("PosixACL { UGOPerms: %O, Mask: %s, Users: %v, Groups: %v }", a.UGOPerms, mask, a.Users, a.Groups) } + +// ACLGetXattr implements getxattr(2) for POSIX ACLs, including permission checks. +func ACLGetXattr(creds *auth.Credentials, opts *GetXattrOptions, mode linux.FileMode, accessACL *PosixACL, defaultACL *PosixACL) (string, error) { + var acl *PosixACL + switch opts.Name { + case linux.XATTR_NAME_POSIX_ACL_ACCESS: + acl = accessACL + case linux.XATTR_NAME_POSIX_ACL_DEFAULT: + acl = defaultACL + default: + return "", linuxerr.EOPNOTSUPP + } + + if mode.FileType() == linux.ModeSymlink { + return "", linuxerr.EOPNOTSUPP + } + + if acl == nil { + return "", linuxerr.ENODATA + } + + // Serialize the access ACL for userspace + return string(acl.Serialize(creds.UserNamespace)), nil +} + +// ACLSetXattr performs permission checking and returns the ACL type and parsed ACL +// that should be set for setxattr(2). +func ACLSetXattr(creds *auth.Credentials, opts *SetXattrOptions, mode linux.FileMode, kuid auth.KUID) (*PosixACL, ACLType, error) { + var aclType ACLType + switch opts.Name { + case linux.XATTR_NAME_POSIX_ACL_ACCESS: + aclType = AccessACL + case linux.XATTR_NAME_POSIX_ACL_DEFAULT: + aclType = DefaultACL + default: + return nil, 0, linuxerr.EOPNOTSUPP + } + + if mode.FileType() == linux.ModeSymlink { + // ACLs cannot be set on symlinks + return nil, 0, linuxerr.EOPNOTSUPP + } + + // Parse the ACL from userspace + acl, err := ParsePosixACL([]byte(opts.Value), creds.UserNamespace) + if err != nil { + return nil, 0, err + } + + if aclType == DefaultACL && !mode.IsDir() { + if acl != nil { + // Default ACL can only be set on directories + return nil, 0, linuxerr.EACCES + } + return nil, aclType, nil + } + + if !CanActAsOwner(creds, kuid) { + return nil, 0, linuxerr.EPERM + } + + return acl, aclType, nil +} + +// ACLRemoveXattr performs permission checking and returns the type of the ACL +// that should be removed by removexattr(2). +func ACLRemoveXattr(creds *auth.Credentials, name string, mode linux.FileMode, kuid auth.KUID) (ACLType, error) { + var aclType ACLType + switch name { + case linux.XATTR_NAME_POSIX_ACL_ACCESS: + aclType = AccessACL + case linux.XATTR_NAME_POSIX_ACL_DEFAULT: + aclType = DefaultACL + default: + return 0, linuxerr.EOPNOTSUPP + } + + if mode.FileType() == linux.ModeSymlink { + return 0, linuxerr.EOPNOTSUPP + } + + // For POSIX ACLs, skip the MayWrite check; instead, this is an ownership check + if !CanActAsOwner(creds, kuid) { + return 0, linuxerr.EPERM + } + + return aclType, nil +} diff --git a/test/syscalls/linux/posix_acl.cc b/test/syscalls/linux/posix_acl.cc index addb5c1e958..640618b2164 100644 --- a/test/syscalls/linux/posix_acl.cc +++ b/test/syscalls/linux/posix_acl.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -28,8 +29,6 @@ #include #include -#include "gmock/gmock.h" -#include "gtest/gtest.h" #include "absl/time/clock.h" #include "absl/time/time.h" #include "test/util/cleanup.h" @@ -40,6 +39,8 @@ #include "test/util/temp_path.h" #include "test/util/test_util.h" #include "test/util/thread_util.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" namespace gvisor { namespace testing { @@ -80,17 +81,17 @@ ACLEntry Ent(int tag, int perm, uint32_t id) { } // BuildACL builds the raw xattr representation for a POSIX ACL. -std::string BuildACL(const std::vector& entries) { +std::string BuildACL(const std::vector &entries) { uint32_t version = kACLVersion; - std::string buf(reinterpret_cast(&version), sizeof(version)); - for (const ACLEntry& e : entries) { - buf.append(reinterpret_cast(&e), sizeof(e)); + std::string buf(reinterpret_cast(&version), sizeof(version)); + for (const ACLEntry &e : entries) { + buf.append(reinterpret_cast(&e), sizeof(e)); } return buf; } // ParseACL parses a raw ACL value into its entries. -std::vector ParseACL(const std::string& blob) { +std::vector ParseACL(const std::string &blob) { std::vector out; for (size_t off = sizeof(uint32_t); off + sizeof(ACLEntry) <= blob.size(); off += sizeof(ACLEntry)) { @@ -103,18 +104,20 @@ std::vector ParseACL(const std::string& blob) { // FindEntryPerm returns the permission of the first entry matching tag (and id // for named user/group tags), or -1 if not found. -int FindEntryPerm(const std::string& blob, uint16_t tag, uint32_t id) { - for (const ACLEntry& e : ParseACL(blob)) { - if (e.tag != tag) continue; - if ((tag == kUser || tag == kGroup) && e.id != id) continue; +int FindEntryPerm(const std::string &blob, uint16_t tag, uint32_t id) { + for (const ACLEntry &e : ParseACL(blob)) { + if (e.tag != tag) + continue; + if ((tag == kUser || tag == kGroup) && e.id != id) + continue; return e.perm; } return -1; } // GetXattrString reads an xattr into a string. -PosixErrorOr GetXattrString(const std::string& path, - const char* name) { +PosixErrorOr GetXattrString(const std::string &path, + const char *name) { char buf[512]; int n = getxattr(path.c_str(), name, buf, sizeof(buf)); if (n < 0) { @@ -124,7 +127,7 @@ PosixErrorOr GetXattrString(const std::string& path, } // ListContainsName reports whether a listxattr(2) result contains name. -bool ListContainsName(const char* list, int len, const char* name) { +bool ListContainsName(const char *list, int len, const char *name) { for (int i = 0; i < len; i += strlen(list + i) + 1) { if (strcmp(list + i, name) == 0) { return true; @@ -133,25 +136,107 @@ bool ListContainsName(const char* list, int len, const char* name) { return false; } -class PosixACLTest : public ::testing::Test { - protected: +// Backend selects the filesystem the ACL tests run against. +// +// When support for POSIX ACLs is added to a new filesystem, it should be added +// here. +enum class Backend { + kTmpfs, // plain tmpfs + kOverlayUpper, // an overlay whose object is created in the upper layer + kOverlayLower, // an overlay whose object starts in the lower layer and is + // copied up on the first modifying operation +}; + +class PosixACLTest : public ::testing::TestWithParam { +protected: void SetUp() override { - // Use /dev/shm to allow the native tests to run without privilege - dir_ = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn("/dev/shm")); - SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(IsTmpfs(dir_.path()))); + // Use /dev/shm to allow tmpfs tests to run without privilege. + base_ = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn("/dev/shm")); + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(IsTmpfs(base_.path()))); + + const auto param = GetParam(); + switch (param) { + case Backend::kTmpfs: + dir_ = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(base_.path())); + break; + case Backend::kOverlayUpper: + case Backend::kOverlayLower: + MountOverlay(param); + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(IsOverlayfs(dir_.path()))); + } file_ = JoinPath(dir_.path(), "posix_acl_test_file"); - ASSERT_NO_ERRNO_AND_VALUE(Open(file_, O_CREAT | O_RDWR, 0644)); - ASSERT_THAT(chmod(file_.c_str(), 0644), SyscallSucceeds()); - subdir_ = JoinPath(dir_.path(), "subdir"); - ASSERT_THAT(mkdir(subdir_.c_str(), 0755), SyscallSucceeds()); + + // For the lower-layer case the objects were already seeded in the lower + // directory by MountOverlay(); the first modifying syscall in each test + // will copy them up. Otherwise create them now: directly on tmpfs, or + // through the merged mount (landing in the upper layer). + if (GetParam() != Backend::kOverlayLower) { + ASSERT_NO_ERRNO_AND_VALUE(Open(file_, O_CREAT | O_RDWR, 0644)); + ASSERT_THAT(chmod(file_.c_str(), 0644), SyscallSucceeds()); + ASSERT_THAT(mkdir(subdir_.c_str(), 0755), SyscallSucceeds()); + } uid_ = getuid(); gid_ = getgid(); } + // MountOverlay mounts an overlay at dir_. + void MountOverlay(const ParamType param) { + // Mounting an overlay requires CAP_SYS_ADMIN + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + + // gVisor doesn't accept /dev/shm as an overlayfs layer. We need + // CAP_SYS_ADMIN for this case anyways, so just mount a separate tmpfs. + layers_ = JoinPath(base_.path(), "layers"); + ASSERT_THAT(mkdir(layers_.c_str(), 0755), SyscallSucceeds()); + ASSERT_THAT(mount("tmpfs", layers_.c_str(), "tmpfs", 0, "mode=0755"), + SyscallSucceeds()); + tmpfs_mounted_ = true; + + const std::string lower = JoinPath(layers_, "lower"); + const std::string upper = JoinPath(layers_, "upper"); + const std::string work = JoinPath(layers_, "work"); + for (const std::string &d : {lower, upper, work}) { + ASSERT_THAT(mkdir(d.c_str(), 0755), SyscallSucceeds()); + } + + // dir_ is the merged mount point. + dir_ = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(base_.path())); + + if (param == Backend::kOverlayLower) { + const std::string lfile = JoinPath(lower, "posix_acl_test_file"); + ASSERT_NO_ERRNO_AND_VALUE(Open(lfile, O_CREAT | O_RDWR, 0644)); + ASSERT_THAT(chmod(lfile.c_str(), 0644), SyscallSucceeds()); + ASSERT_THAT(mkdir(JoinPath(lower, "subdir").c_str(), 0755), + SyscallSucceeds()); + } + + const std::string opts = + "lowerdir=" + lower + ",upperdir=" + upper + ",workdir=" + work; + ASSERT_THAT( + mount("overlay", dir_.path().c_str(), "overlay", 0, opts.c_str()), + SyscallSucceeds()); + overlay_mounted_ = true; + } + + void TearDown() override { + // Unmount inner-to-outer, before the TempPath destructors remove the (now + // empty) mount points. + if (overlay_mounted_) { + EXPECT_THAT(umount2(dir_.path().c_str(), MNT_DETACH), SyscallSucceeds()); + } + if (tmpfs_mounted_) { + EXPECT_THAT(umount2(layers_.c_str(), MNT_DETACH), SyscallSucceeds()); + } + } + + TempPath base_; TempPath dir_; + std::string layers_; + bool tmpfs_mounted_ = false; + bool overlay_mounted_ = false; std::string file_; std::string subdir_; uid_t uid_; @@ -159,7 +244,7 @@ class PosixACLTest : public ::testing::Test { }; // Setting an access ACL and reading it back returns an identical blob. -TEST_F(PosixACLTest, SetGetAccessACL) { +TEST_P(PosixACLTest, SetGetAccessACL) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kUser, kR, uid_), @@ -177,7 +262,7 @@ TEST_F(PosixACLTest, SetGetAccessACL) { } // getxattr reports the size when passed a zero-length buffer. -TEST_F(PosixACLTest, GetAccessACLSize) { +TEST_P(PosixACLTest, GetAccessACLSize) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kUser, kR, uid_), @@ -192,20 +277,20 @@ TEST_F(PosixACLTest, GetAccessACLSize) { } // getxattr on a file with no ACL returns ENODATA. -TEST_F(PosixACLTest, GetAccessACLNoData) { +TEST_P(PosixACLTest, GetAccessACLNoData) { EXPECT_THAT(getxattr(file_.c_str(), kAccessACL, nullptr, 0), SyscallFailsWithErrno(ENODATA)); } // Setting an extended access ACL updates the file mode: the owner/other bits // come from USER_OBJ/OTHER, and the group bits reflect the mask. -TEST_F(PosixACLTest, AccessACLUpdatesMode) { +TEST_P(PosixACLTest, AccessACLUpdatesMode) { const std::string acl = BuildACL({ - Ent(kUserObj, kR | kW, kUndef), // owner rw- - Ent(kUser, kR, uid_), // - Ent(kGroupObj, kR, kUndef), // group_obj r-- - Ent(kMask, kR | kW | kX, kUndef), // mask rwx (surfaces as group bits) - Ent(kOther, kR, kUndef), // other r-- + Ent(kUserObj, kR | kW, kUndef), // owner rw- + Ent(kUser, kR, uid_), // + Ent(kGroupObj, kR, kUndef), // group_obj r-- + Ent(kMask, kR | kW | kX, kUndef), // mask rwx (surfaces as group bits) + Ent(kOther, kR, kUndef), // other r-- }); ASSERT_THAT(setxattr(file_.c_str(), kAccessACL, acl.data(), acl.size(), 0), SyscallSucceeds()); @@ -217,7 +302,7 @@ TEST_F(PosixACLTest, AccessACLUpdatesMode) { // A minimal ACL (only the three base entries, no mask/named entries) is // equivalent to a mode: it is folded into the mode and not stored. -TEST_F(PosixACLTest, MinimalAccessACLFoldsToMode) { +TEST_P(PosixACLTest, MinimalAccessACLFoldsToMode) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW | kX, kUndef), Ent(kGroupObj, kR | kX, kUndef), @@ -236,7 +321,7 @@ TEST_F(PosixACLTest, MinimalAccessACLFoldsToMode) { } // An extended access ACL appears in listxattr; removing it makes it disappear. -TEST_F(PosixACLTest, ListAndRemoveAccessACL) { +TEST_P(PosixACLTest, ListAndRemoveAccessACL) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kUser, kR, uid_), @@ -262,7 +347,7 @@ TEST_F(PosixACLTest, ListAndRemoveAccessACL) { } // A default ACL cannot be set on a non-directory. -TEST_F(PosixACLTest, DefaultACLOnFileFails) { +TEST_P(PosixACLTest, DefaultACLOnFileFails) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kGroupObj, kR, kUndef), @@ -274,7 +359,7 @@ TEST_F(PosixACLTest, DefaultACLOnFileFails) { // A default ACL can be set on and read back from a directory, and appears in // listxattr. -TEST_F(PosixACLTest, SetGetDefaultACLOnDir) { +TEST_P(PosixACLTest, SetGetDefaultACLOnDir) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW | kX, kUndef), Ent(kUser, kR | kX, uid_), @@ -297,7 +382,7 @@ TEST_F(PosixACLTest, SetGetDefaultACLOnDir) { // A file created in a directory with a default ACL inherits an access ACL // derived from that default ACL. A subdirectory also inherits the default ACL. -TEST_F(PosixACLTest, DefaultACLInheritance) { +TEST_P(PosixACLTest, DefaultACLInheritance) { const std::string dacl = BuildACL({ Ent(kUserObj, kR | kW | kX, kUndef), Ent(kUser, kR | kX, uid_), @@ -333,12 +418,12 @@ TEST_F(PosixACLTest, DefaultACLInheritance) { // chmod on a file with an extended ACL updates USER_OBJ, the mask, and OTHER // (not the GROUP_OBJ entry). -TEST_F(PosixACLTest, ChmodUpdate) { +TEST_P(PosixACLTest, ChmodUpdate) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kUser, kR | kW, uid_), - Ent(kGroupObj, kR, kUndef), // group_obj r-- - Ent(kMask, kR, kUndef), // mask r-- + Ent(kGroupObj, kR, kUndef), // group_obj r-- + Ent(kMask, kR, kUndef), // mask r-- Ent(kOther, kR, kUndef), }); ASSERT_THAT(setxattr(file_.c_str(), kAccessACL, acl.data(), acl.size(), 0), @@ -364,7 +449,7 @@ TEST_F(PosixACLTest, ChmodUpdate) { // A named-user ACL entry grants access that the mode bits alone would deny, and // the mask caps that access. -TEST_F(PosixACLTest, NamedUserEnforcement) { +TEST_P(PosixACLTest, NamedUserEnforcement) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETUID))); // Without an ACL granting access, "nobody" cannot read a 0600 file. @@ -379,9 +464,9 @@ TEST_F(PosixACLTest, NamedUserEnforcement) { // Grant "nobody" read via a named-user entry; the mask allows only read. const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), - Ent(kUser, kR | kW | kX, kNobody), // capped by the mask + Ent(kUser, kR | kW | kX, kNobody), // capped by the mask Ent(kGroupObj, 0, kUndef), - Ent(kMask, kR, kUndef), // mask r-- : nobody effectively gets r-- only + Ent(kMask, kR, kUndef), // mask r-- : nobody effectively gets r-- only Ent(kOther, 0, kUndef), }); ASSERT_THAT(setxattr(file_.c_str(), kAccessACL, acl.data(), acl.size(), 0), @@ -392,17 +477,19 @@ TEST_F(PosixACLTest, NamedUserEnforcement) { // Read is granted by the ACL. int rfd = open(file_.c_str(), O_RDONLY); EXPECT_THAT(rfd, SyscallSucceeds()); - if (rfd >= 0) close(rfd); + if (rfd >= 0) + close(rfd); // Write is denied because the mask limits nobody to read. int wfd = open(file_.c_str(), O_WRONLY); EXPECT_THAT(wfd, SyscallFailsWithErrno(EACCES)); - if (wfd >= 0) close(wfd); + if (wfd >= 0) + close(wfd); }); } // A named-group ACL entry grants access to a process in that group that the // mode bits alone would deny, capped by the mask. -TEST_F(PosixACLTest, NamedGroupEnforcement) { +TEST_P(PosixACLTest, NamedGroupEnforcement) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETUID))); SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETGID))); @@ -411,8 +498,8 @@ TEST_F(PosixACLTest, NamedGroupEnforcement) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kGroupObj, 0, kUndef), - Ent(kGroup, kR | kW, kNobody), // capped by the mask - Ent(kMask, kR, kUndef), // mask r-- : group class limited to read + Ent(kGroup, kR | kW, kNobody), // capped by the mask + Ent(kMask, kR, kUndef), // mask r-- : group class limited to read Ent(kOther, 0, kUndef), }); ASSERT_THAT(setxattr(file_.c_str(), kAccessACL, acl.data(), acl.size(), 0), @@ -426,17 +513,19 @@ TEST_F(PosixACLTest, NamedGroupEnforcement) { // Read is granted via the named group. int rfd = open(file_.c_str(), O_RDONLY); EXPECT_THAT(rfd, SyscallSucceeds()); - if (rfd >= 0) close(rfd); + if (rfd >= 0) + close(rfd); // Write is denied due to the mask. int wfd = open(file_.c_str(), O_WRONLY); EXPECT_THAT(wfd, SyscallFailsWithErrno(EACCES)); - if (wfd >= 0) close(wfd); + if (wfd >= 0) + close(wfd); }); } // A restrictive USER_OBJ locks the user out of their own file, even // if group/other would otherwise grant the access. -TEST_F(PosixACLTest, UserObjLocksSelfOutEnforcement) { +TEST_P(PosixACLTest, UserObjLocksSelfOutEnforcement) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETUID))); SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETGID))); @@ -466,7 +555,7 @@ TEST_F(PosixACLTest, UserObjLocksSelfOutEnforcement) { // A restrictive named USER locks the user out of their own file, even // if group/other would otherwise grant the access. -TEST_F(PosixACLTest, NamedUserLocksSelfOutEnforcement) { +TEST_P(PosixACLTest, NamedUserLocksSelfOutEnforcement) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETUID))); SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETGID))); @@ -499,7 +588,7 @@ TEST_F(PosixACLTest, NamedUserLocksSelfOutEnforcement) { // Only the file owner (or a suitably privileged process) may set an ACL, even // with write permission on the file. -TEST_F(PosixACLTest, SetACLRequiresOwnership) { +TEST_P(PosixACLTest, SetACLRequiresOwnership) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SETUID))); // World-writable file owned by the (root) test process. @@ -520,7 +609,7 @@ TEST_F(PosixACLTest, SetACLRequiresOwnership) { } // POSIX access ACLs should work properly with symlinks. -TEST_F(PosixACLTest, SetAccessACLSymlink) { +TEST_P(PosixACLTest, SetAccessACLSymlink) { // Create a symlink to a file auto sym = JoinPath(dir_.path(), "posix_acl_test_symlink"); ASSERT_THAT(symlink(file_.c_str(), sym.c_str()), SyscallSucceeds()); @@ -557,7 +646,7 @@ TEST_F(PosixACLTest, SetAccessACLSymlink) { } // POSIX default ACLs should work properly with symlinks. -TEST_F(PosixACLTest, SetDefaultACLSymlink) { +TEST_P(PosixACLTest, SetDefaultACLSymlink) { // Create a symlink to a directory auto sym = JoinPath(dir_.path(), "posix_acl_test_symlink"); ASSERT_THAT(symlink(dir_.path().c_str(), sym.c_str()), SyscallSucceeds()); @@ -604,7 +693,7 @@ static bool IsTimespecLater(struct timespec a, struct timespec b) { return false; } -TEST_F(PosixACLTest, SetACLUpdatesCTime) { +TEST_P(PosixACLTest, SetACLUpdatesCTime) { // Fetch the original ctime struct stat st = {}; ASSERT_THAT(stat(file_.c_str(), &st), SyscallSucceeds()); @@ -627,7 +716,7 @@ TEST_F(PosixACLTest, SetACLUpdatesCTime) { EXPECT_TRUE(IsTimespecLater(st.st_ctim, old_ctime)); } -TEST_F(PosixACLTest, RemoveACLUpdatesCTime) { +TEST_P(PosixACLTest, RemoveACLUpdatesCTime) { // Set an ACL on the file. const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), @@ -653,7 +742,7 @@ TEST_F(PosixACLTest, RemoveACLUpdatesCTime) { EXPECT_TRUE(IsTimespecLater(st.st_ctim, old_ctime)); } -TEST_F(PosixACLTest, SetACLClearsSGID) { +TEST_P(PosixACLTest, SetACLClearsSGID) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_FSETID))); SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_CHOWN))); @@ -701,7 +790,7 @@ TEST_F(PosixACLTest, SetACLClearsSGID) { ASSERT_FALSE(st.st_mode & S_ISGID); } -TEST_F(PosixACLTest, RemoveACLClearsSGID) { +TEST_P(PosixACLTest, RemoveACLClearsSGID) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_FSETID))); SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_CHOWN))); @@ -738,7 +827,7 @@ TEST_F(PosixACLTest, RemoveACLClearsSGID) { ASSERT_FALSE(st.st_mode & S_ISGID); } -TEST_F(PosixACLTest, SetACLEmpty) { +TEST_P(PosixACLTest, SetACLEmpty) { // Set an ACL on the file std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), @@ -777,7 +866,7 @@ TEST_F(PosixACLTest, SetACLEmpty) { SyscallSucceeds()); } -TEST_F(PosixACLTest, SetACLEmptyHeaderOnly) { +TEST_P(PosixACLTest, SetACLEmptyHeaderOnly) { std::string emptyACL = BuildACL({}); // Set an ACL on the file @@ -822,14 +911,14 @@ TEST_F(PosixACLTest, SetACLEmptyHeaderOnly) { SyscallSucceeds()); } -TEST_F(PosixACLTest, SetACLIncompleteHeader) { +TEST_P(PosixACLTest, SetACLIncompleteHeader) { // Set an ACL on the file char xattr[1] = {}; EXPECT_THAT(setxattr(file_.c_str(), kAccessACL, xattr, 1, 0), SyscallFailsWithErrno(EINVAL)); } -TEST_F(PosixACLTest, SetACLWrongVersion) { +TEST_P(PosixACLTest, SetACLWrongVersion) { std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kUser, kR, uid_), @@ -845,7 +934,7 @@ TEST_F(PosixACLTest, SetACLWrongVersion) { SyscallFailsWithErrno(EOPNOTSUPP)); } -TEST_F(PosixACLTest, SetACLNonWholeNumberEntries) { +TEST_P(PosixACLTest, SetACLNonWholeNumberEntries) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kUser, kR, uid_), @@ -860,7 +949,7 @@ TEST_F(PosixACLTest, SetACLNonWholeNumberEntries) { SyscallFailsWithErrno(EINVAL)); } -TEST_F(PosixACLTest, SetACLInvalidPermissionBits) { +TEST_P(PosixACLTest, SetACLInvalidPermissionBits) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kUser, 10, uid_), @@ -874,7 +963,7 @@ TEST_F(PosixACLTest, SetACLInvalidPermissionBits) { SyscallFailsWithErrno(EINVAL)); } -TEST_F(PosixACLTest, SetACLMultipleObj) { +TEST_P(PosixACLTest, SetACLMultipleObj) { std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kUserObj, kR | kW, kUndef), @@ -928,7 +1017,7 @@ TEST_F(PosixACLTest, SetACLMultipleObj) { SyscallFailsWithErrno(EINVAL)); } -TEST_F(PosixACLTest, SetACLNonUniqueID) { +TEST_P(PosixACLTest, SetACLNonUniqueID) { // acl(5) documents this as causing an ACL to be invalid, however // Linux does not enforce this. So we won't either. @@ -957,7 +1046,7 @@ TEST_F(PosixACLTest, SetACLNonUniqueID) { SyscallSucceeds()); } -TEST_F(PosixACLTest, SetACLNoObj) { +TEST_P(PosixACLTest, SetACLNoObj) { const std::string acl = BuildACL({ Ent(kUser, kR, uid_), Ent(kUser, kR, uid_), @@ -970,7 +1059,7 @@ TEST_F(PosixACLTest, SetACLNoObj) { SyscallFailsWithErrno(EINVAL)); } -TEST_F(PosixACLTest, SetACLNoMask) { +TEST_P(PosixACLTest, SetACLNoMask) { const std::string acl = BuildACL({ Ent(kUserObj, kR | kW, kUndef), Ent(kUser, kR, uid_), @@ -984,7 +1073,23 @@ TEST_F(PosixACLTest, SetACLNoMask) { SyscallFailsWithErrno(EINVAL)); } -} // namespace +INSTANTIATE_TEST_SUITE_P( + All, PosixACLTest, + ::testing::Values(Backend::kTmpfs, Backend::kOverlayUpper, + Backend::kOverlayLower), + [](const ::testing::TestParamInfo &info) -> std::string { + switch (info.param) { + case Backend::kTmpfs: + return "Tmpfs"; + case Backend::kOverlayUpper: + return "OverlayUpper"; + case Backend::kOverlayLower: + return "OverlayLower"; + } + return "Unknown"; + }); + +} // namespace -} // namespace testing -} // namespace gvisor +} // namespace testing +} // namespace gvisor