fusefrontend: make GetXAttr() symlink-safe on Linux

Uses the /proc/self/fd trick, which does not work
on Darwin.

5 files changed, 86 insertions, 13 deletions

diff --git a/internal/fusefrontend/names.go b/internal/fusefrontend/names.go
index 6997177..5d6951c 100644
--- a/internal/fusefrontend/names.go
+++ b/internal/fusefrontend/names.go
@@ -35,6 +35,7 @@ func (fs *FS) isFiltered(path string) bool {
 // from the relative plaintext path "relPath"
 //
 // TODO: this function is NOT symlink-safe.
+// TODO: Move to xattr_darwin.go.
 func (fs *FS) getBackingPath(relPath string) (string, error) {
 	cPath, err := fs.encryptPath(relPath)
 	if err != nil {
diff --git a/internal/fusefrontend/xattr.go b/internal/fusefrontend/xattr.go
index 74b3790..caf1e15 100644
--- a/internal/fusefrontend/xattr.go
+++ b/internal/fusefrontend/xattr.go
@@ -25,24 +25,22 @@ var xattrStorePrefix = "user.gocryptfs."
 
 // GetXAttr - FUSE call. Reads the value of extended attribute "attr".
 //
-// TODO: Make symlink-safe. Blocker: package xattr does not provide fgetxattr(2).
-func (fs *FS) GetXAttr(path string, attr string, context *fuse.Context) ([]byte, fuse.Status) {
-	if fs.isFiltered(path) {
+// This function is symlink-safe on Linux.
+// Darwin does not have fgetxattr(2) nor /proc. How to implement this on Darwin
+// in a symlink-safe way?
+func (fs *FS) GetXAttr(relPath string, attr string, context *fuse.Context) ([]byte, fuse.Status) {
+	if fs.isFiltered(relPath) {
 		return nil, fuse.EPERM
 	}
 	if disallowedXAttrName(attr) {
 		return nil, _EOPNOTSUPP
 	}
 	cAttr := fs.encryptXattrName(attr)
-	cPath, err := fs.getBackingPath(path)
-	if err != nil {
-		return nil, fuse.ToStatus(err)
-	}
-	encryptedData, err := xattr.LGet(cPath, cAttr)
-	if err != nil {
-		return nil, unpackXattrErr(err)
+	cData, status := fs.getXattr(relPath, cAttr, context)
+	if !status.Ok() {
+		return nil, status
 	}
-	data, err := fs.decryptXattrValue(encryptedData)
+	data, err := fs.decryptXattrValue(cData)
 	if err != nil {
 		tlog.Warn.Printf("GetXAttr: %v", err)
 		return nil, fuse.EIO
diff --git a/internal/fusefrontend/xattr_darwin.go b/internal/fusefrontend/xattr_darwin.go
index b626006..cf48d13 100644
--- a/internal/fusefrontend/xattr_darwin.go
+++ b/internal/fusefrontend/xattr_darwin.go
@@ -3,7 +3,11 @@
 // Package fusefrontend interfaces directly with the go-fuse library.
 package fusefrontend
 
-import "github.com/pkg/xattr"
+import (
+	"github.com/pkg/xattr"
+
+	"github.com/hanwen/go-fuse/fuse"
+)
 
 func disallowedXAttrName(attr string) bool {
 	return false
@@ -13,3 +17,16 @@ func disallowedXAttrName(attr string) bool {
 func filterXattrSetFlags(flags int) int {
 	return flags &^ xattr.XATTR_NOSECURITY
 }
+
+// This function is NOT symlink-safe because Darwin lacks fgetxattr().
+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
+}
diff --git a/internal/fusefrontend/xattr_linux.go b/internal/fusefrontend/xattr_linux.go
index 61b90e3..5a189db 100644
--- a/internal/fusefrontend/xattr_linux.go
+++ b/internal/fusefrontend/xattr_linux.go
@@ -3,7 +3,17 @@
 // Package fusefrontend interfaces directly with the go-fuse library.
 package fusefrontend
 
-import "strings"
+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
 // these may be interpreted by the system, and we don't want to cause
@@ -17,3 +27,44 @@ func disallowedXAttrName(attr string) bool {
 func filterXattrSetFlags(flags int) int {
 	return flags
 }
+
+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.
+//
+// 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, 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 the file at relative
+// plaintext path "relPath". Returns the encrypted xattr value.
+//
+// This function is symlink-safe.
+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
+}
diff --git a/tests/xattr/xattr_fd_test.go b/tests/xattr/xattr_fd_test.go
index 7d37a57..ad0b7ed 100644
--- a/tests/xattr/xattr_fd_test.go
+++ b/tests/xattr/xattr_fd_test.go
@@ -34,6 +34,9 @@ func TestFdXattr(t *testing.T) {
 	val1 := []byte("123456789")
 	unix.Fsetxattr(fd, attr, val1, 0)
 	sz, err = unix.Flistxattr(fd, buf)
+	if err != nil {
+		t.Fatal(err)
+	}
 	// Length of "user.attr" + terminating null byte
 	expectedSz := len(attr) + 1
 	if sz != expectedSz {
@@ -45,6 +48,9 @@ func TestFdXattr(t *testing.T) {
 	}
 	// Check content
 	sz, err = unix.Fgetxattr(fd, attr, buf)
+	if err != nil {
+		t.Fatal(err)
+	}
 	str = string(buf[:sz])
 	if str != string(val1) {
 		t.Errorf("expected val %q, got %q", val1, str)