From b214be5e3f76dd17efc9832131f4a7e0414b4cea Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Wed, 2 Jan 2019 20:48:46 +0100 Subject: [PATCH] fusefrontend: xattr: fix operations on files without read permissions * listxattr is fixed via the /proc/self/fd trick * setxattr,removexattr are fixed by opening the file O_WRONLY Fixes https://github.com/rfjakob/gocryptfs/issues/308 --- internal/fusefrontend/fs.go | 11 ++++ internal/fusefrontend/xattr.go | 88 +++++++++++++--------------- internal/syscallcompat/sys_common.go | 34 +++++++++-- 3 files changed, 82 insertions(+), 51 deletions(-) diff --git a/internal/fusefrontend/fs.go b/internal/fusefrontend/fs.go index e7c3903..5c52a19 100644 --- a/internal/fusefrontend/fs.go +++ b/internal/fusefrontend/fs.go @@ -160,6 +160,17 @@ func (fs *FS) Open(path string, flags uint32, context *fuse.Context) (fuseFile n return NewFile(f, fs) } +// openBackingFile opens the ciphertext file that backs relative plaintext +// path "relPath". Always adds O_NOFOLLOW to the flags. +func (fs *FS) openBackingFile(relPath string, flags int) (fd int, err error) { + dirfd, cName, err := fs.openBackingDir(relPath) + if err != nil { + return -1, err + } + defer syscall.Close(dirfd) + return syscallcompat.Openat(dirfd, cName, flags|syscall.O_NOFOLLOW, 0) +} + // Due to RMW, we always need read permissions on the backing file. This is a // problem if the file permissions do not allow reading (i.e. 0200 permissions). // This function works around that problem by chmod'ing the file, obtaining a fd, diff --git a/internal/fusefrontend/xattr.go b/internal/fusefrontend/xattr.go index 045f963..7ffec25 100644 --- a/internal/fusefrontend/xattr.go +++ b/internal/fusefrontend/xattr.go @@ -2,6 +2,8 @@ package fusefrontend import ( + "fmt" + "runtime" "strings" "syscall" @@ -34,11 +36,12 @@ func (fs *FS) GetXAttr(relPath string, attr string, context *fuse.Context) ([]by return nil, _EOPNOTSUPP } - file, fd, status := fs.getFileFd(relPath, context) - if !status.Ok() { - return nil, status + // O_NONBLOCK to not block on FIFOs. + fd, err := fs.openBackingFile(relPath, syscall.O_RDONLY|syscall.O_NONBLOCK) + if err != nil { + return nil, fuse.ToStatus(err) } - defer file.Release() + defer syscall.Close(fd) cAttr := fs.encryptXattrName(attr) @@ -66,17 +69,18 @@ func (fs *FS) SetXAttr(relPath string, attr string, data []byte, flags int, cont return _EOPNOTSUPP } - file, fd, status := fs.getFileFd(relPath, context) - if !status.Ok() { - return status + // O_NONBLOCK to not block on FIFOs. + fd, err := fs.openBackingFile(relPath, syscall.O_WRONLY|syscall.O_NONBLOCK) + if err != nil { + return fuse.ToStatus(err) } - defer file.Release() + defer syscall.Close(fd) flags = filterXattrSetFlags(flags) cAttr := fs.encryptXattrName(attr) cData := fs.encryptXattrValue(data) - err := unix.Fsetxattr(fd, cAttr, cData, flags) + err = unix.Fsetxattr(fd, cAttr, cData, flags) if err != nil { return fuse.ToStatus(err) } @@ -94,14 +98,15 @@ func (fs *FS) RemoveXAttr(relPath string, attr string, context *fuse.Context) fu return _EOPNOTSUPP } - file, fd, status := fs.getFileFd(relPath, context) - if !status.Ok() { - return status + // O_NONBLOCK to not block on FIFOs. + fd, err := fs.openBackingFile(relPath, syscall.O_WRONLY|syscall.O_NONBLOCK) + if err != nil { + return fuse.ToStatus(err) } - defer file.Release() + defer syscall.Close(fd) cAttr := fs.encryptXattrName(attr) - err := unix.Fremovexattr(fd, cAttr) + err = unix.Fremovexattr(fd, cAttr) if err != nil { return fuse.ToStatus(err) } @@ -115,19 +120,30 @@ func (fs *FS) ListXAttr(relPath string, context *fuse.Context) ([]string, fuse.S if fs.isFiltered(relPath) { return nil, fuse.EPERM } - - file, fd, status := fs.getFileFd(relPath, context) - // On a symlink, getFileFd fails with ELOOP. Let's pretend there - // can be no xattrs on symlinks, and always return an empty result. - if status == fuse.Status(syscall.ELOOP) { - return nil, fuse.OK + var cNames []string + var err error + if runtime.GOOS == "linux" { + dirfd, cName, err2 := fs.openBackingDir(relPath) + if err2 != nil { + return nil, fuse.ToStatus(err2) + } + defer syscall.Close(dirfd) + procPath := fmt.Sprintf("/proc/self/fd/%d/%s", dirfd, cName) + cNames, err = syscallcompat.Llistxattr(procPath) + } else { + // O_NONBLOCK to not block on FIFOs. + fd, err2 := fs.openBackingFile(relPath, syscall.O_WRONLY|syscall.O_NONBLOCK) + // On a symlink, openBackingFile fails with ELOOP. Let's pretend there + // can be no xattrs on symlinks, and always return an empty result. + if err2 == syscall.ELOOP { + return nil, fuse.OK + } + if err2 != nil { + return nil, fuse.ToStatus(err2) + } + defer syscall.Close(fd) + cNames, err = syscallcompat.Flistxattr(fd) } - if !status.Ok() { - return nil, status - } - defer file.Release() - - cNames, err := syscallcompat.Flistxattr(fd) if err != nil { return nil, fuse.ToStatus(err) } @@ -199,23 +215,3 @@ func (fs *FS) decryptXattrValue(cData []byte) (data []byte, err error) { } return fs.contentEnc.DecryptBlock([]byte(cData), 0, nil) } - -// getFileFd calls fs.Open() on relative plaintext path "relPath" and returns -// the resulting fusefrontend.*File along with the underlying fd. The caller -// MUST call file.Release() when done with the file. The O_NONBLOCK flag is -// used to not block on FIFOs. -// -// Used by xattrGet() and friends. -func (fs *FS) getFileFd(relPath string, context *fuse.Context) (*File, int, fuse.Status) { - fuseFile, status := fs.Open(relPath, syscall.O_RDONLY|syscall.O_NONBLOCK, context) - if !status.Ok() { - return nil, -1, status - } - file, ok := fuseFile.(*File) - if !ok { - tlog.Warn.Printf("BUG: xattrGet: cast to *File failed") - fuseFile.Release() - return nil, -1, fuse.EIO - } - return file, file.intFd(), fuse.OK -} diff --git a/internal/syscallcompat/sys_common.go b/internal/syscallcompat/sys_common.go index 3a0d5a1..4ce0208 100644 --- a/internal/syscallcompat/sys_common.go +++ b/internal/syscallcompat/sys_common.go @@ -64,7 +64,7 @@ func Fgetxattr(fd int, attr string) (val []byte, err error) { // only happen on MacOS). // // See https://github.com/pkg/xattr for a smarter solution. - // TODO: be smarter? + // TODO: smarter buffer sizing? buf := make([]byte, XATTR_BUFSZ) sz, err := unix.Fgetxattr(fd, attr, buf) if err == syscall.ERANGE { @@ -84,11 +84,11 @@ func Fgetxattr(fd int, attr string) (val []byte, err error) { return val, nil } -// Flistxattr is a wrapper unix.Flistxattr that handles buffer sizing and +// Flistxattr is a wrapper for unix.Flistxattr that handles buffer sizing and // parsing the returned blob to a string slice. func Flistxattr(fd int) (attrs []string, err error) { // See the buffer sizing comments in Fgetxattr. - // TODO: be smarter? + // TODO: smarter buffer sizing? buf := make([]byte, XATTR_BUFSZ) sz, err := unix.Flistxattr(fd, buf) if err == syscall.ERANGE { @@ -101,7 +101,31 @@ func Flistxattr(fd int) (attrs []string, err error) { if sz >= XATTR_SIZE_MAX { return nil, syscall.EOVERFLOW } - buf = buf[:sz] + attrs = parseListxattrBlob(buf[:sz]) + return attrs, nil +} + +// Llistxattr is a wrapper for unix.Llistxattr that handles buffer sizing and +// parsing the returned blob to a string slice. +func Llistxattr(path string) (attrs []string, err error) { + // TODO: smarter buffer sizing? + buf := make([]byte, XATTR_BUFSZ) + sz, err := unix.Llistxattr(path, buf) + if err == syscall.ERANGE { + // Do NOT return ERANGE - the user might retry ad inifinitum! + return nil, syscall.EOVERFLOW + } + if err != nil { + return nil, err + } + if sz >= XATTR_SIZE_MAX { + return nil, syscall.EOVERFLOW + } + attrs = parseListxattrBlob(buf[:sz]) + return attrs, nil +} + +func parseListxattrBlob(buf []byte) (attrs []string) { parts := bytes.Split(buf, []byte{0}) for _, part := range parts { if len(part) == 0 { @@ -110,5 +134,5 @@ func Flistxattr(fd int) (attrs []string, err error) { } attrs = append(attrs, string(part)) } - return attrs, nil + return attrs }