aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakob Unterwurzacher2021-02-07 20:01:16 +0100
committerJakob Unterwurzacher2021-02-07 20:01:16 +0100
commiteaca820e876bfcdc67323eac6dd43ecc420968f2 (patch)
tree7d5fe2fef822778d6a78010fa2fa6f5c021f54d5
parentbb2484f1520102f4ab6c84400f9eef53d4cdd2ad (diff)
fusefrontend: do not encrypt ACLs
Pass through system.posix_acl_access and system.posix_acl_default unencrypted to fix "cp -a" problems. "cp -a" uses "setxattr" even to set normal permissions, see https://www.spinics.net/lists/linux-nfs/msg63986.html . Fixes https://github.com/rfjakob/gocryptfs/issues/543
-rw-r--r--README.md5
-rw-r--r--internal/fusefrontend/node_xattr.go74
-rw-r--r--tests/defaults/acl_test.go202
3 files changed, 271 insertions, 10 deletions
diff --git a/README.md b/README.md
index 7846ad3..b44071e 100644
--- a/README.md
+++ b/README.md
@@ -196,6 +196,9 @@ vNEXT, in progress
* Make `gocryptfs.diriv` and `gocryptfs.xxx.name` files world-readable to make encrypted backups easier
when mounting via [/etc/fstab](Documentation/MANPAGE.md#fstab) ([#539](https://github.com/rfjakob/gocryptfs/issues/539))
* Make it work with MacFUSE v4.x ([#524](https://github.com/rfjakob/gocryptfs/issues/524))
+* **Disable ACL encryption**, it causes a lot of problems ([#543](https://github.com/rfjakob/gocryptfs/issues/543), [#536](https://github.com/rfjakob/gocryptfs/issues/536))
+ * Old encrypted ACLs are reported by `gocryptfs -fsck` but otherwise ignored
+ * This fixes inheritance, but does not yet enforce them correctly
v2.0-beta2, 2020-11-14
* Improve [performance](Documentation/performance.txt#L69)
@@ -222,7 +225,7 @@ v2.0-beta1, 2020-10-15
v1.8.0, 2020-05-09
* Enable ACL support ([#453](https://github.com/rfjakob/gocryptfs/issues/453))
- * **Warning 2021-02-07: This feature is incomplete!**
+ * **Warning 2021-02-07**: This feature is incomplete! Do not use ACLs before gocryptfs v2.0 final!
Reading and writing ACLs works, but they are not enforced or inherited ([#542](https://github.com/rfjakob/gocryptfs/issues/542))
* Ignore `.nfsXXX` temporary files
([#367](https://github.com/rfjakob/gocryptfs/issues/431))
diff --git a/internal/fusefrontend/node_xattr.go b/internal/fusefrontend/node_xattr.go
index cbc5804..3855b55 100644
--- a/internal/fusefrontend/node_xattr.go
+++ b/internal/fusefrontend/node_xattr.go
@@ -10,6 +10,9 @@ import (
"github.com/rfjakob/gocryptfs/internal/tlog"
)
+// -1 as uint32
+const minus1 = ^uint32(0)
+
// xattr names are encrypted like file names, but with a fixed IV.
// Padded with "_xx" for length 16.
var xattrNameIV = []byte("xattr_name_iv_xx")
@@ -22,6 +25,13 @@ var xattrStorePrefix = "user.gocryptfs."
// see https://github.com/rfjakob/gocryptfs/issues/515 for details.
var xattrCapability = "security.capability"
+// isAcl returns true if the attribute name is for storing ACLs
+//
+// ACLs are passed through without encryption
+func isAcl(attr string) bool {
+ return attr == "system.posix_acl_access" || attr == "system.posix_acl_default"
+}
+
// GetXAttr - FUSE call. Reads the value of extended attribute "attr".
//
// This function is symlink-safe through Fgetxattr.
@@ -36,15 +46,34 @@ func (n *Node) Getxattr(ctx context.Context, attr string, dest []byte) (uint32,
// and it did not cause trouble. Seems cleaner than saying ENODATA.
return 0, syscall.EOPNOTSUPP
}
- cAttr := rn.encryptXattrName(attr)
- cData, errno := n.getXAttr(cAttr)
- if errno != 0 {
- return 0, errno
+ var data []byte
+ // ACLs are passed through without encryption
+ if isAcl(attr) {
+ var errno syscall.Errno
+ data, errno = n.getXAttr(attr)
+ if errno != 0 {
+ return minus1, errno
+ }
+ } else {
+ // encrypted user xattr
+ cAttr := rn.encryptXattrName(attr)
+ cData, errno := n.getXAttr(cAttr)
+ if errno != 0 {
+ return 0, errno
+ }
+ var err error
+ data, err = rn.decryptXattrValue(cData)
+ if err != nil {
+ tlog.Warn.Printf("GetXAttr: %v", err)
+ return minus1, syscall.EIO
+ }
+ }
+ // Caller passes size zero to find out how large their buffer should be
+ if len(dest) == 0 {
+ return uint32(len(data)), 0
}
- data, err := rn.decryptXattrValue(cData)
- if err != nil {
- tlog.Warn.Printf("GetXAttr: %v", err)
- return ^uint32(0), syscall.EIO
+ if len(dest) < len(data) {
+ return minus1, syscall.ERANGE
}
l := copy(dest, data)
return uint32(l), 0
@@ -56,6 +85,12 @@ func (n *Node) Getxattr(ctx context.Context, attr string, dest []byte) (uint32,
func (n *Node) Setxattr(ctx context.Context, attr string, data []byte, flags uint32) syscall.Errno {
rn := n.rootNode()
flags = uint32(filterXattrSetFlags(int(flags)))
+
+ // ACLs are passed through without encryption
+ if isAcl(attr) {
+ return n.setXAttr(attr, data, flags)
+ }
+
cAttr := rn.encryptXattrName(attr)
cData := rn.encryptXattrValue(data)
return n.setXAttr(cAttr, cData, flags)
@@ -66,6 +101,12 @@ func (n *Node) Setxattr(ctx context.Context, attr string, data []byte, flags uin
// This function is symlink-safe through Fremovexattr.
func (n *Node) Removexattr(ctx context.Context, attr string) syscall.Errno {
rn := n.rootNode()
+
+ // ACLs are passed through without encryption
+ if isAcl(attr) {
+ return n.removeXAttr(attr)
+ }
+
cAttr := rn.encryptXattrName(attr)
return n.removeXAttr(cAttr)
}
@@ -81,6 +122,11 @@ func (n *Node) Listxattr(ctx context.Context, dest []byte) (uint32, syscall.Errn
rn := n.rootNode()
var buf bytes.Buffer
for _, curName := range cNames {
+ // ACLs are passed through without encryption
+ if isAcl(curName) {
+ buf.WriteString(curName + "\000")
+ continue
+ }
if !strings.HasPrefix(curName, xattrStorePrefix) {
continue
}
@@ -90,10 +136,20 @@ func (n *Node) Listxattr(ctx context.Context, dest []byte) (uint32, syscall.Errn
rn.reportMitigatedCorruption(curName)
continue
}
+ // We *used to* encrypt ACLs, which caused a lot of problems.
+ if isAcl(name) {
+ tlog.Warn.Printf("ListXAttr: ignoring deprecated encrypted ACL %q = %q", curName, name)
+ rn.reportMitigatedCorruption(curName)
+ continue
+ }
buf.WriteString(name + "\000")
}
+ // Caller passes size zero to find out how large their buffer should be
+ if len(dest) == 0 {
+ return uint32(buf.Len()), 0
+ }
if buf.Len() > len(dest) {
- return ^uint32(0), syscall.ERANGE
+ return minus1, syscall.ERANGE
}
return uint32(copy(dest, buf.Bytes())), 0
}
diff --git a/tests/defaults/acl_test.go b/tests/defaults/acl_test.go
new file mode 100644
index 0000000..b3826e8
--- /dev/null
+++ b/tests/defaults/acl_test.go
@@ -0,0 +1,202 @@
+package defaults
+
+import (
+ "io/ioutil"
+ "math/rand"
+ "os"
+ "os/exec"
+ "path/filepath"
+ "syscall"
+ "testing"
+ "time"
+
+ "golang.org/x/sys/unix"
+
+ "github.com/pkg/xattr"
+
+ "github.com/rfjakob/gocryptfs/tests/test_helpers"
+)
+
+// https://github.com/rfjakob/gocryptfs/issues/543
+func TestCpA(t *testing.T) {
+ fn1 := filepath.Join(test_helpers.TmpDir, t.Name())
+ fn2 := filepath.Join(test_helpers.DefaultPlainDir, t.Name())
+
+ rand.Seed(int64(time.Now().Nanosecond()))
+
+ {
+ // Need unrestricted umask
+ old := syscall.Umask(000)
+ defer syscall.Umask(old)
+ }
+
+ for i := 0; i < 10; i++ {
+ // Random permissions (except owner read, which cp needs)
+ var modeWant os.FileMode = os.FileMode(rand.Int31n(0777+1) | 0400)
+
+ // Create file outside mount
+ err := ioutil.WriteFile(fn1, nil, modeWant)
+ if err != nil {
+ t.Fatal(err)
+ }
+ // Verify perms (umask problems?)
+ fi, err := os.Stat(fn1)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if fi.Mode() != modeWant {
+ t.Errorf("ioutil.WriteFile created wrong permissions: want %o have %o", modeWant, fi.Mode())
+ }
+
+ // "cp -a" from outside to inside mount
+ c := exec.Command("cp", "-a", fn1, fn2)
+ c.Stderr = os.Stderr
+ c.Stdout = os.Stdout
+ err = c.Run()
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ // Check perms
+ fi, err = os.Stat(fn2)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if fi.Mode() != modeWant {
+ t.Errorf("cp -a did not preserve permissions: want %o have %o", modeWant, fi.Mode())
+ }
+
+ syscall.Unlink(fn1)
+ syscall.Unlink(fn2)
+ }
+}
+
+func getfacl(fn string) (string, error) {
+ c := exec.Command("getfacl", "-c", "--", fn)
+ out, err := c.Output()
+ return string(out), err
+}
+
+// https://github.com/rfjakob/gocryptfs/issues/543
+func TestAcl543(t *testing.T) {
+ fn1 := test_helpers.TmpDir + "/TestAcl543"
+ fn2 := test_helpers.DefaultPlainDir + "/TestAcl543"
+
+ var c *exec.Cmd
+
+ var modeWant os.FileMode = 0777
+
+ {
+ // Need unrestricted umask
+ old := syscall.Umask(000)
+ defer syscall.Umask(old)
+ }
+
+ // Set acl on file outside gocryptfs mount
+ err := ioutil.WriteFile(fn1, nil, modeWant)
+ if err != nil {
+ t.Fatal(err)
+ }
+ c = exec.Command("setfacl", "-m", "u:daemon:rwx", fn1)
+ c.Stderr = os.Stderr
+ c.Stdout = os.Stdout
+ err = c.Run()
+ if err != nil {
+ t.Skip(err)
+ }
+ aclWant, err := getfacl(fn1)
+ if err != nil {
+ t.Fatal(err)
+ }
+ fi, err := os.Stat(fn1)
+ if fi.Mode() != modeWant {
+ t.Fatalf("mode changed from %o to %o", modeWant, fi.Mode())
+ }
+
+ // Set acl on file inside gocryptfs mount
+ err = ioutil.WriteFile(fn2, nil, modeWant)
+ if err != nil {
+ t.Fatal(err)
+ }
+ c = exec.Command("setfacl", "-m", "u:daemon:rwx", fn2)
+ c.Stderr = os.Stderr
+ c.Stdout = os.Stdout
+ err = c.Run()
+ if err != nil {
+ t.Fatal(err)
+ }
+ aclHave1, err := getfacl(fn1)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if aclHave1 != aclWant {
+ t.Error(aclHave1)
+ }
+ os.Remove(fn2)
+
+ // "cp -a" from outside to inside mount
+ c = exec.Command("cp", "-a", fn1, fn2)
+ c.Stderr = os.Stderr
+ c.Stdout = os.Stdout
+ err = c.Run()
+ if err != nil {
+ t.Fatal(err)
+ }
+ fi, err = os.Stat(fn2)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if fi.Mode() != modeWant {
+ t.Errorf("cp -a did not preserve permissions: want %o have %o", modeWant, fi.Mode())
+ }
+ aclHave2, err := getfacl(fn2)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if aclHave2 != aclWant {
+ t.Errorf("cp -a did not preserve acl: %q", aclHave1)
+ }
+}
+
+// Check that we handle zero-sized and undersized buffers correctly
+func TestXattrOverflow(t *testing.T) {
+ fn := filepath.Join(test_helpers.DefaultPlainDir, t.Name())
+ ioutil.WriteFile(fn, nil, 0600)
+
+ attr := "user.foo123"
+ val := []byte("12341234")
+ err := xattr.LSet(fn, attr, val)
+ if err != nil {
+ t.Skip(err)
+ }
+
+ // Getxattr
+ sz, err := unix.Lgetxattr(fn, attr, nil)
+ if err != nil {
+ t.Error(err)
+ }
+ if sz != len(val) {
+ t.Errorf("wrong sz: want %d have %d", len(val), sz)
+ }
+ sz, err = unix.Lgetxattr(fn, attr, make([]byte, 1))
+ if err != syscall.ERANGE {
+ t.Error(err)
+ }
+
+ // Listxattr
+ szWant, err := unix.Llistxattr(fn, make([]byte, 64*1024))
+ if err != nil {
+ t.Fatal(err)
+ }
+ sz, err = unix.Llistxattr(fn, nil)
+ if err != nil {
+ t.Error(err)
+ }
+ if sz != szWant {
+ t.Errorf("wrong sz: want %d have %d", szWant, sz)
+ }
+ sz, err = unix.Llistxattr(fn, make([]byte, 1))
+ if err != syscall.ERANGE {
+ t.Error(err)
+ }
+}