From 232a2d627c54a95737a1491c7d381323f1c9690b Mon Sep 17 00:00:00 2001 From: Percy Wegmann Date: Thu, 29 Feb 2024 21:24:42 -0600 Subject: [PATCH] tailfs: only impersonate unprivileged user if able to sudo -u as that user When serving TailFS shares, tailscaled executes another tailscaled to act as a file server. It attempts to execute this child process as an unprivileged user using sudo -u. This is important to avoid accessing files as root, which would result in potential privilege escalation. Previously, tailscaled assumed that it was running as someone who can sudo -u, and would fail if it was unable to sudo -u. With this commit, if tailscaled is unable to sudo -u as the requested user, and tailscaled is not running as root, then tailscaled executes the the file server process under the same identity that ran tailscaled, since this is already an unprivileged identity. In the unlikely event that tailscaled is running as root but is unable to sudo -u, it will refuse to run the child file server process in order to avoid privilege escalation. Updates tailscale/corp#16827 Signed-off-by: Percy Wegmann --- tailfs/tailfsimpl/remote_impl.go | 86 +++++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 17 deletions(-) diff --git a/tailfs/tailfsimpl/remote_impl.go b/tailfs/tailfsimpl/remote_impl.go index 43b1c1e9b..471f7f9d1 100644 --- a/tailfs/tailfsimpl/remote_impl.go +++ b/tailfs/tailfsimpl/remote_impl.go @@ -5,6 +5,7 @@ package tailfsimpl import ( "bufio" + "context" "encoding/hex" "fmt" "log" @@ -15,6 +16,7 @@ import ( "net/url" "os" "os/exec" + "os/user" "strings" "sync" "time" @@ -66,12 +68,21 @@ func (s *FileSystemForRemote) SetFileServerAddr(addr string) { func (s *FileSystemForRemote) SetShares(shares map[string]*tailfs.Share) { userServers := make(map[string]*userServer) if tailfs.AllowShareAs() { - // set up per-user server + // Set up per-user server by running the current executable as an + // unprivileged user in order to avoid privilege escalation. + executable, err := os.Executable() + if err != nil { + s.logf("can't find executable: %v", err) + return + } + for _, share := range shares { p, found := userServers[share.As] if !found { p = &userServer{ - logf: s.logf, + logf: s.logf, + username: share.As, + executable: executable, } userServers[share.As] = p } @@ -227,8 +238,10 @@ func (s *FileSystemForRemote) Close() error { // given Shares. All Shares are assumed to have the same Share.As, and the // content is served as that Share.As user. type userServer struct { - logf logger.Logf - shares []*tailfs.Share + logf logger.Logf + shares []*tailfs.Share + username string + executable string // mu guards the below values. Acquire a write lock before updating any of // them, acquire a read lock before reading any of them. @@ -251,11 +264,6 @@ func (s *userServer) Close() error { } func (s *userServer) runLoop() { - executable, err := os.Executable() - if err != nil { - s.logf("can't find executable: %v", err) - return - } maxSleepTime := 30 * time.Second consecutiveFailures := float64(0) var timeOfLastFailure time.Time @@ -267,7 +275,7 @@ func (s *userServer) runLoop() { return } - err := s.run(executable) + err := s.run() now := time.Now() timeSinceLastFailure := now.Sub(timeOfLastFailure) timeOfLastFailure = now @@ -280,22 +288,37 @@ func (s *userServer) runLoop() { if sleepTime > maxSleepTime { sleepTime = maxSleepTime } - s.logf("user server % v stopped with error %v, will try again in %v", executable, err, sleepTime) + s.logf("user server % v stopped with error %v, will try again in %v", s.executable, err, sleepTime) time.Sleep(sleepTime) } } -// Run runs the executable (tailscaled). This function only works on UNIX systems, -// but those are the only ones on which we use userServers anyway. -func (s *userServer) run(executable string) error { +// Run runs the user server using the configured executable. This function only +// works on UNIX systems, but those are the only ones on which we use +// userServers anyway. +func (s *userServer) run() error { // set up the command args := []string{"serve-tailfs"} for _, s := range s.shares { args = append(args, s.Name, s.Path) } - allArgs := []string{"-u", s.shares[0].As, executable} - allArgs = append(allArgs, args...) - cmd := exec.Command("sudo", allArgs...) + var cmd *exec.Cmd + if s.canSudo() { + s.logf("starting TailFS file server as user %q", s.username) + allArgs := []string{"-n", "-u", s.username, s.executable} + allArgs = append(allArgs, args...) + cmd = exec.Command("sudo", allArgs...) + } else { + // If we were root, we should have been able to sudo as a specific + // user, but let's check just to make sure, since we never want to + // access shared folders as root. + err := s.assertNotRoot() + if err != nil { + return err + } + s.logf("starting TailFS file server as ourselves") + cmd = exec.Command(s.executable, args...) + } stdout, err := cmd.StdoutPipe() if err != nil { return fmt.Errorf("stdout pipe: %w", err) @@ -350,3 +373,32 @@ var writeMethods = map[string]bool{ "MOVE": true, "PROPPATCH": true, } + +// canSudo checks wether we can sudo -u the configured executable as the +// configured user by attempting to call the executable with the '-h' flag to +// print help. +func (s *userServer) canSudo() bool { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + if err := exec.CommandContext(ctx, "sudo", "-n", "-u", s.username, s.executable, "-h").Run(); err != nil { + return false + } + return true +} + +// assertNotRoot returns an error if the current user has UID 0 or if we cannot +// determine the current user. +// +// On Linux, root users will always have UID 0. +// +// On BSD, root users should always have UID 0. +func (s *userServer) assertNotRoot() error { + u, err := user.Current() + if err != nil { + return fmt.Errorf("assertNotRoot failed to find current user: %s", err) + } + if u.Uid == "0" { + return fmt.Errorf("%q is root", u.Name) + } + return nil +}