fusefrontend_reverse: use OpenNofollow in findLongnameParent

Protects findLongnameParent against symlink races.

Also add comments to several functions along the way.

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

3 files changed, 25 insertions, 9 deletions

diff --git a/internal/fusefrontend_reverse/reverse_longnames.go b/internal/fusefrontend_reverse/reverse_longnames.go
index 2b4324e..4ddff2b 100644
--- a/internal/fusefrontend_reverse/reverse_longnames.go
+++ b/internal/fusefrontend_reverse/reverse_longnames.go
@@ -2,7 +2,6 @@ package fusefrontend_reverse
 
 import (
 	"log"
-	"os"
 	"path/filepath"
 	"sync"
 	"syscall"
@@ -13,6 +12,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"
 )
 
@@ -27,6 +27,9 @@ const (
 	shortNameMax = 175
 )
 
+// longnameParentCache maps dir+"/"+longname to plaintextname.
+// Yes, the combination of relative plaintext dir path and encrypted
+// longname is strange, but works fine as a map index.
 var longnameParentCache map[string]string
 var longnameCacheLock sync.Mutex
 
@@ -48,7 +51,9 @@ func initLongnameCache() {
 	go longnameCacheCleaner()
 }
 
-// findLongnameParent converts "gocryptfs.longname.XYZ" to the plaintext name
+// findLongnameParent converts "longname" = "gocryptfs.longname.XYZ" to the
+// plaintext name. "dir" = relative plaintext path to the directory the
+// longname file is in, "dirIV" = directory IV of the directory.
 func (rfs *ReverseFS) findLongnameParent(dir string, dirIV []byte, longname string) (plaintextName string, err error) {
 	longnameCacheLock.Lock()
 	hit := longnameParentCache[dir+"/"+longname]
@@ -56,26 +61,27 @@ func (rfs *ReverseFS) findLongnameParent(dir string, dirIV []byte, longname stri
 	if hit != "" {
 		return hit, nil
 	}
-	absDir := filepath.Join(rfs.args.Cipherdir, dir)
-	dirfd, err := os.Open(absDir)
+	fd, err := syscallcompat.OpenNofollow(rfs.args.Cipherdir, dir, syscall.O_RDONLY|syscall.O_DIRECTORY, 0)
 	if err != nil {
 		tlog.Warn.Printf("findLongnameParent: opendir failed: %v\n", err)
 		return "", err
 	}
-	dirEntries, err := dirfd.Readdirnames(-1)
-	dirfd.Close()
+	dirEntries, err := syscallcompat.Getdents(fd)
+	syscall.Close(fd)
 	if err != nil {
-		tlog.Warn.Printf("findLongnameParent: Readdirnames failed: %v\n", err)
+		tlog.Warn.Printf("findLongnameParent: Getdents failed: %v\n", err)
 		return "", err
 	}
 	longnameCacheLock.Lock()
 	defer longnameCacheLock.Unlock()
-	for _, plaintextName = range dirEntries {
+	for _, entry := range dirEntries {
+		plaintextName := entry.Name
 		if len(plaintextName) <= shortNameMax {
 			continue
 		}
 		cName := rfs.nameTransform.EncryptName(plaintextName, dirIV)
 		if len(cName) <= syscall.NAME_MAX {
+			// Entry should have been skipped by the "continue" above
 			log.Panic("logic error or wrong shortNameMax constant?")
 		}
 		hName := rfs.nameTransform.HashLongName(cName)
diff --git a/internal/fusefrontend_reverse/rpath.go b/internal/fusefrontend_reverse/rpath.go
index 2dc76e1..b783686 100644
--- a/internal/fusefrontend_reverse/rpath.go
+++ b/internal/fusefrontend_reverse/rpath.go
@@ -24,6 +24,9 @@ func (rfs *ReverseFS) abs(relPath string, err error) (string, error) {
 	return filepath.Join(rfs.args.Cipherdir, relPath), nil
 }
 
+// rDecryptName decrypts the ciphertext name "cName", given the dirIV of the
+// directory "cName" lies in. The relative plaintext path to the directory
+// "pDir" is used if a "gocryptfs.longname.XYZ.name" must be resolved.
 func (rfs *ReverseFS) rDecryptName(cName string, dirIV []byte, pDir string) (pName string, err error) {
 	nameType := nametransform.NameType(cName)
 	if nameType == nametransform.LongNameNone {
@@ -58,6 +61,8 @@ func (rfs *ReverseFS) rDecryptName(cName string, dirIV []byte, pDir string) (pNa
 	return pName, nil
 }
 
+// decryptPath decrypts a relative ciphertext path to a relative plaintext
+// path.
 func (rfs *ReverseFS) decryptPath(relPath string) (string, error) {
 	if rfs.args.PlaintextNames || relPath == "" {
 		return relPath, nil
diff --git a/internal/fusefrontend_reverse/rpath_cache.go b/internal/fusefrontend_reverse/rpath_cache.go
index f90b357..221f578 100644
--- a/internal/fusefrontend_reverse/rpath_cache.go
+++ b/internal/fusefrontend_reverse/rpath_cache.go
@@ -17,6 +17,8 @@ type rPathCacheContainer struct {
 	dirIV []byte
 }
 
+// lookup relative ciphertext path "cPath". Returns dirIV, relative
+// plaintext path.
 func (c *rPathCacheContainer) lookup(cPath string) ([]byte, string) {
 	c.Lock()
 	defer c.Unlock()
@@ -28,7 +30,9 @@ func (c *rPathCacheContainer) lookup(cPath string) ([]byte, string) {
 	return nil, ""
 }
 
-// store - write entry for "cPath" into the cache
+// store - write entry for the directory at relative ciphertext path "cPath"
+// into the cache.
+// "dirIV" = directory IV of the directory, "pPath" = relative plaintext path
 func (c *rPathCacheContainer) store(cPath string, dirIV []byte, pPath string) {
 	c.Lock()
 	defer c.Unlock()
@@ -37,4 +41,5 @@ func (c *rPathCacheContainer) store(cPath string, dirIV []byte, pPath string) {
 	c.pPath = pPath
 }
 
+// rPathCache: see rPathCacheContainer above for a detailed description
 var rPathCache rPathCacheContainer