From 9bceda18110059d326917657f23bcd6c10e8a001 Mon Sep 17 00:00:00 2001 From: Sylvain Zimmer Date: Wed, 23 Sep 2026 22:52:23 +0200 Subject: [PATCH 1/2] 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) + } + } +} From a7bbfb34b6c615dd71f23b4d5051658e7aecf978 Mon Sep 17 00:00:00 2001 From: Sylvain Zimmer Date: Wed, 23 Sep 2026 22:54:09 +0200 Subject: [PATCH 2/2] reversesshfs: add ExpectRemove ExpectRemove(hostPath) makes the next Remove or Rmdir of hostPath from the guest succeed without touching the host, if it arrives within 5 seconds. It lets the caller relay a deletion done on the host: the guest removes the path in the mount, so the guest kernel emits IN_DELETE, while the host keeps the path if it was created again in the meantime. It returns false when unsupported, i.e. without the rooted builtin server of Linux and macOS hosts. Assisted-by: Claude Opus 5.5 (1M context) Signed-off-by: Sylvain Zimmer --- pkg/reversesshfs/reversesshfs.go | 17 +++++++ pkg/reversesshfs/rooted.go | 42 ++++++++++++++++ pkg/reversesshfs/rooted_e2e_test.go | 75 +++++++++++++++++++++++++++++ pkg/reversesshfs/rooted_others.go | 2 + pkg/reversesshfs/rooted_test.go | 61 ++++++++++++++++++++++- 5 files changed, 196 insertions(+), 1 deletion(-) diff --git a/pkg/reversesshfs/reversesshfs.go b/pkg/reversesshfs/reversesshfs.go index 238f914..e84e237 100644 --- a/pkg/reversesshfs/reversesshfs.go +++ b/pkg/reversesshfs/reversesshfs.go @@ -13,6 +13,7 @@ import ( "runtime" "strconv" "strings" + "sync/atomic" "github.com/lima-vm/sshocker/pkg/ssh" "github.com/lima-vm/sshocker/pkg/util" @@ -44,6 +45,21 @@ type ReverseSSHFS struct { // 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 + + rooted atomic.Pointer[rootedHandlers] +} + +// ExpectRemove makes the next removal of hostPath requested by the remote, within a few seconds, +// succeed without touching hostPath. It is used to relay a local deletion to the remote, +// by removing the path there, which emits an inotify event on the remote. +// It returns false when unsupported, i.e., when not using DriverBuiltin on Linux or macOS. +func (rsf *ReverseSSHFS) ExpectRemove(hostPath string) bool { + h := rsf.rooted.Load() + if h == nil { + return false + } + h.expectRemove(hostPath) + return true } func (rsf *ReverseSSHFS) Prepare() error { @@ -209,6 +225,7 @@ func (rsf *ReverseSSHFS) Start() error { builtinSftpServer, err = sftp.NewServer(stdio, sftpOpts...) } else { builtinSftpServer, rooted, err = newRootedServer(stdio, rsf.LocalPath, rsf.Readonly, rsf.ReadonlyNames) + rsf.rooted.Store(rooted) } if err != nil { return err diff --git a/pkg/reversesshfs/rooted.go b/pkg/reversesshfs/rooted.go index f6796a1..be3be23 100644 --- a/pkg/reversesshfs/rooted.go +++ b/pkg/reversesshfs/rooted.go @@ -9,6 +9,8 @@ import ( "path" "path/filepath" "strings" + "sync" + "time" "github.com/pkg/sftp" "golang.org/x/sys/unix" @@ -27,8 +29,14 @@ type rootedHandlers struct { rootFD int readonly bool readonlyNames []string + + mu sync.Mutex + noopRemovals map[string]time.Time // expiry, keyed by request path } +// noopRemovalTTL bounds how long an ExpectRemove token waits for the guest. +const noopRemovalTTL = 5 * time.Second + func newRootedServer(rwc io.ReadWriteCloser, localPath string, readonly bool, readonlyNames []string) (*sftp.RequestServer, *rootedHandlers, error) { root, err := os.OpenRoot(localPath) if err != nil { @@ -45,6 +53,7 @@ func newRootedServer(rwc io.ReadWriteCloser, localPath string, readonly bool, re rootFD: rootFD, readonly: readonly, readonlyNames: readonlyNames, + noopRemovals: make(map[string]time.Time), } handlers := sftp.Handlers{FileGet: h, FilePut: h, FileCmd: h, FileList: h} srv := sftp.NewRequestServer(rwc, handlers, sftp.WithStartDirectory(h.rootPath)) @@ -55,6 +64,33 @@ func (h *rootedHandlers) Close() error { return errors.Join(h.root.Close(), unix.Close(h.rootFD)) } +// expectRemove makes the next Remove or Rmdir request for p, within noopRemovalTTL, +// succeed without touching the host. p is a host path under the root. +func (h *rootedHandlers) expectRemove(p string) { + p = path.Clean(filepath.ToSlash(p)) + now := time.Now() + h.mu.Lock() + defer h.mu.Unlock() + for k, expiry := range h.noopRemovals { + if now.After(expiry) { + delete(h.noopRemovals, k) + } + } + h.noopRemovals[p] = now.Add(noopRemovalTTL) +} + +func (h *rootedHandlers) consumeNoopRemoval(p string) bool { + p = path.Clean(p) + h.mu.Lock() + defer h.mu.Unlock() + expiry, ok := h.noopRemovals[p] + if !ok { + return false + } + delete(h.noopRemovals, p) + return time.Now().Before(expiry) +} + // 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) { @@ -213,6 +249,12 @@ func (h *rootedHandlers) Filecmd(r *sftp.Request) error { 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) }) + case "Remove", "Rmdir": + // The guest agent removes a path deleted on the host, so that the guest emits IN_DELETE. + // The path may have been created again on the host since, so it must not be removed. + if h.consumeNoopRemoval(r.Filepath) { + return nil + } } // Symlink has the link target in Filepath and the link path in Target. p := r.Filepath diff --git a/pkg/reversesshfs/rooted_e2e_test.go b/pkg/reversesshfs/rooted_e2e_test.go index d572646..5f6d8a6 100644 --- a/pkg/reversesshfs/rooted_e2e_test.go +++ b/pkg/reversesshfs/rooted_e2e_test.go @@ -146,6 +146,81 @@ func TestRootedSSHFS(t *testing.T) { t.Errorf("%s: no IN_ATTRIB event in the mount", f) } } + + // Relay a host deletion: the hostagent calls ExpectRemove, then the guest agent removes the path. + for _, tc := range []struct { + name string + recreate bool + guestSeen bool + guestListed bool + delay time.Duration + }{ + {name: "src/gone.txt", guestSeen: true}, + {name: "src/listed.txt", guestListed: true}, + {name: ".git/index.lock", guestSeen: true}, + {name: "src/later.txt", guestSeen: true, delay: 2 * time.Second}, + {name: "src/again.txt", guestSeen: true, recreate: true}, + {name: "src/unseen.txt"}, + } { + hostPath := filepath.Join(root, tc.name) + if err := os.WriteFile(hostPath, []byte("old"), 0o644); err != nil { + t.Fatal(err) + } + if tc.guestSeen { + if _, err := os.Stat(filepath.Join(mnt, tc.name)); err != nil { + t.Fatal(err) + } + } + if tc.guestListed { + if _, err := os.ReadDir(filepath.Dir(filepath.Join(mnt, tc.name))); err != nil { + t.Fatal(err) + } + } + if err := os.Remove(hostPath); err != nil { + t.Fatal(err) + } + if tc.recreate { + if err := os.WriteFile(hostPath, []byte("new"), 0o644); err != nil { + t.Fatal(err) + } + } + time.Sleep(tc.delay) + h.expectRemove(hostPath) + got := gotDeleteEvent(t, filepath.Join(mnt, tc.name)) + t.Logf("%s (seen: %v, listed: %v, delay: %v): IN_DELETE: %v", tc.name, tc.guestSeen, tc.guestListed, tc.delay, got) + if tc.guestSeen && !got { + t.Errorf("%s: no IN_DELETE event in the mount", tc.name) + } + if tc.recreate { + if b, err := os.ReadFile(hostPath); err != nil || string(b) != "new" { + t.Errorf("%s: the recreated file was modified: %q, %v", tc.name, b, err) + } + } + } +} + +func gotDeleteEvent(t *testing.T, p string) 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, filepath.Dir(p), unix.IN_DELETE); err != nil { + t.Fatal(err) + } + if err := os.Remove(p); err != nil { + t.Logf("remove %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 } func gotAttribEvent(t *testing.T, p string, mtime time.Time) bool { diff --git a/pkg/reversesshfs/rooted_others.go b/pkg/reversesshfs/rooted_others.go index 1a59e81..6a952bc 100644 --- a/pkg/reversesshfs/rooted_others.go +++ b/pkg/reversesshfs/rooted_others.go @@ -16,3 +16,5 @@ func newRootedServer(io.ReadWriteCloser, string, bool, []string) (*sftp.RequestS } func (*rootedHandlers) Close() error { return nil } + +func (*rootedHandlers) expectRemove(string) {} diff --git a/pkg/reversesshfs/rooted_test.go b/pkg/reversesshfs/rooted_test.go index 00f9130..b8b5edb 100644 --- a/pkg/reversesshfs/rooted_test.go +++ b/pkg/reversesshfs/rooted_test.go @@ -24,6 +24,12 @@ var ( // 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() + c, root, _ := setupRootedHandlers(t, readonly) + return c, root +} + +func setupRootedHandlers(t *testing.T, readonly bool) (*sftp.Client, string, *rootedHandlers) { t.Helper() tmp := t.TempDir() root := filepath.Join(tmp, "root") @@ -64,7 +70,7 @@ func setupRooted(t *testing.T, readonly bool) (*sftp.Client, string) { client.Close() <-done }) - return client, root + return client, root, h } func assertUnchanged(t *testing.T, root string) { @@ -290,6 +296,59 @@ func TestRootedNoopTimes(t *testing.T) { assertUnchanged(t, root) } +// TestRootedNoopRemoval checks the removal used to relay host deletions. +func TestRootedNoopRemoval(t *testing.T) { + c, root, h := setupRootedHandlers(t, false) + p := func(s string) string { return filepath.Join(root, s) } + + // The path was created again on the host before the guest removed it. + h.expectRemove(p("src/main.go")) + if err := c.Remove(p("src/main.go")); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(p("src/main.go")); err != nil { + t.Fatalf("the file was removed: %v", err) + } + // The token is consumed. + if err := c.Remove(p("src/main.go")); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(p("src/main.go")); err == nil { + t.Fatal("the file was not removed") + } + + h.expectRemove(p("src")) + if err := c.RemoveDirectory(p("src")); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(p("src")); err != nil { + t.Fatalf("the directory was removed: %v", err) + } + + // A git lock file deleted on the host. + h.expectRemove(p(".git/index.lock")) + if err := c.Remove(p(".git/index.lock")); err != nil { + t.Fatal(err) + } + if err := c.Remove(p(".git/index.lock")); err == nil { + t.Fatal("expected an error") + } + h.expectRemove(p(".git/config")) + if err := c.Remove(p(".git/config")); err != nil { + t.Fatal(err) + } + + // An expired token. + h.expectRemove(p(".git/config")) + h.mu.Lock() + h.noopRemovals[p(".git/config")] = time.Now().Add(-time.Second) + h.mu.Unlock() + if err := c.Remove(p(".git/config")); err == nil { + t.Fatal("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 {