diff options
author | Fabricio Voznika <fvoznika@google.com> | 2020-08-19 18:03:15 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-08-19 18:05:54 -0700 |
commit | 6335704625819914768800c16e82623a65fa6755 (patch) | |
tree | d68a1d75fd2f281b932212770bde241635a2102c /runsc/fsgofer/fsgofer_test.go | |
parent | 25babd63519151eb6e70d847d8fd0172c1d7090f (diff) |
Remove path walk from localFile.Mknod
Replace mknod call with mknodat equivalent to protect
against symlink attacks. Also added Mknod tests.
Remove goferfs reliance on gofer to check for file
existence before creating a synthetic entry.
Updates #2923
PiperOrigin-RevId: 327544516
Diffstat (limited to 'runsc/fsgofer/fsgofer_test.go')
-rw-r--r-- | runsc/fsgofer/fsgofer_test.go | 147 |
1 files changed, 96 insertions, 51 deletions
diff --git a/runsc/fsgofer/fsgofer_test.go b/runsc/fsgofer/fsgofer_test.go index 8ed703584..c91cfd094 100644 --- a/runsc/fsgofer/fsgofer_test.go +++ b/runsc/fsgofer/fsgofer_test.go @@ -21,9 +21,9 @@ import ( "os" "path" "path/filepath" - "syscall" "testing" + "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/p9" "gvisor.dev/gvisor/pkg/test/testutil" @@ -32,7 +32,7 @@ import ( var allOpenFlags = []p9.OpenFlags{p9.ReadOnly, p9.WriteOnly, p9.ReadWrite} var ( - allTypes = []uint32{syscall.S_IFREG, syscall.S_IFDIR, syscall.S_IFLNK} + allTypes = []uint32{unix.S_IFREG, unix.S_IFDIR, unix.S_IFLNK} // allConfs is set in init(). allConfs []Config @@ -83,7 +83,7 @@ func testReadWrite(f p9.File, flags p9.OpenFlags, content []byte) error { } want = append(want, b...) } else { - if e, ok := err.(syscall.Errno); !ok || e != syscall.EBADF { + if e, ok := err.(unix.Errno); !ok || e != unix.EBADF { return fmt.Errorf("WriteAt() should have failed, got: %d, want: EBADFD", err) } } @@ -101,7 +101,7 @@ func testReadWrite(f p9.File, flags p9.OpenFlags, content []byte) error { return fmt.Errorf("ReadAt() wrong data, got: %s, want: %s", string(rBuf), want) } } else { - if e, ok := err.(syscall.Errno); !ok || e != syscall.EBADF { + if e, ok := err.(unix.Errno); !ok || e != unix.EBADF { return fmt.Errorf("ReadAt() should have failed, got: %d, want: EBADFD", err) } } @@ -121,11 +121,11 @@ func (s state) String() string { func typeName(fileType uint32) string { switch fileType { - case syscall.S_IFREG: + case unix.S_IFREG: return "file" - case syscall.S_IFDIR: + case unix.S_IFDIR: return "directory" - case syscall.S_IFLNK: + case unix.S_IFLNK: return "symlink" default: panic(fmt.Sprintf("invalid file type for test: %d", fileType)) @@ -195,19 +195,19 @@ func setup(fileType uint32) (string, string, error) { var name string switch fileType { - case syscall.S_IFREG: + case unix.S_IFREG: name = "file" _, f, _, _, err := root.Create(name, p9.ReadWrite, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())) if err != nil { return "", "", fmt.Errorf("createFile(root, %q) failed, err: %v", "test", err) } defer f.Close() - case syscall.S_IFDIR: + case unix.S_IFDIR: name = "dir" if _, err := root.Mkdir(name, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != nil { return "", "", fmt.Errorf("root.MkDir(%q) failed, err: %v", name, err) } - case syscall.S_IFLNK: + case unix.S_IFLNK: name = "symlink" if _, err := root.Symlink("/some/target", name, p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != nil { return "", "", fmt.Errorf("root.Symlink(%q) failed, err: %v", name, err) @@ -227,7 +227,7 @@ func createFile(dir *localFile, name string) (*localFile, error) { } func TestReadWrite(t *testing.T) { - runCustom(t, []uint32{syscall.S_IFDIR}, rwConfs, func(t *testing.T, s state) { + runCustom(t, []uint32{unix.S_IFDIR}, rwConfs, func(t *testing.T, s state) { child, err := createFile(s.file, "test") if err != nil { t.Fatalf("%v: createFile() failed, err: %v", s, err) @@ -246,9 +246,13 @@ func TestReadWrite(t *testing.T) { if err != nil { t.Fatalf("%v: Walk(%s) failed, err: %v", s, "test", err) } - if _, _, _, err := l.Open(flags); err != nil { + fd, _, _, err := l.Open(flags) + if err != nil { t.Fatalf("%v: Open(%v) failed, err: %v", s, flags, err) } + if fd != nil { + defer fd.Close() + } if err := testReadWrite(l, flags, want); err != nil { t.Fatalf("%v: testReadWrite(%v) failed: %v", s, flags, err) } @@ -257,14 +261,14 @@ func TestReadWrite(t *testing.T) { } func TestCreate(t *testing.T) { - runCustom(t, []uint32{syscall.S_IFDIR}, rwConfs, func(t *testing.T, s state) { + runCustom(t, []uint32{unix.S_IFDIR}, rwConfs, func(t *testing.T, s state) { for i, flags := range allOpenFlags { _, l, _, _, err := s.file.Create(fmt.Sprintf("test-%d", i), flags, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())) if err != nil { t.Fatalf("%v, %v: WriteAt() failed, err: %v", s, flags, err) } - if err := testReadWrite(l, flags, []byte{}); err != nil { + if err := testReadWrite(l, flags, nil); err != nil { t.Fatalf("%v: testReadWrite(%v) failed: %v", s, flags, err) } } @@ -274,7 +278,7 @@ func TestCreate(t *testing.T) { // TestReadWriteDup tests that a file opened in any mode can be dup'ed and // reopened in any other mode. func TestReadWriteDup(t *testing.T) { - runCustom(t, []uint32{syscall.S_IFDIR}, rwConfs, func(t *testing.T, s state) { + runCustom(t, []uint32{unix.S_IFDIR}, rwConfs, func(t *testing.T, s state) { child, err := createFile(s.file, "test") if err != nil { t.Fatalf("%v: createFile() failed, err: %v", s, err) @@ -304,9 +308,13 @@ func TestReadWriteDup(t *testing.T) { t.Fatalf("%v: Walk(<empty>) failed: %v", s, err) } defer dup.Close() - if _, _, _, err := dup.Open(dupFlags); err != nil { + fd, _, _, err := dup.Open(dupFlags) + if err != nil { t.Fatalf("%v: Open(%v) failed: %v", s, flags, err) } + if fd != nil { + defer fd.Close() + } if err := testReadWrite(dup, dupFlags, want); err != nil { t.Fatalf("%v: testReadWrite(%v) failed: %v", s, dupFlags, err) } @@ -316,19 +324,19 @@ func TestReadWriteDup(t *testing.T) { } func TestUnopened(t *testing.T) { - runCustom(t, []uint32{syscall.S_IFREG}, allConfs, func(t *testing.T, s state) { + runCustom(t, []uint32{unix.S_IFREG}, allConfs, func(t *testing.T, s state) { b := []byte("foobar") - if _, err := s.file.WriteAt(b, 0); err != syscall.EBADF { - t.Errorf("%v: WriteAt() should have failed, got: %v, expected: syscall.EBADF", s, err) + if _, err := s.file.WriteAt(b, 0); err != unix.EBADF { + t.Errorf("%v: WriteAt() should have failed, got: %v, expected: unix.EBADF", s, err) } - if _, err := s.file.ReadAt(b, 0); err != syscall.EBADF { - t.Errorf("%v: ReadAt() should have failed, got: %v, expected: syscall.EBADF", s, err) + if _, err := s.file.ReadAt(b, 0); err != unix.EBADF { + t.Errorf("%v: ReadAt() should have failed, got: %v, expected: unix.EBADF", s, err) } - if _, err := s.file.Readdir(0, 100); err != syscall.EBADF { - t.Errorf("%v: Readdir() should have failed, got: %v, expected: syscall.EBADF", s, err) + if _, err := s.file.Readdir(0, 100); err != unix.EBADF { + t.Errorf("%v: Readdir() should have failed, got: %v, expected: unix.EBADF", s, err) } - if err := s.file.FSync(); err != syscall.EBADF { - t.Errorf("%v: FSync() should have failed, got: %v, expected: syscall.EBADF", s, err) + if err := s.file.FSync(); err != unix.EBADF { + t.Errorf("%v: FSync() should have failed, got: %v, expected: unix.EBADF", s, err) } }) } @@ -338,7 +346,7 @@ func TestUnopened(t *testing.T) { // was open with O_PATH, but Open() was not checking for it and allowing the // control file to be reused. func TestOpenOPath(t *testing.T) { - runCustom(t, []uint32{syscall.S_IFREG}, rwConfs, func(t *testing.T, s state) { + runCustom(t, []uint32{unix.S_IFREG}, rwConfs, func(t *testing.T, s state) { // Fist remove all permissions on the file. if err := s.file.SetAttr(p9.SetAttrMask{Permissions: true}, p9.SetAttr{Permissions: p9.FileMode(0)}); err != nil { t.Fatalf("SetAttr(): %v", err) @@ -353,7 +361,7 @@ func TestOpenOPath(t *testing.T) { if newFile.(*localFile).controlReadable { t.Fatalf("control file didn't open with O_PATH: %+v", newFile) } - if _, _, _, err := newFile.Open(p9.ReadOnly); err != syscall.EACCES { + if _, _, _, err := newFile.Open(p9.ReadOnly); err != unix.EACCES { t.Fatalf("Open() should have failed, got: %v, wanted: EACCES", err) } }) @@ -375,7 +383,7 @@ func TestSetAttrPerm(t *testing.T) { valid := p9.SetAttrMask{Permissions: true} attr := p9.SetAttr{Permissions: 0777} got, err := SetGetAttr(s.file, valid, attr) - if s.fileType == syscall.S_IFLNK { + if s.fileType == unix.S_IFLNK { if err == nil { t.Fatalf("%v: SetGetAttr(valid, %v) should have failed", s, attr.Permissions) } @@ -396,7 +404,7 @@ func TestSetAttrSize(t *testing.T) { valid := p9.SetAttrMask{Size: true} attr := p9.SetAttr{Size: size} got, err := SetGetAttr(s.file, valid, attr) - if s.fileType == syscall.S_IFLNK || s.fileType == syscall.S_IFDIR { + if s.fileType == unix.S_IFLNK || s.fileType == unix.S_IFDIR { if err == nil { t.Fatalf("%v: SetGetAttr(valid, %v) should have failed", s, attr.Permissions) } @@ -478,9 +486,9 @@ func TestLink(t *testing.T) { } err = dir.Link(s.file, linkFile) - if s.fileType == syscall.S_IFDIR { - if err != syscall.EPERM { - t.Errorf("%v: Link(target, %s) should have failed, got: %v, expected: syscall.EPERM", s, linkFile, err) + if s.fileType == unix.S_IFDIR { + if err != unix.EPERM { + t.Errorf("%v: Link(target, %s) should have failed, got: %v, expected: unix.EPERM", s, linkFile, err) } return } @@ -491,9 +499,12 @@ func TestLink(t *testing.T) { } func TestROMountChecks(t *testing.T) { - const want = syscall.EROFS + const want = unix.EROFS + uid := p9.UID(os.Getuid()) + gid := p9.GID(os.Getgid()) + runCustom(t, allTypes, roConfs, func(t *testing.T, s state) { - if s.fileType != syscall.S_IFLNK { + if s.fileType != unix.S_IFLNK { if _, _, _, err := s.file.Open(p9.WriteOnly); err != want { t.Errorf("Open() should have failed, got: %v, expected: %v", err, want) } @@ -512,16 +523,16 @@ func TestROMountChecks(t *testing.T) { } } - if _, _, _, _, err := s.file.Create("some_file", p9.ReadWrite, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != want { + if _, _, _, _, err := s.file.Create("some_file", p9.ReadWrite, 0777, uid, gid); err != want { t.Errorf("Create() should have failed, got: %v, expected: %v", err, want) } - if _, err := s.file.Mkdir("some_dir", 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != want { + if _, err := s.file.Mkdir("some_dir", 0777, uid, gid); err != want { t.Errorf("MkDir() should have failed, got: %v, expected: %v", err, want) } if err := s.file.RenameAt("some_file", s.file, "other_file"); err != want { t.Errorf("Rename() should have failed, got: %v, expected: %v", err, want) } - if _, err := s.file.Symlink("some_place", "some_symlink", p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != want { + if _, err := s.file.Symlink("some_place", "some_symlink", uid, gid); err != want { t.Errorf("Symlink() should have failed, got: %v, expected: %v", err, want) } if err := s.file.UnlinkAt("some_file", 0); err != want { @@ -530,6 +541,9 @@ func TestROMountChecks(t *testing.T) { if err := s.file.Link(s.file, "some_link"); err != want { t.Errorf("Link() should have failed, got: %v, expected: %v", err, want) } + if _, err := s.file.Mknod("some-nod", 0777, 1, 2, uid, gid); err != want { + t.Errorf("Mknod() should have failed, got: %v, expected: %v", err, want) + } valid := p9.SetAttrMask{Size: true} attr := p9.SetAttr{Size: 0} @@ -541,16 +555,20 @@ func TestROMountChecks(t *testing.T) { func TestROMountPanics(t *testing.T) { conf := Config{ROMount: true, PanicOnWrite: true} + uid := p9.UID(os.Getuid()) + gid := p9.GID(os.Getgid()) + runCustom(t, allTypes, []Config{conf}, func(t *testing.T, s state) { - if s.fileType != syscall.S_IFLNK { + if s.fileType != unix.S_IFLNK { assertPanic(t, func() { s.file.Open(p9.WriteOnly) }) } - assertPanic(t, func() { s.file.Create("some_file", p9.ReadWrite, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())) }) - assertPanic(t, func() { s.file.Mkdir("some_dir", 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())) }) + assertPanic(t, func() { s.file.Create("some_file", p9.ReadWrite, 0777, uid, gid) }) + assertPanic(t, func() { s.file.Mkdir("some_dir", 0777, uid, gid) }) assertPanic(t, func() { s.file.RenameAt("some_file", s.file, "other_file") }) - assertPanic(t, func() { s.file.Symlink("some_place", "some_symlink", p9.UID(os.Getuid()), p9.GID(os.Getgid())) }) + assertPanic(t, func() { s.file.Symlink("some_place", "some_symlink", uid, gid) }) assertPanic(t, func() { s.file.UnlinkAt("some_file", 0) }) assertPanic(t, func() { s.file.Link(s.file, "some_link") }) + assertPanic(t, func() { s.file.Mknod("some-nod", 0777, 1, 2, uid, gid) }) valid := p9.SetAttrMask{Size: true} attr := p9.SetAttr{Size: 0} @@ -559,9 +577,9 @@ func TestROMountPanics(t *testing.T) { } func TestWalkNotFound(t *testing.T) { - runCustom(t, []uint32{syscall.S_IFDIR}, allConfs, func(t *testing.T, s state) { - if _, _, err := s.file.Walk([]string{"nobody-here"}); err != syscall.ENOENT { - t.Errorf("%v: Walk(%q) should have failed, got: %v, expected: syscall.ENOENT", s, "nobody-here", err) + runCustom(t, []uint32{unix.S_IFDIR}, allConfs, func(t *testing.T, s state) { + if _, _, err := s.file.Walk([]string{"nobody-here"}); err != unix.ENOENT { + t.Errorf("%v: Walk(%q) should have failed, got: %v, expected: unix.ENOENT", s, "nobody-here", err) } }) } @@ -580,7 +598,7 @@ func TestWalkDup(t *testing.T) { } func TestReaddir(t *testing.T) { - runCustom(t, []uint32{syscall.S_IFDIR}, rwConfs, func(t *testing.T, s state) { + runCustom(t, []uint32{unix.S_IFDIR}, rwConfs, func(t *testing.T, s state) { name := "dir" if _, err := s.file.Mkdir(name, 0777, p9.UID(os.Getuid()), p9.GID(os.Getgid())); err != nil { t.Fatalf("%v: MkDir(%s) failed, err: %v", s, name, err) @@ -705,7 +723,7 @@ func TestAttachInvalidType(t *testing.T) { defer os.RemoveAll(dir) fifo := filepath.Join(dir, "fifo") - if err := syscall.Mkfifo(fifo, 0755); err != nil { + if err := unix.Mkfifo(fifo, 0755); err != nil { t.Fatalf("Mkfifo(%q): %v", fifo, err) } @@ -766,16 +784,16 @@ func TestDoubleAttachError(t *testing.T) { } func TestTruncate(t *testing.T) { - runCustom(t, []uint32{syscall.S_IFDIR}, rwConfs, func(t *testing.T, s state) { + runCustom(t, []uint32{unix.S_IFDIR}, rwConfs, func(t *testing.T, s state) { child, err := createFile(s.file, "test") if err != nil { - t.Fatalf("createFile() failed, err: %v", err) + t.Fatalf("createFile() failed: %v", err) } defer child.Close() want := []byte("foobar") w, err := child.WriteAt(want, 0) if err != nil { - t.Fatalf("Write() failed, err: %v", err) + t.Fatalf("Write() failed: %v", err) } if w != len(want) { t.Fatalf("Write() was partial, got: %d, expected: %d", w, len(want)) @@ -783,12 +801,15 @@ func TestTruncate(t *testing.T) { _, l, err := s.file.Walk([]string{"test"}) if err != nil { - t.Fatalf("Walk(%s) failed, err: %v", "test", err) + t.Fatalf("Walk(%s) failed: %v", "test", err) } if _, _, _, err := l.Open(p9.ReadOnly | p9.OpenTruncate); err != nil { - t.Fatalf("Open() failed, err: %v", err) + t.Fatalf("Open() failed: %v", err) } _, mask, attr, err := l.GetAttr(p9.AttrMask{Size: true}) + if err != nil { + t.Fatalf("GetAttr() failed: %v", err) + } if !mask.Size { t.Fatalf("GetAttr() didn't return size: %+v", mask) } @@ -797,3 +818,27 @@ func TestTruncate(t *testing.T) { } }) } + +func TestMknod(t *testing.T) { + runCustom(t, []uint32{unix.S_IFDIR}, rwConfs, func(t *testing.T, s state) { + _, err := s.file.Mknod("test", p9.ModeRegular|0777, 1, 2, p9.UID(os.Getuid()), p9.GID(os.Getgid())) + if err != nil { + t.Fatalf("Mknod() failed: %v", err) + } + + _, f, err := s.file.Walk([]string{"test"}) + if err != nil { + t.Fatalf("Walk() failed: %v", err) + } + fd, _, _, err := f.Open(p9.ReadWrite) + if err != nil { + t.Fatalf("Open() failed: %v", err) + } + if fd != nil { + defer fd.Close() + } + if err := testReadWrite(f, p9.ReadWrite, nil); err != nil { + t.Fatalf("testReadWrite() failed: %v", err) + } + }) +} |