diff options
| -rw-r--r-- | README.md | 5 | ||||
| -rw-r--r-- | internal/fusefrontend/node_xattr.go | 74 | ||||
| -rw-r--r-- | tests/defaults/acl_test.go | 202 |
3 files changed, 271 insertions, 10 deletions
@@ -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) + } +} |