ssh/tailssh: handle terminal opcodes

Updates #3802 #4146

Signed-off-by: Maisem Ali <maisem@tailscale.com>

ssh/tailssh/incubator.go

@@ -29,6 +29,7 @@ import (
 	"github.com/creack/pty"
 	"github.com/tailscale/ssh"
 	"github.com/u-root/u-root/pkg/termios"
+	gossh "golang.org/x/crypto/ssh"
 	"golang.org/x/sys/unix"
 	"tailscale.com/cmd/tailscaled/childproc"
 	"tailscale.com/types/logger"
@@ -178,7 +179,7 @@ func (srv *server) launchProcess(ctx context.Context, s ssh.Session, ci *sshConn
 		stdin, stdout, stderr, err = startWithStdPipes(cmd)
 		return
 	}
-	pty, err := startWithPTY(cmd, ptyReq)
+	pty, err := srv.startWithPTY(cmd, ptyReq)
 	if err != nil {
 		return nil, nil, nil, nil, err
 	}
@@ -196,8 +197,70 @@ func resizeWindow(f *os.File, winCh <-chan ssh.Window) {
 	}
 }
 
+// opcodeShortName is a mapping of SSH opcode
+// to mnemonic names expected by the termios packaage.
+// These are meant to be platform independent.
+var opcodeShortName = map[uint8]string{
+	gossh.VINTR:         "intr",
+	gossh.VQUIT:         "quit",
+	gossh.VERASE:        "erase",
+	gossh.VKILL:         "kill",
+	gossh.VEOF:          "eof",
+	gossh.VEOL:          "eol",
+	gossh.VEOL2:         "eol2",
+	gossh.VSTART:        "start",
+	gossh.VSTOP:         "stop",
+	gossh.VSUSP:         "susp",
+	gossh.VDSUSP:        "dsusp",
+	gossh.VREPRINT:      "rprnt",
+	gossh.VWERASE:       "werase",
+	gossh.VLNEXT:        "lnext",
+	gossh.VFLUSH:        "flush",
+	gossh.VSWTCH:        "swtch",
+	gossh.VSTATUS:       "status",
+	gossh.VDISCARD:      "discard",
+	gossh.IGNPAR:        "ignpar",
+	gossh.PARMRK:        "parmrk",
+	gossh.INPCK:         "inpck",
+	gossh.ISTRIP:        "istrip",
+	gossh.INLCR:         "inlcr",
+	gossh.IGNCR:         "igncr",
+	gossh.ICRNL:         "icrnl",
+	gossh.IUCLC:         "iuclc",
+	gossh.IXON:          "ixon",
+	gossh.IXANY:         "ixany",
+	gossh.IXOFF:         "ixoff",
+	gossh.IMAXBEL:       "imaxbel",
+	gossh.IUTF8:         "iutf8",
+	gossh.ISIG:          "isig",
+	gossh.ICANON:        "icanon",
+	gossh.XCASE:         "xcase",
+	gossh.ECHO:          "echo",
+	gossh.ECHOE:         "echoe",
+	gossh.ECHOK:         "echok",
+	gossh.ECHONL:        "echonl",
+	gossh.NOFLSH:        "noflsh",
+	gossh.TOSTOP:        "tostop",
+	gossh.IEXTEN:        "iexten",
+	gossh.ECHOCTL:       "echoctl",
+	gossh.ECHOKE:        "echoke",
+	gossh.PENDIN:        "pendin",
+	gossh.OPOST:         "opost",
+	gossh.OLCUC:         "olcuc",
+	gossh.ONLCR:         "onlcr",
+	gossh.OCRNL:         "ocrnl",
+	gossh.ONOCR:         "onocr",
+	gossh.ONLRET:        "onlret",
+	gossh.CS7:           "cs7",
+	gossh.CS8:           "cs8",
+	gossh.PARENB:        "parenb",
+	gossh.PARODD:        "parodd",
+	gossh.TTY_OP_ISPEED: "tty_op_ispeed",
+	gossh.TTY_OP_OSPEED: "tty_op_ospeed",
+}
+
 // 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) {
+func (srv *server) startWithPTY(cmd *exec.Cmd, ptyReq ssh.Pty) (ptyFile *os.File, err error) {
 	var tty *os.File
 	ptyFile, tty, err = pty.Open()
 	if err != nil {
@@ -210,7 +273,7 @@ func startWithPTY(cmd *exec.Cmd, ptyReq ssh.Pty) (ptyFile *os.File, err error) {
 			tty.Close()
 		}
 	}()
-	ptyRawConn, err := ptyFile.SyscallConn()
+	ptyRawConn, err := tty.SyscallConn()
 	if err != nil {
 		return nil, fmt.Errorf("SyscallConn: %w", err)
 	}
@@ -228,21 +291,30 @@ func startWithPTY(cmd *exec.Cmd, ptyReq ssh.Pty) (ptyFile *os.File, err error) {
 		tios.Row = int(ptyReq.Window.Height)
 		tios.Col = int(ptyReq.Window.Width)
 
-		// And these are just stumbling around in the dark temporarily
-		// while we try to match OpenSSH settings. Empirically this makes
-		// stty -a output be the same, but we're still having problems:
-		// https://github.com/tailscale/tailscale/issues/4146
-		// TODO(bradfitz): figure all this out and do something more principled
-		// and confident and documented, once we have a clue.
-		tios.Ispeed = 9600
-		tios.Ospeed = 9600
-		tios.CC["eol"] = 255
-		tios.CC["eol2"] = 255
-		tios.Opts["echok"] = false
-		tios.Opts["imaxbel"] = true
-		tios.Opts["iutf8"] = true
-		tios.Opts["ixany"] = true
-		tios.Opts["pendin"] = true
+		for c, v := range ptyReq.Modes {
+			if c == gossh.TTY_OP_ISPEED {
+				tios.Ispeed = int(v)
+				continue
+			}
+			if c == gossh.TTY_OP_OSPEED {
+				tios.Ospeed = int(v)
+				continue
+			}
+			k, ok := opcodeShortName[c]
+			if !ok {
+				srv.logf("unknown opcode: %d", c)
+				continue
+			}
+			if _, ok := tios.CC[k]; ok {
+				tios.CC[k] = uint8(v)
+				continue
+			}
+			if _, ok := tios.Opts[k]; ok {
+				tios.Opts[k] = v > 0
+				continue
+			}
+			srv.logf("unsupported opcode: %v(%d)=%v", k, c, v)
+		}
 
 		// Save PTY settings.
 		if _, err := tios.STTY(int(fd)); err != nil {

ssh/tailssh/tailssh.go

@@ -19,7 +19,6 @@ import (
 	"os"
 	"os/exec"
 	"os/user"
-	"reflect"
 	"strings"
 	"sync"
 	"time"
@@ -353,6 +352,10 @@ func (srv *server) handleAcceptedSSH(ctx context.Context, s ssh.Session, ci *ssh
 		}
 	}
 
+	// Take control of the PTY so that we can configure it below.
+	// See https://github.com/tailscale/tailscale/issues/4146
+	s.DisablePTYEmulation()
+
 	cmd, stdin, stdout, stderr, err := srv.launchProcess(ctx, s, ci, lu)
 	if err != nil {
 		logf("start failed: %v", err.Error())
@@ -376,14 +379,7 @@ func (srv *server) handleAcceptedSSH(ctx context.Context, s ssh.Session, ci *ssh
 		stdin.Close()
 	}()
 	go func() {
-		// Write to s.Channel directly, avoiding gliderlab/ssh's (*session).Write
-		// call that translates newline endings, which we don't need.
-		// See https://github.com/tailscale/tailscale/issues/4146.
-		// TODO(bradfitz,maisem): remove this reflect hackery once gliderlab/ssh changes
-		// are all in.
-		// s is an gliderlabs/ssh.(*session); write to its Channel field.
-		sshChan := reflect.ValueOf(s).Elem().FieldByName("Channel").Interface().(io.Writer)
-		_, err := io.Copy(sshChan, stdout)
+		_, err := io.Copy(s, stdout)
 		if err != nil {
 			// TODO: don't log in the success case.
 			logf("ssh: stdout copy: %v", err)