From 9bceda18110059d326917657f23bcd6c10e8a001 Mon Sep 17 00:00:00 2001 From: Sylvain Zimmer Date: Wed, 23 Sep 2026 22:52:23 +0200 Subject: [PATCH] reversesshfs: serve only the mounted directory, add ReadonlyNames The builtin driver used sftp.NewServer, which serves the whole host file system to the guest. On Linux and macOS hosts, it now uses sftp.NewRequestServer with handlers rooted at LocalPath: reads go through os.Root, and writes open each parent directory with O_NOFOLLOW, so they never follow a symlink. A path outside the root gets EACCES. Windows keeps sftp.NewServer. Setting the mode fails on a symlink, on macOS as on Linux. On Linux kernels without fchmodat2 (< 6.6), it goes through /proc/self/fd of an O_PATH fd, as glibc does, so it needs no read permission. ReadonlyNames makes a path read-only when any of its components matches one of these names, compared case-insensitively and ignoring the code points that HFS+ ignores, as git does. For example, [".git"] keeps the working tree writable while hooks and config stay read-only. Setting ReadonlyNames selects the builtin driver in auto mode, and fails with the OpenSSH driver or on Windows. Setting the times of a read-only name to its current mtime is a no-op that succeeds, because this is how the Lima guest agent triggers IN_ATTRIB for host changes (mountInotify). Addresses #6 for Linux and macOS hosts. Assisted-by: Claude Opus 5.5 (1M context) Signed-off-by: Sylvain Zimmer --- go.mod | 2 +- pkg/reversesshfs/reversesshfs.go | 40 ++- pkg/reversesshfs/rooted.go | 448 +++++++++++++++++++++++++ pkg/reversesshfs/rooted_darwin.go | 34 ++ pkg/reversesshfs/rooted_darwin_test.go | 10 + pkg/reversesshfs/rooted_e2e_test.go | 173 ++++++++++ pkg/reversesshfs/rooted_linux.go | 53 +++ pkg/reversesshfs/rooted_linux_test.go | 54 +++ pkg/reversesshfs/rooted_others.go | 18 + pkg/reversesshfs/rooted_test.go | 360 ++++++++++++++++++++ 10 files changed, 1182 insertions(+), 10 deletions(-) create mode 100644 pkg/reversesshfs/rooted.go create mode 100644 pkg/reversesshfs/rooted_darwin.go create mode 100644 pkg/reversesshfs/rooted_darwin_test.go create mode 100644 pkg/reversesshfs/rooted_e2e_test.go create mode 100644 pkg/reversesshfs/rooted_linux.go create mode 100644 pkg/reversesshfs/rooted_linux_test.go create mode 100644 pkg/reversesshfs/rooted_others.go create mode 100644 pkg/reversesshfs/rooted_test.go diff --git a/go.mod b/go.mod index 450e3da..252115b 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/pkg/sftp v1.13.11 github.com/sirupsen/logrus v1.9.4 github.com/urfave/cli/v2 v2.27.7 + golang.org/x/sys v0.47.0 ) require ( @@ -14,5 +15,4 @@ require ( github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 // indirect golang.org/x/crypto v0.54.0 // indirect - golang.org/x/sys v0.47.0 // indirect ) diff --git a/pkg/reversesshfs/reversesshfs.go b/pkg/reversesshfs/reversesshfs.go index aa70cea..238f914 100644 --- a/pkg/reversesshfs/reversesshfs.go +++ b/pkg/reversesshfs/reversesshfs.go @@ -40,6 +40,10 @@ type ReverseSSHFS struct { sshCmd *exec.Cmd opensshSftpServerCmd *exec.Cmd SSHFSAdditionalArgs []string + + // ReadonlyNames makes a path read-only when any of its components is one of these names + // (compared case-insensitively). Requires DriverBuiltin on Linux or macOS. + ReadonlyNames []string } func (rsf *ReverseSSHFS) Prepare() error { @@ -158,6 +162,10 @@ func (rsf *ReverseSSHFS) Start() error { case DriverBuiltin, DriverOpensshSftpServer: // NOP case "", DriverAuto: + if len(rsf.ReadonlyNames) > 0 { + driver = DriverBuiltin + break + } var err error driver, opensshSftpServerBinary, err = DetectDriver(opensshSftpServerBinary) if err != nil { @@ -167,7 +175,13 @@ func (rsf *ReverseSSHFS) Start() error { default: return fmt.Errorf("unknown driver %q", driver) } - var builtinSftpServer *sftp.Server + if len(rsf.ReadonlyNames) > 0 && driver != DriverBuiltin { + return fmt.Errorf("ReadonlyNames requires driver %q, got %q", DriverBuiltin, driver) + } + var ( + builtinSftpServer interface{ Serve() error } + rooted *rootedHandlers + ) switch driver { case DriverBuiltin: stdinPipe, err := rsf.sshCmd.StdinPipe() @@ -182,15 +196,20 @@ func (rsf *ReverseSSHFS) Start() error { ReadCloser: stdoutPipe, WriteCloser: stdinPipe, } - var sftpOpts []sftp.ServerOption - if rsf.Readonly { - sftpOpts = append(sftpOpts, sftp.ReadOnly()) + if runtime.GOOS == "windows" { + if len(rsf.ReadonlyNames) > 0 { + return errors.New("ReadonlyNames is not supported on Windows") + } + var sftpOpts []sftp.ServerOption + if rsf.Readonly { + sftpOpts = append(sftpOpts, sftp.ReadOnly()) + } + // NOTE: sftp.NewServer doesn't support specifying the root. + // https://github.com/pkg/sftp/pull/238 + builtinSftpServer, err = sftp.NewServer(stdio, sftpOpts...) + } else { + builtinSftpServer, rooted, err = newRootedServer(stdio, rsf.LocalPath, rsf.Readonly, rsf.ReadonlyNames) } - // NOTE: sftp.NewServer doesn't support specifying the root. - // https://github.com/pkg/sftp/pull/238 - // - // TODO: use sftp.NewRequestServer with custom handlers to mitigate potential vulnerabilities. - builtinSftpServer, err = sftp.NewServer(stdio, sftpOpts...) if err != nil { return err } @@ -234,6 +253,9 @@ func (rsf *ReverseSSHFS) Start() error { switch driver { case DriverBuiltin: go func() { + if rooted != nil { + defer rooted.Close() + } if srvErr := builtinSftpServer.Serve(); srvErr != nil { if errors.Is(srvErr, io.EOF) { logrus.WithError(srvErr).Debugf("sftp server for %v exited with EOF (negligible)", rsf.LocalPath) diff --git a/pkg/reversesshfs/rooted.go b/pkg/reversesshfs/rooted.go new file mode 100644 index 0000000..f6796a1 --- /dev/null +++ b/pkg/reversesshfs/rooted.go @@ -0,0 +1,448 @@ +//go:build linux || darwin + +package reversesshfs + +import ( + "errors" + "io" + "os" + "path" + "path/filepath" + "strings" + + "github.com/pkg/sftp" + "golang.org/x/sys/unix" +) + +// rootedHandlers serves only the files under rootPath. +// +// Reads go through os.Root, which follows symlinks but never outside the root. +// Writes open the parent directory one component at a time with O_NOFOLLOW, +// so they never follow a symlink, and are denied when any path component +// matches readonlyNames. Following no symlink on writes is what prevents +// the client from swapping a directory for a symlink into a read-only one. +type rootedHandlers struct { + rootPath string // slash-separated, cleaned + root *os.Root + rootFD int + readonly bool + readonlyNames []string +} + +func newRootedServer(rwc io.ReadWriteCloser, localPath string, readonly bool, readonlyNames []string) (*sftp.RequestServer, *rootedHandlers, error) { + root, err := os.OpenRoot(localPath) + if err != nil { + return nil, nil, err + } + rootFD, err := unix.Open(localPath, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) + if err != nil { + root.Close() + return nil, nil, &os.PathError{Op: "open", Path: localPath, Err: err} + } + h := &rootedHandlers{ + rootPath: path.Clean(filepath.ToSlash(localPath)), + root: root, + rootFD: rootFD, + readonly: readonly, + readonlyNames: readonlyNames, + } + handlers := sftp.Handlers{FileGet: h, FilePut: h, FileCmd: h, FileList: h} + srv := sftp.NewRequestServer(rwc, handlers, sftp.WithStartDirectory(h.rootPath)) + return srv, h, nil +} + +func (h *rootedHandlers) Close() error { + return errors.Join(h.root.Close(), unix.Close(h.rootFD)) +} + +// rel maps a request path to a path relative to the root. +// The result has no "." or ".." component, except "." for the root itself. +func (h *rootedHandlers) rel(p string) (string, error) { + if !path.IsAbs(p) { + p = path.Join(h.rootPath, p) + } + p = path.Clean(p) + if p == h.rootPath { + return ".", nil + } + prefix := h.rootPath + if prefix != "/" { + prefix += "/" + } + if r, ok := strings.CutPrefix(p, prefix); ok { + return r, nil + } + return "", unix.EACCES +} + +func (h *rootedHandlers) writableRel(p string) (string, error) { + if h.readonly { + return "", unix.EACCES + } + r, err := h.rel(p) + if err != nil { + return "", err + } + if h.isReadonlyName(r) { + return "", unix.EACCES + } + return r, nil +} + +func (h *rootedHandlers) isReadonlyName(rel string) bool { + for _, c := range strings.Split(rel, "/") { + for _, name := range h.readonlyNames { + if sameName(c, name) { + return true + } + } + } + return false +} + +// sameName reports whether a file name may refer to the same entry as name +// on a case-insensitive (APFS, HFS+) file system. +// The ignored code points are the ones listed in next_hfs_char() of git's utf8.c. +func sameName(s, name string) bool { + s = strings.Map(func(r rune) rune { + switch { + case r >= 0x200c && r <= 0x200f, r >= 0x202a && r <= 0x202e, r >= 0x206a && r <= 0x206f, r == 0xfeff: + return -1 + } + return r + }, s) + return strings.EqualFold(s, name) +} + +// openParent returns a directory fd for the parent of rel, and the base name. +// The caller must close the fd. +func (h *rootedHandlers) openParent(rel string) (int, string, error) { + fd, err := unix.Dup(h.rootFD) + if err != nil { + return -1, "", err + } + if rel == "." { + return fd, ".", nil + } + dir, base := path.Split(rel) + if dir != "" { + for _, c := range strings.Split(strings.TrimSuffix(dir, "/"), "/") { + if c == "" || c == "." || c == ".." { + unix.Close(fd) + return -1, "", unix.EACCES + } + next, err := unix.Openat(fd, c, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC|unix.O_NONBLOCK, 0) + unix.Close(fd) + if err != nil { + return -1, "", err + } + fd = next + } + } + return fd, base, nil +} + +// Fileread implements sftp.FileReader. +func (h *rootedHandlers) Fileread(r *sftp.Request) (io.ReaderAt, error) { + rel, err := h.rel(r.Filepath) + if err != nil { + return nil, err + } + return h.root.Open(rel) +} + +// Filewrite implements sftp.FileWriter. +func (h *rootedHandlers) Filewrite(r *sftp.Request) (io.WriterAt, error) { + return h.openFile(r) +} + +// OpenFile implements sftp.OpenFileWriter. +func (h *rootedHandlers) OpenFile(r *sftp.Request) (sftp.WriterAtReaderAt, error) { + return h.openFile(r) +} + +func (h *rootedHandlers) openFile(r *sftp.Request) (*os.File, error) { + rel, err := h.writableRel(r.Filepath) + if err != nil { + return nil, err + } + pf := r.Pflags() + flags := unix.O_NOFOLLOW | unix.O_CLOEXEC + switch { + case pf.Read && (pf.Write || pf.Append): + flags |= unix.O_RDWR + case pf.Write || pf.Append: + flags |= unix.O_WRONLY + default: + flags |= unix.O_RDONLY + } + // O_APPEND is not set, as it conflicts with WriteAt; the client sends offsets. + if pf.Creat { + flags |= unix.O_CREAT + } + if pf.Trunc { + flags |= unix.O_TRUNC + } + if pf.Excl { + flags |= unix.O_EXCL + } + var mode uint32 = 0o644 + if r.AttrFlags().Permissions { + mode = r.Attributes().Mode & 0o7777 + } + dirfd, base, err := h.openParent(rel) + if err != nil { + return nil, err + } + defer unix.Close(dirfd) + fd, err := unix.Openat(dirfd, base, flags, mode) + if err != nil { + return nil, &os.PathError{Op: "open", Path: r.Filepath, Err: err} + } + return os.NewFile(uintptr(fd), r.Filepath), nil +} + +// Filecmd implements sftp.FileCmder. +func (h *rootedHandlers) Filecmd(r *sftp.Request) error { + switch r.Method { + case "Setstat": + return h.setstat(r) + case "Rename": + return h.rename(r, true) + case "Link": + return h.twoPaths(r.Filepath, r.Target, func(oldfd int, oldBase string, newfd int, newBase string) error { + return unix.Linkat(oldfd, oldBase, newfd, newBase, 0) + }) + } + // Symlink has the link target in Filepath and the link path in Target. + p := r.Filepath + if r.Method == "Symlink" { + p = r.Target + } + rel, err := h.writableRel(p) + if err != nil { + return err + } + dirfd, base, err := h.openParent(rel) + if err != nil { + return err + } + defer unix.Close(dirfd) + switch r.Method { + case "Rmdir": + err = unix.Unlinkat(dirfd, base, unix.AT_REMOVEDIR) + case "Remove": + err = unix.Unlinkat(dirfd, base, 0) + case "Mkdir": + var mode uint32 = 0o755 + if r.AttrFlags().Permissions { + mode = r.Attributes().Mode & 0o7777 + } + err = unix.Mkdirat(dirfd, base, mode) + case "Symlink": + err = unix.Symlinkat(r.Filepath, dirfd, base) + default: + return sftp.ErrSSHFxOpUnsupported + } + if err != nil { + return &os.PathError{Op: strings.ToLower(r.Method), Path: p, Err: err} + } + return nil +} + +// PosixRename implements sftp.PosixRenameFileCmder. +func (h *rootedHandlers) PosixRename(r *sftp.Request) error { + return h.rename(r, false) +} + +func (h *rootedHandlers) rename(r *sftp.Request, noReplace bool) error { + return h.twoPaths(r.Filepath, r.Target, func(oldfd int, oldBase string, newfd int, newBase string) error { + if noReplace { + var st unix.Stat_t + if err := unix.Fstatat(newfd, newBase, &st, unix.AT_SYMLINK_NOFOLLOW); err == nil { + return os.ErrExist + } + } + return unix.Renameat(oldfd, oldBase, newfd, newBase) + }) +} + +func (h *rootedHandlers) twoPaths(oldPath, newPath string, f func(oldfd int, oldBase string, newfd int, newBase string) error) error { + oldRel, err := h.writableRel(oldPath) + if err != nil { + return err + } + newRel, err := h.writableRel(newPath) + if err != nil { + return err + } + oldfd, oldBase, err := h.openParent(oldRel) + if err != nil { + return err + } + defer unix.Close(oldfd) + newfd, newBase, err := h.openParent(newRel) + if err != nil { + return err + } + defer unix.Close(newfd) + if err := f(oldfd, oldBase, newfd, newBase); err != nil { + return &os.LinkError{Op: "rename", Old: oldPath, New: newPath, Err: err} + } + return nil +} + +func (h *rootedHandlers) setstat(r *sftp.Request) error { + if h.isNoopTimes(r) { + return nil + } + rel, err := h.writableRel(r.Filepath) + if err != nil { + return err + } + dirfd, base, err := h.openParent(rel) + if err != nil { + return err + } + defer unix.Close(dirfd) + flags := r.AttrFlags() + attrs := r.Attributes() + if flags.Size { + fd, err := unix.Openat(dirfd, base, unix.O_WRONLY|unix.O_NOFOLLOW|unix.O_CLOEXEC|unix.O_NONBLOCK, 0) + if err != nil { + return &os.PathError{Op: "truncate", Path: r.Filepath, Err: err} + } + err = unix.Ftruncate(fd, int64(attrs.Size)) + unix.Close(fd) + if err != nil { + return &os.PathError{Op: "truncate", Path: r.Filepath, Err: err} + } + } + if flags.Permissions { + if err := fchmodatNoFollow(dirfd, base, attrs.Mode&0o7777); err != nil { + return &os.PathError{Op: "chmod", Path: r.Filepath, Err: err} + } + } + if flags.UidGid { + if err := unix.Fchownat(dirfd, base, int(attrs.UID), int(attrs.GID), unix.AT_SYMLINK_NOFOLLOW); err != nil { + return &os.PathError{Op: "chown", Path: r.Filepath, Err: err} + } + } + if flags.Acmodtime { + ts := []unix.Timespec{ + unix.NsecToTimespec(int64(attrs.Atime) * 1e9), + unix.NsecToTimespec(int64(attrs.Mtime) * 1e9), + } + if err := unix.UtimesNanoAt(dirfd, base, ts, unix.AT_SYMLINK_NOFOLLOW); err != nil { + return &os.PathError{Op: "chtimes", Path: r.Filepath, Err: err} + } + } + return nil +} + +// isNoopTimes reports whether r only sets the access and modification times of a +// read-only name to its current modification time. Such a request is answered +// without touching the file, so that the guest kernel still emits IN_ATTRIB: +// this is how the guest agent relays host inotify events (mountInotify). +func (h *rootedHandlers) isNoopTimes(r *sftp.Request) bool { + flags := r.AttrFlags() + if h.readonly || flags.Size || flags.UidGid || flags.Permissions || !flags.Acmodtime { + return false + } + rel, err := h.rel(r.Filepath) + if err != nil || !h.isReadonlyName(rel) { + return false + } + fi, err := h.root.Lstat(rel) + if err != nil { + return false + } + // SFTP v3 times are in seconds. + mtime := uint32(fi.ModTime().Unix()) + attrs := r.Attributes() + return attrs.Atime == mtime && attrs.Mtime == mtime +} + +// StatVFS implements sftp.StatVFSFileCmder. +func (h *rootedHandlers) StatVFS(r *sftp.Request) (*sftp.StatVFS, error) { + if _, err := h.rel(r.Filepath); err != nil { + return nil, err + } + var st unix.Statfs_t + if err := unix.Fstatfs(h.rootFD, &st); err != nil { + return nil, err + } + return statVFS(&st), nil +} + +// Filelist implements sftp.FileLister. +func (h *rootedHandlers) Filelist(r *sftp.Request) (sftp.ListerAt, error) { + rel, err := h.rel(r.Filepath) + if err != nil { + return nil, err + } + switch r.Method { + case "List": + f, err := h.root.Open(rel) + if err != nil { + return nil, err + } + defer f.Close() + fis, err := f.Readdir(-1) + if err != nil { + return nil, err + } + return listerAt(fis), nil + case "Stat": + fi, err := h.root.Stat(rel) + if err != nil { + return nil, err + } + return listerAt{fi}, nil + } + return nil, sftp.ErrSSHFxOpUnsupported +} + +// Lstat implements sftp.LstatFileLister. +func (h *rootedHandlers) Lstat(r *sftp.Request) (sftp.ListerAt, error) { + rel, err := h.rel(r.Filepath) + if err != nil { + return nil, err + } + fi, err := h.root.Lstat(rel) + if err != nil { + return nil, err + } + return listerAt{fi}, nil +} + +// Readlink implements sftp.ReadlinkFileLister. +func (h *rootedHandlers) Readlink(p string) (string, error) { + rel, err := h.rel(p) + if err != nil { + return "", err + } + return h.root.Readlink(rel) +} + +// RealPath implements sftp.RealPathFileLister. +// It does not resolve symlinks, and does not access the file system. +func (h *rootedHandlers) RealPath(p string) (string, error) { + if !path.IsAbs(p) { + p = path.Join(h.rootPath, p) + } + return path.Clean(p), nil +} + +type listerAt []os.FileInfo + +func (l listerAt) ListAt(ls []os.FileInfo, offset int64) (int, error) { + if offset >= int64(len(l)) { + return 0, io.EOF + } + n := copy(ls, l[offset:]) + if n < len(ls) { + return n, io.EOF + } + return n, nil +} diff --git a/pkg/reversesshfs/rooted_darwin.go b/pkg/reversesshfs/rooted_darwin.go new file mode 100644 index 0000000..d81d06e --- /dev/null +++ b/pkg/reversesshfs/rooted_darwin.go @@ -0,0 +1,34 @@ +package reversesshfs + +import ( + "github.com/pkg/sftp" + "golang.org/x/sys/unix" +) + +func statVFS(st *unix.Statfs_t) *sftp.StatVFS { + return &sftp.StatVFS{ + Bsize: uint64(st.Bsize), + Frsize: uint64(st.Bsize), + Blocks: st.Blocks, + Bfree: st.Bfree, + Bavail: st.Bavail, + Files: st.Files, + Ffree: st.Ffree, + Favail: st.Ffree, + Flag: uint64(st.Flags), + Namemax: 1024, + } +} + +// fchmodatNoFollow never follows a symlink at base, and fails on one, as on Linux. +// If base is replaced by a symlink after the check, fchmodat changes the mode of the symlink itself. +func fchmodatNoFollow(dirfd int, base string, mode uint32) error { + var st unix.Stat_t + if err := unix.Fstatat(dirfd, base, &st, unix.AT_SYMLINK_NOFOLLOW); err != nil { + return err + } + if st.Mode&unix.S_IFMT == unix.S_IFLNK { + return unix.ELOOP + } + return unix.Fchmodat(dirfd, base, mode, unix.AT_SYMLINK_NOFOLLOW) +} diff --git a/pkg/reversesshfs/rooted_darwin_test.go b/pkg/reversesshfs/rooted_darwin_test.go new file mode 100644 index 0000000..62e7b8a --- /dev/null +++ b/pkg/reversesshfs/rooted_darwin_test.go @@ -0,0 +1,10 @@ +package reversesshfs + +import ( + "os" + "syscall" +) + +func ctime(fi os.FileInfo) syscall.Timespec { + return fi.Sys().(*syscall.Stat_t).Ctimespec +} diff --git a/pkg/reversesshfs/rooted_e2e_test.go b/pkg/reversesshfs/rooted_e2e_test.go new file mode 100644 index 0000000..d572646 --- /dev/null +++ b/pkg/reversesshfs/rooted_e2e_test.go @@ -0,0 +1,173 @@ +//go:build linux + +package reversesshfs + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/lima-vm/sshocker/pkg/util" + "golang.org/x/sys/unix" +) + +// TestRootedSSHFS mounts the rooted server with a real sshfs in slave mode. +// It runs only when $SSHFS names an sshfs binary and FUSE is usable. +func TestRootedSSHFS(t *testing.T) { + sshfs := os.Getenv("SSHFS") + if sshfs == "" { + t.Skip("SSHFS is not set") + } + tmp := t.TempDir() + root := filepath.Join(tmp, "root") + mnt := filepath.Join(tmp, "mnt") + for _, d := range []string{filepath.Join(root, "src"), mnt} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + git := func(dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", dir}, args...)...) + cmd.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_AUTHOR_NAME=a", "GIT_AUTHOR_EMAIL=a@a", "GIT_COMMITTER_NAME=a", "GIT_COMMITTER_EMAIL=a@a") + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + return string(out) + } + if err := os.WriteFile(filepath.Join(root, "src", "main.go"), []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + git(root, "init", "-q") + git(root, "add", ".") + git(root, "commit", "-q", "-m", "init") + + cmd := exec.Command(sshfs, ":"+root, mnt, "-f", "-o", "slave") + stdin, err := cmd.StdinPipe() + if err != nil { + t.Fatal(err) + } + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + cmd.Stderr = os.Stderr + srv, h, err := newRootedServer(&util.RWC{ReadCloser: stdout, WriteCloser: stdin}, root, false, []string{".git"}) + if err != nil { + t.Fatal(err) + } + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + go func() { + _ = srv.Serve() + _ = h.Close() + }() + t.Cleanup(func() { + _ = exec.Command("fusermount3", "-u", mnt).Run() + _ = cmd.Wait() + }) + for i := 0; ; i++ { + if _, err := os.Stat(filepath.Join(mnt, "src", "main.go")); err == nil { + break + } + if i == 50 { + t.Fatal("sshfs did not mount") + } + time.Sleep(100 * time.Millisecond) + } + + sh := func(script string) error { + c := exec.Command("sh", "-euc", script) + c.Dir = mnt + out, err := c.CombinedOutput() + t.Logf("%s: %v: %s", script, err, out) + return err + } + allowed := []string{ + "echo x > src/new.go && cat src/new.go", + "mkdir -p a/b/c && touch a/b/c/f && rm -rf a", + "sed -i s/main/foo/ src/main.go", + "cp -a src src2 && rm -r src2", + "chmod 600 src/new.go && truncate -s 0 src/new.go", + "ln -s main.go src/link && readlink src/link && cat src/link && rm src/link", + "mv src/new.go src/renamed.go && rm src/renamed.go", + "df .", + "cat .git/HEAD && ls .git/hooks", + "git status --short", + "git log --oneline", + } + for _, s := range allowed { + if err := sh(s); err != nil { + t.Errorf("%s: %v", s, err) + } + } + denied := []string{ + "echo evil > .git/hooks/pre-commit", + "echo evil >> .git/config", + "mv .git x", + "rm .git/HEAD", + "chmod 777 .git/config", + // Not @0: sshfs replaces a zero time with the current time. + "touch -d @1 .git/config", + "mkdir src/.git", + "ln -s .git g && echo evil > g/hooks/post-checkout", + "git add src", + "git commit -q --allow-empty -m evil", + } + for _, s := range denied { + if err := sh(s); err == nil { + t.Errorf("%s: expected an error", s) + } + } + if fi, err := os.Stat(filepath.Join(root, ".git", "config")); err != nil || fi.ModTime().Unix() == 1 { + t.Errorf(".git/config times were changed: %v, %v", fi.ModTime(), err) + } + if _, err := os.Stat(filepath.Join(root, ".git", "hooks", "pre-commit")); err == nil { + t.Error("hook was written") + } + if n := git(root, "rev-list", "--count", "HEAD"); n != "1\n" { + t.Errorf("commit count changed: %q", n) + } + + // Relay a host change the way Lima's guest agent does for mountInotify. + for _, f := range []string{".git/HEAD", "src/main.go"} { + if err := os.WriteFile(filepath.Join(root, f), []byte("ref: refs/heads/other\n"), 0o644); err != nil { + t.Fatal(err) + } + fi, err := os.Stat(filepath.Join(root, f)) + if err != nil { + t.Fatal(err) + } + if !gotAttribEvent(t, filepath.Join(mnt, f), fi.ModTime()) { + t.Errorf("%s: no IN_ATTRIB event in the mount", f) + } + } +} + +func gotAttribEvent(t *testing.T, p string, mtime time.Time) bool { + t.Helper() + fd, err := unix.InotifyInit1(unix.IN_CLOEXEC | unix.IN_NONBLOCK) + if err != nil { + t.Fatal(err) + } + defer unix.Close(fd) + if _, err := unix.InotifyAddWatch(fd, p, unix.IN_ATTRIB); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(p, mtime, mtime); err != nil { + t.Logf("chtimes %s: %v", p, err) + return false + } + buf := make([]byte, 4096) + for range 20 { + if n, err := unix.Read(fd, buf); err == nil && n > 0 { + return true + } + time.Sleep(50 * time.Millisecond) + } + return false +} diff --git a/pkg/reversesshfs/rooted_linux.go b/pkg/reversesshfs/rooted_linux.go new file mode 100644 index 0000000..7ad5fd0 --- /dev/null +++ b/pkg/reversesshfs/rooted_linux.go @@ -0,0 +1,53 @@ +package reversesshfs + +import ( + "errors" + "strconv" + + "github.com/pkg/sftp" + "golang.org/x/sys/unix" +) + +func statVFS(st *unix.Statfs_t) *sftp.StatVFS { + return &sftp.StatVFS{ + Bsize: uint64(st.Bsize), + Frsize: uint64(st.Frsize), + Blocks: st.Blocks, + Bfree: st.Bfree, + Bavail: st.Bavail, + Files: st.Files, + Ffree: st.Ffree, + Favail: st.Ffree, + Flag: uint64(st.Flags), + Namemax: uint64(st.Namelen), + } +} + +// fchmodatNoFollow never follows a symlink at base, and fails on one. +// Only fchmodat2 (kernel 6.6+) supports AT_SYMLINK_NOFOLLOW, and it fails with EOPNOTSUPP on a symlink. +func fchmodatNoFollow(dirfd int, base string, mode uint32) error { + err := unix.Fchmodat(dirfd, base, mode, unix.AT_SYMLINK_NOFOLLOW) + if !errors.Is(err, unix.EOPNOTSUPP) && !errors.Is(err, unix.ENOSYS) { + return err + } + return fchmodatOPath(dirfd, base, mode) +} + +// fchmodatOPath is the fallback of fchmodatNoFollow, as in glibc: +// fchmod does not work on an O_PATH fd, but chmod on its /proc/self/fd entry does, +// and unlike opening base for reading, needs no read permission. +func fchmodatOPath(dirfd int, base string, mode uint32) error { + fd, err := unix.Openat(dirfd, base, unix.O_PATH|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) + if err != nil { + return err + } + defer unix.Close(fd) + var st unix.Stat_t + if err := unix.Fstat(fd, &st); err != nil { + return err + } + if st.Mode&unix.S_IFMT == unix.S_IFLNK { + return unix.ELOOP + } + return unix.Chmod("/proc/self/fd/"+strconv.Itoa(fd), mode) +} diff --git a/pkg/reversesshfs/rooted_linux_test.go b/pkg/reversesshfs/rooted_linux_test.go new file mode 100644 index 0000000..b3140d4 --- /dev/null +++ b/pkg/reversesshfs/rooted_linux_test.go @@ -0,0 +1,54 @@ +package reversesshfs + +import ( + "errors" + "os" + "path/filepath" + "syscall" + "testing" + + "golang.org/x/sys/unix" +) + +func ctime(fi os.FileInfo) syscall.Timespec { + return fi.Sys().(*syscall.Stat_t).Ctim +} + +// TestFchmodatOPath runs the fallback for kernels without fchmodat2 (< 6.6) directly, +// as newer kernels only take it for symlinks. +func TestFchmodatOPath(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses permission checks") + } + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "file"), []byte("orig"), 0); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(filepath.Join(dir, "dir"), 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(filepath.Join(dir, "dir"), 0o755) }) + if err := os.Symlink("file", filepath.Join(dir, "link")); err != nil { + t.Fatal(err) + } + dirfd, err := unix.Open(dir, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) + if err != nil { + t.Fatal(err) + } + defer unix.Close(dirfd) + + for _, name := range []string{"file", "dir"} { + if err := fchmodatOPath(dirfd, name, 0o755); err != nil { + t.Fatalf("%s: %v", name, err) + } + if fi, err := os.Lstat(filepath.Join(dir, name)); err != nil || fi.Mode().Perm() != 0o755 { + t.Fatalf("%s: %v, %v", name, fi, err) + } + } + if err := fchmodatOPath(dirfd, "link", 0o600); !errors.Is(err, unix.ELOOP) { + t.Fatalf("link: expected ELOOP, got %v", err) + } + if fi, err := os.Stat(filepath.Join(dir, "file")); err != nil || fi.Mode().Perm() != 0o755 { + t.Fatalf("symlink target changed: %v, %v", fi, err) + } +} diff --git a/pkg/reversesshfs/rooted_others.go b/pkg/reversesshfs/rooted_others.go new file mode 100644 index 0000000..1a59e81 --- /dev/null +++ b/pkg/reversesshfs/rooted_others.go @@ -0,0 +1,18 @@ +//go:build !linux && !darwin + +package reversesshfs + +import ( + "errors" + "io" + + "github.com/pkg/sftp" +) + +type rootedHandlers struct{} + +func newRootedServer(io.ReadWriteCloser, string, bool, []string) (*sftp.RequestServer, *rootedHandlers, error) { + return nil, nil, errors.New("the rooted builtin sftp server is supported only on Linux and macOS") +} + +func (*rootedHandlers) Close() error { return nil } diff --git a/pkg/reversesshfs/rooted_test.go b/pkg/reversesshfs/rooted_test.go new file mode 100644 index 0000000..00f9130 --- /dev/null +++ b/pkg/reversesshfs/rooted_test.go @@ -0,0 +1,360 @@ +//go:build linux || darwin + +package reversesshfs + +import ( + "io" + "os" + "path/filepath" + "sync" + "testing" + "time" + + "github.com/lima-vm/sshocker/pkg/util" + "github.com/pkg/sftp" +) + +var ( + zwnj = string(rune(0x200c)) // ignored by HFS+ + zwj = string(rune(0x200d)) + bom = string(rune(0xfeff)) +) + +// setupRooted serves /root with ".git" read-only, and returns a client +// that plays the role of a compromised guest sending arbitrary requests. +// It also creates the symlinks "gitlink" -> ".git" and "configlink" -> ".git/config". +func setupRooted(t *testing.T, readonly bool) (*sftp.Client, string) { + t.Helper() + tmp := t.TempDir() + root := filepath.Join(tmp, "root") + for _, d := range []string{filepath.Join(root, ".git", "hooks"), filepath.Join(root, "src"), filepath.Join(tmp, "outside")} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + for _, f := range []string{filepath.Join(root, ".git", "config"), filepath.Join(root, "src", "main.go"), filepath.Join(tmp, "outside", "secret")} { + if err := os.WriteFile(f, []byte("orig"), 0o644); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink(".git", filepath.Join(root, "gitlink")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(".git", "config"), filepath.Join(root, "configlink")); err != nil { + t.Fatal(err) + } + c2sR, c2sW := io.Pipe() + s2cR, s2cW := io.Pipe() + srv, h, err := newRootedServer(&util.RWC{ReadCloser: c2sR, WriteCloser: s2cW}, root, readonly, []string{".git"}) + if err != nil { + t.Fatal(err) + } + done := make(chan struct{}) + go func() { + defer close(done) + _ = srv.Serve() + _ = h.Close() + }() + client, err := sftp.NewClientPipe(s2cR, c2sW) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + srv.Close() + client.Close() + <-done + }) + return client, root +} + +func assertUnchanged(t *testing.T, root string) { + t.Helper() + fi, err := os.Lstat(filepath.Join(root, ".git")) + if err != nil || !fi.IsDir() { + t.Fatalf(".git is no longer a directory: %v, %v", fi, err) + } + b, err := os.ReadFile(filepath.Join(root, ".git", "config")) + if err != nil || string(b) != "orig" { + t.Fatalf(".git/config changed: %q, %v", b, err) + } + fi, err = os.Stat(filepath.Join(root, ".git", "config")) + if err != nil || fi.Mode().Perm() != 0o644 { + t.Fatalf(".git/config mode changed: %v, %v", fi, err) + } + b, err = os.ReadFile(filepath.Join(filepath.Dir(root), "outside", "secret")) + if err != nil || string(b) != "orig" { + t.Fatalf("outside/secret changed: %q, %v", b, err) + } + for dir, n := range map[string]int{".git": 2, ".git/hooks": 0, "../outside": 1} { + entries, err := os.ReadDir(filepath.Join(root, dir)) + if err != nil || len(entries) != n { + t.Fatalf("%s entries changed: %v, %v", dir, entries, err) + } + } +} + +func TestRootedAllowsNormalOperations(t *testing.T) { + c, root := setupRooted(t, false) + p := func(s string) string { return filepath.Join(root, s) } + + f, err := c.Create(p("src/new.go")) + if err != nil { + t.Fatal(err) + } + if _, err := f.Write([]byte("hello")); err != nil { + t.Fatal(err) + } + if err := f.Chmod(0o600); err != nil { + t.Fatal(err) + } + f.Close() + if err := c.Truncate(p("src/new.go"), 2); err != nil { + t.Fatal(err) + } + mtime := time.Unix(1_000_000_000, 0) + if err := c.Chtimes(p("src/new.go"), mtime, mtime); err != nil { + t.Fatal(err) + } + if err := c.Mkdir(p("dir")); err != nil { + t.Fatal(err) + } + if err := c.PosixRename(p("src/new.go"), p("dir/renamed.go")); err != nil { + t.Fatal(err) + } + if err := c.Rename(p("dir/renamed.go"), p("src/main.go")); err == nil { + t.Fatal("Rename must not replace an existing file") + } + if err := c.Symlink("renamed.go", p("dir/link")); err != nil { + t.Fatal(err) + } + if target, err := c.ReadLink(p("dir/link")); err != nil || target != "renamed.go" { + t.Fatalf("ReadLink: %q, %v", target, err) + } + fi, err := c.Stat(p("dir/link")) + if err != nil { + t.Fatal(err) + } + if fi.Size() != 2 || fi.Mode().Perm() != 0o600 || !fi.ModTime().Equal(mtime) { + t.Fatalf("unexpected stat: size=%d mode=%v mtime=%v", fi.Size(), fi.Mode(), fi.ModTime()) + } + if err := c.Link(p("dir/renamed.go"), p("dir/hardlink")); err != nil { + t.Fatal(err) + } + entries, err := c.ReadDir(p("dir")) + if err != nil || len(entries) != 3 { + t.Fatalf("ReadDir: %v, %v", entries, err) + } + if _, err := c.StatVFS(root); err != nil { + t.Fatal(err) + } + for _, s := range []string{"dir/link", "dir/hardlink", "dir/renamed.go"} { + if err := c.Remove(p(s)); err != nil { + t.Fatal(err) + } + } + if err := c.RemoveDirectory(p("dir")); err != nil { + t.Fatal(err) + } + // Reading the protected directory, also through an in-root symlink, is allowed. + rf, err := c.Open(p("configlink")) + if err != nil { + t.Fatal(err) + } + b, err := io.ReadAll(rf) + rf.Close() + if err != nil || string(b) != "orig" { + t.Fatalf("read .git/config: %q, %v", b, err) + } + assertUnchanged(t, root) +} + +func TestRootedChmodWithoutReadPermission(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses permission checks") + } + c, root := setupRooted(t, false) + for _, s := range []string{"src/main.go", "src"} { + p := filepath.Join(root, s) + if err := os.Chmod(p, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(p, 0o755) }) + if err := c.Chmod(p, 0o755); err != nil { + t.Fatalf("%s: %v", s, err) + } + if fi, err := os.Stat(p); err != nil || fi.Mode().Perm() != 0o755 { + t.Fatalf("%s: %v, %v", s, fi, err) + } + } +} + +func TestRootedDeniesProtectedWrites(t *testing.T) { + type env struct { + c *sftp.Client + root string + } + p := func(e env, s string) string { return filepath.Join(e.root, s) } + open := func(e env, s string, flags int) error { + f, err := e.c.OpenFile(p(e, s), flags) + if err == nil { + f.Close() + } + return err + } + create := func(e env, s string) error { return open(e, s, os.O_RDWR|os.O_CREATE|os.O_TRUNC) } + withOutsideLink := func(f func(e env) error) func(e env) error { + return func(e env) error { + if err := os.Symlink(filepath.Join(e.root, "..", "outside"), p(e, "out")); err != nil { + panic(err) + } + return f(e) + } + } + attempts := []struct { + name string + f func(e env) error + }{ + {"create hook", func(e env) error { return create(e, ".git/hooks/pre-commit") }}, + {"open config for write", func(e env) error { return open(e, ".git/config", os.O_WRONLY) }}, + // On a case-insensitive file system, these would name the existing .git. + {"upper case", func(e env) error { return e.c.Mkdir(p(e, "src/.GIT")) }}, + {"hfs ignorable", func(e env) error { return e.c.Mkdir(p(e, "src/.g"+zwnj+"it")) }}, + {"nested repo", func(e env) error { return e.c.Mkdir(p(e, "src/.git")) }}, + {"nested gitfile", func(e env) error { return create(e, "src/.git") }}, + {"via dir symlink", func(e env) error { return create(e, "gitlink/hooks/pre-commit") }}, + {"via file symlink", func(e env) error { return open(e, "configlink", os.O_WRONLY|os.O_TRUNC) }}, + {"hardlink from config", func(e env) error { return e.c.Link(p(e, ".git/config"), p(e, "src/config")) }}, + {"hardlink into hooks", func(e env) error { return e.c.Link(p(e, "src/main.go"), p(e, ".git/hooks/x")) }}, + {"symlink in hooks", func(e env) error { return e.c.Symlink("/bin/sh", p(e, ".git/hooks/x")) }}, + {"rename .git away", func(e env) error { return e.c.PosixRename(p(e, ".git"), p(e, "old")) }}, + {"rename into .git", func(e env) error { return e.c.PosixRename(p(e, "src/main.go"), p(e, ".git/hooks/x")) }}, + {"rename to nested gitfile", func(e env) error { return e.c.PosixRename(p(e, "src/main.go"), p(e, "src/.git")) }}, + {"remove config", func(e env) error { return e.c.Remove(p(e, ".git/config")) }}, + {"rmdir hooks", func(e env) error { return e.c.RemoveDirectory(p(e, ".git/hooks")) }}, + {"chmod config", func(e env) error { return e.c.Chmod(p(e, ".git/config"), 0o777) }}, + {"chmod via symlink", func(e env) error { return e.c.Chmod(p(e, "configlink"), 0o777) }}, + {"truncate config", func(e env) error { return e.c.Truncate(p(e, ".git/config"), 0) }}, + {"truncate via symlink", func(e env) error { return e.c.Truncate(p(e, "configlink"), 0) }}, + {"chtimes config", func(e env) error { return e.c.Chtimes(p(e, ".git/config"), time.Unix(0, 0), time.Unix(0, 0)) }}, + {"write outside", func(e env) error { return create(e, "../outside/secret") }}, + {"read outside", func(e env) error { return open(e, "../outside/secret", os.O_RDONLY) }}, + {"write through outside symlink", withOutsideLink(func(e env) error { return create(e, "out/secret") })}, + {"read through outside symlink", withOutsideLink(func(e env) error { return open(e, "out/secret", os.O_RDONLY) })}, + } + for _, a := range attempts { + t.Run(a.name, func(t *testing.T) { + c, root := setupRooted(t, false) + if err := a.f(env{c, root}); err == nil { + t.Error("expected an error") + } + assertUnchanged(t, root) + }) + } +} + +// TestRootedNoopTimes checks the utimes request used by Lima's mountInotify. +func TestRootedNoopTimes(t *testing.T) { + c, root := setupRooted(t, false) + config := filepath.Join(root, ".git", "config") + mtime := time.Unix(1_000_000_000, 0) + if err := os.Chtimes(config, mtime, mtime); err != nil { + t.Fatal(err) + } + before, err := os.Lstat(config) + if err != nil { + t.Fatal(err) + } + time.Sleep(10 * time.Millisecond) + if err := c.Chtimes(config, mtime, mtime); err != nil { + t.Fatalf("utimes to the current mtime: %v", err) + } + after, err := os.Lstat(config) + if err != nil { + t.Fatal(err) + } + if ctime(before) != ctime(after) || !after.ModTime().Equal(mtime) { + t.Fatalf("file was modified: ctime %v -> %v, mtime %v", ctime(before), ctime(after), after.ModTime()) + } + for _, times := range [][2]time.Time{ + {mtime, mtime.Add(time.Second)}, + {mtime.Add(time.Second), mtime}, + {mtime.Add(time.Second), mtime.Add(time.Second)}, + } { + if err := c.Chtimes(config, times[0], times[1]); err == nil { + t.Errorf("utimes to %v: expected an error", times) + } + } + if err := c.Chtimes(filepath.Join(root, ".git", "missing"), mtime, mtime); err == nil { + t.Error("utimes on a missing file: expected an error") + } + assertUnchanged(t, root) +} + +func TestRootedReadonly(t *testing.T) { + c, root := setupRooted(t, true) + if _, err := c.Create(filepath.Join(root, "src", "new.go")); err == nil { + t.Error("expected an error") + } + if err := c.Mkdir(filepath.Join(root, "dir")); err == nil { + t.Error("expected an error") + } + if _, err := c.Stat(filepath.Join(root, "src", "main.go")); err != nil { + t.Error(err) + } +} + +// TestRootedSymlinkSwapRace swaps a directory for a symlink to .git +// while the client keeps writing into it. +func TestRootedSymlinkSwapRace(t *testing.T) { + c, root := setupRooted(t, false) + d := filepath.Join(root, "d") + stop := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + for { + select { + case <-stop: + return + default: + } + _ = os.RemoveAll(d) + _ = os.Mkdir(d, 0o755) + _ = os.RemoveAll(d) + _ = os.Symlink(".git", d) + } + }() + var created int + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + if f, err := c.Create(filepath.Join(d, "pwn")); err == nil { + f.Close() + created++ + } + _ = c.Mkdir(filepath.Join(d, "pwndir")) + _ = c.Symlink("x", filepath.Join(d, "pwnlink")) + _ = c.Chmod(filepath.Join(d, "config"), 0o777) + _ = c.Truncate(filepath.Join(d, "config"), 0) + _ = c.Remove(filepath.Join(d, "config")) + } + close(stop) + wg.Wait() + if created == 0 { + t.Fatal("no write reached d while it was a directory, the race was not exercised") + } + assertUnchanged(t, root) +} + +func TestSameName(t *testing.T) { + for _, s := range []string{".git", ".GIT", ".Git", ".g" + zwnj + "it", bom + ".git", ".git" + zwj} { + if !sameName(s, ".git") { + t.Errorf("%q should match .git", s) + } + } + for _, s := range []string{".gitignore", "git", ".git.", "x.git", ".gi"} { + if sameName(s, ".git") { + t.Errorf("%q should not match .git", s) + } + } +}