Use relevant SSHProxyAddr after Login (#2657)

Login could change the SSH proxy address based
on the information from "ping" endpoint

The function ConnectToProxy was saving
old version of the variable and kept using
it during login procedure.

This commit fixes that so the relevant
variable is always referenced.

lib/client/api.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 Gravitational, Inc.
+Copyright 2016-2019 Gravitational, Inc.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -70,7 +70,7 @@ var log = logrus.WithFields(logrus.Fields{
 })
 
 const (
-	// Directory location where tsh profiles (and session keys) are stored
+	// ProfileDir is a directory location where tsh profiles (and session keys) are stored
 	ProfileDir = ".tsh"
 )
 
@@ -446,6 +446,9 @@ func Status(profileDir string, proxyHost string) (*ProfileStatus, []*ProfileStat
 		if file.IsDir() {
 			continue
 		}
+		if file.Mode()&os.ModeSymlink != 0 {
+			continue
+		}
 		if !strings.HasSuffix(file.Name(), ".yaml") {
 			continue
 		}
@@ -525,7 +528,7 @@ func (c *Config) LoadProfile(profileDir string, proxyName string) error {
 
 // SaveProfile updates the given profiles directory with the current configuration
 // If profileDir is an empty string, the default ~/.tsh is used
-func (c *Config) SaveProfile(profileDir string, profileOptions ...ProfileOptions) error {
+func (c *Config) SaveProfile(profileAliasHost, profileDir string, profileOptions ...ProfileOptions) error {
 	if c.WebProxyAddr == "" {
 		return nil
 	}
@@ -535,6 +538,11 @@ func (c *Config) SaveProfile(profileDir string, profileOptions ...ProfileOptions
 	profileDir = FullProfilePath(profileDir)
 	profilePath := path.Join(profileDir, webProxyHost) + ".yaml"
 
+	profileAliasPath := ""
+	if profileAliasHost != "" {
+		profileAliasPath = path.Join(profileDir, profileAliasHost) + ".yaml"
+	}
+
 	var cp ClientProfile
 	cp.Username = c.Username
 	cp.WebProxyAddr = c.WebProxyAddr
@@ -553,7 +561,7 @@ func (c *Config) SaveProfile(profileDir string, profileOptions ...ProfileOptions
 			opts |= flag
 		}
 	}
-	if err := cp.SaveTo(profilePath, opts); err != nil {
+	if err := cp.SaveTo(profileAliasPath, profilePath, opts); err != nil {
 		return trace.Wrap(err)
 	}
 	return nil
@@ -562,7 +570,7 @@ func (c *Config) SaveProfile(profileDir string, profileOptions ...ProfileOptions
 // ParseProxyHost parses the proxyHost string and updates the config.
 //
 // Format of proxyHost string:
-//   proxy_web_addr:<proxy_web_port>,<proxy_ssh_portt>
+//   proxy_web_addr:<proxy_web_port>,<proxy_ssh_port>
 func (c *Config) ParseProxyHost(proxyHost string) error {
 	host, port, err := net.SplitHostPort(proxyHost)
 	if err != nil {
@@ -1408,7 +1416,6 @@ func (tc *TeleportClient) connectToProxy(ctx context.Context) (*ProxyClient, err
 	var err error
 
 	proxyPrincipal := tc.getProxySSHPrincipal()
-	proxyAddr := tc.Config.SSHProxyAddr
 	sshConfig := &ssh.ClientConfig{
 		User:            proxyPrincipal,
 		HostKeyCallback: tc.HostKeyCallback,
@@ -1419,7 +1426,7 @@ func (tc *TeleportClient) connectToProxy(ctx context.Context) (*ProxyClient, err
 		return &ProxyClient{
 			teleportClient:  tc,
 			Client:          sshClient,
-			proxyAddress:    proxyAddr,
+			proxyAddress:    tc.Config.SSHProxyAddr,
 			proxyPrincipal:  proxyPrincipal,
 			hostKeyCallback: sshConfig.HostKeyCallback,
 			authMethod:      m,
@@ -1428,14 +1435,14 @@ func (tc *TeleportClient) connectToProxy(ctx context.Context) (*ProxyClient, err
 			clientAddr:      tc.ClientAddr,
 		}
 	}
-	successMsg := fmt.Sprintf("Successful auth with proxy %v", proxyAddr)
+	successMsg := fmt.Sprintf("Successful auth with proxy %v", tc.Config.SSHProxyAddr)
 	// try to authenticate using every non interactive auth method we have:
 	for i, m := range tc.authMethods() {
-		log.Infof("Connecting proxy=%v login='%v' method=%d", proxyAddr, sshConfig.User, i)
+		log.Infof("Connecting proxy=%v login='%v' method=%d", tc.Config.SSHProxyAddr, sshConfig.User, i)
 		var sshClient *ssh.Client
 
 		sshConfig.Auth = []ssh.AuthMethod{m}
-		sshClient, err = ssh.Dial("tcp", proxyAddr, sshConfig)
+		sshClient, err = ssh.Dial("tcp", tc.Config.SSHProxyAddr, sshConfig)
 		if err != nil {
 			if utils.IsHandshakeFailedError(err) {
 				log.Warn(err)
@@ -1450,35 +1457,38 @@ func (tc *TeleportClient) connectToProxy(ctx context.Context) (*ProxyClient, err
 	// is disabled in configuration, or the user refused connecting to untrusted hosts
 	if tc.Config.SkipLocalAuth || tc.localAgent.UserRefusedHosts() {
 		if err == nil {
-			err = trace.BadParameter("failed to authenticate with proxy %v", proxyAddr)
+			err = trace.BadParameter("failed to authenticate with proxy %v", tc.Config.SSHProxyAddr)
 		}
 		return nil, trace.Wrap(err)
 	}
 	// if we get here, it means we failed to authenticate using stored keys
 	// and we need to ask for the login information
+	// Login reaches out to ping endpoint of the proxy
+	// and has a side effect of updating tc.Config.SSHProxyAddr
+	// and other parameters, that's why they are referenced directly below
 	key, err := tc.Login(ctx, true)
 	if err != nil {
 		// we need to communicate directly to user here,
 		// otherwise user will see endless loop with no explanation
 		if trace.IsTrustError(err) {
-			fmt.Printf("Refusing to connect to untrusted proxy %v without --insecure flag\n", proxyAddr)
+			fmt.Printf("Refusing to connect to untrusted proxy %v without --insecure flag\n", tc.Config.SSHProxyAddr)
 		}
 		return nil, trace.Wrap(err)
 	}
 	// Save profile to record proxy credentials
-	if err := tc.SaveProfile("", ProfileCreateNew); err != nil {
+	if err := tc.SaveProfile(key.ProxyHost, "", ProfileCreateNew|ProfileMakeCurrent); err != nil {
 		log.Warningf("Failed to save profile: %v", err)
 	}
 	authMethod, err := key.AsAuthMethod()
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-
 	// After successful login we have local agent updated with latest
 	// and greatest auth information, try it now
 	sshConfig.Auth = []ssh.AuthMethod{authMethod}
-	sshConfig.User = proxyPrincipal
-	sshClient, err := ssh.Dial("tcp", proxyAddr, sshConfig)
+	sshConfig.User = tc.getProxySSHPrincipal()
+	log.Infof("Connecting proxy=%v login='%v' using local agent.", tc.Config.SSHProxyAddr, sshConfig.User)
+	sshClient, err := ssh.Dial("tcp", tc.Config.SSHProxyAddr, sshConfig)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
@@ -1535,6 +1545,9 @@ func (tc *TeleportClient) Login(ctx context.Context, activateKey bool) (*Key, er
 		}
 	}
 
+	// preserve original web proxy host that could have
+	webProxyHost, _ := tc.WebProxyHostPort()
+
 	if err := tc.applyProxySettings(pr.Proxy); err != nil {
 		return nil, trace.Wrap(err)
 	}
@@ -1592,6 +1605,7 @@ func (tc *TeleportClient) Login(ctx context.Context, activateKey bool) (*Key, er
 	// extract the new certificate out of the response
 	key.Cert = response.Cert
 	key.TLSCert = response.TLSCert
+	key.ProxyHost = webProxyHost
 
 	if len(response.HostSigners) <= 0 {
 		return nil, trace.BadParameter("bad response from the server: expected at least one certificate, got 0")

lib/client/profile.go

@@ -131,18 +131,29 @@ func ProfileFromFile(filePath string) (*ClientProfile, error) {
 }
 
 // SaveTo saves the profile into a given filename, optionally overwriting it.
-func (cp *ClientProfile) SaveTo(filePath string, opts ProfileOptions) error {
+func (cp *ClientProfile) SaveTo(profileAliasPath, filePath string, opts ProfileOptions) error {
 	bytes, err := yaml.Marshal(&cp)
 	if err != nil {
 		return trace.Wrap(err)
 	}
 	if err = ioutil.WriteFile(filePath, bytes, 0660); err != nil {
 		return trace.Wrap(err)
 	}
+	if profileAliasPath != "" && filepath.Base(profileAliasPath) != filepath.Base(filePath) {
+		if err := os.Remove(profileAliasPath); err != nil {
+			log.Warningf("Failed to remove symlink alias: %v", err)
+		}
+		err := os.Symlink(filepath.Base(filePath), profileAliasPath)
+		if err != nil {
+			log.Warningf("Failed to create profile alias: %v", err)
+		}
+	}
 	// set 'current' symlink:
 	if opts&ProfileMakeCurrent != 0 {
 		symlink := filepath.Join(filepath.Dir(filePath), CurrentProfileSymlink)
-		os.Remove(symlink)
+		if err := os.Remove(symlink); err != nil {
+			log.Warningf("Failed to remove symlink: %v", err)
+		}
 		err = os.Symlink(filepath.Base(filePath), symlink)
 	}
 	return trace.Wrap(err)

lib/client/profile_test.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 Gravitational, Inc.
+Copyright 2016-2019 Gravitational, Inc.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -42,11 +42,11 @@ func (s *ProfileTestSuite) TestEverything(c *check.C) {
 	pfile := path.Join(home, "test.yaml")
 
 	// save to a file:
-	err := p.SaveTo(pfile, 0)
+	err := p.SaveTo("", pfile, 0)
 	c.Assert(err, check.IsNil)
 
 	// try to save to non-existent dir, should get an error
-	err = p.SaveTo("/bad/directory/profile.yaml", 0)
+	err = p.SaveTo("", "/bad/directory/profile.yaml", 0)
 	c.Assert(err, check.NotNil)
 
 	// make sure there is no symlink:
@@ -55,7 +55,7 @@ func (s *ProfileTestSuite) TestEverything(c *check.C) {
 	c.Assert(os.IsNotExist(err), check.Equals, true)
 
 	// save again, this time with a symlink:
-	p.SaveTo(pfile, ProfileMakeCurrent)
+	p.SaveTo("", pfile, ProfileMakeCurrent)
 	stat, err := os.Stat(symlink)
 	c.Assert(err, check.IsNil)
 	c.Assert(stat.Size() > 10, check.Equals, true)
@@ -69,4 +69,14 @@ func (s *ProfileTestSuite) TestEverything(c *check.C) {
 	clone, err = ProfileFromDir(home, "test")
 	c.Assert(err, check.IsNil)
 	c.Assert(*clone, check.DeepEquals, *p)
+
+	// Save with alias
+	aliasPath := path.Join(home, "alias.yaml")
+	err = p.SaveTo(aliasPath, pfile, ProfileMakeCurrent)
+	c.Assert(err, check.IsNil)
+
+	// Load from alias works
+	clone, err = ProfileFromDir(home, "alias")
+	c.Assert(err, check.IsNil)
+	c.Assert(*clone, check.DeepEquals, *p)
 }

tool/tsh/tsh.go

@@ -1,5 +1,5 @@
 /*
-Copyright 2016 Gravitational, Inc.
+Copyright 2016-2019 Gravitational, Inc.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -398,7 +398,7 @@ func onLogin(cf *CLIConf) {
 		// but cluster is specified, treat this as selecting a new cluster
 		// for the same proxy
 		case (cf.Proxy == "" || host(cf.Proxy) == host(profile.ProxyURL.Host)) && cf.SiteName != "":
-			tc.SaveProfile("")
+			tc.SaveProfile("", "")
 			if err := kubeclient.UpdateKubeconfig(tc); err != nil {
 				utils.FatalError(err)
 			}
@@ -442,7 +442,7 @@ func onLogin(cf *CLIConf) {
 	}
 
 	// Regular login without -i flag.
-	tc.SaveProfile("")
+	tc.SaveProfile(key.ProxyHost, "")
 
 	// Print status to show information of the logged in user. Update the
 	// command line flag (used to print status) for the proxy to make sure any
@@ -495,7 +495,10 @@ func onLogout(cf *CLIConf) {
 		utils.FatalError(err)
 		return
 	}
-	profiles := append(available, active)
+	profiles := append([]*client.ProfileStatus{}, available...)
+	if active != nil {
+		profiles = append(profiles, active)
+	}
 
 	// Extract the proxy name.
 	proxyHost, _, err := net.SplitHostPort(cf.Proxy)
@@ -847,15 +850,16 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (tc *client.TeleportClient, e
 			fmt.Printf("WARNING: Failed to load tsh profile for %q: %v\n", cf.Proxy, err)
 		}
 	}
-
 	// 3: override with the CLI flags
 	if cf.Namespace != "" {
 		c.Namespace = cf.Namespace
 	}
 	if cf.Username != "" {
 		c.Username = cf.Username
 	}
-	if cf.Proxy != "" {
+	// if proxy is set, and proxy is not equal to profile's
+	// loaded addresses, override the values
+	if cf.Proxy != "" && c.WebProxyAddr == "" {
 		err = c.ParseProxyHost(cf.Proxy)
 		if err != nil {
 			return nil, trace.Wrap(err)