From 1549e72be59b11a1625ecdb6f28b380de2e213dc Mon Sep 17 00:00:00 2001 From: Alexander Klizhentas Date: Wed, 24 Apr 2019 11:11:30 -0700 Subject: [PATCH] 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 | 46 +++++++++++++++++++++++++------------- lib/client/profile.go | 15 +++++++++++-- lib/client/profile_test.go | 18 +++++++++++---- tool/tsh/tsh.go | 16 ++++++++----- 4 files changed, 67 insertions(+), 28 deletions(-) diff --git a/lib/client/api.go b/lib/client/api.go index 8e80464aeb018..ca855577d2ca7 100644 --- a/lib/client/api.go +++ b/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_addr:, 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") diff --git a/lib/client/profile.go b/lib/client/profile.go index 9d34e8cc658ca..1bb250a1d46df 100644 --- a/lib/client/profile.go +++ b/lib/client/profile.go @@ -131,7 +131,7 @@ 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) @@ -139,10 +139,21 @@ func (cp *ClientProfile) SaveTo(filePath string, opts ProfileOptions) error { 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) diff --git a/lib/client/profile_test.go b/lib/client/profile_test.go index bda1da1e3eb69..c6f92f7559fa0 100644 --- a/lib/client/profile_test.go +++ b/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) } diff --git a/tool/tsh/tsh.go b/tool/tsh/tsh.go index 3c9fbf5949560..cc4b59d994472 100644 --- a/tool/tsh/tsh.go +++ b/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,7 +850,6 @@ 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 @@ -855,7 +857,9 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (tc *client.TeleportClient, e 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)