fusefrontend_reverse: secure Access against symlink races (somewhat)

Unfortunately, faccessat in Linux ignores AT_SYMLINK_NOFOLLOW,
so this is not completely atomic.

Given that the information you get from access is not very
interesting, it seems good enough.

https://github.com/rfjakob/gocryptfs/issues/165

3 files changed, 60 insertions, 2 deletions

diff --git a/internal/fusefrontend_reverse/rfs.go b/internal/fusefrontend_reverse/rfs.go
index db10ce0..d87a936 100644
--- a/internal/fusefrontend_reverse/rfs.go
+++ b/internal/fusefrontend_reverse/rfs.go
@@ -201,11 +201,16 @@ func (rfs *ReverseFS) Access(relPath string, mode uint32, context *fuse.Context)
 		}
 		return fuse.EPERM
 	}
-	absPath, err := rfs.abs(rfs.decryptPath(relPath))
+	dirfd, name, err := rfs.openBackingDir(relPath)
 	if err != nil {
 		return fuse.ToStatus(err)
 	}
-	return fuse.ToStatus(syscall.Access(absPath, mode))
+	err = syscallcompat.Faccessat(dirfd, name, mode)
+	if err != nil {
+		fmt.Printf("name=%q err=%v", name, err)
+	}
+	syscall.Close(dirfd)
+	return fuse.ToStatus(err)
 }
 
 // Open - FUSE call
diff --git a/internal/fusefrontend_reverse/rpath.go b/internal/fusefrontend_reverse/rpath.go
index fa7680f..2dc76e1 100644
--- a/internal/fusefrontend_reverse/rpath.go
+++ b/internal/fusefrontend_reverse/rpath.go
@@ -8,6 +8,7 @@ import (
 
 	"github.com/rfjakob/gocryptfs/internal/nametransform"
 	"github.com/rfjakob/gocryptfs/internal/pathiv"
+	"github.com/rfjakob/gocryptfs/internal/syscallcompat"
 	"github.com/rfjakob/gocryptfs/internal/tlog"
 )
 
@@ -89,3 +90,23 @@ func (rfs *ReverseFS) decryptPath(relPath string) (string, error) {
 	rPathCache.store(cDir, dirIV, nametransform.Dir(pRelPath))
 	return pRelPath, nil
 }
+
+// openBackingDir decrypt the relative ciphertext path "cRelPath", opens
+// the directory that contains the target file/dir and returns the fd to
+// the directory and the decrypted name of the target file.
+// The fd/name pair is intended for use with fchownat and friends.
+func (rfs *ReverseFS) openBackingDir(cRelPath string) (dirfd int, pName string, err error) {
+	// Decrypt relative path
+	pRelPath, err := rfs.decryptPath(cRelPath)
+	if err != nil {
+		return -1, "", err
+	}
+	// Open directory, safe against symlink races
+	pDir := filepath.Dir(pRelPath)
+	dirfd, err = syscallcompat.OpenNofollow(rfs.args.Cipherdir, pDir, syscall.O_RDONLY|syscall.O_DIRECTORY, 0)
+	if err != nil {
+		return -1, "", err
+	}
+	pName = filepath.Base(pRelPath)
+	return dirfd, pName, nil
+}
diff --git a/tests/reverse/correctness_test.go b/tests/reverse/correctness_test.go
index 63043b7..15eb18b 100644
--- a/tests/reverse/correctness_test.go
+++ b/tests/reverse/correctness_test.go
@@ -8,7 +8,10 @@ import (
 	"syscall"
 	"testing"
 
+	"golang.org/x/sys/unix"
+
 	"github.com/rfjakob/gocryptfs/internal/ctlsock"
+	"github.com/rfjakob/gocryptfs/internal/syscallcompat"
 	"github.com/rfjakob/gocryptfs/tests/test_helpers"
 )
 
@@ -138,6 +141,35 @@ func TestAccessVirtual(t *testing.T) {
 	}
 }
 
+// Check that the access() syscall works on regular files
+func TestAccess(t *testing.T) {
+	f, err := os.Create(dirA + "/testaccess1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.Close()
+	f, err = os.Open(dirB)
+	if err != nil {
+		t.Fatal(err)
+	}
+	names, err := f.Readdirnames(0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, n := range names {
+		// Check if file exists - this should never fail
+		err = syscallcompat.Faccessat(unix.AT_FDCWD, dirB+"/"+n, unix.F_OK)
+		if err != nil {
+			t.Errorf("%s: %v", n, err)
+		}
+		// Check if file is readable
+		err = syscallcompat.Faccessat(unix.AT_FDCWD, dirB+"/"+n, unix.R_OK)
+		if err != nil {
+			t.Logf("%s: %v", n, err)
+		}
+	}
+}
+
 // Opening a nonexistant file name should return ENOENT
 // and not EBADMSG or EIO or anything else.
 func TestEnoent(t *testing.T) {