syscallcompat: untangle OpenNofollow and rename to OpenDirNofollow

The function used to do two things:

1) Walk the directory tree in a manner safe from symlink attacks
2) Open the final component in the mode requested by the caller

This change drops (2), which was only used once, and lets the caller
handle it. This simplifies the function and makes it fit for reuse in
forward mode in openBackingPath(), and for using O_PATH on Linux.
This commit is contained in:
Jakob Unterwurzacher 2018-09-08 17:41:17 +02:00
parent bc14f8dcb6
commit 9ec9d0c49c
7 changed files with 28 additions and 22 deletions

View File

@ -63,7 +63,7 @@ func (rfs *ReverseFS) findLongnameParent(dir string, dirIV []byte, longname stri
if hit != "" { if hit != "" {
return hit, nil return hit, nil
} }
fd, err := syscallcompat.OpenNofollow(rfs.args.Cipherdir, dir, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) fd, err := syscallcompat.OpenDirNofollow(rfs.args.Cipherdir, dir)
if err != nil { if err != nil {
tlog.Warn.Printf("findLongnameParent: opendir failed: %v\n", err) tlog.Warn.Printf("findLongnameParent: opendir failed: %v\n", err)
return "", err return "", err

View File

@ -4,6 +4,7 @@ import (
"bytes" "bytes"
"io" "io"
"os" "os"
"path/filepath"
"syscall" "syscall"
// In newer Go versions, this has moved to just "sync/syncmap". // In newer Go versions, this has moved to just "sync/syncmap".
@ -46,7 +47,13 @@ func (rfs *ReverseFS) newFile(relPath string) (*reverseFile, fuse.Status) {
if err != nil { if err != nil {
return nil, fuse.ToStatus(err) return nil, fuse.ToStatus(err)
} }
fd, err := syscallcompat.OpenNofollow(rfs.args.Cipherdir, pRelPath, syscall.O_RDONLY, 0) dir := filepath.Dir(pRelPath)
dirfd, err := syscallcompat.OpenDirNofollow(rfs.args.Cipherdir, dir)
if err != nil {
return nil, fuse.ToStatus(err)
}
fd, err := syscallcompat.Openat(dirfd, filepath.Base(pRelPath), syscall.O_RDONLY|syscall.O_NOFOLLOW, 0)
syscall.Close(dirfd)
if err != nil { if err != nil {
return nil, fuse.ToStatus(err) return nil, fuse.ToStatus(err)
} }

View File

@ -305,7 +305,7 @@ func (rfs *ReverseFS) OpenDir(cipherPath string, context *fuse.Context) ([]fuse.
return nil, fuse.ToStatus(err) return nil, fuse.ToStatus(err)
} }
// Read plaintext dir // Read plaintext dir
fd, err := syscallcompat.OpenNofollow(rfs.args.Cipherdir, relPath, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) fd, err := syscallcompat.OpenDirNofollow(rfs.args.Cipherdir, relPath)
if err != nil { if err != nil {
return nil, fuse.ToStatus(err) return nil, fuse.ToStatus(err)
} }

View File

@ -108,7 +108,7 @@ func (rfs *ReverseFS) openBackingDir(cRelPath string) (dirfd int, pName string,
} }
// Open directory, safe against symlink races // Open directory, safe against symlink races
pDir := filepath.Dir(pRelPath) pDir := filepath.Dir(pRelPath)
dirfd, err = syscallcompat.OpenNofollow(rfs.args.Cipherdir, pDir, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) dirfd, err = syscallcompat.OpenDirNofollow(rfs.args.Cipherdir, pDir)
if err != nil { if err != nil {
return -1, "", err return -1, "", err
} }

View File

@ -91,7 +91,7 @@ func (f *virtualFile) Read(buf []byte, off int64) (resultData fuse.ReadResult, s
// GetAttr - FUSE call // GetAttr - FUSE call
func (f *virtualFile) GetAttr(a *fuse.Attr) fuse.Status { func (f *virtualFile) GetAttr(a *fuse.Attr) fuse.Status {
dir := filepath.Dir(f.parentFile) dir := filepath.Dir(f.parentFile)
dirfd, err := syscallcompat.OpenNofollow(f.cipherdir, dir, syscall.O_RDONLY|syscall.O_DIRECTORY, 0) dirfd, err := syscallcompat.OpenDirNofollow(f.cipherdir, dir)
if err != nil { if err != nil {
return fuse.ToStatus(err) return fuse.ToStatus(err)
} }

View File

@ -8,18 +8,18 @@ import (
"github.com/rfjakob/gocryptfs/internal/tlog" "github.com/rfjakob/gocryptfs/internal/tlog"
) )
// OpenNofollow opens the file/dir at "relPath" in a way that is secure against // OpenDirNofollow opens the dir at "relPath" in a way that is secure against
// symlink attacks. Symlinks that are part of "relPath" are never followed. // symlink attacks. Symlinks that are part of "relPath" are never followed.
// This function is implemented by walking the directory tree, starting at // This function is implemented by walking the directory tree, starting at
// "baseDir", using the Openat syscall with the O_NOFOLLOW flag. // "baseDir", using the Openat syscall with the O_NOFOLLOW flag.
// Symlinks that are part of the "baseDir" path are followed. // Symlinks that are part of the "baseDir" path are followed.
func OpenNofollow(baseDir string, relPath string, flags int, mode uint32) (fd int, err error) { func OpenDirNofollow(baseDir string, relPath string) (fd int, err error) {
if !filepath.IsAbs(baseDir) { if !filepath.IsAbs(baseDir) {
tlog.Warn.Printf("BUG: OpenNofollow called with relative baseDir=%q", baseDir) tlog.Warn.Printf("BUG: OpenDirNofollow called with relative baseDir=%q", baseDir)
return -1, syscall.EINVAL return -1, syscall.EINVAL
} }
if filepath.IsAbs(relPath) { if filepath.IsAbs(relPath) {
tlog.Warn.Printf("BUG: OpenNofollow called with absolute relPath=%q", relPath) tlog.Warn.Printf("BUG: OpenDirNofollow called with absolute relPath=%q", relPath)
return -1, syscall.EINVAL return -1, syscall.EINVAL
} }
// Open the base dir (following symlinks) // Open the base dir (following symlinks)
@ -31,14 +31,11 @@ func OpenNofollow(baseDir string, relPath string, flags int, mode uint32) (fd in
if relPath == "" { if relPath == "" {
return dirfd, nil return dirfd, nil
} }
// Split the path into components and separate intermediate directories // Split the path into components
// and the final basename
parts := strings.Split(relPath, "/") parts := strings.Split(relPath, "/")
dirs := parts[:len(parts)-1] // Walk the directory tree
final := parts[len(parts)-1]
// Walk intermediate directories
var dirfd2 int var dirfd2 int
for _, name := range dirs { for _, name := range parts {
dirfd2, err = Openat(dirfd, name, syscall.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_DIRECTORY, 0) dirfd2, err = Openat(dirfd, name, syscall.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_DIRECTORY, 0)
syscall.Close(dirfd) syscall.Close(dirfd)
if err != nil { if err != nil {
@ -46,8 +43,6 @@ func OpenNofollow(baseDir string, relPath string, flags int, mode uint32) (fd in
} }
dirfd = dirfd2 dirfd = dirfd2
} }
defer syscall.Close(dirfd) // Return fd to final directory
// Open the final component with the flags and permissions requested by return dirfd, nil
// the user plus forced NOFOLLOW.
return Openat(dirfd, final, flags|syscall.O_NOFOLLOW, mode)
} }

View File

@ -12,7 +12,11 @@ func TestOpenNofollow(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
// Create a file // Create a file
fd, err := OpenNofollow(tmpDir, "d1/d2/d3/f1", syscall.O_RDWR|syscall.O_CREAT|syscall.O_EXCL, 0600) dirfd, err := OpenDirNofollow(tmpDir, "d1/d2/d3")
if err != nil {
t.Fatal(err)
}
fd, err := Openat(dirfd, "f1", syscall.O_RDWR|syscall.O_CREAT|syscall.O_EXCL, 0600)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -27,7 +31,7 @@ func TestOpenNofollow(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
os.Symlink(tmpDir+"/d1.renamed", tmpDir+"/d1") os.Symlink(tmpDir+"/d1.renamed", tmpDir+"/d1")
fd, err = OpenNofollow(tmpDir, "d1/d2/d3/f1", syscall.O_RDWR|syscall.O_CREAT, 0600) fd, err = OpenDirNofollow(tmpDir, "d1/d2/d3")
if err == nil { if err == nil {
t.Fatalf("should have failed") t.Fatalf("should have failed")
} }
@ -35,7 +39,7 @@ func TestOpenNofollow(t *testing.T) {
t.Errorf("expected ELOOP or ENOTDIR, got %v", err) t.Errorf("expected ELOOP or ENOTDIR, got %v", err)
} }
// Check to see that the base dir can be opened as well // Check to see that the base dir can be opened as well
fd, err = OpenNofollow(tmpDir, "", syscall.O_RDONLY, 0) fd, err = OpenDirNofollow(tmpDir, "")
if err != nil { if err != nil {
t.Errorf("cannot open base dir: %v", err) t.Errorf("cannot open base dir: %v", err)
} else { } else {