Fix underlying reader not a terminal issues

build.assets/tooling/cmd/difftest/main.go

@@ -55,6 +55,10 @@ var (
 		// TestSSHOnMultipleNodes and its successor TestSSHWithMFA take ~10-15s to run which prevents
 		// it from ever completing the 100 runs successfully.
 		"TestSSHOnMultipleNodes", "TestSSHWithMFA",
+
+		// TestProxySSH takes around 10-15s to run, largely due to the 7-10 seconds it takes to create a
+		// tsh test suite. This prevents it from ever completing the 100 runs successfully.
+		"TestProxySSH",
 	}
 )
 

lib/client/api.go

@@ -551,6 +551,10 @@ func RetryWithRelogin(ctx context.Context, tc *TeleportClient, fn func() error)
 
 	key, err := tc.Login(ctx)
 	if err != nil {
+		if errors.Is(err, prompt.ErrNotTerminal) {
+			log.WithError(err).Debugf("Relogin is not available in this environment")
+			return trace.Wrap(fnErr)
+		}
 		if trace.IsTrustError(err) {
 			return trace.Wrap(err, "refusing to connect to untrusted proxy %v without --insecure flag\n", tc.SSHProxyAddr)
 		}
@@ -3697,7 +3701,7 @@ func (tc *TeleportClient) headlessLogin(ctx context.Context, priv *keys.PrivateK
 
 	tshApprove := fmt.Sprintf("tsh headless approve --user=%v --proxy=%v %v", tc.Username, tc.WebProxyAddr, headlessAuthenticationID)
 
-	fmt.Fprintf(tc.Stdout, "Complete headless authentication in your local web browser:\n\n%s\n"+
+	fmt.Fprintf(tc.Stderr, "Complete headless authentication in your local web browser:\n\n%s\n"+
 		"\nor execute this command in your local terminal:\n\n%s\n", webUILink, tshApprove)
 
 	response, err := SSHAgentHeadlessLogin(ctx, SSHLoginHeadless{
@@ -3819,7 +3823,7 @@ func (tc *TeleportClient) Ping(ctx context.Context) (*webclient.PingResponse, er
 	// If version checking was requested and the server advertises a minimum version.
 	if tc.CheckVersions && pr.MinClientVersion != "" {
 		if err := utils.CheckVersion(teleport.Version, pr.MinClientVersion); err != nil && trace.IsBadParameter(err) {
-			fmt.Fprintf(tc.Config.Stderr, `
+			fmt.Fprintf(tc.Stderr, `
 			WARNING
 			Detected potentially incompatible client and server versions.
 			Minimum client version supported by the server is %v but you are using %v.
@@ -3863,15 +3867,19 @@ func (tc *TeleportClient) ShowMOTD(ctx context.Context) error {
 	}
 
 	if motd.Text != "" {
-		fmt.Fprintf(tc.Config.Stderr, "%s\nPress [ENTER] to continue.\n", motd.Text)
-		// We're re-using the password reader for user acknowledgment for
-		// aesthetic purposes, because we want to hide any garbage the
-		// use might enter at the prompt. Whatever the user enters will
-		// be simply discarded, and the user can still CTRL+C out if they
-		// disagree.
-		_, err := prompt.Stdin().ReadPassword(context.Background())
-		if err != nil {
-			return trace.Wrap(err)
+		fmt.Fprintln(tc.Stderr, motd.Text)
+
+		// If possible, prompt the user for acknowledement before continuing.
+		if stdin := prompt.Stdin(); stdin.IsTerminal() {
+			// We're re-using the password reader for user acknowledgment for
+			// aesthetic purposes, because we want to hide any garbage the
+			// user might enter at the prompt. Whatever the user enters will
+			// be simply discarded, and the user can still CTRL+C out if they
+			// disagree.
+			fmt.Fprintln(tc.Stderr, "Press [ENTER] to continue.")
+			if _, err := stdin.ReadPassword(ctx); err != nil {
+				return trace.Wrap(err)
+			}
 		}
 	}
 
@@ -4228,13 +4236,21 @@ func Username() (string, error) {
 
 // AskOTP prompts the user to enter the OTP token.
 func (tc *TeleportClient) AskOTP(ctx context.Context) (token string, err error) {
+	stdin := prompt.Stdin()
+	if !stdin.IsTerminal() {
+		return "", trace.Wrap(prompt.ErrNotTerminal, "cannot perform OTP login without a terminal")
+	}
 	return prompt.Password(ctx, tc.Stderr, prompt.Stdin(), "Enter your OTP token")
 }
 
 // AskPassword prompts the user to enter the password
 func (tc *TeleportClient) AskPassword(ctx context.Context) (pwd string, err error) {
+	stdin := prompt.Stdin()
+	if !stdin.IsTerminal() {
+		return "", trace.Wrap(prompt.ErrNotTerminal, "cannot perform password login without a terminal")
+	}
 	return prompt.Password(
-		ctx, tc.Stderr, prompt.Stdin(), fmt.Sprintf("Enter password for Teleport user %v", tc.Config.Username))
+		ctx, tc.Stderr, stdin, fmt.Sprintf("Enter password for Teleport user %v", tc.Config.Username))
 }
 
 // LoadTLSConfig returns the user's TLS configuration, either from static

lib/utils/prompt/context_reader.go

@@ -184,6 +184,11 @@ func (cr *ContextReader) processReads() {
 	}
 }
 
+// IsTerminal returns whether the given reader is a terminal.
+func (cr *ContextReader) IsTerminal() bool {
+	return cr.term.IsTerminal(cr.fd)
+}
+
 // handleInterrupt restores terminal state on interrupts.
 // Called only on global ContextReaders, such as Stdin.
 func (cr *ContextReader) handleInterrupt() {

lib/utils/prompt/mock.go

@@ -34,6 +34,10 @@ func NewFakeReader() *FakeReader {
 	return &FakeReader{}
 }
 
+func (r *FakeReader) IsTerminal() bool {
+	return true
+}
+
 func (r *FakeReader) AddReply(fn FakeReplyFunc) *FakeReader {
 	r.mu.Lock()
 	defer r.mu.Unlock()

lib/utils/prompt/stdin.go

@@ -28,6 +28,7 @@ var (
 
 // StdinReader contains ContextReader methods applicable to stdin.
 type StdinReader interface {
+	IsTerminal() bool
 	Reader
 	SecureReader
 }

tool/tsh/proxy.go

@@ -188,15 +188,28 @@ func sshProxy(ctx context.Context, tc *libclient.TeleportClient, sp sshProxyPara
 	}
 	defer upstreamConn.Close()
 
+	signers, err := tc.LocalAgent().Signers()
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	if len(signers) == 0 {
+		return trace.BadParameter("no SSH auth methods loaded, are you logged in?")
+	}
+
 	remoteProxyAddr := net.JoinHostPort(sp.proxyHost, sp.proxyPort)
 	client, err := makeSSHClient(ctx, upstreamConn, remoteProxyAddr, &ssh.ClientConfig{
-		User: tc.HostLogin,
-		Auth: []ssh.AuthMethod{
-			ssh.PublicKeysCallback(tc.LocalAgent().Signers),
-		},
+		User:            tc.HostLogin,
+		Auth:            []ssh.AuthMethod{ssh.PublicKeys(signers...)},
 		HostKeyCallback: tc.HostKeyCallback,
 	})
 	if err != nil {
+		if utils.IsHandshakeFailedError(err) {
+			// TODO(codingllama): Improve error message below for device trust.
+			//  An alternative we have here is querying the cluster to check if device
+			//  trust is required, a check similar to `IsMFARequired`.
+			log.Infof("Access denied to %v connecting to %v: %v", tc.HostLogin, remoteProxyAddr, err)
+			return trace.AccessDenied(`access denied to %v connecting to %v`, tc.HostLogin, remoteProxyAddr)
+		}
 		return trace.Wrap(err)
 	}
 	defer client.Close()

tool/tsh/proxy_test.go

@@ -47,7 +47,6 @@ import (
 	"github.com/gravitational/teleport/lib/events"
 	"github.com/gravitational/teleport/lib/service/servicecfg"
 	"github.com/gravitational/teleport/lib/teleagent"
-	"github.com/gravitational/teleport/lib/utils"
 )
 
 // TestSSH verifies "tsh ssh" command.
@@ -253,7 +252,9 @@ func TestSSHLoadAllCAs(t *testing.T) {
 
 // TestProxySSH verifies "tsh proxy ssh" functionality
 func TestProxySSH(t *testing.T) {
-	createAgent(t)
+	// ssh agent can cause race conditions in parallel tests.
+	disableAgent(t)
+	ctx := context.Background()
 
 	tests := []struct {
 		name string
@@ -280,7 +281,9 @@ func TestProxySSH(t *testing.T) {
 	}
 
 	for _, tc := range tests {
+		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
 			s := newTestSuite(t, tc.opts...)
 
 			proxyRequest := fmt.Sprintf("%s.%s:%d",
@@ -289,35 +292,27 @@ func TestProxySSH(t *testing.T) {
 				s.root.Config.SSH.Addr.Port(defaults.SSHServerListenPort))
 
 			runProxySSH := func(proxyRequest string, opts ...cliOption) error {
-				return Run(context.Background(), []string{
+				return Run(ctx, []string{
 					"--insecure",
 					"--proxy", s.root.Config.Proxy.WebAddr.Addr,
 					"proxy", "ssh", proxyRequest,
 				}, opts...)
 			}
 
-			t.Run("login", func(t *testing.T) {
-				t.Parallel()
-
-				// Should fail without login
-				err := runProxySSH(proxyRequest, setHomePath(t.TempDir()))
-				require.Error(t, err)
+			// login to Teleport
+			homePath, kubeConfigPath := mustLogin(t, s)
 
-				// login into Teleport
-				homePath, kubeConfigPath := mustLogin(t, s)
+			t.Run("logged in", func(t *testing.T) {
+				t.Parallel()
 
-				// Should succeed with login
-				err = runProxySSH(proxyRequest, setHomePath(homePath), setKubeConfigPath(kubeConfigPath))
+				err := runProxySSH(proxyRequest, setHomePath(homePath), setKubeConfigPath(kubeConfigPath))
 				require.NoError(t, err)
 			})
 
 			t.Run("re-login", func(t *testing.T) {
 				t.Parallel()
 
-				// login into Teleport
-				homePath, kubeConfigPath := mustLogin(t, s)
-
-				err := runProxySSH(proxyRequest, setHomePath(homePath), setKubeConfigPath(kubeConfigPath), setMockSSOLogin(t, s))
+				err := runProxySSH(proxyRequest, setHomePath(t.TempDir()), setKubeConfigPath(filepath.Join(t.TempDir(), teleport.KubeConfigFile)), setMockSSOLogin(t, s))
 				require.NoError(t, err)
 			})
 
@@ -332,13 +327,9 @@ func TestProxySSH(t *testing.T) {
 				t.Parallel()
 
 				invalidLoginRequest := fmt.Sprintf("%s@%s", "invalidUser", proxyRequest)
-
-				// login into Teleport
-				homePath, kubeConfigPath := mustLogin(t, s)
-
 				err := runProxySSH(invalidLoginRequest, setHomePath(homePath), setKubeConfigPath(kubeConfigPath), setMockSSOLogin(t, s))
 				require.Error(t, err)
-				require.True(t, utils.IsHandshakeFailedError(err), "expected handshake error, got %v", err)
+				require.True(t, trace.IsAccessDenied(err), "expected access denied, got %v", err)
 			})
 		})
 	}
@@ -704,6 +695,11 @@ func createAgent(t *testing.T) string {
 	return teleAgent.Path
 }
 
+func disableAgent(t *testing.T) {
+	t.Helper()
+	t.Setenv(teleport.SSHAuthSock, "")
+}
+
 func setMockSSOLogin(t *testing.T, s *suite) cliOption {
 	return func(cf *CLIConf) error {
 		cf.mockSSOLogin = mockSSOLogin(t, s.root.GetAuthServer(), s.user)