Rmdir: handle creating and removing unreadable directories

This patch also splits off Mkdir and Rmdir into its own file.

Fixes issue #8, thanks to @diseq for the bug report.

5 files changed, 196 insertions, 100 deletions

diff --git a/cryptfs/nonce.go b/cryptfs/nonce.go
index 9bdbb09..a122ea5 100644
--- a/cryptfs/nonce.go
+++ b/cryptfs/nonce.go
@@ -1,6 +1,7 @@
 package cryptfs
 
 import (
+	"encoding/binary"
 	"bytes"
 	"crypto/rand"
 	"encoding/hex"
@@ -17,6 +18,12 @@ func RandBytes(n int) []byte {
 	return b
 }
 
+// Return a secure random uint64
+func RandUint64() uint64 {
+	b := RandBytes(8)
+	return binary.BigEndian.Uint64(b)
+}
+
 var gcmNonce nonce96
 
 type nonce96 struct {
diff --git a/integration_tests/helpers.go b/integration_tests/helpers.go
index b269069..750283d 100644
--- a/integration_tests/helpers.go
+++ b/integration_tests/helpers.go
@@ -127,6 +127,38 @@ func testMkdirRmdir(t *testing.T, plainDir string) {
 	if err != nil {
 		t.Fatal(err)
 	}
+
+	// Removing a non-empty dir should fail with ENOTEMPTY
+	if os.Mkdir(dir, 0777) != nil {
+		t.Fatal(err)
+	}
+	f, err := os.Create(dir + "/file")
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.Close()
+	err = syscall.Rmdir(dir)
+	errno := err.(syscall.Errno)
+	if errno != syscall.ENOTEMPTY {
+		t.Errorf("Should have gotten ENOTEMPTY, go %v", errno)
+	}
+	if syscall.Unlink(dir + "/file") != nil {
+		t.Fatal(err)
+	}
+	if syscall.Rmdir(dir) != nil {
+		t.Fatal(err)
+	}
+
+	// We should also be able to remove a directory we do not have permissions to
+	// read or write
+	err = os.Mkdir(dir, 0000)
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = syscall.Rmdir(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
 }
 
 // Create and rename a file
diff --git a/main.go b/main.go
index 21d8ed6..49b13d1 100644
--- a/main.go
+++ b/main.go
@@ -159,7 +159,7 @@ func main() {
 		"Setting this to a lower value speeds up mounting but makes the password susceptible to brute-force attacks")
 	flagSet.Parse(os.Args[1:])
 
-	// Fork a child into the background if "-f" is not set and we are mounting a filesystem
+	// Fork a child into the background if "-f" is not set AND we are mounting a filesystem
 	if !args.foreground && flagSet.NArg() == 2 {
 		forkChild() // does not return
 	}
@@ -339,6 +339,11 @@ func pathfsFrontend(key []byte, args argContainer, confFile *cryptfs.ConfFile) *
 	}
 	srv.SetDebug(args.fusedebug)
 
+	// All FUSE file and directory create calls carry explicit permission
+	// information. We need an unrestricted umask to create the files and
+	// directories with the requested permissions.
+	syscall.Umask(0000)
+
 	return srv
 }
 
diff --git a/pathfs_frontend/fs.go b/pathfs_frontend/fs.go
index 8d05c02..c7a9888 100644
--- a/pathfs_frontend/fs.go
+++ b/pathfs_frontend/fs.go
@@ -2,7 +2,6 @@ package pathfs_frontend
 
 import (
 	"encoding/base64"
-	"fmt"
 	"os"
 	"path/filepath"
 	"sync"
@@ -245,40 +244,7 @@ func (fs *FS) Readlink(path string, context *fuse.Context) (out string, status f
 	return string(target), fuse.OK
 }
 
-func (fs *FS) Mkdir(relPath string, mode uint32, context *fuse.Context) (code fuse.Status) {
-	if fs.isFiltered(relPath) {
-		return fuse.EPERM
-	}
-	encPath, err := fs.getBackingPath(relPath)
-	if err != nil {
-		return fuse.ToStatus(err)
-	}
-	if !fs.args.DirIV {
-		return fuse.ToStatus(os.Mkdir(encPath, os.FileMode(mode)))
-	}
 
-	// The new directory may take the place of an older one that is still in the cache
-	fs.CryptFS.DirIVCacheEnc.Clear()
-	// Create directory
-	fs.dirIVLock.Lock()
-	defer fs.dirIVLock.Unlock()
-	err = os.Mkdir(encPath, os.FileMode(mode))
-	if err != nil {
-		return fuse.ToStatus(err)
-	}
-	// Create gocryptfs.diriv inside
-	err = cryptfs.WriteDirIV(encPath)
-	if err != nil {
-		// This should not happen
-		cryptfs.Warn.Printf("Creating %s in dir %s failed: %v\n", cryptfs.DIRIV_FILENAME, encPath, err)
-		err2 := syscall.Rmdir(encPath)
-		if err2 != nil {
-			cryptfs.Warn.Printf("Mkdir: Rollback failed: %v\n", err2)
-		}
-		return fuse.ToStatus(err)
-	}
-	return fuse.OK
-}
 
 func (fs *FS) Unlink(path string, context *fuse.Context) (code fuse.Status) {
 	if fs.isFiltered(path) {
@@ -291,71 +257,6 @@ func (fs *FS) Unlink(path string, context *fuse.Context) (code fuse.Status) {
 	return fuse.ToStatus(syscall.Unlink(cPath))
 }
 
-func (fs *FS) Rmdir(name string, context *fuse.Context) (code fuse.Status) {
-	encPath, err := fs.getBackingPath(name)
-	if err != nil {
-		return fuse.ToStatus(err)
-	}
-	if !fs.args.DirIV {
-		return fuse.ToStatus(syscall.Rmdir(encPath))
-	}
-
-	// If the directory is not empty besides gocryptfs.diriv, do not even
-	// attempt the dance around gocryptfs.diriv.
-	fd, err := os.Open(encPath)
-	if err != nil {
-		return fuse.ToStatus(err)
-	}
-	defer fd.Close()
-	list, err := fd.Readdirnames(10)
-	if err != nil {
-		return fuse.ToStatus(err)
-	}
-	if len(list) > 1 {
-		return fuse.ToStatus(syscall.ENOTEMPTY)
-	}
-
-	// Move "gocryptfs.diriv" to the parent dir under name "gocryptfs.diriv.rmdir.INODENUMBER"
-	var st syscall.Stat_t
-	err = syscall.Fstat(int(fd.Fd()), &st)
-	if err != nil {
-		return fuse.ToStatus(err)
-	}
-	dirivPath := filepath.Join(encPath, cryptfs.DIRIV_FILENAME)
-	parentDir := filepath.Dir(encPath)
-	tmpName := fmt.Sprintf("gocryptfs.diriv.rmdir.%d", st.Ino)
-	tmpDirivPath := filepath.Join(parentDir, tmpName)
-	cryptfs.Debug.Printf("Rmdir: Renaming %s to %s\n", cryptfs.DIRIV_FILENAME, tmpDirivPath)
-	// The directory is in an inconsistent state between rename and rmdir. Protect against
-	// concurrent readers.
-	fs.dirIVLock.Lock()
-	defer fs.dirIVLock.Unlock()
-	err = os.Rename(dirivPath, tmpDirivPath)
-	if err != nil {
-		cryptfs.Warn.Printf("Rmdir: Renaming %s to %s failed: %v\n", cryptfs.DIRIV_FILENAME, tmpDirivPath, err)
-		return fuse.ToStatus(err)
-	}
-	// Actual Rmdir
-	err = syscall.Rmdir(encPath)
-	if err != nil {
-		// This can happen if another file in the directory was created in the
-		// meantime, undo the rename
-		err2 := os.Rename(tmpDirivPath, dirivPath)
-		if err2 != nil {
-			cryptfs.Warn.Printf("Rmdir: Rollback failed: %v\n", err2)
-		}
-		return fuse.ToStatus(err)
-	}
-	// Delete "gocryptfs.diriv.rmdir.INODENUMBER"
-	err = syscall.Unlink(tmpDirivPath)
-	if err != nil {
-		cryptfs.Warn.Printf("Rmdir: Could not clean up %s: %v\n", tmpName, err)
-	}
-	// The now-deleted directory may have been in the DirIV cache. Clear it.
-	fs.CryptFS.DirIVCacheEnc.Clear()
-	return fuse.OK
-}
-
 func (fs *FS) Symlink(target string, linkName string, context *fuse.Context) (code fuse.Status) {
 	cryptfs.Debug.Printf("Symlink(\"%s\", \"%s\")\n", target, linkName)
 	if fs.isFiltered(linkName) {
diff --git a/pathfs_frontend/fs_dir.go b/pathfs_frontend/fs_dir.go
new file mode 100644
index 0000000..95257bb
--- /dev/null
+++ b/pathfs_frontend/fs_dir.go
@@ -0,0 +1,151 @@
+package pathfs_frontend
+
+import (
+	"os"
+	"path/filepath"
+	"syscall"
+	"fmt"
+
+	"github.com/hanwen/go-fuse/fuse"
+	"github.com/rfjakob/gocryptfs/cryptfs"
+)
+
+func (fs *FS) Mkdir(relPath string, mode uint32, context *fuse.Context) (code fuse.Status) {
+	if fs.isFiltered(relPath) {
+		return fuse.EPERM
+	}
+	encPath, err := fs.getBackingPath(relPath)
+	if err != nil {
+		return fuse.ToStatus(err)
+	}
+	if !fs.args.DirIV {
+		return fuse.ToStatus(os.Mkdir(encPath, os.FileMode(mode)))
+	}
+
+	// We need write and execute permissions to create gocryptfs.diriv
+	origMode := mode
+	mode = mode | 0300
+
+	// The new directory may take the place of an older one that is still in the cache
+	fs.CryptFS.DirIVCacheEnc.Clear()
+	// Create directory
+	fs.dirIVLock.Lock()
+	defer fs.dirIVLock.Unlock()
+	err = os.Mkdir(encPath, os.FileMode(mode))
+	if err != nil {
+		return fuse.ToStatus(err)
+	}
+	// Create gocryptfs.diriv inside
+	err = cryptfs.WriteDirIV(encPath)
+	if err != nil {
+		// This should not happen
+		cryptfs.Warn.Printf("Mkdir: WriteDirIV failed: %v\n", err)
+		err2 := syscall.Rmdir(encPath)
+		if err2 != nil {
+			cryptfs.Warn.Printf("Mkdir: Rmdir rollback failed: %v\n", err2)
+		}
+		return fuse.ToStatus(err)
+	}
+
+	// Set permissions back to what the user wanted
+	if origMode != mode {
+		err = os.Chmod(encPath, os.FileMode(origMode))
+		if err != nil {
+			cryptfs.Warn.Printf("Mkdir: Chmod failed: %v\n", err)
+		}
+	}
+
+	return fuse.OK
+}
+
+func (fs *FS) Rmdir(name string, context *fuse.Context) (code fuse.Status) {
+	encPath, err := fs.getBackingPath(name)
+	if err != nil {
+		return fuse.ToStatus(err)
+	}
+	if !fs.args.DirIV {
+		return fuse.ToStatus(syscall.Rmdir(encPath))
+	}
+
+	// If the directory is not empty besides gocryptfs.diriv, do not even
+	// attempt the dance around gocryptfs.diriv.
+	fd, err := os.Open(encPath)
+	if perr, ok := err.(*os.PathError); ok && perr.Err == syscall.EACCES {
+		// We need permission to read and modify the directory
+		cryptfs.Debug.Printf("Rmdir: handling EACCESS\n")
+		fi, err2 := os.Stat(encPath)
+		if err2 != nil {
+			cryptfs.Debug.Printf("Rmdir: Stat: %v\n", err2)
+			return fuse.ToStatus(err2)
+		}
+		origMode := fi.Mode()
+		newMode := origMode | 0700
+		err2 = os.Chmod(encPath, newMode)
+		if err2 != nil {
+			cryptfs.Debug.Printf("Rmdir: Chmod failed: %v\n", err2)
+			return fuse.ToStatus(err)
+		}
+		defer func () {
+			if code != fuse.OK {
+				// Undo the chmod if removing the directory failed
+				err3 := os.Chmod(encPath, origMode)
+				if err3 != nil {
+					cryptfs.Warn.Printf("Rmdir: Chmod rollback failed: %v\n", err2)
+				}
+			}
+		}()
+		// Retry open
+		fd, err = os.Open(encPath)
+	}
+	if err != nil {
+		cryptfs.Debug.Printf("Rmdir: Open: %v\n", err)
+		return fuse.ToStatus(err)
+	}
+	list, err := fd.Readdirnames(10)
+	fd.Close()
+	if err != nil {
+		cryptfs.Debug.Printf("Rmdir: Readdirnames: %v\n", err)
+		return fuse.ToStatus(err)
+	}
+	if len(list) > 1 {
+		return fuse.ToStatus(syscall.ENOTEMPTY)
+	} else if len(list) == 0 {
+		cryptfs.Warn.Printf("Rmdir: gocryptfs.diriv missing, allowing deletion\n")
+		return fuse.ToStatus(syscall.Rmdir(encPath))
+	}
+
+	// Move "gocryptfs.diriv" to the parent dir as "gocryptfs.diriv.rmdir.XYZ"
+	dirivPath := filepath.Join(encPath, cryptfs.DIRIV_FILENAME)
+	parentDir := filepath.Dir(encPath)
+	tmpName := fmt.Sprintf("gocryptfs.diriv.rmdir.%d", cryptfs.RandUint64())
+	tmpDirivPath := filepath.Join(parentDir, tmpName)
+	cryptfs.Debug.Printf("Rmdir: Renaming %s to %s\n", cryptfs.DIRIV_FILENAME, tmpDirivPath)
+	// The directory is in an inconsistent state between rename and rmdir. Protect against
+	// concurrent readers.
+	fs.dirIVLock.Lock()
+	defer fs.dirIVLock.Unlock()
+	err = os.Rename(dirivPath, tmpDirivPath)
+	if err != nil {
+		cryptfs.Warn.Printf("Rmdir: Renaming %s to %s failed: %v\n", cryptfs.DIRIV_FILENAME, tmpDirivPath, err)
+		return fuse.ToStatus(err)
+	}
+	// Actual Rmdir
+	err = syscall.Rmdir(encPath)
+	if err != nil {
+		// This can happen if another file in the directory was created in the
+		// meantime, undo the rename
+		err2 := os.Rename(tmpDirivPath, dirivPath)
+		if err2 != nil {
+			cryptfs.Warn.Printf("Rmdir: Rename rollback failed: %v\n", err2)
+		}
+		return fuse.ToStatus(err)
+	}
+	// Delete "gocryptfs.diriv.rmdir.INODENUMBER"
+	err = syscall.Unlink(tmpDirivPath)
+	if err != nil {
+		cryptfs.Warn.Printf("Rmdir: Could not clean up %s: %v\n", tmpName, err)
+	}
+	// The now-deleted directory may have been in the DirIV cache. Clear it.
+	fs.CryptFS.DirIVCacheEnc.Clear()
+	return fuse.OK
+}