ssh/tailssh: only chdir incubator process to user's homedir when necessary and possible
Instead of changing the working directory before launching the incubator process, this now just changes the working directory after dropping privileges, at which point we're more likely to be able to enter the user's home directory since we're running as the user. For paths that use the 'login' or 'su -l' commands, those already take care of changing the working directory to the user's home directory. Fixes #13120 Signed-off-by: Percy Wegmann <percy@tailscale.com>
This commit is contained in:
parent
af3d3c433b
commit
4b525fdda0
|
@ -115,6 +115,7 @@ func (ss *sshSession) newIncubatorCommand(logf logger.Logf) (cmd *exec.Cmd, err
|
||||||
"--gid=" + lu.Gid,
|
"--gid=" + lu.Gid,
|
||||||
"--groups=" + groups,
|
"--groups=" + groups,
|
||||||
"--local-user=" + lu.Username,
|
"--local-user=" + lu.Username,
|
||||||
|
"--home-dir=" + lu.HomeDir,
|
||||||
"--remote-user=" + remoteUser,
|
"--remote-user=" + remoteUser,
|
||||||
"--remote-ip=" + ci.src.Addr().String(),
|
"--remote-ip=" + ci.src.Addr().String(),
|
||||||
"--has-tty=false", // updated in-place by startWithPTY
|
"--has-tty=false", // updated in-place by startWithPTY
|
||||||
|
@ -179,6 +180,7 @@ type incubatorArgs struct {
|
||||||
gid int
|
gid int
|
||||||
gids []int
|
gids []int
|
||||||
localUser string
|
localUser string
|
||||||
|
homeDir string
|
||||||
remoteUser string
|
remoteUser string
|
||||||
remoteIP string
|
remoteIP string
|
||||||
ttyName string
|
ttyName string
|
||||||
|
@ -201,6 +203,7 @@ func parseIncubatorArgs(args []string) (incubatorArgs, error) {
|
||||||
flags.IntVar(&ia.gid, "gid", 0, "the gid of local-user")
|
flags.IntVar(&ia.gid, "gid", 0, "the gid of local-user")
|
||||||
flags.StringVar(&groups, "groups", "", "comma-separated list of gids of local-user")
|
flags.StringVar(&groups, "groups", "", "comma-separated list of gids of local-user")
|
||||||
flags.StringVar(&ia.localUser, "local-user", "", "the user to run as")
|
flags.StringVar(&ia.localUser, "local-user", "", "the user to run as")
|
||||||
|
flags.StringVar(&ia.homeDir, "home-dir", "/", "the user's home directory")
|
||||||
flags.StringVar(&ia.remoteUser, "remote-user", "", "the remote user/tags")
|
flags.StringVar(&ia.remoteUser, "remote-user", "", "the remote user/tags")
|
||||||
flags.StringVar(&ia.remoteIP, "remote-ip", "", "the remote Tailscale IP")
|
flags.StringVar(&ia.remoteIP, "remote-ip", "", "the remote Tailscale IP")
|
||||||
flags.StringVar(&ia.ttyName, "tty-name", "", "the tty name (pts/3)")
|
flags.StringVar(&ia.ttyName, "tty-name", "", "the tty name (pts/3)")
|
||||||
|
@ -565,7 +568,7 @@ const (
|
||||||
// dropPrivileges calls doDropPrivileges with uid, gid, and gids from the given
|
// dropPrivileges calls doDropPrivileges with uid, gid, and gids from the given
|
||||||
// incubatorArgs.
|
// incubatorArgs.
|
||||||
func dropPrivileges(dlogf logger.Logf, ia incubatorArgs) error {
|
func dropPrivileges(dlogf logger.Logf, ia incubatorArgs) error {
|
||||||
return doDropPrivileges(dlogf, ia.uid, ia.gid, ia.gids)
|
return doDropPrivileges(dlogf, ia.uid, ia.gid, ia.gids, ia.homeDir)
|
||||||
}
|
}
|
||||||
|
|
||||||
// doDropPrivileges contains all the logic for dropping privileges to a different
|
// doDropPrivileges contains all the logic for dropping privileges to a different
|
||||||
|
@ -578,7 +581,7 @@ func dropPrivileges(dlogf logger.Logf, ia incubatorArgs) error {
|
||||||
// be done by running:
|
// be done by running:
|
||||||
//
|
//
|
||||||
// go test -c ./ssh/tailssh/ && sudo ./tailssh.test -test.v -test.run TestDoDropPrivileges
|
// go test -c ./ssh/tailssh/ && sudo ./tailssh.test -test.v -test.run TestDoDropPrivileges
|
||||||
func doDropPrivileges(dlogf logger.Logf, wantUid, wantGid int, supplementaryGroups []int) error {
|
func doDropPrivileges(dlogf logger.Logf, wantUid, wantGid int, supplementaryGroups []int, homeDir string) error {
|
||||||
dlogf("dropping privileges")
|
dlogf("dropping privileges")
|
||||||
fatalf := func(format string, args ...any) {
|
fatalf := func(format string, args ...any) {
|
||||||
dlogf("[unexpected] error dropping privileges: "+format, args...)
|
dlogf("[unexpected] error dropping privileges: "+format, args...)
|
||||||
|
@ -664,6 +667,13 @@ func doDropPrivileges(dlogf logger.Logf, wantUid, wantGid int, supplementaryGrou
|
||||||
// TODO(andrew-d): assert that our supplementary groups are correct
|
// TODO(andrew-d): assert that our supplementary groups are correct
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Prefer to run in user's homedir if possible. We ignore a failure to Chdir,
|
||||||
|
// which just leaves us at "/" where we launched in the first place.
|
||||||
|
dlogf("attempting to chdir to user's home directory %q", homeDir)
|
||||||
|
if err := os.Chdir(homeDir); err != nil {
|
||||||
|
dlogf("failed to chdir to user's home directory %q, continuing in current directory", homeDir)
|
||||||
|
}
|
||||||
|
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -680,16 +690,7 @@ func (ss *sshSession) launchProcess() error {
|
||||||
}
|
}
|
||||||
|
|
||||||
cmd := ss.cmd
|
cmd := ss.cmd
|
||||||
homeDir := ss.conn.localUser.HomeDir
|
cmd.Dir = "/"
|
||||||
if _, err := os.Stat(homeDir); err == nil {
|
|
||||||
cmd.Dir = homeDir
|
|
||||||
} else if os.IsNotExist(err) {
|
|
||||||
// If the home directory doesn't exist, we can't chdir to it.
|
|
||||||
// Instead, we'll chdir to the root directory.
|
|
||||||
cmd.Dir = "/"
|
|
||||||
} else {
|
|
||||||
return err
|
|
||||||
}
|
|
||||||
cmd.Env = envForUser(ss.conn.localUser)
|
cmd.Env = envForUser(ss.conn.localUser)
|
||||||
for _, kv := range ss.Environ() {
|
for _, kv := range ss.Environ() {
|
||||||
if acceptEnvPair(kv) {
|
if acceptEnvPair(kv) {
|
||||||
|
|
|
@ -49,7 +49,7 @@ func TestDoDropPrivileges(t *testing.T) {
|
||||||
f := os.NewFile(3, "out.json")
|
f := os.NewFile(3, "out.json")
|
||||||
|
|
||||||
// We're in our subprocess; actually drop privileges now.
|
// We're in our subprocess; actually drop privileges now.
|
||||||
doDropPrivileges(t.Logf, input.UID, input.GID, input.AdditionalGroups)
|
doDropPrivileges(t.Logf, input.UID, input.GID, input.AdditionalGroups, "/")
|
||||||
|
|
||||||
additional, _ := syscall.Getgroups()
|
additional, _ := syscall.Getgroups()
|
||||||
|
|
||||||
|
|
|
@ -122,13 +122,13 @@ func TestIntegrationSSH(t *testing.T) {
|
||||||
{
|
{
|
||||||
cmd: "pwd",
|
cmd: "pwd",
|
||||||
want: []string{homeDir},
|
want: []string{homeDir},
|
||||||
skip: !fallbackToSUAvailable(),
|
skip: os.Getenv("SKIP_FILE_OPS") == "1" || !fallbackToSUAvailable(),
|
||||||
forceV1Behavior: false,
|
forceV1Behavior: false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
cmd: "echo 'hello'",
|
cmd: "echo 'hello'",
|
||||||
want: []string{"hello"},
|
want: []string{"hello"},
|
||||||
skip: !fallbackToSUAvailable(),
|
skip: os.Getenv("SKIP_FILE_OPS") == "1" || !fallbackToSUAvailable(),
|
||||||
forceV1Behavior: false,
|
forceV1Behavior: false,
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
|
@ -25,7 +25,7 @@ COPY tailssh.test .
|
||||||
|
|
||||||
RUN chmod 755 tailscaled
|
RUN chmod 755 tailscaled
|
||||||
|
|
||||||
# RUN echo "First run tests normally."
|
RUN echo "First run tests normally."
|
||||||
RUN eval `ssh-agent -s` && TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestSSHAgentForwarding
|
RUN eval `ssh-agent -s` && TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestSSHAgentForwarding
|
||||||
RUN if echo "$BASE" | grep "ubuntu:"; then rm -Rf /home/testuser; fi
|
RUN if echo "$BASE" | grep "ubuntu:"; then rm -Rf /home/testuser; fi
|
||||||
RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegrationSFTP
|
RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegrationSFTP
|
||||||
|
@ -35,9 +35,14 @@ RUN if echo "$BASE" | grep "ubuntu:"; then rm -Rf /home/testuser; fi
|
||||||
RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegrationSSH
|
RUN TAILSCALED_PATH=`pwd`tailscaled ./tailssh.test -test.v -test.run TestIntegrationSSH
|
||||||
|
|
||||||
RUN echo "Then run tests as non-root user testuser and make sure tests still pass."
|
RUN echo "Then run tests as non-root user testuser and make sure tests still pass."
|
||||||
|
RUN touch /tmp/tailscalessh.log
|
||||||
RUN chown testuser:groupone /tmp/tailscalessh.log
|
RUN chown testuser:groupone /tmp/tailscalessh.log
|
||||||
RUN TAILSCALED_PATH=`pwd`tailscaled eval `su -m testuser -c ssh-agent -s` && su -m testuser -c "./tailssh.test -test.v -test.run TestSSHAgentForwarding"
|
RUN TAILSCALED_PATH=`pwd`tailscaled eval `su -m testuser -c ssh-agent -s` && su -m testuser -c "./tailssh.test -test.v -test.run TestSSHAgentForwarding"
|
||||||
RUN TAILSCALED_PATH=`pwd`tailscaled su -m testuser -c "./tailssh.test -test.v -test.run TestIntegration TestDoDropPrivileges"
|
RUN TAILSCALED_PATH=`pwd`tailscaled su -m testuser -c "./tailssh.test -test.v -test.run TestIntegration TestDoDropPrivileges"
|
||||||
|
RUN echo "Also, deny everyone access to the user's home directory and make sure non file-related tests still pass."
|
||||||
|
RUN mkdir -p /home/testuser && chown testuser:groupone /home/testuser && chmod 0000 /home/testuser
|
||||||
|
RUN TAILSCALED_PATH=`pwd`tailscaled SKIP_FILE_OPS=1 su -m testuser -c "./tailssh.test -test.v -test.run TestIntegrationSSH"
|
||||||
|
RUN chmod 0755 /home/testuser
|
||||||
RUN chown root:root /tmp/tailscalessh.log
|
RUN chown root:root /tmp/tailscalessh.log
|
||||||
|
|
||||||
RUN if echo "$BASE" | grep "ubuntu:"; then \
|
RUN if echo "$BASE" | grep "ubuntu:"; then \
|
||||||
|
|
Loading…
Reference in New Issue