diff options
author | Jakob Unterwurzacher | 2017-11-26 21:59:24 +0100 |
---|---|---|
committer | Jakob Unterwurzacher | 2017-11-27 21:04:45 +0100 |
commit | 72b975867a3b9bdf53fc2da62e2ba4a328d7e4ab (patch) | |
tree | 8282cffdf1361228827851ca62f40589b4bb4a55 | |
parent | 1bb47b6796c7a2cfb64e6cdff37c43c03c473a81 (diff) |
fusefronted: allow_other: close race between mknod and chown
If the user manages to replace the directory with
a symlink at just the right time, we could be tricked
into chown'ing the wrong file.
This change fixes the race by using fchownat, which
unfortunately is not available on darwin, hence a compat
wrapper is added.
Scenario, as described by @slackner at
https://github.com/rfjakob/gocryptfs/issues/177 :
1. Create a forward mount point with `plaintextnames` enabled
2. Mount as root user with `allow_other`
3. For testing purposes create a file `/tmp/file_owned_by_root`
which is owned by the root user
4. As a regular user run inside of the GoCryptFS mount:
```
mkdir tempdir
mknod tempdir/file_owned_by_root p &
mv tempdir tempdir2
ln -s /tmp tempdir
```
When the steps are done fast enough and in the right order
(run in a loop!), the device file will be created in
`tempdir`, but the `lchown` will be executed by following
the symlink. As a result, the ownership of the file located
at `/tmp/file_owned_by_root` will be changed.
-rw-r--r-- | internal/fusefrontend/fs.go | 20 | ||||
-rw-r--r-- | internal/syscallcompat/sys_darwin.go | 16 | ||||
-rw-r--r-- | internal/syscallcompat/sys_linux.go | 5 | ||||
-rw-r--r-- | tests/matrix/matrix_test.go | 9 |
4 files changed, 41 insertions, 9 deletions
diff --git a/internal/fusefrontend/fs.go b/internal/fusefrontend/fs.go index b4819fd..8f77538 100644 --- a/internal/fusefrontend/fs.go +++ b/internal/fusefrontend/fs.go @@ -10,6 +10,8 @@ import ( "syscall" "time" + "golang.org/x/sys/unix" + "github.com/hanwen/go-fuse/fuse" "github.com/hanwen/go-fuse/fuse/nodefs" "github.com/hanwen/go-fuse/fuse/pathfs" @@ -280,15 +282,14 @@ func (fs *FS) Mknod(path string, mode uint32, dev uint32, context *fuse.Context) if err != nil { return fuse.ToStatus(err) } + dirfd, err := os.Open(filepath.Dir(cPath)) + if err != nil { + return fuse.ToStatus(err) + } + defer dirfd.Close() // Create ".name" file to store long file name cName := filepath.Base(cPath) if nametransform.IsLongContent(cName) { - var dirfd *os.File - dirfd, err = os.Open(filepath.Dir(cPath)) - if err != nil { - return fuse.ToStatus(err) - } - defer dirfd.Close() err = fs.nameTransform.WriteLongName(dirfd, cName, path) if err != nil { return fuse.ToStatus(err) @@ -300,16 +301,17 @@ func (fs *FS) Mknod(path string, mode uint32, dev uint32, context *fuse.Context) } } else { // Create regular device node - err = syscall.Mknod(cPath, mode, int(dev)) + err = syscallcompat.Mknodat(int(dirfd.Fd()), cName, mode, int(dev)) } if err != nil { return fuse.ToStatus(err) } // Set owner if fs.args.PreserveOwner { - err = os.Lchown(cPath, int(context.Owner.Uid), int(context.Owner.Gid)) + err = syscallcompat.Fchownat(int(dirfd.Fd()), cName, int(context.Owner.Uid), + int(context.Owner.Gid), unix.AT_SYMLINK_NOFOLLOW) if err != nil { - tlog.Warn.Printf("Mknod: Lchown failed: %v", err) + tlog.Warn.Printf("Mknod: Fchownat failed: %v", err) } } return fuse.OK diff --git a/internal/syscallcompat/sys_darwin.go b/internal/syscallcompat/sys_darwin.go index e77cd98..5bc5fba 100644 --- a/internal/syscallcompat/sys_darwin.go +++ b/internal/syscallcompat/sys_darwin.go @@ -131,3 +131,19 @@ func Dup3(oldfd int, newfd int, flags int) (err error) { } return syscall.Dup2(oldfd, newfd) } + +// Poor man's Fchownat. +func Fchownat(dirfd int, path string, uid int, gid int, flags int) (err error) { + cwd, err := syscall.Open(".", syscall.O_RDONLY, 0) + if err != nil { + return err + } + chdirMutex.Lock() + defer chdirMutex.Unlock() + err = syscall.Fchdir(dirfd) + if err != nil { + return err + } + defer syscall.Fchdir(cwd) + return syscall.Lchown(path, uid, gid) +} diff --git a/internal/syscallcompat/sys_linux.go b/internal/syscallcompat/sys_linux.go index 441f904..8a64a81 100644 --- a/internal/syscallcompat/sys_linux.go +++ b/internal/syscallcompat/sys_linux.go @@ -68,3 +68,8 @@ func Mknodat(dirfd int, path string, mode uint32, dev int) (err error) { func Dup3(oldfd int, newfd int, flags int) (err error) { return syscall.Dup3(oldfd, newfd, flags) } + +// Fchownat syscall. +func Fchownat(dirfd int, path string, uid int, gid int, flags int) (err error) { + return syscall.Fchownat(dirfd, path, uid, gid, flags) +} diff --git a/tests/matrix/matrix_test.go b/tests/matrix/matrix_test.go index 9b9cb1b..170f8ba 100644 --- a/tests/matrix/matrix_test.go +++ b/tests/matrix/matrix_test.go @@ -781,3 +781,12 @@ func TestUtimesNanoFd(t *testing.T) { procPath := fmt.Sprintf("/proc/self/fd/%d", f.Fd()) doTestUtimesNano(t, procPath) } + +// Make sure the Mknod call works by creating a fifo (named pipe) +func TestMkfifo(t *testing.T) { + path := test_helpers.DefaultPlainDir + "/fifo1" + err := syscall.Mkfifo(path, 0700) + if err != nil { + t.Fatal(err) + } +} |