Print errors to stderr

finds out what happens if multiple
gocryptfs mounts write to one file concurrently
(usually, nothing good).

This use case is relevant for HPC clusters.

Makefile

@@ -10,7 +10,11 @@ test:
 .phony: root_test
 root_test:
 	./build.bash
-	cd tests/root_test && go test -c && sudo ./root_test.test -test.v
+
+# Need to use TMPDIR=/var/tmp as TestOverlay fails on tmpfs.
+	cd tests/root_test && go test -c && sudo TMPDIR=/var/tmp ./root_test.test -test.v
+
+	cd tests/cli && go test -c && sudo ./cli.test -test.v -test.run=TestDirectMount
 
 .phony: format
 format:

README.md

@@ -195,6 +195,14 @@ RM:    2,367
 Changelog
 ---------
 
+#### v2.4.0, 2023-06-10
+* Try the `mount(2)` syscall before falling back to `fusermount(1)`. This means we
+  don't need `fusermount(1)` at all if running as root or in a root-like namespace
+  ([#697](https://github.com/rfjakob/gocryptfs/issues/697))
+* Fix `-extpass` mis-parsing commas ([#730](https://github.com/rfjakob/gocryptfs/issues/730))
+* Fix `rm -R` mis-reporting `write-protected directory` on gocryptfs on sshfs
+  ([commit](https://github.com/rfjakob/gocryptfs/commit/09954c4bdecf0ca6da65776f176dc934ffced2b0))
+
 #### v2.3.2, 2023-04-29
 * Fix incorrect file size reported after hard link creation
   ([#724](https://github.com/rfjakob/gocryptfs/issues/724))

contrib/gocryptfssh

@@ -0,0 +1,86 @@
+#!/bin/sh
+
+# This script mounts an gocryptfs filesystem, starts a shell in the mounted
+# directory, and then unmounts the filesystem when the shell exits. This is an
+# equivalent of the encfssh script by by David Rosenstrauch.
+
+canonicalize() {
+	cd "$1" || return
+	pwd
+}
+
+
+case $1 in "" | -h | --help)
+	echo "Usage: gocryptfssh encrypted_directory [unencrypted-directory [-p]]"
+	echo "  -p   mount the unencrypted directory as public"
+	exit 1
+	;;
+esac
+
+enc_dir=$1
+unenc_dir_given=false
+mount_public=false
+if [ ! -z "$2" ]; then
+    unenc_dir_given=true
+    unenc_dir=$2
+    for arg in "$@" ; do
+	if [ "$arg" = "-p" ]; then
+	    mount_public=true
+	fi
+    done
+    [ -d "$unenc_dir" ] || mkdir -- "$unenc_dir"
+else
+    unenc_dir=$(mktemp -d .XXXXXXXX)
+fi
+
+if [ ! -d "$enc_dir" ]; then
+    mkdir -- "$enc_dir"
+fi
+
+enc_dir=$(canonicalize "$enc_dir")
+unenc_dir=$(canonicalize "$unenc_dir")
+
+options=
+if [ "$unenc_dir_given" = "true" ]; then
+    if [ "$mount_public" = "true" ]; then
+	options="-- -o allow_other"
+    fi
+fi
+
+# Attach the directory and change into it
+
+if gocryptfs "$enc_dir" "$unenc_dir" $options; then :; else
+    echo "gocryptfs failed"
+    rmdir -- "$unenc_dir"
+    exit 1
+fi
+if ! [ "$unenc_dir_given" = "true" ]; then
+    chmod 700 "$unenc_dir"
+fi
+echo "Directory is $unenc_dir" >&2
+cd "$unenc_dir" || exit
+
+# Fall back to umount if fusermount is not available (e.g., on OS X)
+fuse_umount() {
+	if command -v fusermount >/dev/null 2>&1; then
+		fusermount -u "$@"
+	else
+		umount "$@" # MacOS case
+	fi
+}
+
+# Honor the SHELL environment variable to select a shell to run
+"$SHELL"; retval=$?
+
+# ensure that this shell isn't itself holding the mounted directory open
+# ...but avoid terminating on failure, *or* causing a shellcheck warning for
+#    failing to check exit status from cd.
+cd / ||:
+
+# if unmount fails, skip rmdir, always use exit status of failure
+fuse_umount "$unenc_dir" || exit
+
+if ! [ "$unenc_dir_given" = true ]; then
+	rmdir -- "$unenc_dir"
+fi
+exit "$retval"

go.mod

@@ -5,6 +5,7 @@ go 1.16
 require (
 	github.com/aperturerobotics/jacobsa-crypto v1.0.0
 	github.com/hanwen/go-fuse/v2 v2.3.0
+	github.com/moby/sys/mountinfo v0.6.2
 	github.com/pkg/xattr v0.4.3
 	github.com/rfjakob/eme v1.1.2
 	github.com/sabhiram/go-gitignore v0.0.0-20201211210132-54b8a0bf510f

gocryptfs-xray/paths_ctlsock.go

@@ -22,7 +22,7 @@ func transformPaths(socketPath string, req *ctlsock.RequestStruct, in *string, s
 	errorCount := 0
 	c, err := ctlsock.New(socketPath)
 	if err != nil {
-		fmt.Printf("fatal: %v\n", err)
+		fmt.Fprintf(os.Stderr, "fatal: %v\n", err)
 		os.Exit(1)
 	}
 	line := 1

gocryptfs-xray/xray_main.go

@@ -107,7 +107,7 @@ func main() {
 
 	s := sum(args.dumpmasterkey, args.decryptPaths, args.encryptPaths)
 	if s > 1 {
-		fmt.Printf("fatal: %d operations were requested\n", s)
+		fmt.Fprintf(os.Stderr, "fatal: %d operations were requested\n", s)
 		os.Exit(1)
 	}
 	if flag.NArg() != 1 {
@@ -183,7 +183,7 @@ func inspectCiphertext(args *argContainer, fd *os.File) {
 		fmt.Println("empty file")
 		os.Exit(0)
 	} else if err == io.EOF {
-		fmt.Printf("incomplete file header: read %d bytes, want %d\n", n, contentenc.HeaderLen)
+		fmt.Fprintf(os.Stderr, "incomplete file header: read %d bytes, want %d\n", n, contentenc.HeaderLen)
 		os.Exit(1)
 	} else if err != nil {
 		errExit(err)

internal/fusefrontend/node.go

@@ -99,6 +99,17 @@ func (n *Node) Getattr(ctx context.Context, f fs.FileHandle, out *fuse.AttrOut)
 	return 0
 }
 
+func (n *Node) Access(ctx context.Context, mode uint32) syscall.Errno {
+	dirfd, cName, errno := n.prepareAtSyscallMyself()
+	if errno != 0 {
+		return errno
+	}
+	defer syscall.Close(dirfd)
+
+	err := syscallcompat.Faccessat(dirfd, cName, mode)
+	return fs.ToErrno(err)
+}
+
 // Unlink - FUSE call. Delete a file.
 //
 // Symlink-safe through use of Unlinkat().

mount.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"bytes"
-	"fmt"
 	"log"
 	"log/syslog"
 	"math"
@@ -388,7 +387,6 @@ func initGoFuse(rootNode fs.InodeEmbedder, args *argContainer) *fuse.Server {
 		// have it >128kiB. We cannot handle more than 128kiB, so we tell
 		// the kernel to limit the size explicitly.
 		MaxWrite: fuse.MAX_KERNEL_WRITE,
-		Options:  []string{fmt.Sprintf("max_read=%d", fuse.MAX_KERNEL_WRITE)},
 		Debug:    args.fusedebug,
 		// The kernel usually submits multiple read requests in parallel,
 		// which means we serve them in any order. Out-of-order reads are
@@ -398,6 +396,9 @@ func initGoFuse(rootNode fs.InodeEmbedder, args *argContainer) *fuse.Server {
 		// Setting SyncRead disables FUSE_CAP_ASYNC_READ. This makes the kernel
 		// do everything in-order without parallelism.
 		SyncRead: args.serialize_reads,
+		// Attempt to directly call mount(2) before trying fusermount. This means we
+		// can do without fusermount if running as root.
+		DirectMount: true,
 	}
 
 	mOpts := &fuseOpts.MountOptions
@@ -427,7 +428,7 @@ func initGoFuse(rootNode fs.InodeEmbedder, args *argContainer) *fuse.Server {
 		tlog.Warn.Printf("Warning: %q will be displayed as %q in \"df -T\"", fsname, fsname2)
 		fsname = fsname2
 	}
-	mOpts.Options = append(mOpts.Options, "fsname="+fsname)
+	mOpts.FsName = fsname
 	// Second column, "Type", will be shown as "fuse." + Name
 	mOpts.Name = "gocryptfs"
 	if args.reverse {

tests/canonical-benchmarks.bash

@@ -35,7 +35,13 @@ function etime {
 }
 
 echo -n "WRITE: "
-dd if=/dev/zero of=zero bs=131072 count=2000 2>&1 | tail -n 1
+dd if=/dev/zero of=zero bs=131072 count=2000 conv=fsync 2>&1 | tail -n 1
+
+# Drop cache of file "zero", otherwise we are benchmarking the
+# page cache. Borrowed from
+# https://www.gnu.org/software/coreutils/manual/html_node/dd-invocation.html#index-nocache
+dd if=zero iflag=nocache count=0 status=none
+
 sleep 0.1
 echo -n "READ:  "
 dd if=zero of=/dev/null bs=131072 count=2000 2>&1 | tail -n 1

tests/cli/directmount_test.go

@@ -0,0 +1,91 @@
+package cli
+
+import (
+	"fmt"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/moby/sys/mountinfo"
+
+	"github.com/rfjakob/gocryptfs/v2/tests/test_helpers"
+)
+
+// TestDirectMount checks that the effective mount options are what we expect.
+//
+// This test should be run twice:
+// 1) As a normal user (uses fusermount):          make test
+// 2) As root (mount syscall is called directly):  make root_test
+func TestDirectMount(t *testing.T) {
+	type testCase struct {
+		allow_other bool
+		noexec      bool
+		suid        bool
+		dev         bool
+	}
+	table := []testCase{
+		{ /* all false */ },
+		{allow_other: true},
+		{noexec: true},
+		{suid: true},
+		{dev: true},
+	}
+
+	dir := test_helpers.InitFS(t)
+	mnt := dir + ".mnt"
+
+	checkOptionPresent := func(t *testing.T, opts string, option string, want bool) {
+		split := strings.Split(opts, ",")
+		have := false
+		for _, v := range split {
+			if strings.HasPrefix(v, option) {
+				have = true
+				break
+			}
+		}
+		if want != have {
+			t.Errorf("checkOptionPresent: %s: want=%v have=%v. Full string: %s", option, want, have, opts)
+		}
+	}
+
+	doTestMountInfo := func(t *testing.T, row testCase) {
+		test_helpers.MountOrFatal(t, dir, mnt,
+			"-extpass=echo test",
+			fmt.Sprintf("-allow_other=%v", row.allow_other),
+			fmt.Sprintf("-noexec=%v", row.noexec),
+			fmt.Sprintf("-dev=%v", row.dev),
+			fmt.Sprintf("-suid=%v", row.suid))
+		defer test_helpers.UnmountErr(mnt)
+
+		mounts, err := mountinfo.GetMounts(mountinfo.SingleEntryFilter(mnt))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if len(mounts) != 1 {
+			t.Fatalf("Could not find mountpoint %q in /proc/self/mountinfo", mnt)
+		}
+		info := mounts[0]
+
+		if info.FSType != "fuse.gocryptfs" {
+			t.Errorf("wrong FSType: %q", info.FSType)
+		}
+		if info.Source != dir {
+			t.Errorf("wrong Source: have %q, want %q", info.Source, dir)
+		}
+		checkOptionPresent(t, info.VFSOptions, "max_read=", true)
+		checkOptionPresent(t, info.VFSOptions, "allow_other", row.allow_other)
+		// gocryptfs enables default_permissions when allow_other is enabled
+		checkOptionPresent(t, info.VFSOptions, "default_permissions", row.allow_other)
+		checkOptionPresent(t, info.Options, "noexec", row.noexec)
+		// Enabling suid and dev only works as root
+		if os.Getuid() == 0 {
+			checkOptionPresent(t, info.Options, "nosuid", !row.suid)
+			checkOptionPresent(t, info.Options, "nodev", !row.dev)
+		}
+	}
+
+	for _, row := range table {
+		doTestMountInfo(t, row)
+	}
+
+}

tests/cluster/cluster_test.go

@@ -0,0 +1,111 @@
+// package cluster_test finds out what happens if multiple
+// gocryptfs mounts write to one file concurrently
+// (usually, nothing good).
+//
+// This use case is relevant for HPC clusters.
+package cluster_test
+
+import (
+	"bytes"
+	"math/rand"
+	"os"
+	"sync"
+	"testing"
+
+	"github.com/rfjakob/gocryptfs/v2/tests/test_helpers"
+)
+
+// This test passes on XFS but fails on ext4 and tmpfs!!!
+//
+// Quoting https://lists.samba.org/archive/samba-technical/2019-March/133050.html
+//
+// > It turns out that xfs respects POSIX w.r.t "atomic read/write" and
+// > this is implemented by taking a file-wide shared lock on every
+// > buffered read.
+// > This behavior is unique to XFS on Linux and is not optional.
+// > Other Linux filesystems only guaranty page level atomicity for
+// > buffered read/write.
+//
+// See also:
+// * https://lore.kernel.org/linux-xfs/20190325001044.GA23020@dastard/
+//   Dave Chinner: XFS is the only linux filesystem that provides this behaviour.
+func TestClusterConcurrentRW(t *testing.T) {
+	if os.Getenv("ENABLE_CLUSTER_TEST") != "1" {
+		t.Skipf("This test is disabled by default because it fails unless on XFS.\n" +
+			"Run it like this: ENABLE_CLUSTER_TEST=1 go test\n" +
+			"Choose a backing directory by setting TMPDIR.")
+	}
+
+	const blocksize = 4096
+	const fileSize = 25 * blocksize // 100 kiB
+
+	cDir := test_helpers.InitFS(t)
+	mnt1 := cDir + ".mnt1"
+	mnt2 := cDir + ".mnt2"
+	test_helpers.MountOrFatal(t, cDir, mnt1, "-extpass=echo test", "-wpanic=0")
+	defer test_helpers.UnmountPanic(mnt1)
+	test_helpers.MountOrFatal(t, cDir, mnt2, "-extpass=echo test", "-wpanic=0")
+	defer test_helpers.UnmountPanic(mnt2)
+
+	f1, err := os.Create(mnt1 + "/foo")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f1.Close()
+	// Preallocate space
+	_, err = f1.WriteAt(make([]byte, fileSize), 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	f2, err := os.OpenFile(mnt2+"/foo", os.O_RDWR, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f2.Close()
+
+	var wg sync.WaitGroup
+
+	const loops = 10000
+	writeThread := func(f *os.File) {
+		defer wg.Done()
+		buf := make([]byte, blocksize)
+		for i := 0; i < loops; i++ {
+			if t.Failed() {
+				return
+			}
+			off := rand.Int63n(fileSize / blocksize)
+			_, err := f.WriteAt(buf, off)
+			if err != nil {
+				t.Errorf("writeThread iteration %d: WriteAt failed: %v", i, err)
+				return
+			}
+		}
+	}
+	readThread := func(f *os.File) {
+		defer wg.Done()
+		zeroBlock := make([]byte, blocksize)
+		buf := make([]byte, blocksize)
+		for i := 0; i < loops; i++ {
+			if t.Failed() {
+				return
+			}
+			off := rand.Int63n(fileSize / blocksize)
+			_, err := f.ReadAt(buf, off)
+			if err != nil {
+				t.Errorf("readThread iteration %d: ReadAt failed: %v", i, err)
+				return
+			}
+			if !bytes.Equal(buf, zeroBlock) {
+				t.Errorf("readThread iteration %d: data mismatch", i)
+				return
+			}
+		}
+	}
+
+	wg.Add(4)
+	go writeThread(f1)
+	go writeThread(f2)
+	go readThread(f1)
+	go readThread(f2)
+	wg.Wait()
+}

tests/root_test/root_test.go

@@ -375,7 +375,7 @@ func TestOverlay(t *testing.T) {
 		t.Skip("must run as root")
 	}
 	cDir := test_helpers.InitFS(t)
-	if syscallcompat.DetectQuirks(cDir)|syscallcompat.QuirkNoUserXattr != 0 {
+	if syscallcompat.DetectQuirks(cDir)&syscallcompat.QuirkNoUserXattr != 0 {
 		t.Logf("No user xattrs! overlay mount will likely fail.")
 	}
 	os.Chmod(cDir, 0755)

tests/stress_tests/fsstress-gocryptfs.bash

@@ -1,9 +1,13 @@
 #!/bin/bash
 #
-# Mount a go-fuse loopback filesystem in /tmp and run fsstress against it
+# Mount a gocryptfs filesystem in /var/tmp and run fsstress against it
 # in an infinite loop, only exiting on errors.
 #
-# When called as "fsstress-gocryptfs.bash", a gocryptfs filesystem is tested
+# Replicates what xfstests generic/013 does
+# ( https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/tests/generic/013 ),
+# but in an infinite loop.
+#
+# When called as "fsstress-loopback.bash", a go-fuse loopback filesystem is tested
 # instead.
 #
 # This test used to fail on older go-fuse versions after a few iterations with