Fix teleport parsing to support IPV6

This commit fixes #2124

lib/client/api.go

@@ -808,7 +808,16 @@ func (tc *TeleportClient) getTargetNodes(ctx context.Context, proxy *ProxyClient
 		}
 	}
 	if len(nodes) == 0 {
-		retval = append(retval, net.JoinHostPort(tc.Host, strconv.Itoa(tc.HostPort)))
+		// detect the common error when users use host:port address format
+		_, port, err := net.SplitHostPort(tc.Host)
+		// client has used host:port notation
+		if err == nil {
+			return nil, trace.BadParameter(
+				"please use ssh subcommand with '--port=%v' flag instead of semicolon",
+				port)
+		}
+		addr := net.JoinHostPort(tc.Host, strconv.Itoa(tc.HostPort))
+		retval = append(retval, addr)
 	}
 	return retval, nil
 }
@@ -851,7 +860,7 @@ func (tc *TeleportClient) SSH(ctx context.Context, command []string, runLocally
 	}
 	nodeClient, err := proxyClient.ConnectToNode(
 		ctx,
-		nodeAddrs[0]+"@"+tc.Namespace+"@"+siteInfo.Name,
+		NodeAddr{Addr: nodeAddrs[0], Namespace: tc.Namespace, Cluster: siteInfo.Name},
 		tc.Config.HostLogin,
 		false)
 	if err != nil {
@@ -973,11 +982,11 @@ func (tc *TeleportClient) Join(ctx context.Context, namespace string, sessionID
 		return trace.NotFound(notFoundErrorMessage)
 	}
 	// connect to server:
-	fullNodeAddr := node.GetAddr()
-	if tc.SiteName != "" {
-		fullNodeAddr = fmt.Sprintf("%s@%s@%s", node.GetAddr(), tc.Namespace, tc.SiteName)
-	}
-	nc, err := proxyClient.ConnectToNode(ctx, fullNodeAddr, tc.Config.HostLogin, false)
+	nc, err := proxyClient.ConnectToNode(ctx, NodeAddr{
+		Addr:      node.GetAddr(),
+		Namespace: tc.Namespace,
+		Cluster:   tc.SiteName,
+	}, tc.Config.HostLogin, false)
 	if err != nil {
 		return trace.Wrap(err)
 	}
@@ -1112,7 +1121,7 @@ func (tc *TeleportClient) ExecuteSCP(ctx context.Context, cmd scp.Command) (err
 
 	nodeClient, err := proxyClient.ConnectToNode(
 		ctx,
-		nodeAddrs[0]+"@"+tc.Namespace+"@"+clusterInfo.Name,
+		NodeAddr{Addr: nodeAddrs[0], Namespace: tc.Namespace, Cluster: clusterInfo.Name},
 		tc.Config.HostLogin,
 		false)
 	if err != nil {
@@ -1164,7 +1173,9 @@ func (tc *TeleportClient) SCP(ctx context.Context, args []string, port int, recu
 		if err != nil {
 			return nil, trace.Wrap(err)
 		}
-		return proxyClient.ConnectToNode(ctx, addr+"@"+tc.Namespace+"@"+siteInfo.Name, tc.HostLogin, false)
+		return proxyClient.ConnectToNode(ctx,
+			NodeAddr{Addr: addr, Namespace: tc.Namespace, Cluster: siteInfo.Name},
+			tc.HostLogin, false)
 	}
 
 	var progressWriter io.Writer
@@ -1191,11 +1202,14 @@ func (tc *TeleportClient) SCP(ctx context.Context, args []string, port int, recu
 			directoryMode = true
 		}
 
-		login, host, dest := scp.ParseSCPDestination(last)
-		if login != "" {
-			tc.HostLogin = login
+		dest, err := scp.ParseSCPDestination(last)
+		if err != nil {
+			return trace.Wrap(err)
 		}
-		addr := net.JoinHostPort(host, strconv.Itoa(port))
+		if dest.Login != "" {
+			tc.HostLogin = dest.Login
+		}
+		addr := net.JoinHostPort(dest.Host.Host(), strconv.Itoa(port))
 
 		client, err := connectToNode(addr)
 		if err != nil {
@@ -1207,7 +1221,7 @@ func (tc *TeleportClient) SCP(ctx context.Context, args []string, port int, recu
 			scpConfig := scp.Config{
 				User:           tc.Username,
 				ProgressWriter: progressWriter,
-				RemoteLocation: dest,
+				RemoteLocation: dest.Path,
 				Flags: scp.Flags{
 					Target:        []string{src},
 					Recursive:     recursive,
@@ -1227,10 +1241,13 @@ func (tc *TeleportClient) SCP(ctx context.Context, args []string, port int, recu
 		}
 		// download:
 	} else {
-		login, host, src := scp.ParseSCPDestination(first)
-		addr := net.JoinHostPort(host, strconv.Itoa(port))
-		if login != "" {
-			tc.HostLogin = login
+		src, err := scp.ParseSCPDestination(first)
+		if err != nil {
+			return trace.Wrap(err)
+		}
+		addr := net.JoinHostPort(src.Host.Host(), strconv.Itoa(port))
+		if src.Login != "" {
+			tc.HostLogin = src.Login
 		}
 		client, err := connectToNode(addr)
 		if err != nil {
@@ -1244,7 +1261,7 @@ func (tc *TeleportClient) SCP(ctx context.Context, args []string, port int, recu
 					Recursive: recursive,
 					Target:    []string{dest},
 				},
-				RemoteLocation: src,
+				RemoteLocation: src.Path,
 				ProgressWriter: progressWriter,
 			}
 
@@ -1302,7 +1319,9 @@ func (tc *TeleportClient) runCommand(
 				resultsC <- err
 			}()
 			var nodeClient *NodeClient
-			nodeClient, err = proxyClient.ConnectToNode(ctx, address+"@"+tc.Namespace+"@"+siteName, tc.Config.HostLogin, false)
+			nodeClient, err = proxyClient.ConnectToNode(ctx,
+				NodeAddr{Addr: address, Namespace: tc.Namespace, Cluster: siteName},
+				tc.Config.HostLogin, false)
 			if err != nil {
 				fmt.Fprintln(tc.Stderr, err)
 				return

lib/client/client.go

@@ -402,17 +402,49 @@ func (proxy *ProxyClient) dialAuthServer(ctx context.Context, clusterName string
 	), nil
 }
 
+// NodeAddr is a full node address
+type NodeAddr struct {
+	// Addr is an address to dial
+	Addr string
+	// Namespace is the node namespace
+	Namespace string
+	// Cluster is the name of the target cluster
+	Cluster string
+}
+
+// String returns a user-friendly name
+func (n NodeAddr) String() string {
+	parts := []string{nodeName(n.Addr)}
+	if n.Cluster != "" {
+		parts = append(parts, "on cluster", n.Cluster)
+	}
+	return strings.Join(parts, " ")
+}
+
+// ProxyFormat returns the address in the format
+// used by the proxy subsystem
+func (n *NodeAddr) ProxyFormat() string {
+	parts := []string{n.Addr}
+	if n.Namespace != "" {
+		parts = append(parts, n.Namespace)
+	}
+	if n.Cluster != "" {
+		parts = append(parts, n.Cluster)
+	}
+	return strings.Join(parts, "@")
+}
+
 // ConnectToNode connects to the ssh server via Proxy.
 // It returns connected and authenticated NodeClient
-func (proxy *ProxyClient) ConnectToNode(ctx context.Context, nodeAddress string, user string, quiet bool) (*NodeClient, error) {
-	log.Infof("Client=%v connecting to node=%s", proxy.clientAddr, nodeAddress)
+func (proxy *ProxyClient) ConnectToNode(ctx context.Context, nodeAddress NodeAddr, user string, quiet bool) (*NodeClient, error) {
+	log.Infof("Client=%v connecting to node=%v", proxy.clientAddr, nodeAddress)
 
 	// parse destination first:
 	localAddr, err := utils.ParseAddr("tcp://" + proxy.proxyAddress)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	fakeAddr, err := utils.ParseAddr("tcp://" + nodeAddress)
+	fakeAddr, err := utils.ParseAddr("tcp://" + nodeAddress.Addr)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
@@ -465,13 +497,13 @@ func (proxy *ProxyClient) ConnectToNode(ctx context.Context, nodeAddress string,
 		}
 	}
 
-	err = proxySession.RequestSubsystem("proxy:" + nodeAddress)
+	err = proxySession.RequestSubsystem("proxy:" + nodeAddress.ProxyFormat())
 	if err != nil {
 		// read the stderr output from the failed SSH session and append
 		// it to the end of our own message:
 		serverErrorMsg, _ := ioutil.ReadAll(proxyErr)
 		return nil, trace.ConnectionProblem(err, "failed connecting to node %v. %s",
-			nodeName(strings.Split(nodeAddress, "@")[0]), serverErrorMsg)
+			nodeName(nodeAddress.Addr), serverErrorMsg)
 	}
 	pipeNetConn := utils.NewPipeNetConn(
 		proxyReader,
@@ -485,16 +517,11 @@ func (proxy *ProxyClient) ConnectToNode(ctx context.Context, nodeAddress string,
 		Auth:            []ssh.AuthMethod{proxy.authMethod},
 		HostKeyCallback: proxy.hostKeyCallback,
 	}
-	conn, chans, reqs, err := newClientConn(ctx, pipeNetConn, nodeAddress, sshConfig)
+	conn, chans, reqs, err := newClientConn(ctx, pipeNetConn, nodeAddress.ProxyFormat(), sshConfig)
 	if err != nil {
 		if utils.IsHandshakeFailedError(err) {
 			proxySession.Close()
-			parts := strings.Split(nodeAddress, "@")
-			hostname := parts[0]
-			if len(hostname) == 0 && len(parts) > 1 {
-				hostname = "cluster " + parts[1]
-			}
-			return nil, trace.Errorf(`access denied to %v connecting to %v`, user, nodeName(hostname))
+			return nil, trace.AccessDenied(`access denied to %v connecting to %v`, user, nodeAddress)
 		}
 		return nil, trace.Wrap(err)
 	}

lib/service/service.go

@@ -1088,7 +1088,7 @@ func (process *TeleportProcess) initAuthService() error {
 		if aport == "" {
 			aport = port
 		}
-		authAddr = fmt.Sprintf("%v:%v", ahost, aport)
+		authAddr = net.JoinHostPort(ahost, aport)
 	} else {
 		// advertise-ip is not set, while the CA is listening on 0.0.0.0? lets try
 		// to guess the 'advertise ip' then:

lib/srv/regular/proxy.go

@@ -282,7 +282,7 @@ func (t *proxySubsys) proxyToHost(
 
 		ip, port, err := net.SplitHostPort(servers[i].GetAddr())
 		if err != nil {
-			t.log.Error(err)
+			t.log.Errorf("Failed to parse address %q: %v.", servers[i].GetAddr(), err)
 			continue
 		}
 		if t.host == ip || t.host == servers[i].GetHostname() || utils.SliceContainsStr(ips, ip) {

lib/srv/regular/sshserver.go

@@ -588,7 +588,7 @@ func (s *Server) AdvertiseAddr() string {
 	if aport == "" {
 		aport = port
 	}
-	return fmt.Sprintf("%v:%v", ahost, aport)
+	return net.JoinHostPort(ahost, port)
 }
 
 func (s *Server) getRole() teleport.Role {

lib/sshutils/scp/scp.go

@@ -23,6 +23,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -642,7 +643,7 @@ func (r *reader) read() error {
 		return trace.Wrap(err)
 	}
 	if n < 1 {
-		return trace.Errorf("unexpected error, read 0 bytes")
+		return trace.BadParameter("unexpected error, read 0 bytes")
 	}
 
 	switch r.b[0] {
@@ -653,29 +654,52 @@ func (r *reader) read() error {
 		if err := r.s.Err(); err != nil {
 			return trace.Wrap(err)
 		}
-		return trace.Errorf(r.s.Text())
-	}
-	return trace.Errorf("unrecognized command: %#v", r.b)
+		return trace.BadParameter(r.s.Text())
+	}
+	return trace.BadParameter("unrecognized command: %v", r.b)
+}
+
+var reSCP = regexp.MustCompile(
+	// optional username, note that outside group
+	// is a non-capturing as it includes @ sign we don't want
+	`(?:(?P<username>[^@]+)@)?` +
+		// either some stuff in brackets - [ipv6]
+		// or some stuff without brackets and colons
+		`(?P<host>` +
+		// this says: [stuff in brackets that is not brackets] - loose definition of the IP address
+		`(?:\[[^\[\]]+\])` +
+		// or
+		`|` +
+		// some stuff without brackets or colons to make sure the OR condition
+		// is not ambiguous
+		`(?:[^\[\:\]]+)` +
+		`)` +
+		// after colon, there is a path that could consist technically of
+		// any char
+		`:(?P<path>.+)`,
+)
+
+// Destination is scp destination to copy to or from
+type Destination struct {
+	// Login is an optional login username
+	Login string
+	// Host is a host to copy to/from
+	Host utils.NetAddr
+	// Path is a path to copy to/from
+	Path string
 }
 
 // ParseSCPDestination takes a string representing a remote resource for SCP
 // to download/upload, like "user@host:/path/to/resource.txt" and returns
 // 3 components of it
-func ParseSCPDestination(s string) (login, host, dest string) {
-	parts := strings.SplitN(s, "@", 2)
-	if len(parts) > 1 {
-		login = parts[0]
-		host = parts[1]
-	} else {
-		host = parts[0]
+func ParseSCPDestination(s string) (*Destination, error) {
+	out := reSCP.FindStringSubmatch(s)
+	if len(out) == 0 {
+		return nil, trace.BadParameter("failed to parse %q, try form user@host:/path", s)
 	}
-	parts = strings.SplitN(host, ":", 2)
-	if len(parts) > 1 {
-		host = parts[0]
-		dest = parts[1]
-	}
-	if len(dest) == 0 {
-		dest = "."
+	addr, err := utils.ParseAddr(out[2])
+	if err != nil {
+		return nil, trace.Wrap(err)
 	}
-	return login, host, dest
+	return &Destination{Login: out[1], Host: *addr, Path: out[3]}, nil
 }