fusefrontend: use Fsetxattr/Fgetxattr/etc on all platforms

Darwin now also has these functions, use them. Simplifies
the code and makes it symlink-safe on Darwin as well.

3 files changed, 75 insertions, 209 deletions

diff --git a/internal/fusefrontend/xattr.go b/internal/fusefrontend/xattr.go
index 816754c..045f963 100644
--- a/internal/fusefrontend/xattr.go
+++ b/internal/fusefrontend/xattr.go
@@ -1,15 +1,15 @@
 // Package fusefrontend interfaces directly with the go-fuse library.
 package fusefrontend
 
-// FUSE operations on paths
-
 import (
 	"strings"
 	"syscall"
 
+	"golang.org/x/sys/unix"
+
 	"github.com/hanwen/go-fuse/fuse"
-	"github.com/pkg/xattr"
 
+	"github.com/rfjakob/gocryptfs/internal/syscallcompat"
 	"github.com/rfjakob/gocryptfs/internal/tlog"
 )
 
@@ -25,9 +25,7 @@ var xattrStorePrefix = "user.gocryptfs."
 
 // GetXAttr - FUSE call. Reads the value of extended attribute "attr".
 //
-// This function is symlink-safe on Linux.
-// Darwin does not have fgetxattr(2) nor /proc/self/fd. How to implement this
-// on Darwin in a symlink-safe way?
+// This function is symlink-safe through Fgetxattr.
 func (fs *FS) GetXAttr(relPath string, attr string, context *fuse.Context) ([]byte, fuse.Status) {
 	if fs.isFiltered(relPath) {
 		return nil, fuse.EPERM
@@ -35,11 +33,20 @@ func (fs *FS) GetXAttr(relPath string, attr string, context *fuse.Context) ([]by
 	if disallowedXAttrName(attr) {
 		return nil, _EOPNOTSUPP
 	}
-	cAttr := fs.encryptXattrName(attr)
-	cData, status := fs.getXattr(relPath, cAttr, context)
+
+	file, fd, status := fs.getFileFd(relPath, context)
 	if !status.Ok() {
 		return nil, status
 	}
+	defer file.Release()
+
+	cAttr := fs.encryptXattrName(attr)
+
+	cData, err := syscallcompat.Fgetxattr(fd, cAttr)
+	if err != nil {
+		return nil, fuse.ToStatus(err)
+	}
+
 	data, err := fs.decryptXattrValue(cData)
 	if err != nil {
 		tlog.Warn.Printf("GetXAttr: %v", err)
@@ -50,9 +57,7 @@ func (fs *FS) GetXAttr(relPath string, attr string, context *fuse.Context) ([]by
 
 // SetXAttr - FUSE call. Set extended attribute.
 //
-// This function is symlink-safe on Linux.
-// Darwin does not have fsetxattr(2) nor /proc/self/fd. How to implement this
-// on Darwin in a symlink-safe way?
+// This function is symlink-safe through Fsetxattr.
 func (fs *FS) SetXAttr(relPath string, attr string, data []byte, flags int, context *fuse.Context) fuse.Status {
 	if fs.isFiltered(relPath) {
 		return fuse.EPERM
@@ -60,17 +65,27 @@ func (fs *FS) SetXAttr(relPath string, attr string, data []byte, flags int, cont
 	if disallowedXAttrName(attr) {
 		return _EOPNOTSUPP
 	}
+
+	file, fd, status := fs.getFileFd(relPath, context)
+	if !status.Ok() {
+		return status
+	}
+	defer file.Release()
+
 	flags = filterXattrSetFlags(flags)
 	cAttr := fs.encryptXattrName(attr)
 	cData := fs.encryptXattrValue(data)
-	return fs.setXattr(relPath, cAttr, cData, flags, context)
+
+	err := unix.Fsetxattr(fd, cAttr, cData, flags)
+	if err != nil {
+		return fuse.ToStatus(err)
+	}
+	return fuse.OK
 }
 
 // RemoveXAttr - FUSE call.
 //
-// This function is symlink-safe on Linux.
-// Darwin does not have fremovexattr(2) nor /proc/self/fd. How to implement this
-// on Darwin in a symlink-safe way?
+// This function is symlink-safe through Fremovexattr.
 func (fs *FS) RemoveXAttr(relPath string, attr string, context *fuse.Context) fuse.Status {
 	if fs.isFiltered(relPath) {
 		return fuse.EPERM
@@ -78,23 +93,45 @@ func (fs *FS) RemoveXAttr(relPath string, attr string, context *fuse.Context) fu
 	if disallowedXAttrName(attr) {
 		return _EOPNOTSUPP
 	}
+
+	file, fd, status := fs.getFileFd(relPath, context)
+	if !status.Ok() {
+		return status
+	}
+	defer file.Release()
+
 	cAttr := fs.encryptXattrName(attr)
-	return fs.removeXAttr(relPath, cAttr, context)
+	err := unix.Fremovexattr(fd, cAttr)
+	if err != nil {
+		return fuse.ToStatus(err)
+	}
+	return fuse.OK
 }
 
 // ListXAttr - FUSE call. Lists extended attributes on the file at "relPath".
 //
-// This function is symlink-safe on Linux.
-// Darwin does not have flistxattr(2) nor /proc/self/fd. How to implement this
-// on Darwin in a symlink-safe way?
+// This function is symlink-safe through Flistxattr.
 func (fs *FS) ListXAttr(relPath string, context *fuse.Context) ([]string, fuse.Status) {
 	if fs.isFiltered(relPath) {
 		return nil, fuse.EPERM
 	}
-	cNames, status := fs.listXAttr(relPath, context)
+
+	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
+	}
 	if !status.Ok() {
 		return nil, status
 	}
+	defer file.Release()
+
+	cNames, err := syscallcompat.Flistxattr(fd)
+	if err != nil {
+		return nil, fuse.ToStatus(err)
+	}
+
 	names := make([]string, 0, len(cNames))
 	for _, curName := range cNames {
 		if !strings.HasPrefix(curName, xattrStorePrefix) {
@@ -163,16 +200,22 @@ func (fs *FS) decryptXattrValue(cData []byte) (data []byte, err error) {
 	return fs.contentEnc.DecryptBlock([]byte(cData), 0, nil)
 }
 
-// unpackXattrErr unpacks an error value that we got from xattr.LGet/LSet/etc
-// and converts it to a fuse status. If err == nil, it returns fuse.OK.
-func unpackXattrErr(err error) fuse.Status {
-	if err == nil {
-		return fuse.OK
+// 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
 	}
-	err2, ok := err.(*xattr.Error)
+	file, ok := fuseFile.(*File)
 	if !ok {
-		tlog.Warn.Printf("unpackXattrErr: cannot unpack err=%v", err)
-		return fuse.EIO
+		tlog.Warn.Printf("BUG: xattrGet: cast to *File failed")
+		fuseFile.Release()
+		return nil, -1, fuse.EIO
 	}
-	return fuse.ToStatus(err2.Err)
+	return file, file.intFd(), fuse.OK
 }
diff --git a/internal/fusefrontend/xattr_darwin.go b/internal/fusefrontend/xattr_darwin.go
index c756486..8760480 100644
--- a/internal/fusefrontend/xattr_darwin.go
+++ b/internal/fusefrontend/xattr_darwin.go
@@ -3,86 +3,14 @@
 // Package fusefrontend interfaces directly with the go-fuse library.
 package fusefrontend
 
-import (
-	"path/filepath"
-
-	"github.com/pkg/xattr"
-
-	"github.com/hanwen/go-fuse/fuse"
-
-	"github.com/rfjakob/gocryptfs/internal/tlog"
-)
-
 func disallowedXAttrName(attr string) bool {
 	return false
 }
 
 // On Darwin it is needed to unset XATTR_NOSECURITY 0x0008
 func filterXattrSetFlags(flags int) int {
-	return flags &^ xattr.XATTR_NOSECURITY
-}
-
-// This function is NOT symlink-safe because Darwin lacks
-// both fgetxattr() and /proc/self/fd.
-func (fs *FS) getXattr(relPath string, cAttr string, context *fuse.Context) ([]byte, fuse.Status) {
-	cPath, err := fs.getBackingPath(relPath)
-	if err != nil {
-		return nil, fuse.ToStatus(err)
-	}
-	cData, err := xattr.LGet(cPath, cAttr)
-	if err != nil {
-		return nil, unpackXattrErr(err)
-	}
-	return cData, fuse.OK
-}
-
-// This function is NOT symlink-safe because Darwin lacks
-// both fsetxattr() and /proc/self/fd.
-func (fs *FS) setXattr(relPath string, cAttr string, cData []byte, flags int, context *fuse.Context) fuse.Status {
-	cPath, err := fs.getBackingPath(relPath)
-	if err != nil {
-		return fuse.ToStatus(err)
-	}
-	err = xattr.LSetWithFlags(cPath, cAttr, cData, flags)
-	return unpackXattrErr(err)
-}
-
-// This function is NOT symlink-safe because Darwin lacks
-// both fremovexattr() and /proc/self/fd.
-func (fs *FS) removeXAttr(relPath string, cAttr string, context *fuse.Context) fuse.Status {
-	cPath, err := fs.getBackingPath(relPath)
-	if err != nil {
-		return fuse.ToStatus(err)
-	}
-	err = xattr.LRemove(cPath, cAttr)
-	return unpackXattrErr(err)
-}
-
-// This function is NOT symlink-safe because Darwin lacks
-// both flistxattr() and /proc/self/fd.
-func (fs *FS) listXAttr(relPath string, context *fuse.Context) ([]string, fuse.Status) {
-	cPath, err := fs.getBackingPath(relPath)
-	if err != nil {
-		return nil, fuse.ToStatus(err)
-	}
-	cNames, err := xattr.LList(cPath)
-	if err != nil {
-		return nil, unpackXattrErr(err)
-	}
-	return cNames, fuse.OK
-}
+	// See https://opensource.apple.com/source/xnu/xnu-1504.15.3/bsd/sys/xattr.h.auto.html
+	const XATTR_NOSECURITY = 0x0008
 
-// getBackingPath - get the absolute encrypted path of the backing file
-// from the relative plaintext path "relPath"
-//
-// This function is NOT symlink-safe. Darwin needs it because it lacks
-// fgetxattr(2) and friends.
-func (fs *FS) getBackingPath(relPath string) (string, error) {
-	cPath, err := fs.encryptPath(relPath)
-	if err != nil {
-		return "", err
-	}
-	cAbsPath := filepath.Join(fs.args.Cipherdir, cPath)
-	tlog.Debug.Printf("getBackingPath: %s + %s -> %s", fs.args.Cipherdir, relPath, cAbsPath)
-	return cAbsPath, nil
+	return flags &^ XATTR_NOSECURITY
 }
diff --git a/internal/fusefrontend/xattr_linux.go b/internal/fusefrontend/xattr_linux.go
index a96a147..fd3b7e5 100644
--- a/internal/fusefrontend/xattr_linux.go
+++ b/internal/fusefrontend/xattr_linux.go
@@ -4,15 +4,7 @@
 package fusefrontend
 
 import (
-	"fmt"
 	"strings"
-	"syscall"
-
-	"github.com/hanwen/go-fuse/fuse"
-
-	"github.com/pkg/xattr"
-
-	"github.com/rfjakob/gocryptfs/internal/tlog"
 )
 
 // Only allow the "user" namespace, block "trusted" and "security", as
@@ -27,100 +19,3 @@ func disallowedXAttrName(attr string) bool {
 func filterXattrSetFlags(flags int) int {
 	return flags
 }
-
-// procFd returns the path to file descriptor "fd" in /proc/self/fd.
-func procFd(fd int) string {
-	return fmt.Sprintf("/proc/self/fd/%d", fd)
-}
-
-// 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
-}
-
-// getXattr - read encrypted xattr name "cAttr" from relative
-// plaintext path "relPath". Returns the encrypted xattr value.
-//
-// This function is symlink-safe by using /proc/self/fd.
-func (fs *FS) getXattr(relPath string, cAttr string, context *fuse.Context) ([]byte, fuse.Status) {
-	file, fd, status := fs.getFileFd(relPath, context)
-	if !status.Ok() {
-		return nil, status
-	}
-	defer file.Release()
-
-	cData, err := xattr.Get(procFd(fd), cAttr)
-	if err != nil {
-		return nil, unpackXattrErr(err)
-	}
-	return cData, fuse.OK
-}
-
-// setXattr - set encrypted xattr name "cAttr" to value "cData" on plaintext
-// path "relPath".
-//
-// This function is symlink-safe by using /proc/self/fd.
-func (fs *FS) setXattr(relPath string, cAttr string, cData []byte, flags int, context *fuse.Context) fuse.Status {
-	file, fd, status := fs.getFileFd(relPath, context)
-	if !status.Ok() {
-		return status
-	}
-	defer file.Release()
-
-	err := xattr.SetWithFlags(procFd(fd), cAttr, cData, flags)
-	return unpackXattrErr(err)
-}
-
-// removeXAttr - remove encrypted xattr name "cAttr" from
-// plaintext path "relPath".
-//
-// This function is symlink-safe on Linux by using /proc/self/fd.
-func (fs *FS) removeXAttr(relPath string, cAttr string, context *fuse.Context) fuse.Status {
-	file, fd, status := fs.getFileFd(relPath, context)
-	if !status.Ok() {
-		return status
-	}
-	defer file.Release()
-
-	err := xattr.Remove(procFd(fd), cAttr)
-	return unpackXattrErr(err)
-}
-
-// listXAttr - list encrypted xattr names on plaintext path "relPath".
-//
-// This function is symlink-safe on Linux by using /proc/self/fd.
-func (fs *FS) listXAttr(relPath string, context *fuse.Context) ([]string, fuse.Status) {
-	file, fd, status := fs.getFileFd(relPath, context)
-	if !status.Ok() {
-		// If relPath is a symlink, getFileFd fails with ELOOP. As setXattr()
-		// also fails with ELOOP, there is no way to set xattrs on symlinks,
-		// and we can assume that the file does not have any.
-		if status == fuse.Status(syscall.ELOOP) {
-			return nil, fuse.OK
-		}
-		return nil, status
-	}
-	defer file.Release()
-
-	cNames, err := xattr.List(procFd(fd))
-	if err != nil {
-		status := unpackXattrErr(err)
-		return nil, status
-	}
-	return cNames, fuse.OK
-}