From e7b59aa1b6a9b2e2fe21522b7d710f3aaf218bfa Mon Sep 17 00:00:00 2001 From: Jing Chen Date: Wed, 28 Feb 2024 23:58:10 -0800 Subject: [PATCH] Implement Getxattr for directfs and lisafs. It allows us to read the executable binary's security.capability. PiperOrigin-RevId: 611364676 --- pkg/sentry/fsimpl/gofer/dentry_impl.go | 3 +-- pkg/sentry/fsimpl/gofer/directfs_dentry.go | 8 ++++++++ pkg/sentry/syscalls/linux/sys_xattr.go | 10 +++++++--- runsc/boot/filter/config/config_main.go | 6 ++++++ runsc/fsgofer/filter/config.go | 1 - runsc/fsgofer/filter/filter.go | 3 +++ runsc/fsgofer/lisafs.go | 4 +++- 7 files changed, 28 insertions(+), 7 deletions(-) diff --git a/pkg/sentry/fsimpl/gofer/dentry_impl.go b/pkg/sentry/fsimpl/gofer/dentry_impl.go index 9c97667e4..0f264245c 100644 --- a/pkg/sentry/fsimpl/gofer/dentry_impl.go +++ b/pkg/sentry/fsimpl/gofer/dentry_impl.go @@ -306,8 +306,7 @@ func (d *dentry) getXattrImpl(ctx context.Context, opts *vfs.GetXattrOptions) (s case *lisafsDentry: return dt.controlFD.GetXattr(ctx, opts.Name, opts.Size) case *directfsDentry: - // Consistent with runsc/fsgofer. - return "", linuxerr.EOPNOTSUPP + return dt.getXattr(opts.Name, opts.Size) default: panic("unknown dentry implementation") } diff --git a/pkg/sentry/fsimpl/gofer/directfs_dentry.go b/pkg/sentry/fsimpl/gofer/directfs_dentry.go index 7b5c7518c..165ea1cc5 100644 --- a/pkg/sentry/fsimpl/gofer/directfs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/directfs_dentry.go @@ -423,6 +423,14 @@ func (d *directfsDentry) getHostChild(name string) (*dentry, error) { return d.fs.newDirectfsDentry(childFD) } +func (d *directfsDentry) getXattr(name string, size uint64) (string, error) { + data := make([]byte, size) + if _, err := unix.Fgetxattr(d.controlFD, name, data); err != nil { + return "", err + } + return string(data), nil +} + // getCreatedChild opens the newly created child, sets its uid/gid, constructs // a disconnected dentry and returns it. func (d *directfsDentry) getCreatedChild(name string, uid, gid int, isDir bool) (*dentry, error) { diff --git a/pkg/sentry/syscalls/linux/sys_xattr.go b/pkg/sentry/syscalls/linux/sys_xattr.go index 1f86a610c..bddf6ed73 100644 --- a/pkg/sentry/syscalls/linux/sys_xattr.go +++ b/pkg/sentry/syscalls/linux/sys_xattr.go @@ -101,6 +101,9 @@ func getxattr(t *kernel.Task, args arch.SyscallArguments, shouldFollowFinalSymli valueAddr := args[2].Pointer() size := args[3].SizeT() + if size > linux.XATTR_SIZE_MAX { + size = linux.XATTR_SIZE_MAX + } path, err := copyInPath(t, pathAddr) if err != nil { return 0, nil, err @@ -137,6 +140,10 @@ func Fgetxattr(t *kernel.Task, sysno uintptr, args arch.SyscallArguments) (uintp valueAddr := args[2].Pointer() size := args[3].SizeT() + if size > linux.XATTR_SIZE_MAX { + size = linux.XATTR_SIZE_MAX + } + file := t.GetFile(fd) if file == nil { return 0, nil, linuxerr.EBADF @@ -339,9 +346,6 @@ func copyInXattrValue(t *kernel.Task, valueAddr hostarch.Addr, size uint) (strin } func copyOutXattrValue(t *kernel.Task, valueAddr hostarch.Addr, size uint, value string) (int, error) { - if size > linux.XATTR_SIZE_MAX { - size = linux.XATTR_SIZE_MAX - } if size == 0 { // Return the size that would be required to accommodate the value. return len(value), nil diff --git a/runsc/boot/filter/config/config_main.go b/runsc/boot/filter/config/config_main.go index ba66f0c35..e7c611c23 100644 --- a/runsc/boot/filter/config/config_main.go +++ b/runsc/boot/filter/config/config_main.go @@ -408,5 +408,11 @@ func hostFilesystemFilters() seccomp.SyscallRules { seccomp.AnyValue{}, seccomp.AnyValue{}, }, + unix.SYS_FGETXATTR: seccomp.PerArg{ + seccomp.NonNegativeFD{}, + seccomp.AnyValue{}, + seccomp.AnyValue{}, + seccomp.AnyValue{}, + }, }) } diff --git a/runsc/fsgofer/filter/config.go b/runsc/fsgofer/filter/config.go index 48bf50f12..8eada32d7 100644 --- a/runsc/fsgofer/filter/config.go +++ b/runsc/fsgofer/filter/config.go @@ -233,5 +233,4 @@ var udsCreateSyscalls = seccomp.MakeSyscallRules(map[uintptr]seccomp.SyscallRule var xattrSyscalls = seccomp.MakeSyscallRules(map[uintptr]seccomp.SyscallRule{ unix.SYS_FGETXATTR: seccomp.MatchAll{}, - unix.SYS_FSETXATTR: seccomp.MatchAll{}, }) diff --git a/runsc/fsgofer/filter/filter.go b/runsc/fsgofer/filter/filter.go index 710aeb760..d5b856391 100644 --- a/runsc/fsgofer/filter/filter.go +++ b/runsc/fsgofer/filter/filter.go @@ -53,6 +53,9 @@ func Install(opt Options) error { // when not enabled. s.Merge(instrumentationFilters()) + // TODO(b/317993245): add HostFilesystem to Options. + s.Merge(xattrSyscalls) + return seccomp.Install(s, seccomp.DenyNewExecMappings, seccomp.DefaultProgramOptions()) } diff --git a/runsc/fsgofer/lisafs.go b/runsc/fsgofer/lisafs.go index 76aa106e2..9bb3a97e9 100644 --- a/runsc/fsgofer/lisafs.go +++ b/runsc/fsgofer/lisafs.go @@ -901,7 +901,9 @@ func (fd *controlFDLisa) Renamed() { // GetXattr implements lisafs.ControlFDImpl.GetXattr. func (fd *controlFDLisa) GetXattr(name string, size uint32, getValueBuf func(uint32) []byte) (uint16, error) { - return 0, unix.EOPNOTSUPP + data := getValueBuf(size) + xattrSize, err := unix.Fgetxattr(fd.hostFD, name, data) + return uint16(xattrSize), err } // SetXattr implements lisafs.ControlFDImpl.SetXattr.