Improved xattr handling on non-linux systems (#227)

* Fixed xattr filtering for MacOS. "system." and "user." prefixes are only relevant for Linux.
* Small cleanup and additional tests.

5 files changed, 51 insertions, 34 deletions

diff --git a/internal/fusefrontend/xattr.go b/internal/fusefrontend/xattr.go
index faaebd4..f3e7532 100644
--- a/internal/fusefrontend/xattr.go
+++ b/internal/fusefrontend/xattr.go
@@ -16,11 +16,6 @@ import (
 // xattr names are encrypted like file names, but with a fixed IV.
 var xattrNameIV = []byte("xattr_name_iv_xx")
 
-// Only allow the "user" namespace, block "trusted" and "security", as
-// these may be interpreted by the system, and we don't want to cause
-// trouble with our encrypted garbage.
-var xattrUserPrefix = "user."
-
 // We store encrypted xattrs under this prefix plus the base64-encoded
 // encrypted original name.
 var xattrStorePrefix = "user.gocryptfs."
@@ -31,15 +26,12 @@ func (fs *FS) GetXAttr(path string, attr string, context *fuse.Context) ([]byte,
 	if fs.isFiltered(path) {
 		return nil, fuse.EPERM
 	}
-	if !strings.HasPrefix(attr, xattrUserPrefix) {
+	if disallowedXAttrName(attr) {
 		// "ls -l" queries security.selinux, system.posix_acl_access, system.posix_acl_default
 		// and throws error messages if it gets something else than ENODATA.
 		return nil, fuse.ENODATA
 	}
-	cAttr, err := fs.encryptXattrName(attr)
-	if err != nil {
-		return nil, fuse.ToStatus(err)
-	}
+	cAttr := fs.encryptXattrName(attr)
 	cPath, err := fs.getBackingPath(path)
 	if err != nil {
 		return nil, fuse.ToStatus(err)
@@ -65,14 +57,15 @@ func (fs *FS) SetXAttr(path string, attr string, data []byte, flags int, context
 	if flags != 0 {
 		return fuse.EPERM
 	}
-	cPath, err := fs.getBackingPath(path)
-	if err != nil {
-		return fuse.ToStatus(err)
+	if disallowedXAttrName(attr) {
+		return fuse.EPERM
 	}
-	cAttr, err := fs.encryptXattrName(attr)
+
+	cPath, err := fs.getBackingPath(path)
 	if err != nil {
 		return fuse.ToStatus(err)
 	}
+	cAttr := fs.encryptXattrName(attr)
 	// xattr data is encrypted like a symlink target
 	cData64 := []byte(fs.encryptSymlinkTarget(string(data)))
 	return unpackXattrErr(xattr.Set(cPath, cAttr, cData64))
@@ -83,14 +76,14 @@ func (fs *FS) RemoveXAttr(path string, attr string, context *fuse.Context) fuse.
 	if fs.isFiltered(path) {
 		return fuse.EPERM
 	}
-	cPath, err := fs.getBackingPath(path)
-	if err != nil {
-		return fuse.ToStatus(err)
+	if disallowedXAttrName(attr) {
+		return fuse.EPERM
 	}
-	cAttr, err := fs.encryptXattrName(attr)
+	cPath, err := fs.getBackingPath(path)
 	if err != nil {
 		return fuse.ToStatus(err)
 	}
+	cAttr := fs.encryptXattrName(attr)
 	return unpackXattrErr(xattr.Remove(cPath, cAttr))
 }
 
@@ -124,14 +117,10 @@ func (fs *FS) ListXAttr(path string, context *fuse.Context) ([]string, fuse.Stat
 }
 
 // encryptXattrName transforms "user.foo" to "user.gocryptfs.a5sAd4XAa47f5as6dAf"
-func (fs *FS) encryptXattrName(attr string) (cAttr string, err error) {
-	// Reject anything that does not start with "user."
-	if !strings.HasPrefix(attr, xattrUserPrefix) {
-		return "", syscall.EPERM
-	}
+func (fs *FS) encryptXattrName(attr string) (cAttr string) {
 	// xattr names are encrypted like file names, but with a fixed IV.
 	cAttr = xattrStorePrefix + fs.nameTransform.EncryptName(attr, xattrNameIV)
-	return cAttr, nil
+	return cAttr
 }
 
 func (fs *FS) decryptXattrName(cAttr string) (attr string, err error) {
diff --git a/internal/fusefrontend/xattr_linux.go b/internal/fusefrontend/xattr_linux.go
new file mode 100644
index 0000000..ebe42b1
--- /dev/null
+++ b/internal/fusefrontend/xattr_linux.go
@@ -0,0 +1,15 @@
+// +build linux
+
+// Package fusefrontend interfaces directly with the go-fuse library.
+package fusefrontend
+
+import "strings"
+
+// Only allow the "user" namespace, block "trusted" and "security", as
+// these may be interpreted by the system, and we don't want to cause
+// trouble with our encrypted garbage.
+const xattrUserPrefix = "user."
+
+func disallowedXAttrName(attr string) bool {
+	return !strings.HasPrefix(attr, xattrUserPrefix)
+}
diff --git a/internal/fusefrontend/xattr_linux_unit_test.go b/internal/fusefrontend/xattr_linux_unit_test.go
new file mode 100644
index 0000000..5fea58b
--- /dev/null
+++ b/internal/fusefrontend/xattr_linux_unit_test.go
@@ -0,0 +1,13 @@
+// +build linux
+
+package fusefrontend
+
+import (
+	"testing"
+)
+
+func TestDisallowedLinuxAttributes(t *testing.T) {
+	if !disallowedXAttrName("xxxx") {
+		t.Fatalf("Names that don't start with 'user.' should fail")
+	}
+}
diff --git a/internal/fusefrontend/xattr_notlinux.go b/internal/fusefrontend/xattr_notlinux.go
new file mode 100644
index 0000000..aa9c981
--- /dev/null
+++ b/internal/fusefrontend/xattr_notlinux.go
@@ -0,0 +1,8 @@
+// +build !linux
+
+// Package fusefrontend interfaces directly with the go-fuse library.
+package fusefrontend
+
+func disallowedXAttrName(attr string) bool {
+	return false
+}
diff --git a/internal/fusefrontend/xattr_unit_test.go b/internal/fusefrontend/xattr_unit_test.go
index ea5d3bb..c5c9360 100644
--- a/internal/fusefrontend/xattr_unit_test.go
+++ b/internal/fusefrontend/xattr_unit_test.go
@@ -4,7 +4,6 @@ package fusefrontend
 // "xattr_integration_test.go" in the test/xattr package.
 
 import (
-	"syscall"
 	"testing"
 
 	"github.com/rfjakob/gocryptfs/internal/contentenc"
@@ -24,18 +23,11 @@ func newTestFS() *FS {
 
 func TestEncryptDecryptXattrName(t *testing.T) {
 	fs := newTestFS()
-	_, err := fs.encryptXattrName("xxxx")
-	if err != syscall.EPERM {
-		t.Fatalf("Names that don't start with 'user.' should fail")
-	}
 	attr1 := "user.foo123456789"
-	cAttr, err := fs.encryptXattrName(attr1)
-	if err != nil {
-		t.Fatal(err)
-	}
+	cAttr := fs.encryptXattrName(attr1)
 	t.Logf("cAttr=%v", cAttr)
 	attr2, err := fs.decryptXattrName(cAttr)
-	if attr1 != attr2 {
+	if attr1 != attr2 || err != nil {
 		t.Fatalf("Decrypt mismatch: %v != %v", attr1, attr2)
 	}
 }