Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 74 additions & 4 deletions host/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ type RelayWarning struct {
Host string
}

// SFTPWriteLimits bounds optional rooted uploads by expected filename,
// per-file extent and lifetime write bytes, and host-wide lifetime write bytes.
// Each filename has one non-reusable writable instance. Nil keeps the existing
// rooted behavior; non-nil limits require SFTPRoot.
type SFTPWriteLimits = internal.SFTPWriteLimits

type Host struct {
Host string
// KeepAliveDuration paces the guest sshd's keepalive to each guest. The
Expand Down Expand Up @@ -325,6 +331,16 @@ type Host struct {

// SFTP configuration
SFTPDisabled bool // Disable SFTP subsystem entirely (--no-sftp)
// SFTPRoot confines SFTP content and metadata to this directory handle.
// Nil preserves the ordinary filesystem/consent behavior. The caller owns
// the root and must keep it open until Run has returned AND WaitSFTP succeeds.
// Rooted hosts are single-use. Configure this before Run; do not change it.
SFTPRoot *os.Root
// SFTPWriteLimits is copied at Run entry; configure it before Run.
SFTPWriteLimits *SFTPWriteLimits
sftpMu sync.Mutex
sftpRunDone chan struct{}
sftpServer *internal.Server
// PermissionChecker supplies custom consent policy. Nil denies ungranted operations.
PermissionChecker consent.PermissionChecker
// TerminalConsent selects consent on the primary local terminal; mutually exclusive with PermissionChecker.
Expand Down Expand Up @@ -600,13 +616,60 @@ func (c *Host) claimTimeout() time.Duration {
return ClaimTimeout
}

// WaitSFTP certifies that a rooted host has returned from Run, closed SFTP
// admission, and joined every rooted request worker and open handle. Concurrent
// waits are safe. Before Run it returns an error; failed startup with no server
// is already quiescent. Nil-root hosts have nothing to wait for.
//
// On timeout the caller must retain the root and its directory and may wait
// again. Cancellation cannot force a custom permission checker or blocked OS
// filesystem call to return, so Run returning alone is not this guarantee.
func (c *Host) WaitSFTP(ctx context.Context) error {
if c.SFTPRoot == nil {
return nil
}
c.sftpMu.Lock()
done := c.sftpRunDone
c.sftpMu.Unlock()
if done == nil {
return errors.New("rooted host has not run")
}
select {
case <-done:
case <-ctx.Done():
return ctx.Err()
}
c.sftpMu.Lock()
server := c.sftpServer
c.sftpMu.Unlock()
if server == nil {
return nil
}
return server.WaitSFTP(ctx)
}

// Run hosts one session and returns when it ends.
//
// It may be called again afterwards: everything Run claims, it gives back
// before returning, including the two fields it fills in on the Host itself.
// A caller that supplied AdminSocketFile keeps it across runs, since managing
// the path is what supplying it means.
// Without SFTPRoot it may be called again afterwards: everything Run claims,
// it gives back before returning, including the two fields it fills in on the
// Host itself. A caller that supplied AdminSocketFile keeps it across runs,
// since managing the path is what supplying it means. Rooted hosts are single-
// use and require WaitSFTP before the caller closes their root.
func (c *Host) Run(ctx context.Context) (runErr error) {
if c.SFTPRoot != nil {
c.sftpMu.Lock()
if c.sftpRunDone != nil {
c.sftpMu.Unlock()
return errors.New("a host with SFTPRoot cannot be reused")
}
c.sftpRunDone = make(chan struct{})
c.sftpMu.Unlock()
defer close(c.sftpRunDone)
}
sftpWriteLimits, err := internal.CopySFTPWriteLimits(c.SFTPRoot, c.SFTPWriteLimits)
if err != nil {
return err
}
// An empty command is no command, as the guest door reads one.
if len(c.LoginCommand) > 0 && len(c.ForceCommand) > 0 {
return errors.New("LoginCommand and ForceCommand are mutually exclusive")
Expand Down Expand Up @@ -1246,6 +1309,8 @@ func (c *Host) Run(ctx context.Context) (runErr error) {
InitialClientTimeout: c.InitialClientTimeout,
StopGrace: c.StopGrace,
SFTPDisabled: c.SFTPDisabled,
SFTPRoot: c.SFTPRoot,
SFTPWriteLimits: sftpWriteLimits,
PermissionChecker: c.PermissionChecker,
TerminalConsent: c.TerminalConsent,
SFTPAutoAccept: c.AutoAccept || c.SFTPAutoAccept,
Expand All @@ -1263,6 +1328,11 @@ func (c *Host) Run(ctx context.Context) (runErr error) {
logger.Warn("reverse tunnel stopped serving guests; command continues", "error", err)
},
}
if c.SFTPRoot != nil {
c.sftpMu.Lock()
c.sftpServer = &sshServer
c.sftpMu.Unlock()
}
g.Add(func() error {
return sshServer.ServeWithContext(ctx, guestDoor, attachLn)
}, func(err error) {
Expand Down
32 changes: 29 additions & 3 deletions host/internal/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io"
"log/slog"
"net"
"os"
"path/filepath"
"slices"
"strings"
Expand Down Expand Up @@ -131,6 +132,10 @@ type Server struct {

// SFTP configuration
SFTPDisabled bool // Disable SFTP subsystem entirely
// SFTPRoot is caller-owned; WaitSFTP must succeed before it is closed.
SFTPRoot *os.Root
SFTPWriteLimits *SFTPWriteLimits
sftpWorkers rootedSFTPWorkers
// PermissionChecker supplies custom consent policy. Nil denies ungranted operations.
PermissionChecker consent.PermissionChecker
// TerminalConsent selects consent on the primary local terminal; mutually exclusive with PermissionChecker.
Expand Down Expand Up @@ -219,6 +224,16 @@ func logReplayFallback(logger *slog.Logger) func(error) {
// guest, and — when host is not nil — the host door on host, for clients that
// are already on this machine.
func (s *Server) ServeWithContext(ctx context.Context, guest, host net.Listener) error {
if s.SFTPRoot != nil {
defer s.sftpWorkers.stop()
if !rootedSFTPSupported() {
return errors.New("rooted SFTP is unsupported on this platform")
}
}
sftpWriteLimits, err := newRootedSFTPWriteLimits(s.SFTPRoot, s.SFTPWriteLimits)
if err != nil {
return err
}
if s.TerminalConsent && s.PermissionChecker != nil {
return errors.New("TerminalConsent and PermissionChecker are mutually exclusive")
}
Expand Down Expand Up @@ -375,9 +390,14 @@ func (s *Server) ServeWithContext(ctx context.Context, guest, host net.Listener)
kind: kindGuest,
cmdDone: cmdDone,
commandResult: cmd.Result,
sftpRoot: s.SFTPRoot,
sftpWriteLimits: sftpWriteLimits,

onPrivateCommandExit: s.onPrivateCommandExit,
}
if s.SFTPRoot != nil {
sh.sftpWorkers = &s.sftpWorkers
}

ss := []gssh.Signer{s.HostKey}

Expand All @@ -390,9 +410,9 @@ func (s *Server) ServeWithContext(ctx context.Context, guest, host net.Listener)

// Set up subsystem handlers (SFTP). The door admits a subsystem with
// the session's end, as it does a shell (see admitGuest), and SFTP
// hands that admission back as it starts: an SFTP session in progress
// is not waited for, and is cut off when the session ends, as it
// always was.
// hands that admission back as it starts. Rooted sessions separately
// register their workers for WaitSFTP; nil-root sessions retain their
// existing shutdown behavior.
var subsystemHandlers map[string]gssh.SubsystemHandler
if !s.SFTPDisabled {
subsystemHandlers = map[string]gssh.SubsystemHandler{
Expand Down Expand Up @@ -485,6 +505,9 @@ func (s *Server) ServeWithContext(ctx context.Context, guest, host net.Listener)
// door's listener's close. A command-led exit has already closed
// cmdDone by the time this runs.
releaseSessions(cmdDone, timing.outputDrain+guestFlushTimeout, cancel)
if s.SFTPRoot != nil {
s.sftpWorkers.stop()
}

// That cancel hangs up every forced command, and each one's guest is
// sent what it says on the way out and a reset once it has gone.
Expand Down Expand Up @@ -1087,6 +1110,9 @@ type sessionHandler struct {
// SFTP configuration
permissionChecker consent.PermissionChecker
sftpAutoAccept bool
sftpRoot *os.Root
sftpWriteLimits *rootedSFTPWriteLimits
sftpWorkers *rootedSFTPWorkers

// subsystems are the guest door's subsystem handlers, by name, so that
// admitGuest refuses one the door doesn't serve. Nil in a test's handler.
Expand Down
50 changes: 47 additions & 3 deletions host/internal/sftp.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,20 @@ type SFTPSession struct {
logger *slog.Logger
ctx context.Context
autoAccept bool
root *os.Root
writeLimits *rootedSFTPWriteLimits
}

// HandleSFTP handles SFTP subsystem requests
func (h *sessionHandler) HandleSFTP(sess gssh.Session) {
if h.sftpRoot != nil {
finished, ok := h.sftpWorkers.admit(sess)
if !ok {
_ = sess.Exit(1)
return
}
defer finished()
}
sessionID := sess.Context().Value(gssh.ContextKeySessionID).(string)

// Get client info for terminal approval. The fingerprint is the guest's
Expand Down Expand Up @@ -80,6 +90,8 @@ func (h *sessionHandler) HandleSFTP(sess gssh.Session) {
permissionChecker: h.permissionChecker,
clientInfo: clientInfo,
logger: h.logger,
root: h.sftpRoot,
writeLimits: h.sftpWriteLimits,
}

handlers := sftp.Handlers{
Expand All @@ -89,10 +101,18 @@ func (h *sessionHandler) HandleSFTP(sess gssh.Session) {
FileList: &sftpFileLister{session: sftpSession, logger: h.logger},
}

// Get user's home directory for SFTP start directory
userHome, err := os.UserHomeDir()
// Rooted sessions resolve relative requests under their declared root;
// ordinary sessions retain the user's home as their starting directory.
var userHome string
var err error
if h.sftpRoot != nil {
userHome, err = filepath.Abs(h.sftpRoot.Name())
handlers.FileList = &rootedSFTPFileLister{sftpFileLister{session: sftpSession, logger: h.logger}}
} else {
userHome, err = os.UserHomeDir()
}
if err != nil {
h.logger.Error("failed to get user home directory", "error", err)
h.logger.Error("failed to get SFTP start directory", "error", err)
_ = sess.Exit(1)
return
}
Expand Down Expand Up @@ -249,6 +269,9 @@ func sftpOperationError(ctx context.Context, r *sftp.Request, s *SFTPSession) er
// paths already resolve to the home directory, so "notes.txt" covers what
// "~/notes.txt" was reaching for.
func (s *SFTPSession) resolvePath(reqPath string) (string, error) {
if s.root != nil {
return rootedSFTPPath(s.root, reqPath)
}
if runtime.GOOS == "windows" {
reqPath = undoubleDriveLetter(reqPath)
}
Expand Down Expand Up @@ -343,6 +366,9 @@ func (h *sftpFileReader) Fileread(r *sftp.Request) (io.ReaderAt, error) {
if err := sftpOperationError(ctx, r, h.session); err != nil {
return nil, err
}
if h.session.root != nil {
return h.session.root.Open(path)
Comment on lines +369 to +370

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛡️ Codex Security Review · Automatically triggered

P2 Badge Security: Bound rooted SFTP download opens and bytes

When transfer_concurrency is 4 or 8 and the manifest-pinned relay is compromised, rooted SFTP constrains writes, but this path returns a fresh file for every authorized OPEN without read-open or byte accounting. Because the relay may attest the configured guest public key, it can repeatedly open the deterministic transfer file, retain handles to exhaust the runner's descriptors, or repeatedly read it to generate unbudgeted egress. These requests multiplex over an admitted SSH transport, so neither the connection guard nor PayloadBudgetBytes sees them. Apply lifetime per-file/aggregate accounting to reads and cap simultaneous handles.

Useful? React with 👍 / 👎.

}
return os.Open(path)
}

Expand Down Expand Up @@ -401,6 +427,15 @@ func (h *sftpFileWriter) Filewrite(r *sftp.Request) (io.WriterAt, error) {
if err := sftpOperationError(ctx, r, h.session); err != nil {
return nil, err
}
if h.session.root != nil {
if h.session.writeLimits != nil {
if !pflags.Write {
return nil, sftp.ErrSSHFxPermissionDenied
}
return h.session.writeLimits.open(h.session.root, path, osFlags)
}
return h.session.root.OpenFile(path, osFlags, 0600)
}
return os.OpenFile(path, osFlags, 0644)
}

Expand All @@ -411,6 +446,9 @@ type sftpFileCmder struct {
}

func (h *sftpFileCmder) Filecmd(r *sftp.Request) error {
if h.session.root != nil && r.Method != "Remove" {
return sftp.ErrSSHFxOpUnsupported
}
arrived := time.Now()
requestID := nextSFTPRequestID()
ctx, cancel := h.session.operationContext(r.Context())
Expand Down Expand Up @@ -467,6 +505,9 @@ func (h *sftpFileCmder) Filecmd(r *sftp.Request) error {
// Execute the operation
switch r.Method {
case "Remove":
if h.session.root != nil {
return h.session.root.Remove(path)
}
return os.Remove(path)
case "Mkdir":
return os.Mkdir(path, 0755)
Expand Down Expand Up @@ -559,6 +600,9 @@ type sftpFileLister struct {
}

func (h *sftpFileLister) Filelist(r *sftp.Request) (sftp.ListerAt, error) {
if h.session.root != nil {
return h.rootedList(r)
}
path, err := h.session.resolvePath(r.Filepath)
if err != nil {
return nil, sftp.ErrSSHFxPermissionDenied
Expand Down
Loading
Loading