From 12b32aa06c0475d60ee51b3753052ac2e4d09308 Mon Sep 17 00:00:00 2001 From: bolshevik Date: Tue, 17 Apr 2018 20:33:04 +0200 Subject: [PATCH] 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. --- internal/fusefrontend/xattr.go | 37 +++++++------------ internal/fusefrontend/xattr_linux.go | 15 ++++++++ .../fusefrontend/xattr_linux_unit_test.go | 13 +++++++ internal/fusefrontend/xattr_notlinux.go | 8 ++++ internal/fusefrontend/xattr_unit_test.go | 12 +----- 5 files changed, 51 insertions(+), 34 deletions(-) create mode 100644 internal/fusefrontend/xattr_linux.go create mode 100644 internal/fusefrontend/xattr_linux_unit_test.go create mode 100644 internal/fusefrontend/xattr_notlinux.go 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 } + if disallowedXAttrName(attr) { + return fuse.EPERM + } + cPath, err := fs.getBackingPath(path) if err != nil { return fuse.ToStatus(err) } - cAttr, err := fs.encryptXattrName(attr) - 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 } + if disallowedXAttrName(attr) { + return fuse.EPERM + } cPath, err := fs.getBackingPath(path) if err != nil { return fuse.ToStatus(err) } - cAttr, err := fs.encryptXattrName(attr) - 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) } }