From 91e042e2bafbec3271d8c79005f7dc8229a16a10 Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sat, 2 Dec 2017 20:35:44 +0100 Subject: [PATCH] syscallcompat: add OpenNofollow helper OpenNofollow = symlink-race-safe Open Prepares fixing https://github.com/rfjakob/gocryptfs/issues/165 --- internal/syscallcompat/open_nofollow.go | 49 ++++++++++++++++++++ internal/syscallcompat/open_nofollow_test.go | 37 +++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 internal/syscallcompat/open_nofollow.go create mode 100644 internal/syscallcompat/open_nofollow_test.go diff --git a/internal/syscallcompat/open_nofollow.go b/internal/syscallcompat/open_nofollow.go new file mode 100644 index 0000000..c804e12 --- /dev/null +++ b/internal/syscallcompat/open_nofollow.go @@ -0,0 +1,49 @@ +package syscallcompat + +import ( + "path/filepath" + "strings" + "syscall" + + "github.com/rfjakob/gocryptfs/internal/tlog" +) + +// OpenNofollow opens the file/dir at "relPath" in a way that is secure against +// symlink attacks. Symlinks that are part of "relPath" are never followed. +// This function is implemented by walking the directory tree, starting at +// "baseDir", using the Openat syscall with the O_NOFOLLOW flag. +// Symlinks that are part of the "baseDir" path are followed. +func OpenNofollow(baseDir string, relPath string, flags int, mode uint32) (fd int, err error) { + if !filepath.IsAbs(baseDir) { + tlog.Warn.Printf("BUG: OpenNofollow called with relative baseDir=%q", baseDir) + return -1, syscall.EINVAL + } + if filepath.IsAbs(relPath) { + tlog.Warn.Printf("BUG: OpenNofollow called with absolute relPath=%q", relPath) + return -1, syscall.EINVAL + } + // Open the base dir + dirfd, err := syscall.Open(baseDir, syscall.O_RDONLY, 0) + if err != nil { + return -1, err + } + // Split the path into components and separate intermediate directories + // and the final basename + parts := strings.Split(relPath, "/") + dirs := parts[:len(parts)-1] + final := parts[len(parts)-1] + // Walk intermediate directories + var dirfd2 int + for _, name := range dirs { + dirfd2, err = Openat(dirfd, name, syscall.O_RDONLY|syscall.O_NOFOLLOW, 0) + syscall.Close(dirfd) + if err != nil { + return -1, err + } + dirfd = dirfd2 + } + defer syscall.Close(dirfd) + // Open the final component with the flags and permissions requested by + // the user plus forced NOFOLLOW. + return Openat(dirfd, final, flags|syscall.O_NOFOLLOW, mode) +} diff --git a/internal/syscallcompat/open_nofollow_test.go b/internal/syscallcompat/open_nofollow_test.go new file mode 100644 index 0000000..e9cdf77 --- /dev/null +++ b/internal/syscallcompat/open_nofollow_test.go @@ -0,0 +1,37 @@ +package syscallcompat + +import ( + "os" + "syscall" + "testing" +) + +func TestOpenNofollow(t *testing.T) { + err := os.MkdirAll(tmpDir+"/d1/d2/d3", 0700) + if err != nil { + t.Fatal(err) + } + // Create a file + fd, err := OpenNofollow(tmpDir, "d1/d2/d3/f1", syscall.O_RDWR|syscall.O_CREAT|syscall.O_EXCL, 0600) + if err != nil { + t.Fatal(err) + } + syscall.Close(fd) + _, err = os.Stat(tmpDir + "/d1/d2/d3/f1") + if err != nil { + t.Fatal(err) + } + // Replace "d1" with a symlink - open should fail with ELOOP + err = os.Rename(tmpDir+"/d1", tmpDir+"/d1.renamed") + if err != nil { + t.Fatal(err) + } + os.Symlink(tmpDir+"/d1.renamed", tmpDir+"/d1") + fd, err = OpenNofollow(tmpDir, "d1/d2/d3/f1", syscall.O_RDWR|syscall.O_CREAT, 0600) + if err == nil { + t.Fatalf("should have failed") + } + if err != syscall.ELOOP { + t.Errorf("expected ELOOP, got %v", err) + } +}