Better error handling for connecting via reverse tunnel

Prior to this fix Teleport would not relay proxy errors from remote
clusters.

In other words, the following command:

```
$ tsh --cluster=remote ssh non-existing-host
```

Would print an error like:
"Cannot find a remote tunnel connection. ssh subsystem request failed"

Insead, it should say something like:
"dial non-existing-host error: no such host"

This commit fixes it. It works by:

- Sending net.Dial() error from the remote proxy back via stderr over
  reverse tunnel.

- Carefully handling this error to distinguish it from tunnel-related
  network errors.

Makefile

@@ -2,7 +2,7 @@
 # Naming convention:
 #	for stable releases we use "1.0.0" format
 #   for pre-releases, we use   "1.0.0-beta.2" format
-VERSION=1.2.7-beta
+VERSION=1.3.0
 
 # These are standard autotools variables, don't change them please
 BUILDDIR ?= build
@@ -26,7 +26,6 @@ $(eval BUILDFLAGS := $(ADDFLAGS) -ldflags '-w -s')
 #
 .PHONY: all
 all: setver teleport tctl tsh assets
-	#sudo killall teleport
 	cp -f build.assets/release.mk $(BUILDDIR)/Makefile
 
 .PHONY: tctl

lib/client/api.go

@@ -430,6 +430,7 @@ func (tc *TeleportClient) SSH(command []string, runLocally bool) error {
 		tc.Config.HostLogin,
 		false)
 	if err != nil {
+		tc.ExitStatus = 1
 		return trace.Wrap(err)
 	}
 	// proxy local ports (forward incoming connections to remote host ports)
@@ -754,7 +755,9 @@ func (tc *TeleportClient) ListNodes() ([]services.Server, error) {
 }
 
 // runCommand executes a given bash command on a bunch of remote nodes
-func (tc *TeleportClient) runCommand(siteName string, nodeAddresses []string, proxyClient *ProxyClient, command []string) error {
+func (tc *TeleportClient) runCommand(
+	siteName string, nodeAddresses []string, proxyClient *ProxyClient, command []string) error {
+
 	resultsC := make(chan error, len(nodeAddresses))
 	for _, address := range nodeAddresses {
 		go func(address string) {
@@ -783,9 +786,17 @@ func (tc *TeleportClient) runCommand(siteName string, nodeAddresses []string, pr
 				return
 			}
 			if err = nodeSession.runCommand(command, tc.OnShellCreated, tc.Config.Interactive); err != nil {
-				exitErr, ok := err.(*ssh.ExitError)
+				originErr := trace.Unwrap(err)
+				exitErr, ok := originErr.(*ssh.ExitError)
 				if ok {
 					tc.ExitStatus = exitErr.ExitStatus()
+				} else {
+					// if an error occurs, but no exit status is passed back, GoSSH returns
+					// a generic error like this. in this case the error message is printed
+					// to stderr by the remote process so we have to quietly return 1:
+					if strings.Contains(originErr.Error(), "exited without exit status") {
+						tc.ExitStatus = 1
+					}
 				}
 			}
 		}(address)

lib/reversetunnel/agent.go

@@ -195,9 +195,21 @@ func (a *Agent) proxyAccessPoint(ch ssh.Channel, req <-chan *ssh.Request) {
 	wg.Wait()
 }
 
+// proxyTransport runs as a goroutine running inside a reverse tunnel client
+// and it establishes and maintains the following remote connection:
+//
+// tsh -> proxy(also reverse-tunnel-server) -> reverse-tunnel-agent
+//
+// ch   : SSH channel which received "teleport-transport" out-of-band request
+// reqC : request payload
 func (a *Agent) proxyTransport(ch ssh.Channel, reqC <-chan *ssh.Request) {
 	defer ch.Close()
 
+	// always push space into stderr to make sure the caller can always
+	// safely call read(stderr) without blocking. this stderr is only used
+	// to request proxying of TCP/IP via reverse tunnel.
+	fmt.Fprint(ch.Stderr(), " ")
+
 	var req *ssh.Request
 	select {
 	case <-a.broadcastClose.C:
@@ -218,7 +230,12 @@ func (a *Agent) proxyTransport(ch ssh.Channel, reqC <-chan *ssh.Request) {
 
 	conn, err := net.Dial("tcp", server)
 	if err != nil {
-		log.Errorf("failed to dial: %v, err: %v", server, err)
+		log.Error(trace.DebugReport(err))
+		// write the connection error to stderr of the caller (via SSH channel)
+		// so the error will be propagated all the way back to the
+		// client (most likely tsh)
+		fmt.Fprint(ch.Stderr(), err.Error())
+		req.Reply(false, []byte(err.Error()))
 		return
 	}
 	req.Reply(true, []byte("connected"))

lib/reversetunnel/srv.go

@@ -18,6 +18,7 @@ package reversetunnel
 
 import (
 	"fmt"
+	"io/ioutil"
 	"net"
 	"net/http"
 	"strings"
@@ -514,6 +515,12 @@ func (s *tunnelSite) String() string {
 	return fmt.Sprintf("remoteSite(%v)", s.domainName)
 }
 
+func (s *tunnelSite) connectionCount() int {
+	s.Lock()
+	defer s.Unlock()
+	return len(s.connections)
+}
+
 func (s *tunnelSite) nextConn() (*remoteConn, error) {
 	s.Lock()
 	defer s.Unlock()
@@ -668,8 +675,9 @@ func (s *tunnelSite) dialAccessPoint(network, addr string) (net.Conn, error) {
 // Dial is used to connect a requesting client (say, tsh) to an SSH server
 // located in a remote connected site, the connection goes through the
 // reverse proxy tunnel.
-func (s *tunnelSite) Dial(network string, addr string) (net.Conn, error) {
+func (s *tunnelSite) Dial(network string, addr string) (conn net.Conn, err error) {
 	s.log.Infof("[TUNNEL] dialing %v@%v through the tunnel", addr, s.domainName)
+	stop := false
 
 	try := func() (net.Conn, error) {
 		remoteConn, err := s.nextConn()
@@ -682,34 +690,44 @@ func (s *tunnelSite) Dial(network string, addr string) (net.Conn, error) {
 			remoteConn.markInvalid(err)
 			return nil, trace.Wrap(err)
 		}
-		// we're creating a new SSH connection inside reverse SSH connection
-		// as a new SSH channel:
+		stop = true
+		// send a special SSH out-of-band request called "teleport-transport"
+		// the agent on the other side will create a new TCP/IP connection to
+		// 'addr' on its network and will start proxying that connection over
+		// this SSH channel:
 		var dialed bool
 		dialed, err = ch.SendRequest(chanTransportDialReq, true, []byte(addr))
 		if err != nil {
-			remoteConn.markInvalid(err)
 			return nil, trace.Wrap(err)
 		}
 		if !dialed {
-			remoteConn.markInvalid(err)
-			return nil, trace.ConnectionProblem(
-				nil, "remote server %v is not available", addr)
+			defer ch.Close()
+			// pull the error message from the tunnel client (remote cluster)
+			// passed to us via stderr:
+			errMessage, _ := ioutil.ReadAll(ch.Stderr())
+			if errMessage == nil {
+				errMessage = []byte("failed connecting to " + addr)
+			}
+			return nil, trace.Errorf(strings.TrimSpace(string(errMessage)))
 		}
 		return utils.NewChConn(remoteConn.sshConn, ch), nil
 	}
-
-	for {
-		conn, err := try()
-		if err != nil {
-			s.log.Errorf("[TUNNEL] Dial(addr=%v) failed: %v", addr, err)
-			// we interpret it as a "out of connections and will try again"
-			if trace.IsNotFound(err) {
-				return nil, trace.Wrap(err)
-			}
-			continue
+	// loop through existing TCP/IP connections (reverse tunnels) and try
+	// to establish an inbound connection-over-ssh-channel to the remote
+	// cluster (AKA "remotetunnel agent"):
+	for i := 0; i < s.connectionCount() && !stop; i++ {
+		conn, err = try()
+		if err == nil {
+			return conn, nil
 		}
-		return conn, nil
+		s.log.Errorf("[TUNNEL] Dial(addr=%v) failed: %v", addr, err)
 	}
+	// didn't connect and no error? this means we didn't have any connected
+	// tunnels to try
+	if err == nil {
+		err = trace.Errorf("%v is offline", s.GetName())
+	}
+	return nil, err
 }
 
 func (s *tunnelSite) DialServer(addr string) (net.Conn, error) {

lib/srv/sshserver.go

@@ -596,7 +596,7 @@ func (s *Server) handleDirectTCPIPRequest(sconn *ssh.ServerConn, ch ssh.Channel,
 	ctx.Infof("direct-tcpip channel: %#v to --> %v", req, addr)
 	conn, err := net.Dial("tcp", addr)
 	if err != nil {
-		ctx.Infof("failed to connect to: %v, err: %v", addr, err)
+		ctx.Infof("failed connecting to: %v, err: %v", addr, err)
 		return
 	}
 	defer conn.Close()
@@ -807,7 +807,7 @@ func (s *Server) handleSubsystem(ch ssh.Channel, req *ssh.Request, ctx *ctx) err
 	// starting subsystem is blocking to the client,
 	// while collecting its result and waiting is not blocking
 	if err := sb.start(ctx.conn, ch, req, ctx); err != nil {
-		ctx.Warnf("[SSH] failed to execute request, err: %v", err)
+		ctx.Warnf("[SSH] failed executing request: %v", err)
 		ctx.sendSubsystemResult(trace.Wrap(err))
 		return trace.Wrap(err)
 	}

vendor/github.com/gravitational/trace/errors.go

@@ -268,7 +268,10 @@ type ConnectionProblemError struct {
 
 // Error is debug - friendly error message
 func (c *ConnectionProblemError) Error() string {
-	return fmt.Sprintf("%v: %v", c.Message, c.Err)
+	if c.Err == nil {
+		return c.Message
+	}
+	return c.Err.Error()
 }
 
 // IsConnectionProblemError indicates that this error is of ConnectionProblemError type

version.go

@@ -2,7 +2,7 @@
 package teleport
 
 const (
-	Version = "1.2.7-beta"
+	Version = "1.3.0"
 )
 
 var Gitref string