Browse Source

ssh/tailssh: create login sessions for new connections

Signed-off-by: Maisem Ali <[email protected]>
Maisem Ali 4 years ago
parent
commit
06c147d848
5 changed files with 596 additions and 188 deletions
  1. 2 1
      cmd/tailscaled/depaware.txt
  2. 288 0
      ssh/tailssh/incubator.go
  3. 174 0
      ssh/tailssh/incubator_linux.go
  4. 122 179
      ssh/tailssh/tailssh.go
  5. 10 8
      ssh/tailssh/tailssh_test.go

+ 2 - 1
cmd/tailscaled/depaware.txt

@@ -65,7 +65,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
   LD    github.com/gliderlabs/ssh                                    from tailscale.com/ssh/tailssh
    W 💣 github.com/go-ole/go-ole                                     from github.com/go-ole/go-ole/oleutil+
    W 💣 github.com/go-ole/go-ole/oleutil                             from tailscale.com/wgengine/winnet
-   L 💣 github.com/godbus/dbus/v5                                    from tailscale.com/net/dns
+   L 💣 github.com/godbus/dbus/v5                                    from tailscale.com/net/dns+
         github.com/golang/groupcache/lru                             from tailscale.com/net/dnscache
         github.com/google/btree                                      from gvisor.dev/gvisor/pkg/tcpip/header+
    L    github.com/insomniacslk/dhcp/dhcpv4                          from tailscale.com/net/tstun
@@ -367,6 +367,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         io/fs                                                        from crypto/rand+
         io/ioutil                                                    from github.com/aws/aws-sdk-go-v2/aws/protocol/query+
         log                                                          from expvar+
+  LD    log/syslog                                                   from tailscale.com/ssh/tailssh
         math                                                         from compress/flate+
         math/big                                                     from crypto/dsa+
         math/bits                                                    from compress/flate+

+ 288 - 0
ssh/tailssh/incubator.go

@@ -0,0 +1,288 @@
+// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This file contains the code for the incubator process.
+// Taiscaled launches the incubator as the same user as it was launched as.
+// The incbuator then registers a new session with the OS, sets its own UID to
+// the specified `--uid`` and then lauches the requested `--cmd`.
+
+//go:build linux || (darwin && !ios)
+// +build linux darwin,!ios
+
+package tailssh
+
+import (
+	"context"
+	"flag"
+	"fmt"
+	"io"
+	"log"
+	"log/syslog"
+	"os"
+	"os/exec"
+	"os/user"
+	"runtime"
+	"strings"
+	"syscall"
+
+	"github.com/creack/pty"
+	"github.com/gliderlabs/ssh"
+	"golang.org/x/sys/unix"
+	"tailscale.com/cmd/tailscaled/childproc"
+	"tailscale.com/types/logger"
+)
+
+func init() {
+	childproc.Add("ssh", beIncubator)
+}
+
+var ptyName = func(f *os.File) (string, error) {
+	return "", fmt.Errorf("unimplemented")
+}
+
+// maybeStartLoginSession starts a new login session for the specified UID.
+// On success, it may return a non-nil close func which must be closed to
+// release the session.
+// See maybeStartLoginSessionLinux.
+var maybeStartLoginSession = func(logf logger.Logf, uid uint32, localUser, remoteUser, remoteHost, tty string) (close func() error, err error) {
+	return nil, nil
+}
+
+// newIncubatorCommand returns a new exec.Cmd configured with
+// `tailscaled be-child ssh` as the entrypoint.
+// If tailscaled is empty, the desired cmd is executed directly.
+func newIncubatorCommand(ctx context.Context, ci *sshConnInfo, lu *user.User, tailscaled, name string, args []string) *exec.Cmd {
+	if tailscaled == "" {
+		return exec.CommandContext(ctx, name, args...)
+	}
+	remoteUser := ci.uprof.LoginName
+	if len(ci.node.Tags) > 0 {
+		remoteUser = strings.Join(ci.node.Tags, ",")
+	}
+
+	incubatorArgs := []string{
+		"be-child",
+		"ssh",
+		"--uid=" + lu.Uid,
+		"--local-user=" + lu.Name,
+		"--remote-user=" + remoteUser,
+		"--remote-ip=" + ci.src.IP().String(),
+		"--cmd=" + name,
+	}
+
+	if len(args) > 0 {
+		incubatorArgs = append(incubatorArgs, fmt.Sprintf("--cmd-args=%q", strings.Join(args, " ")))
+	}
+	return exec.CommandContext(ctx, tailscaled, incubatorArgs...)
+}
+
+const debugIncubator = false
+
+// beIncubator is the entrypoint to the `tailscaled be-child ssh` subcommand.
+// It is responsible for informing the system of a new login session for the user.
+// This is sometimes necessary for mounting home directories and decrypting file
+// systems.
+//
+// Taiscaled launches the incubator as the same user as it was launched as.
+// The incbuator then registers a new session with the OS, sets its own UID to
+// the specified `--uid`` and then lauches the requested `--cmd`.
+func beIncubator(args []string) error {
+	var (
+		flags      = flag.NewFlagSet("", flag.ExitOnError)
+		uid        = flags.Uint64("uid", 0, "the uid of local-user")
+		localUser  = flags.String("local-user", "", "the user to run as")
+		remoteUser = flags.String("remote-user", "", "the remote user/tags")
+		remoteIP   = flags.String("remote-ip", "", "the remote Tailscale IP")
+		ttyName    = flags.String("tty-name", "", "the tty name (pts/3)")
+		hasTTY     = flags.Bool("has-tty", false, "is the output attached to a tty")
+		cmdName    = flags.String("cmd", "", "the cmd to launch")
+		cmdArgs    = flags.String("cmd-args", "", "the args for cmd")
+	)
+	if err := flags.Parse(args); err != nil {
+		return err
+	}
+	logf := logger.Discard
+	if debugIncubator {
+		// We don't own stdout or stderr, so the only place we can log is syslog.
+		if sl, err := syslog.New(syslog.LOG_INFO|syslog.LOG_DAEMON, "tailscaled-ssh"); err == nil {
+			logf = log.New(sl, "", 0).Printf
+		}
+	}
+
+	euid := uint64(os.Geteuid())
+	// Inform the system that we are about to log someone in.
+	// We can only do this if we are running as root.
+	sessionCloser, err := maybeStartLoginSession(logf, uint32(*uid), *localUser, *remoteUser, *remoteIP, *ttyName)
+	if err == nil && sessionCloser != nil {
+		defer sessionCloser()
+	}
+	if euid != *uid {
+		// Switch users if required before starting the desired process.
+		if err := syscall.Setuid(int(*uid)); err != nil {
+			logf(err.Error())
+			os.Exit(1)
+		}
+	}
+
+	var cArgs []string
+	if *cmdArgs != "" {
+		cArgs = strings.Split(*cmdArgs, " ")
+	}
+
+	cmd := exec.Command(*cmdName, cArgs...)
+	cmd.Stdin = os.Stdin
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+	cmd.Env = os.Environ()
+
+	if *hasTTY {
+		// If we were launched with a tty then we should
+		// mark that as the ctty of the child. However,
+		// as the ctty is being passed from the parent
+		// we set the child to foreground instead which
+		// also passes the ctty.
+		// However, we can not do this if never had a tty to
+		// begin with.
+		cmd.SysProcAttr = &syscall.SysProcAttr{
+			Foreground: true,
+		}
+	}
+	return cmd.Run()
+}
+
+// launchProcess launches an incubator process for the provided session.
+// It is responsible for configuring the process execution environment.
+// The caller can wait for the process to exit by calling cmd.Wait().
+func (srv *server) launchProcess(ctx context.Context, s ssh.Session, ci *sshConnInfo, lu *user.User) (cmd *exec.Cmd, stdin io.WriteCloser, stdout, stderr io.Reader, err error) {
+	shell := loginShell(lu.Uid)
+	var args []string
+	if rawCmd := s.RawCommand(); rawCmd != "" {
+		args = []string{"-c", rawCmd}
+	}
+	ptyReq, winCh, isPty := s.Pty()
+
+	cmd = newIncubatorCommand(ctx, ci, lu, srv.tailscaledPath, shell, args)
+	cmd.Dir = lu.HomeDir
+	cmd.Env = append(cmd.Env, envForUser(lu)...)
+	cmd.Env = append(cmd.Env, s.Environ()...)
+	cmd.Env = append(cmd.Env,
+		fmt.Sprintf("SSH_CLIENT=%s %d %d", ci.src.IP(), ci.src.Port(), ci.dst.Port()),
+		fmt.Sprintf("SSH_CONNECTION=%s %d %s %d", ci.src.IP(), ci.src.Port(), ci.dst.IP(), ci.dst.Port()),
+	)
+	srv.logf("ssh: starting: %+v", cmd.Args)
+
+	if !isPty {
+		stdin, stdout, stderr, err = startWithStdPipes(cmd)
+		return
+	}
+	pty, err := startWithPTY(cmd, ptyReq)
+	if err != nil {
+		return nil, nil, nil, nil, err
+	}
+	go resizeWindow(pty, winCh)
+	// When using a pty we don't get a separate reader for stderr.
+	return cmd, pty, pty, nil, nil
+}
+
+func resizeWindow(f *os.File, winCh <-chan ssh.Window) {
+	for win := range winCh {
+		unix.IoctlSetWinsize(int(f.Fd()), syscall.TIOCSWINSZ, &unix.Winsize{
+			Row: uint16(win.Height),
+			Col: uint16(win.Width),
+		})
+	}
+}
+
+// startWithPTY starts cmd with a psuedo-terminal attached to Stdin, Stdout and Stderr.
+func startWithPTY(cmd *exec.Cmd, ptyReq ssh.Pty) (ptyFile *os.File, err error) {
+	var tty *os.File
+	ptyFile, tty, err = pty.Open()
+	if err != nil {
+		err = fmt.Errorf("pty.Open: %w", err)
+		return
+	}
+	defer func() {
+		if err != nil {
+			ptyFile.Close()
+			tty.Close()
+		}
+	}()
+	if err = pty.Setsize(ptyFile, &pty.Winsize{
+		Rows: uint16(ptyReq.Window.Width),
+		Cols: uint16(ptyReq.Window.Height),
+	}); err != nil {
+		err = fmt.Errorf("pty.Setsize: %w", err)
+		return
+	}
+	cmd.SysProcAttr = &syscall.SysProcAttr{
+		Setctty: true,
+		Setsid:  true,
+	}
+	cmd.Args = append(cmd.Args, "--has-tty=true")
+	if ptyName, err := ptyName(ptyFile); err == nil {
+		cmd.Args = append(cmd.Args, "--tty-name="+ptyName)
+	}
+	if ptyReq.Term != "" {
+		cmd.Env = append(cmd.Env, fmt.Sprintf("TERM=%s", ptyReq.Term))
+	}
+	cmd.Stdin = tty
+	cmd.Stdout = tty
+	cmd.Stderr = tty
+
+	if err = cmd.Start(); err != nil {
+		return
+	}
+	return ptyFile, nil
+}
+
+// startWithStdPipes starts cmd with os.Pipe for Stdin, Stdout and Stderr.
+func startWithStdPipes(cmd *exec.Cmd) (stdin io.WriteCloser, stdout, stderr io.ReadCloser, err error) {
+	defer func() {
+		if err != nil {
+			for _, c := range []io.Closer{stdin, stdout, stderr} {
+				if c != nil {
+					c.Close()
+				}
+			}
+		}
+	}()
+	stdin, err = cmd.StdinPipe()
+	if err != nil {
+		return
+	}
+	stdout, err = cmd.StdoutPipe()
+	if err != nil {
+		return
+	}
+	stderr, err = cmd.StderrPipe()
+	if err != nil {
+		return
+	}
+	err = cmd.Start()
+	return
+}
+
+func loginShell(uid string) string {
+	switch runtime.GOOS {
+	case "linux":
+		out, _ := exec.Command("getent", "passwd", uid).Output()
+		// out is "root:x:0:0:root:/root:/bin/bash"
+		f := strings.SplitN(string(out), ":", 10)
+		if len(f) > 6 {
+			return strings.TrimSpace(f[6]) // shell
+		}
+	}
+	if e := os.Getenv("SHELL"); e != "" {
+		return e
+	}
+	return "/bin/bash"
+}
+
+func envForUser(u *user.User) []string {
+	return []string{
+		fmt.Sprintf("SHELL=" + loginShell(u.Uid)),
+		fmt.Sprintf("USER=" + u.Username),
+		fmt.Sprintf("HOME=" + u.HomeDir),
+	}
+}

+ 174 - 0
ssh/tailssh/incubator_linux.go

@@ -0,0 +1,174 @@
+// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build linux
+// +build linux
+
+package tailssh
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"syscall"
+	"time"
+	"unsafe"
+
+	"github.com/godbus/dbus/v5"
+	"tailscale.com/types/logger"
+)
+
+func init() {
+	ptyName = ptyNameLinux
+	maybeStartLoginSession = maybeStartLoginSessionLinux
+}
+
+func ptyNameLinux(f *os.File) (string, error) {
+	var n uint32
+	_, _, e := syscall.Syscall(syscall.SYS_IOCTL, f.Fd(), syscall.TIOCGPTN, uintptr(unsafe.Pointer(&n)))
+	if e != 0 {
+		return "", e
+	}
+	return fmt.Sprintf("pts/%d", n), nil
+}
+
+// callLogin1 invokes the provided method of the "login1" service over D-Bus.
+// https://www.freedesktop.org/software/systemd/man/org.freedesktop.login1.html
+func callLogin1(method string, flags dbus.Flags, args ...interface{}) (*dbus.Call, error) {
+	conn, err := dbus.SystemBus()
+	if err != nil {
+		// DBus probably not running.
+		return nil, err
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+
+	name, objectPath := "org.freedesktop.login1", "/org/freedesktop/login1"
+	obj := conn.Object(name, dbus.ObjectPath(objectPath))
+	call := obj.CallWithContext(ctx, method, flags, args...)
+	if call.Err != nil {
+		return nil, call.Err
+	}
+	return call, nil
+}
+
+// createSessionArgs is a wrapper struct for the Login1.Manager.CreateSession args.
+// The CreateSession API arguments and response types are defined here:
+// https://www.freedesktop.org/software/systemd/man/org.freedesktop.login1.html
+type createSessionArgs struct {
+	uid        uint32     // User ID being logged in.
+	pid        uint32     // Process ID for the session, 0 means current process.
+	service    string     // Service creating the session.
+	typ        string     // Type of login (oneof unspecified, tty, x11).
+	class      string     // Type of session class (oneof user, greeter, lock-screen).
+	desktop    string     // the desktop environment.
+	seat       string     // the seat this session belongs to, empty otherwise.
+	vtnr       uint32     // the virtual terminal number of the session if there is any, 0 otherwise.
+	tty        string     // the kernel TTY path of the session if this is a text login, empty otherwise.
+	display    string     // the X11 display name if this is a graphical login, empty otherwise.
+	remote     bool       // whether the session is remote.
+	remoteUser string     // the remote user if this is a remote session, empty otherwise.
+	remoteHost string     // the remote host if this is a remote session, empty otherwise.
+	properties []struct { // This is unused and exists just to make the marshaling work
+		S string
+		V dbus.Variant
+	}
+}
+
+func (a createSessionArgs) args() []interface{} {
+	return []interface{}{
+		a.uid,
+		a.pid,
+		a.service,
+		a.typ,
+		a.class,
+		a.desktop,
+		a.seat,
+		a.vtnr,
+		a.tty,
+		a.display,
+		a.remote,
+		a.remoteUser,
+		a.remoteHost,
+		a.properties,
+	}
+}
+
+// createSessionResp is a wrapper struct for the Login1.Manager.CreateSession response.
+// The CreateSession API arguments and response types are defined here:
+// https://www.freedesktop.org/software/systemd/man/org.freedesktop.login1.html
+type createSessionResp struct {
+	sessionID   string
+	objectPath  dbus.ObjectPath
+	runtimePath string
+	fifoFD      dbus.UnixFD
+	uid         uint32
+	seatID      string
+	vtnr        uint32
+	existing    bool // whether a new session was created.
+}
+
+// createSession creates a tty user login session for the provided uid.
+func createSession(uid uint32, remoteUser, remoteHost, tty string) (createSessionResp, error) {
+	a := createSessionArgs{
+		uid:        uid,
+		service:    "tailscaled",
+		typ:        "tty",
+		class:      "user",
+		tty:        tty,
+		remote:     true,
+		remoteUser: remoteUser,
+		remoteHost: remoteHost,
+	}
+
+	call, err := callLogin1("org.freedesktop.login1.Manager.CreateSession", 0, a.args()...)
+	if err != nil {
+		return createSessionResp{}, err
+	}
+
+	return createSessionResp{
+		sessionID:   call.Body[0].(string),
+		objectPath:  call.Body[1].(dbus.ObjectPath),
+		runtimePath: call.Body[2].(string),
+		fifoFD:      call.Body[3].(dbus.UnixFD),
+		uid:         call.Body[4].(uint32),
+		seatID:      call.Body[5].(string),
+		vtnr:        call.Body[6].(uint32),
+		existing:    call.Body[7].(bool),
+	}, nil
+}
+
+// releaseSession releases the session identified by sessionID.
+func releaseSession(sessionID string) error {
+	// https://www.freedesktop.org/software/systemd/man/org.freedesktop.login1.html
+	_, err := callLogin1("org.freedesktop.login1.Manager.ReleaseSession", dbus.FlagNoReplyExpected, sessionID)
+	return err
+}
+
+// maybeStartLoginSessionLinux is the linux implementation of maybeStartLoginSession.
+func maybeStartLoginSessionLinux(logf logger.Logf, uid uint32, localUser, remoteUser, remoteHost, tty string) (func() error, error) {
+	if os.Geteuid() != 0 {
+		return nil, nil
+	}
+	logf("starting session for user %d", uid)
+	// The only way we can actually start a new session is if we are
+	// running outside one and are root, which is typically the case
+	// for systemd managed tailscaled.
+	resp, err := createSession(uint32(uid), remoteUser, remoteHost, tty)
+	if err != nil {
+		// TODO(maisem): figure out if we are running in a session.
+		// We can look at the DBus GetSessionByPID API.
+		// https://www.freedesktop.org/software/systemd/man/org.freedesktop.login1.html
+		// For now best effort is fine.
+		logf("ssh: failed to CreateSession for user %q (%d) %v", localUser, uid, err)
+		return nil, nil
+	}
+	if !resp.existing {
+		return func() error {
+			return releaseSession(resp.sessionID)
+		}, nil
+	}
+	return nil, nil
+}

+ 122 - 179
ssh/tailssh/tailssh.go

@@ -18,16 +18,12 @@ import (
 	"os"
 	"os/exec"
 	"os/user"
-	"runtime"
 	"strings"
-	"syscall"
+	"sync"
 	"time"
-	"unsafe"
 
-	"github.com/creack/pty"
 	"github.com/gliderlabs/ssh"
 	"inet.af/netaddr"
-	"tailscale.com/cmd/tailscaled/childproc"
 	"tailscale.com/envknob"
 	"tailscale.com/ipn/ipnlocal"
 	"tailscale.com/net/tsaddr"
@@ -35,22 +31,17 @@ import (
 	"tailscale.com/types/logger"
 )
 
-func init() {
-	childproc.Add("ssh", sshChild)
-}
-
-func sshChild([]string) error {
-	fmt.Println("TODO(maisem): ssh dbus stuff")
-	return nil
-}
-
 // TODO(bradfitz): this is all very temporary as code is temporarily
 // being moved around; it will be restructured and documented in
 // following commits.
 
 // Handle handles an SSH connection from c.
 func Handle(logf logger.Logf, lb *ipnlocal.LocalBackend, c net.Conn) error {
-	srv := &server{lb, logf}
+	tsd, err := os.Executable()
+	if err != nil {
+		return err
+	}
+	srv := &server{lb, logf, tsd}
 	ss, err := srv.newSSHServer()
 	if err != nil {
 		return err
@@ -86,12 +77,16 @@ func (srv *server) newSSHServer() (*ssh.Server, error) {
 }
 
 type server struct {
-	lb   *ipnlocal.LocalBackend
-	logf logger.Logf
+	lb             *ipnlocal.LocalBackend
+	logf           logger.Logf
+	tailscaledPath string
 }
 
 var debugPolicyFile = envknob.String("TS_DEBUG_SSH_POLICY_FILE")
 
+// sshPolicy returns the SSHPolicy for current node.
+// If there is no SSHPolicy in the netmap, it returns a debugPolicy
+// if one is defined.
 func (srv *server) sshPolicy() (_ *tailcfg.SSHPolicy, ok bool) {
 	lb := srv.lb
 	nm := lb.NetMap()
@@ -117,70 +112,91 @@ func (srv *server) sshPolicy() (_ *tailcfg.SSHPolicy, ok bool) {
 	return nil, false
 }
 
-func (srv *server) handleSSH(s ssh.Session) {
-	lb := srv.lb
-	logf := srv.logf
-	sshUser := s.User()
-	addr := s.RemoteAddr()
-	logf("Handling SSH from %v for user %v", addr, sshUser)
-	ta, ok := addr.(*net.TCPAddr)
+func asTailscaleIPPort(a net.Addr) (netaddr.IPPort, error) {
+	ta, ok := a.(*net.TCPAddr)
 	if !ok {
-		logf("tsshd: rejecting non-TCP addr %T %v", addr, addr)
-		s.Exit(1)
-		return
+		return netaddr.IPPort{}, fmt.Errorf("non-TCP addr %T %v", a, a)
 	}
 	tanetaddr, ok := netaddr.FromStdIP(ta.IP)
 	if !ok {
-		logf("tsshd: rejecting unparseable addr %v", ta.IP)
-		s.Exit(1)
-		return
+		return netaddr.IPPort{}, fmt.Errorf("unparseable addr %v", ta.IP)
 	}
 	if !tsaddr.IsTailscaleIP(tanetaddr) {
-		logf("tsshd: rejecting non-Tailscale addr %v", ta.IP)
-		s.Exit(1)
-		return
+		return netaddr.IPPort{}, fmt.Errorf("non-Tailscale addr %v", ta.IP)
 	}
+	return netaddr.IPPortFrom(tanetaddr, uint16(ta.Port)), nil
+}
+
+// evaluatePolicy returns the SSHAction, sshConnInfo and localUser
+// after evaluating the sshUser and remoteAddr against the SSHPolicy.
+// The remoteAddr and localAddr params must be Tailscale IPs.
+func (srv *server) evaluatePolicy(sshUser string, localAddr, remoteAddr net.Addr) (_ *tailcfg.SSHAction, _ *sshConnInfo, localUser string, _ error) {
+	logf := srv.logf
+	lb := srv.lb
+	logf("Handling SSH from %v for user %v", remoteAddr, sshUser)
 
 	pol, ok := srv.sshPolicy()
 	if !ok {
-		logf("tsshd: rejecting connection; no SSH policy")
-		s.Exit(1)
-		return
+		return nil, nil, "", fmt.Errorf("tsshd: rejecting connection; no SSH policy")
 	}
 
-	srcIPP := netaddr.IPPortFrom(tanetaddr, uint16(ta.Port))
+	srcIPP, err := asTailscaleIPPort(remoteAddr)
+	if err != nil {
+		return nil, nil, "", fmt.Errorf("tsshd: rejecting: %w", err)
+	}
+	dstIPP, err := asTailscaleIPPort(localAddr)
+	if err != nil {
+		return nil, nil, "", err
+	}
 	node, uprof, ok := lb.WhoIs(srcIPP)
 	if !ok {
-		fmt.Fprintf(s, "Hello, %v. I don't know who you are.\n", srcIPP)
-		s.Exit(1)
-		return
+		return nil, nil, "", fmt.Errorf("Hello, %v. I don't know who you are.\n", srcIPP)
 	}
 
-	srcIP := srcIPP.IP()
 	ci := &sshConnInfo{
 		now:     time.Now(),
 		sshUser: sshUser,
-		srcIP:   srcIP,
+		src:     srcIPP,
+		dst:     dstIPP,
 		node:    node,
 		uprof:   &uprof,
 	}
-	action, localUser, ok := evalSSHPolicy(pol, ci)
-	if ok && action.Message != "" {
+	a, localUser, ok := evalSSHPolicy(pol, ci)
+	if !ok {
+		return nil, nil, "", fmt.Errorf("ssh: access denied for %q from %v", uprof.LoginName, ci.src.IP())
+	}
+	return a, ci, localUser, nil
+}
+
+// handleSSH is invoked when a new SSH connection attempt is made.
+func (srv *server) handleSSH(s ssh.Session) {
+	logf := srv.logf
+
+	sshUser := s.User()
+	action, ci, localUser, err := srv.evaluatePolicy(sshUser, s.LocalAddr(), s.RemoteAddr())
+	if err != nil {
+		logf(err.Error())
+		s.Exit(1)
+		return
+	}
+	if action.Message != "" {
 		io.WriteString(s.Stderr(), strings.Replace(action.Message, "\n", "\r\n", -1))
 	}
-	if !ok || action.Reject {
-		logf("ssh: access denied for %q from %v", uprof.LoginName, srcIP)
+	if action.Reject {
+		logf("ssh: access denied for %q from %v", ci.uprof.LoginName, ci.src.IP())
 		s.Exit(1)
 		return
 	}
 	if !action.Accept || action.HoldAndDelegate != "" {
 		fmt.Fprintf(s, "TODO: other SSHAction outcomes")
 		s.Exit(1)
+		return
 	}
 	lu, err := user.Lookup(localUser)
 	if err != nil {
 		logf("ssh: user Lookup %q: %v", localUser, err)
 		s.Exit(1)
+		return
 	}
 
 	var ctx context.Context = context.Background()
@@ -198,6 +214,23 @@ func (srv *server) handleSSH(s ssh.Session) {
 	srv.handleAcceptedSSH(ctx, s, ci, lu)
 }
 
+func (srv *server) handleSessionTermination(ctx context.Context, s ssh.Session, ci *sshConnInfo, cmd *exec.Cmd, exitOnce *sync.Once) {
+	<-ctx.Done()
+	// Either the process has already existed, in which case this does nothing.
+	// Or, the process is still running in which case this will kill it.
+	exitOnce.Do(func() {
+		err := ctx.Err()
+		if serr, ok := err.(SSHTerminationError); ok {
+			msg := serr.SSHTerminationMessage()
+			if msg != "" {
+				io.WriteString(s.Stderr(), "\r\n\r\n"+msg+"\r\n\r\n")
+			}
+		}
+		srv.logf("terminating SSH session from %v: %v", ci.src.IP(), err)
+		cmd.Process.Kill()
+	})
+}
+
 // handleAcceptedSSH handles s once it's been accepted and determined
 // that it should run as local system user lu.
 //
@@ -208,10 +241,6 @@ func (srv *server) handleAcceptedSSH(ctx context.Context, s ssh.Session, ci *ssh
 	logf := srv.logf
 	localUser := lu.Username
 
-	var err error
-	ptyReq, winCh, isPty := s.Pty()
-	logf("ssh: connection from %v %v to %v@ => %q. command = %q, env = %q", ci.srcIP, ci.uprof.LoginName, ci.sshUser, localUser, s.Command(), s.Environ())
-	var cmd *exec.Cmd
 	if euid := os.Geteuid(); euid != 0 {
 		if lu.Uid != fmt.Sprint(euid) {
 			logf("ssh: can't switch to user %q from process euid %v", localUser, euid)
@@ -219,87 +248,53 @@ func (srv *server) handleAcceptedSSH(ctx context.Context, s ssh.Session, ci *ssh
 			s.Exit(1)
 			return
 		}
-		cmd = exec.Command(loginShell(lu.Uid))
-		if rawCmd := s.RawCommand(); rawCmd != "" {
-			cmd.Args = append(cmd.Args, "-c", rawCmd)
-		}
-	} else {
-		if rawCmd := s.RawCommand(); rawCmd != "" {
-			cmd = exec.Command("/usr/bin/env", "su", "-c", rawCmd, localUser)
-			// TODO: and Env for PATH, SSH_CONNECTION, SSH_CLIENT, XDG_SESSION_TYPE, XDG_*, etc
-		} else {
-			cmd = exec.Command("/usr/bin/env", "su", "-", localUser)
-		}
 	}
-	cmd.Dir = lu.HomeDir
-	cmd.Env = append(cmd.Env, s.Environ()...)
-	cmd.Env = append(cmd.Env, envForUser(lu)...)
-	if ptyReq.Term != "" {
-		cmd.Env = append(cmd.Env, fmt.Sprintf("TERM=%s", ptyReq.Term))
-	}
-	logf("Running: %q", cmd.Args)
-	var toCmd io.WriteCloser
-	var fromCmd io.ReadCloser
-	if isPty {
-		f, err := pty.StartWithSize(cmd, &pty.Winsize{
-			Rows: uint16(ptyReq.Window.Width),
-			Cols: uint16(ptyReq.Window.Height),
-		})
+
+	cmd, stdin, stdout, stderr, err := srv.launchProcess(ctx, s, ci, lu)
+	if err != nil {
+		logf("start failed: %v", err.Error())
+		s.Exit(1)
+		return
+	}
+	// We use this sync.Once to ensure that we only terminate the process once,
+	// either it exits itself or is terminated
+	var exitOnce sync.Once
+	if ctx.Done() != nil {
+		ctx, cancel := context.WithCancel(ctx)
+		defer cancel()
+		go srv.handleSessionTermination(ctx, s, ci, cmd, &exitOnce)
+	}
+	go func() {
+		_, err := io.Copy(stdin, s)
 		if err != nil {
-			logf("running shell: %v", err)
-			s.Exit(1)
-			return
+			// TODO: don't log in the success case.
+			logf("ssh: stdin copy: %v", err)
 		}
-		defer f.Close()
-		toCmd = f
-		fromCmd = f
-		go func() {
-			for win := range winCh {
-				setWinsize(f, win.Width, win.Height)
-			}
-		}()
-	} else {
-		stdin, stdout, stderr, err := startWithStdPipes(cmd)
+		stdin.Close()
+	}()
+	go func() {
+		_, err := io.Copy(s, stdout)
 		if err != nil {
-			logf("ssh: start error: %f", err)
-			s.Exit(1)
-			return
+			// TODO: don't log in the success case.
+			logf("ssh: stdout copy: %v", err)
 		}
-		fromCmd, toCmd = stdout, stdin
-		go func() { io.Copy(s.Stderr(), stderr) }()
-	}
-
-	if ctx.Done() != nil {
-		done := make(chan struct{})
-		defer close(done)
+	}()
+	// stderr is nil for ptys.
+	if stderr != nil {
 		go func() {
-			select {
-			case <-done:
-			case <-ctx.Done():
-				err := ctx.Err()
-				if serr, ok := err.(SSHTerminationError); ok {
-					msg := serr.SSHTerminationMessage()
-					if msg != "" {
-						io.WriteString(s.Stderr(), "\r\n\r\n"+msg+"\r\n\r\n")
-					}
-				}
-				logf("terminating SSH session from %v: %v", ci.srcIP, err)
-				cmd.Process.Kill()
+			_, err := io.Copy(s.Stderr(), stderr)
+			if err != nil {
+				// TODO: don't log in the success case.
+				logf("ssh: stderr copy: %v", err)
 			}
 		}()
 	}
-
-	go func() {
-		_, err := io.Copy(toCmd, s) // stdin
-		logf("ssh: stdin copy: %v", err)
-		toCmd.Close()
-	}()
-	go func() {
-		_, err := io.Copy(s, fromCmd) // stdout
-		logf("ssh: stdout copy: %v", err)
-	}()
-
 	err = cmd.Wait()
+	// This will either make the SSH Termination goroutine be a no-op,
+	// or itself will be a no-op because the process was killed by the
+	// aforementioned goroutine.
+	exitOnce.Do(func() {})
+
 	if err == nil {
 		logf("ssh: Wait: ok")
 		s.Exit(0)
@@ -317,11 +312,6 @@ func (srv *server) handleAcceptedSSH(ctx context.Context, s ssh.Session, ci *ssh
 	return
 }
 
-func setWinsize(f *os.File, w, h int) {
-	syscall.Syscall(syscall.SYS_IOCTL, f.Fd(), uintptr(syscall.TIOCSWINSZ),
-		uintptr(unsafe.Pointer(&struct{ h, w, x, y uint16 }{uint16(h), uint16(w), 0, 0})))
-}
-
 type sshConnInfo struct {
 	// now is the time to consider the present moment for the
 	// purposes of rule evaluation.
@@ -330,8 +320,11 @@ type sshConnInfo struct {
 	// sshUser is the requested local SSH username ("root", "alice", etc).
 	sshUser string
 
-	// srcIP is the Tailscale IP that the connection came from.
-	srcIP netaddr.IP
+	// src is the Tailscale IP and port that the connection came from.
+	src netaddr.IPPort
+
+	// dst is the Tailscale IP and port that the connection came for.
+	dst netaddr.IPPort
 
 	// node is srcIP's node.
 	node *tailcfg.Node
@@ -399,7 +392,7 @@ func matchesPrincipal(ps []*tailcfg.SSHPrincipal, ci *sshConnInfo) bool {
 			return true
 		}
 		if p.NodeIP != "" {
-			if ip, _ := netaddr.ParseIP(p.NodeIP); ip == ci.srcIP {
+			if ip, _ := netaddr.ParseIP(p.NodeIP); ip == ci.src.IP() {
 				return true
 			}
 		}
@@ -409,53 +402,3 @@ func matchesPrincipal(ps []*tailcfg.SSHPrincipal, ci *sshConnInfo) bool {
 	}
 	return false
 }
-
-func loginShell(uid string) string {
-	switch runtime.GOOS {
-	case "linux":
-		out, _ := exec.Command("getent", "passwd", uid).Output()
-		// out is "root:x:0:0:root:/root:/bin/bash"
-		f := strings.SplitN(string(out), ":", 10)
-		if len(f) > 6 {
-			return strings.TrimSpace(f[6]) // shell
-		}
-	}
-	if e := os.Getenv("SHELL"); e != "" {
-		return e
-	}
-	return "/bin/bash"
-}
-
-func startWithStdPipes(cmd *exec.Cmd) (stdin io.WriteCloser, stdout, stderr io.ReadCloser, err error) {
-	defer func() {
-		if err != nil {
-			for _, c := range []io.Closer{stdin, stdout, stderr} {
-				if c != nil {
-					c.Close()
-				}
-			}
-		}
-	}()
-	stdin, err = cmd.StdinPipe()
-	if err != nil {
-		return
-	}
-	stdout, err = cmd.StdoutPipe()
-	if err != nil {
-		return
-	}
-	stderr, err = cmd.StderrPipe()
-	if err != nil {
-		return
-	}
-	err = cmd.Start()
-	return
-}
-
-func envForUser(u *user.User) []string {
-	return []string{
-		fmt.Sprintf("SHELL=" + loginShell(u.Uid)),
-		fmt.Sprintf("USER=" + u.Username),
-		fmt.Sprintf("HOME=" + u.HomeDir),
-	}
-}

+ 10 - 8
ssh/tailssh/tailssh_test.go

@@ -26,7 +26,6 @@ import (
 	"tailscale.com/ipn/store/mem"
 	"tailscale.com/net/tsdial"
 	"tailscale.com/tailcfg"
-	"tailscale.com/tstest"
 	"tailscale.com/types/logger"
 	"tailscale.com/util/lineread"
 	"tailscale.com/wgengine"
@@ -131,7 +130,7 @@ func TestMatchRule(t *testing.T) {
 				Principals: []*tailcfg.SSHPrincipal{{NodeIP: "1.2.3.4"}},
 				SSHUsers:   map[string]string{"*": "ubuntu"},
 			},
-			ci:       &sshConnInfo{srcIP: netaddr.MustParseIP("1.2.3.4")},
+			ci:       &sshConnInfo{src: netaddr.MustParseIPPort("1.2.3.4:30343")},
 			wantUser: "ubuntu",
 		},
 		{
@@ -174,8 +173,7 @@ func TestMatchRule(t *testing.T) {
 func timePtr(t time.Time) *time.Time { return &t }
 
 func TestSSH(t *testing.T) {
-	ml := new(tstest.MemLogger)
-	var logf logger.Logf = ml.Logf
+	var logf logger.Logf = t.Logf
 	eng, err := wgengine.NewFakeUserspaceEngine(logf, 0)
 	if err != nil {
 		t.Fatal(err)
@@ -191,7 +189,7 @@ func TestSSH(t *testing.T) {
 	dir := t.TempDir()
 	lb.SetVarRoot(dir)
 
-	srv := &server{lb, logf}
+	srv := &server{lb, logf, ""}
 	ss, err := srv.newSSHServer()
 	if err != nil {
 		t.Fatal(err)
@@ -204,7 +202,8 @@ func TestSSH(t *testing.T) {
 
 	ci := &sshConnInfo{
 		sshUser: "test",
-		srcIP:   netaddr.MustParseIP("1.2.3.4"),
+		src:     netaddr.MustParseIPPort("1.2.3.4:32342"),
+		dst:     netaddr.MustParseIPPort("1.2.3.5:22"),
 		node:    &tailcfg.Node{},
 		uprof:   &tailcfg.UserProfile{},
 	}
@@ -245,8 +244,8 @@ func TestSSH(t *testing.T) {
 	}
 
 	t.Run("env", func(t *testing.T) {
-		cmd := execSSH("env")
-		cmd.Env = append(os.Environ(), "LANG=foo")
+		cmd := execSSH("LANG=foo env")
+		cmd.Env = append(os.Environ(), "LOCAL_ENV=bar")
 		got, err := cmd.CombinedOutput()
 		if err != nil {
 			t.Fatal(err)
@@ -270,6 +269,9 @@ func TestSSH(t *testing.T) {
 		if got, want := m["LANG"], "foo"; got != want {
 			t.Errorf("LANG = %q; want %q", got, want)
 		}
+		if got := m["LOCAL_ENV"]; got != "" {
+			t.Errorf("LOCAL_ENV leaked over ssh: %v", got)
+		}
 		t.Logf("got: %+v", m)
 	})