SCP error messages are proper now

`tsh scp` now reports the same error messages as OpenSSH scp:

- when talking to Teleport servers
- when talking to OpenSSH servers

lib/client/client.go

@@ -213,7 +213,10 @@ func (proxy *ProxyClient) ConnectToNode(nodeAddress string, user string) (*NodeC
 		err = proxySession.RequestSubsystem("proxy:" + nodeAddress)
 		if err != nil {
 			defer printErrors()
-			return nil, trace.Wrap(err)
+
+			parts := strings.Split(nodeAddress, "@")
+			siteName := parts[len(parts)-1]
+			return nil, trace.Errorf("Failed connecting to cluster %v: %v", siteName, err)
 		}
 		pipeNetConn := utils.NewPipeNetConn(
 			proxyReader,
@@ -490,15 +493,6 @@ func (client *NodeClient) scp(scpCommand scp.Command, shellCmd string, errWriter
 		return trace.Wrap(err)
 	}
 
-	stderr, err := session.StderrPipe()
-	if err != nil {
-		return trace.Wrap(err)
-	}
-
-	go func() {
-		io.Copy(errWriter, stderr)
-	}()
-
 	ch := utils.NewPipeNetConn(
 		stdout,
 		stdin,
@@ -507,14 +501,24 @@ func (client *NodeClient) scp(scpCommand scp.Command, shellCmd string, errWriter
 		&net.IPAddr{},
 	)
 
+	closeC := make(chan interface{}, 1)
 	go func() {
 		if err = scpCommand.Execute(ch); err != nil {
 			log.Error(err)
 		}
 		stdin.Close()
+		close(closeC)
 	}()
 
-	return trace.Wrap(session.Run(shellCmd))
+	runErr := session.Run(shellCmd)
+	if runErr != nil && err == nil {
+		err = runErr
+	}
+	<-closeC
+	if trace.IsEOF(err) {
+		err = nil
+	}
+	return trace.Wrap(err)
 }
 
 // listenAndForward listens on a given socket and forwards all incoming connections

lib/client/keyagent.go

@@ -101,7 +101,7 @@ func (a *LocalKeyAgent) AddHostSignersToCache(hostSigners []services.CertAuthori
 func (a *LocalKeyAgent) CheckHostSignature(hostId string, remote net.Addr, key ssh.PublicKey) error {
 	// TODO (ev) remove this!
 	// we're temporarily turning off host validation to experiment with OpenSSH servers
-	return nil
+	//return nil
 
 	cert, ok := key.(*ssh.Certificate)
 	if !ok {

lib/srv/exec.go

@@ -58,7 +58,6 @@ type execResponse struct {
 	cmdName string
 	cmd     *exec.Cmd
 	ctx     *ctx
-	isSCP   bool
 }
 
 // parseExecRequest parses SSH exec request
@@ -68,12 +67,10 @@ func parseExecRequest(req *ssh.Request, ctx *ctx) (*execResponse, error) {
 		return nil, fmt.Errorf("failed to parse exec request, error: %v", err)
 	}
 	// is this scp request?
-	isSCP := false
 	args := strings.Split(e.Command, " ")
 	if len(args) > 0 {
 		_, f := filepath.Split(args[0])
 		if f == "scp" {
-			isSCP = true
 			// for 'scp' requests, we'll fork ourselves with scp parameters:
 			teleportBin, err := osext.Executable()
 			if err != nil {
@@ -89,7 +86,6 @@ func parseExecRequest(req *ssh.Request, ctx *ctx) (*execResponse, error) {
 	return &execResponse{
 		ctx:     ctx,
 		cmdName: e.Command,
-		isSCP:   isSCP,
 	}, nil
 }
 

lib/sshutils/scp/scp.go

@@ -158,17 +158,18 @@ func (cmd *Command) sendFile(r *reader, ch io.ReadWriter, fi os.FileInfo, path s
 			events.SCPAction:  "read",
 		})
 	}
-
 	out := fmt.Sprintf("C%04o %d %s\n", fi.Mode()&os.ModePerm, fi.Size(), fi.Name())
 	log.Infof("sendFile: %v", out)
+
 	_, err := io.WriteString(ch, out)
 	if err != nil {
 		return trace.Wrap(err)
 	}
+
 	if err := r.read(); err != nil {
 		return trace.Wrap(err)
 	}
-	log.Infof("sendFile got OK")
+
 	f, err := os.Open(path)
 	if err != nil {
 		return trace.Wrap(err)
@@ -185,7 +186,7 @@ func (cmd *Command) sendFile(r *reader, ch io.ReadWriter, fi os.FileInfo, path s
 	if err := sendOK(ch); err != nil {
 		return trace.Wrap(err)
 	}
-	return r.read()
+	return trace.Wrap(r.read())
 }
 
 // serveSink executes file uploading, when a remote server sends file(s)
@@ -235,8 +236,7 @@ func (cmd *Command) processCommand(ch io.ReadWriter, st *state, b byte, line str
 	log.Infof("<- %v %v", string(b), line)
 	switch b {
 	case WarnByte:
-		log.Warningf("got warning: %v", line)
-		return nil
+		return trace.Errorf(line)
 	case ErrByte:
 		return trace.Errorf(line)
 	case 'C':
@@ -244,10 +244,11 @@ func (cmd *Command) processCommand(ch io.ReadWriter, st *state, b byte, line str
 		if err != nil {
 			return trace.Wrap(err)
 		}
-		if err := sendOK(ch); err != nil {
+		err = cmd.receiveFile(st, *f, ch)
+		if err != nil {
 			return trace.Wrap(err)
 		}
-		return cmd.receiveFile(st, *f, ch)
+		return nil
 	case 'D':
 		d, err := ParseNewFile(line)
 		if err != nil {
@@ -269,6 +270,8 @@ func (cmd *Command) processCommand(ch io.ReadWriter, st *state, b byte, line str
 }
 
 func (cmd *Command) receiveFile(st *state, fc NewFileCmd, ch io.ReadWriter) error {
+	log.Infof("scp.receiveFile(%v)", cmd.Target)
+
 	// if the dest path is a folder, we should save the file to that folder, but
 	// only if is 'recursive' is set
 	path := cmd.Target
@@ -293,10 +296,17 @@ func (cmd *Command) receiveFile(st *state, fc NewFileCmd, ch io.ReadWriter) erro
 	}
 
 	defer f.Close()
-	n, err := io.CopyN(f, ch, int64(fc.Length))
-	if err != nil {
+
+	if err = sendOK(ch); err != nil {
 		return trace.Wrap(err)
 	}
+
+	n, err := io.CopyN(f, ch, int64(fc.Length))
+	if err != nil {
+		log.Error(err)
+		return trace.Wrap(err)
+	}
+
 	if n != int64(fc.Length) {
 		return trace.Errorf("unexpected file copy length: %v", n)
 	}
@@ -437,6 +447,7 @@ func sendOK(ch io.ReadWriter) error {
 // sendError gets called during all errors during SCP transmission. It does
 // logs the error into Teleport log and also writes it back to the SCP client
 func sendError(ch io.ReadWriter, err error) error {
+	log.Error(err)
 	if err == nil {
 		return nil
 	}
@@ -492,6 +503,9 @@ type reader struct {
 	r io.Reader
 }
 
+// read is used to "ask" for response messages after each SCP transmission
+// it only reads text data until a newline and returns 'nil' for "OK" responses
+// and errors for everything else
 func (r *reader) read() error {
 	n, err := r.r.Read(r.b)
 	if err != nil {
@@ -510,9 +524,6 @@ func (r *reader) read() error {
 		if err := r.s.Err(); err != nil {
 			return trace.Wrap(err)
 		}
-		if r.b[0] == ErrByte {
-			return trace.Wrap(err)
-		}
 		return trace.Errorf(r.s.Text())
 	}
 	return trace.Errorf("unrecognized command: %#v", r.b)