From d162b02060c331c35b786db102455b4cd7f2c690 Mon Sep 17 00:00:00 2001 From: xacrimon Date: Tue, 16 Mar 2021 20:21:19 +0100 Subject: [PATCH] implement rfd 18 --- lib/client/api.go | 43 +++- lib/client/keyagent.go | 49 ++-- lib/client/keyagent_test.go | 20 +- lib/client/keystore.go | 470 +++++++++++++++++++++++------------- lib/client/keystore_test.go | 51 ++++ lib/client/profile.go | 55 ----- lib/client/profile_test.go | 45 ---- tool/tsh/tsh.go | 23 +- 8 files changed, 447 insertions(+), 309 deletions(-) diff --git a/lib/client/api.go b/lib/client/api.go index 93c9ae730d230..4c3de0fd5fbb0 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -69,9 +69,26 @@ import ( const ( // ProfileDir is a directory location where tsh profiles (and session keys) are stored - ProfileDir = ".tsh" + ProfileDir = ".tsh" + AddKeysToAgentAuto = "auto" + AddKeysToAgentNo = "no" + AddKeysToAgentYes = "yes" + AddKeysToAgentOnly = "only" ) +var AllAddKeysOptions = []string{AddKeysToAgentAuto, AddKeysToAgentNo, AddKeysToAgentYes, AddKeysToAgentOnly} + +// ValidateAgentKeyOption validates that a string is a valid option for the AddKeysToAgent parameter. +func ValidateAgentKeyOption(supplied string) error { + for _, option := range AllAddKeysOptions { + if supplied == option { + return nil + } + } + + return trace.BadParameter("invalid value %q, must be one of %v", supplied, AllAddKeysOptions) +} + var log = logrus.WithFields(logrus.Fields{ trace.Component: teleport.ComponentClient, }) @@ -271,9 +288,12 @@ type Config struct { // (not currently implemented), or set to 'none' to suppress browser opening entirely. Browser string - // UseLocalSSHAgent will write user certificates to the local ssh-agent (or - // similar) socket at $SSH_AUTH_SOCK. - UseLocalSSHAgent bool + // AddKeysToAgent specifies how the client handles keys. + // auto - will attempt to add keys to agent if the agent supports it + // only - attempt to load keys into agent but don't write them to disk + // on - attempt to load keys into agent + // off - do not attempt to load keys into agent + AddKeysToAgent string // EnableEscapeSequences will scan Stdin for SSH escape sequences during // command/shell execution. This also requires Stdin to be an interactive @@ -298,7 +318,7 @@ func MakeDefaultConfig() *Config { Stdout: os.Stdout, Stderr: os.Stderr, Stdin: os.Stdin, - UseLocalSSHAgent: true, + AddKeysToAgent: AddKeysToAgentAuto, EnableEscapeSequences: true, } } @@ -945,7 +965,18 @@ func NewClient(c *Config) (tc *TeleportClient, err error) { } else { // initialize the local agent (auth agent which uses local SSH keys signed by the CA): webProxyHost, _ := tc.WebProxyHostPort() - tc.localAgent, err = NewLocalAgent(c.KeysDir, webProxyHost, c.Username, c.UseLocalSSHAgent) + + var keystore LocalKeyStore + if c.AddKeysToAgent != AddKeysToAgentOnly { + keystore, err = NewFSLocalKeyStore(c.KeysDir) + } else { + keystore, err = NewMemLocalKeyStore(c.KeysDir) + } + if err != nil { + return nil, trace.Wrap(err) + } + + tc.localAgent, err = NewLocalAgent(keystore, webProxyHost, c.Username, c.AddKeysToAgent) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/client/keyagent.go b/lib/client/keyagent.go index a0f0569b3022f..6a69c21a9ccd6 100644 --- a/lib/client/keyagent.go +++ b/lib/client/keyagent.go @@ -100,14 +100,25 @@ func NewKeyStoreCertChecker(keyStore LocalKeyStore) ssh.HostKeyCallback { } } -// NewLocalAgent reads all Teleport certificates from disk (using FSLocalKeyStore), -// creates a LocalKeyAgent, loads all certificates into it, and returns the agent. -func NewLocalAgent(keyDir, proxyHost, username string, useLocalSSHAgent bool) (a *LocalKeyAgent, err error) { - keystore, err := NewFSLocalKeyStore(keyDir) - if err != nil { - return nil, trace.Wrap(err) - } +func agentIsPresent() bool { + return os.Getenv(teleport.SSHAuthSock) != "" +} + +// agentSupportsSSHCertificates checks if the running agent supports SSH certificates. +// This detection implementation is as described in RFD 18 and works by simply checking for +// presence of gpg-agent which is a common agent known to not support SSH certificates. +func agentSupportsSSHCertificates() bool { + agent := os.Getenv(teleport.SSHAuthSock) + return !strings.Contains(agent, "gpg-agent") +} + +func shouldAddKeysToAgent(addKeysToAgent string) bool { + return (addKeysToAgent == AddKeysToAgentAuto && agentSupportsSSHCertificates()) || addKeysToAgent == AddKeysToAgentOnly || addKeysToAgent == AddKeysToAgentYes +} +// NewLocalAgent reads all available credentials from the provided LocalKeyStore +// and loads them into the local and system agent +func NewLocalAgent(keystore LocalKeyStore, proxyHost, username string, keysOption string) (a *LocalKeyAgent, err error) { a = &LocalKeyAgent{ log: logrus.WithFields(logrus.Fields{ trace.Component: teleport.ComponentKeyAgent, @@ -119,17 +130,16 @@ func NewLocalAgent(keyDir, proxyHost, username string, useLocalSSHAgent bool) (a proxyHost: proxyHost, } - if useLocalSSHAgent { + if shouldAddKeysToAgent(keysOption) { a.sshAgent = connectToSSHAgent() } else { log.Debug("Skipping connection to the local ssh-agent.") - } - // unload all teleport keys from the agent first to ensure - // we don't leave stale keys in the agent - err = a.UnloadKeys() - if err != nil { - return nil, trace.Wrap(err) + if !agentSupportsSSHCertificates() && agentIsPresent() { + log.Warn(`Certificate was not loaded into agent because the agent at SSH_AUTH_SOCK does not appear +to support SSH certificates. To force load the certificate into the running agent, use +the --add-keys-to-agent=yes flag.`) + } } // read in key for this user in proxy @@ -252,8 +262,7 @@ func (a *LocalKeyAgent) UnloadKeys() error { return nil } -// GetKey returns the key for this user in a proxy from the filesystem keystore -// at ~/.tsh. +// GetKey returns the key for this user in a proxy from the backing key store. // // clusterName is an optional teleport cluster name to load kubernetes // certificates for. @@ -403,7 +412,7 @@ func (a *LocalKeyAgent) defaultHostPromptFunc(host string, key ssh.PublicKey, wr // AddKey activates a new signed session key by adding it into the keystore and also // by loading it into the SSH agent func (a *LocalKeyAgent) AddKey(key *Key) (*agent.AddedKey, error) { - // save it to disk (usually into ~/.tsh) + // save it to the keystore (usually into ~/.tsh) err := a.keyStore.AddKey(a.proxyHost, a.username, key) if err != nil { return nil, trace.Wrap(err) @@ -460,14 +469,14 @@ func (a *LocalKeyAgent) DeleteKeys() error { func (a *LocalKeyAgent) AuthMethods() (m []ssh.AuthMethod) { // combine our certificates with external SSH agent's: var signers []ssh.Signer - if ourCerts, _ := a.Signers(); ourCerts != nil { - signers = append(signers, ourCerts...) - } if a.sshAgent != nil { if sshAgentCerts, _ := a.sshAgent.Signers(); sshAgentCerts != nil { signers = append(signers, sshAgentCerts...) } } + if ourCerts, _ := a.Signers(); ourCerts != nil { + signers = append(signers, ourCerts...) + } // for every certificate create a new "auth method" and return them m = make([]ssh.AuthMethod, 0) for i := range signers { diff --git a/lib/client/keyagent_test.go b/lib/client/keyagent_test.go index 74893849d2b17..e33e9789bd87e 100644 --- a/lib/client/keyagent_test.go +++ b/lib/client/keyagent_test.go @@ -96,7 +96,9 @@ func (s *KeyAgentTestSuite) SetUpTest(c *check.C) { // a teleport key with the teleport username. func (s *KeyAgentTestSuite) TestAddKey(c *check.C) { // make a new local agent - lka, err := NewLocalAgent(s.keyDir, s.hostname, s.username, true) + keystore, err := NewFSLocalKeyStore(s.keyDir) + c.Assert(err, check.IsNil) + lka, err := NewLocalAgent(keystore, s.hostname, s.username, AddKeysToAgentAuto) c.Assert(err, check.IsNil) // add the key to the local agent, this should write the key @@ -155,7 +157,9 @@ func (s *KeyAgentTestSuite) TestLoadKey(c *check.C) { userdata := []byte("hello, world") // make a new local agent - lka, err := NewLocalAgent(s.keyDir, s.hostname, s.username, true) + keystore, err := NewFSLocalKeyStore(s.keyDir) + c.Assert(err, check.IsNil) + lka, err := NewLocalAgent(keystore, s.hostname, s.username, AddKeysToAgentAuto) c.Assert(err, check.IsNil) // unload any keys that might be in the agent for this user @@ -213,7 +217,9 @@ func (s *KeyAgentTestSuite) TestLoadKey(c *check.C) { func (s *KeyAgentTestSuite) TestHostCertVerification(c *check.C) { // Make a new local agent. - lka, err := NewLocalAgent(s.keyDir, s.hostname, s.username, true) + keystore, err := NewFSLocalKeyStore(s.keyDir) + c.Assert(err, check.IsNil) + lka, err := NewLocalAgent(keystore, s.hostname, s.username, AddKeysToAgentAuto) c.Assert(err, check.IsNil) // By default user has not refused any hosts. @@ -294,7 +300,9 @@ func (s *KeyAgentTestSuite) TestHostCertVerification(c *check.C) { func (s *KeyAgentTestSuite) TestHostKeyVerification(c *check.C) { // make a new local agent - lka, err := NewLocalAgent(s.keyDir, s.hostname, s.username, true) + keystore, err := NewFSLocalKeyStore(s.keyDir) + c.Assert(err, check.IsNil) + lka, err := NewLocalAgent(keystore, s.hostname, s.username, AddKeysToAgentAuto) c.Assert(err, check.IsNil) // by default user has not refused any hosts: @@ -348,7 +356,9 @@ func (s *KeyAgentTestSuite) TestHostKeyVerification(c *check.C) { func (s *KeyAgentTestSuite) TestDefaultHostPromptFunc(c *check.C) { keygen := testauthority.New() - a, err := NewLocalAgent(s.keyDir, s.hostname, s.username, true) + keystore, err := NewFSLocalKeyStore(s.keyDir) + c.Assert(err, check.IsNil) + a, err := NewLocalAgent(keystore, s.hostname, s.username, AddKeysToAgentAuto) c.Assert(err, check.IsNil) _, keyBytes, err := keygen.GenerateKeyPair("") diff --git a/lib/client/keystore.go b/lib/client/keystore.go index 1f1a670cb5626..3537fc43a345d 100644 --- a/lib/client/keystore.go +++ b/lib/client/keystore.go @@ -65,6 +65,8 @@ const ( // // The _only_ filesystem-based implementation of LocalKeyStore is declared // below (FSLocalKeyStore) +// +// TODO: add `getKubeTLSCerts`, `deleteKubeTLSCerts`, etc methods to `LocalKeyStore` to avoid handling different keystore implementations inside `KeyOption` implementations. type LocalKeyStore interface { // AddKey adds the given session key for the proxy and username to the // storage backend. @@ -132,11 +134,7 @@ type LocalKeyStore interface { //    ├── bar.pub //    └── bar-x509.pem type FSLocalKeyStore struct { - // log holds the structured logger. - log *logrus.Entry - - // KeyDir is the directory where all keys are stored. - KeyDir string + fsLocalNonSessionKeyStore } // NewFSLocalKeyStore creates a new filesystem-based local keystore object @@ -150,19 +148,22 @@ func NewFSLocalKeyStore(dirPath string) (s *FSLocalKeyStore, err error) { } return &FSLocalKeyStore{ - log: logrus.WithFields(logrus.Fields{ - trace.Component: teleport.ComponentKeyStore, - }), - KeyDir: dirPath, + fsLocalNonSessionKeyStore: fsLocalNonSessionKeyStore{ + log: logrus.WithFields(logrus.Fields{ + trace.Component: teleport.ComponentKeyStore, + }), + KeyDir: dirPath, + }, }, nil } // AddKey adds a new key to the session store. If a key for the host is already // stored, overwrites it. func (fs *FSLocalKeyStore) AddKey(host, username string, key *Key) error { - dirPath, err := fs.dirFor(host, true) - if err != nil { - return trace.Wrap(err) + dirPath := fs.dirFor(host) + if err := os.MkdirAll(dirPath, profileDirPerms); err != nil { + fs.log.Error(err) + return trace.ConvertSystemError(err) } writeBytes := func(fname string, data []byte) error { fp := filepath.Join(dirPath, fname) @@ -172,16 +173,16 @@ func (fs *FSLocalKeyStore) AddKey(host, username string, key *Key) error { } return err } - if err = writeBytes(username+fileExtCert, key.Cert); err != nil { + if err := writeBytes(username+fileExtCert, key.Cert); err != nil { return trace.Wrap(err) } - if err = writeBytes(username+fileExtTLSCert, key.TLSCert); err != nil { + if err := writeBytes(username+fileExtTLSCert, key.TLSCert); err != nil { return trace.Wrap(err) } - if err = writeBytes(username+fileExtPub, key.Pub); err != nil { + if err := writeBytes(username+fileExtPub, key.Pub); err != nil { return trace.Wrap(err) } - if err = writeBytes(username, key.Priv); err != nil { + if err := writeBytes(username, key.Priv); err != nil { return trace.Wrap(err) } // TODO(awly): unit test this. @@ -220,10 +221,7 @@ func (fs *FSLocalKeyStore) AddKey(host, username string, key *Key) error { // DeleteKey deletes a key from the local store func (fs *FSLocalKeyStore) DeleteKey(host, username string, opts ...KeyOption) error { - dirPath, err := fs.dirFor(host, false) - if err != nil { - return trace.Wrap(err) - } + dirPath := fs.dirFor(host) files := []string{ filepath.Join(dirPath, username+fileExtCert), filepath.Join(dirPath, username+fileExtTLSCert), @@ -231,12 +229,12 @@ func (fs *FSLocalKeyStore) DeleteKey(host, username string, opts ...KeyOption) e filepath.Join(dirPath, username), } for _, fn := range files { - if err = os.Remove(fn); err != nil { + if err := os.Remove(fn); err != nil { return trace.Wrap(err) } } for _, o := range opts { - if err := o.deleteKey(dirPath, username); err != nil { + if err := o.deleteKey(fs, keyIndex{proxyHost: host, username: username}); err != nil { return trace.Wrap(err) } } @@ -249,12 +247,8 @@ func (fs *FSLocalKeyStore) DeleteKey(host, username string, opts ...KeyOption) e // Useful when needing to log out of a specific service, like a particular // database proxy. func (fs *FSLocalKeyStore) DeleteKeyOption(host, username string, opts ...KeyOption) error { - dirPath, err := fs.dirFor(host, false) - if err != nil { - return trace.Wrap(err) - } for _, o := range opts { - if err := o.deleteKey(dirPath, username); err != nil { + if err := o.deleteKey(fs, keyIndex{proxyHost: host, username: username}); err != nil { return trace.Wrap(err) } } @@ -276,11 +270,8 @@ func (fs *FSLocalKeyStore) DeleteKeys() error { // GetKey returns a key for a given host. If the key is not found, // returns trace.NotFound error. func (fs *FSLocalKeyStore) GetKey(proxyHost, username string, opts ...KeyOption) (*Key, error) { - dirPath, err := fs.dirFor(proxyHost, false) - if err != nil { - return nil, trace.Wrap(err) - } - _, err = ioutil.ReadDir(dirPath) + dirPath := fs.dirFor(proxyHost) + _, err := ioutil.ReadDir(dirPath) if err != nil { return nil, trace.NotFound("no session keys for %v in %v", username, proxyHost) } @@ -327,7 +318,7 @@ func (fs *FSLocalKeyStore) GetKey(proxyHost, username string, opts ...KeyOption) } for _, o := range opts { - if err := o.getKey(dirPath, username, key); err != nil { + if err := o.getKey(fs, keyIndex{proxyHost: proxyHost, username: username}, key); err != nil { fs.log.Error(err) return nil, trace.Wrap(err) } @@ -359,12 +350,18 @@ func (fs *FSLocalKeyStore) GetKey(proxyHost, username string, opts ...KeyOption) return key, nil } +type keyIndex struct { + proxyHost string + username string + clusterName string +} + // KeyOption is an additional step to run when loading (LocalKeyStore.GetKey) // or deleting (LocalKeyStore.DeleteKey) keys. These are the steps skipped by // default to reduce the amount of work that Get/DeleteKey performs by default. type KeyOption interface { - getKey(dirPath, username string, key *Key) error - deleteKey(dirPath, username string) error + getKey(store LocalKeyStore, idx keyIndex, key *Key) error + deleteKey(store LocalKeyStore, idx keyIndex) error } // WithKubeCerts returns a GetKeyOption to load kubernetes certificates from @@ -378,22 +375,35 @@ type withKubeCerts struct { } // TODO(awly): unit test this. -func (o withKubeCerts) getKey(dirPath, username string, key *Key) error { - kubeDir := filepath.Join(dirPath, username+kubeDirSuffix, o.teleportClusterName) - kubeFiles, err := ioutil.ReadDir(kubeDir) - if err != nil && !os.IsNotExist(err) { - return trace.Wrap(err) - } - if key.KubeTLSCerts == nil { - key.KubeTLSCerts = make(map[string][]byte) - } - for _, fi := range kubeFiles { - data, err := ioutil.ReadFile(filepath.Join(kubeDir, fi.Name())) - if err != nil { +func (o withKubeCerts) getKey(store LocalKeyStore, idx keyIndex, key *Key) error { + switch s := store.(type) { + case *FSLocalKeyStore: + dirPath := s.dirFor(idx.proxyHost) + kubeDir := filepath.Join(dirPath, idx.username+kubeDirSuffix, o.teleportClusterName) + kubeFiles, err := ioutil.ReadDir(kubeDir) + if err != nil && !os.IsNotExist(err) { return trace.Wrap(err) } - kubeCluster := strings.TrimSuffix(filepath.Base(fi.Name()), fileExtTLSCert) - key.KubeTLSCerts[kubeCluster] = data + if key.KubeTLSCerts == nil { + key.KubeTLSCerts = make(map[string][]byte) + } + for _, fi := range kubeFiles { + data, err := ioutil.ReadFile(filepath.Join(kubeDir, fi.Name())) + if err != nil { + return trace.Wrap(err) + } + kubeCluster := strings.TrimSuffix(filepath.Base(fi.Name()), fileExtTLSCert) + key.KubeTLSCerts[kubeCluster] = data + } + case *MemLocalKeyStore: + idx.clusterName = o.teleportClusterName + stored, ok := s.inMem[idx] + if !ok { + return trace.NotFound("key for %v not found", idx) + } + key.KubeTLSCerts = stored.KubeTLSCerts + default: + return trace.BadParameter("unexpected key store type %T", store) } if key.ClusterName == "" { key.ClusterName = o.teleportClusterName @@ -401,10 +411,22 @@ func (o withKubeCerts) getKey(dirPath, username string, key *Key) error { return nil } -func (o withKubeCerts) deleteKey(dirPath, username string) error { - kubeCertsDir := filepath.Join(dirPath, username+kubeDirSuffix, o.teleportClusterName) - if err := os.RemoveAll(kubeCertsDir); err != nil { - return trace.Wrap(err) +func (o withKubeCerts) deleteKey(store LocalKeyStore, idx keyIndex) error { + switch s := store.(type) { + case *FSLocalKeyStore: + dirPath := s.dirFor(idx.proxyHost) + kubeCertsDir := filepath.Join(dirPath, idx.username+kubeDirSuffix, o.teleportClusterName) + if err := os.RemoveAll(kubeCertsDir); err != nil { + return trace.Wrap(err) + } + case *MemLocalKeyStore: + idx.clusterName = o.teleportClusterName + stored, ok := s.inMem[idx] + if ok { + stored.KubeTLSCerts = nil + } + default: + return trace.BadParameter("unexpected key store type %T", store) } return nil } @@ -419,22 +441,35 @@ type withDBCerts struct { teleportClusterName, dbName string } -func (o withDBCerts) getKey(dirPath, username string, key *Key) error { - dbDir := filepath.Join(dirPath, username+dbDirSuffix, o.teleportClusterName) - dbFiles, err := ioutil.ReadDir(dbDir) - if err != nil && !os.IsNotExist(err) { - return trace.Wrap(err) - } - if key.DBTLSCerts == nil { - key.DBTLSCerts = make(map[string][]byte) - } - for _, fi := range dbFiles { - data, err := ioutil.ReadFile(filepath.Join(dbDir, fi.Name())) - if err != nil { +func (o withDBCerts) getKey(store LocalKeyStore, idx keyIndex, key *Key) error { + switch s := store.(type) { + case *FSLocalKeyStore: + dirPath := s.dirFor(idx.proxyHost) + dbDir := filepath.Join(dirPath, idx.username+dbDirSuffix, o.teleportClusterName) + dbFiles, err := ioutil.ReadDir(dbDir) + if err != nil && !os.IsNotExist(err) { return trace.Wrap(err) } - dbName := strings.TrimSuffix(filepath.Base(fi.Name()), fileExtTLSCert) - key.DBTLSCerts[dbName] = data + if key.DBTLSCerts == nil { + key.DBTLSCerts = make(map[string][]byte) + } + for _, fi := range dbFiles { + data, err := ioutil.ReadFile(filepath.Join(dbDir, fi.Name())) + if err != nil { + return trace.Wrap(err) + } + dbName := strings.TrimSuffix(filepath.Base(fi.Name()), fileExtTLSCert) + key.DBTLSCerts[dbName] = data + } + case *MemLocalKeyStore: + idx.clusterName = o.teleportClusterName + stored, ok := s.inMem[idx] + if !ok { + return trace.NotFound("key for %v not found", idx) + } + key.DBTLSCerts = stored.DBTLSCerts + default: + return trace.BadParameter("unexpected key store type %T", store) } if key.ClusterName == "" { key.ClusterName = o.teleportClusterName @@ -442,46 +477,81 @@ func (o withDBCerts) getKey(dirPath, username string, key *Key) error { return nil } -func (o withDBCerts) deleteKey(dirPath, username string) error { +func (o withDBCerts) deleteKey(store LocalKeyStore, idx keyIndex) error { // If database name is specified, remove only that cert, otherwise remove // certs for all databases a user is logged into. - if o.dbName != "" { - return os.Remove(filepath.Join(dirPath, username+dbDirSuffix, o.teleportClusterName, o.dbName+fileExtTLSCert)) + switch s := store.(type) { + case *FSLocalKeyStore: + dirPath := s.dirFor(idx.proxyHost) + if o.dbName != "" { + return os.Remove(filepath.Join(dirPath, idx.username+dbDirSuffix, o.teleportClusterName, o.dbName+fileExtTLSCert)) + } + return os.RemoveAll(filepath.Join(dirPath, idx.username+dbDirSuffix, o.teleportClusterName)) + case *MemLocalKeyStore: + idx.clusterName = o.teleportClusterName + stored, ok := s.inMem[idx] + if !ok { + return trace.NotFound("key for %v not found", idx) + } + if o.dbName != "" { + stored.DBTLSCerts[o.dbName] = nil + } else { + stored.DBTLSCerts = nil + } + default: + return trace.BadParameter("unexpected key store type %T", store) } - return os.RemoveAll(filepath.Join(dirPath, username+dbDirSuffix, o.teleportClusterName)) + return nil } -// SaveCerts saves trusted TLS certificates of certificate authorities -func (fs *FSLocalKeyStore) SaveCerts(proxy string, cas []auth.TrustedCerts) (retErr error) { - dir, err := fs.dirFor(proxy, true) - if err != nil { - return trace.Wrap(err) +// initKeysDir initializes the keystore root directory. Usually it is ~/.tsh +func initKeysDir(dirPath string) (string, error) { + var err error + // not specified? use `~/.tsh` + if dirPath == "" { + u, err := user.Current() + if err != nil { + dirPath = os.TempDir() + } else { + dirPath = u.HomeDir + } + dirPath = filepath.Join(dirPath, defaultKeyDir) } - fp, err := os.OpenFile(filepath.Join(dir, fileNameTLSCerts), os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0640) + // create if doesn't exist: + _, err = os.Stat(dirPath) if err != nil { - return trace.ConvertSystemError(err) - } - defer utils.StoreErrorOf(fp.Close, &retErr) - for _, ca := range cas { - for _, cert := range ca.TLSCertificates { - if _, err := fp.Write(cert); err != nil { - return trace.ConvertSystemError(err) - } - if _, err := fmt.Fprintln(fp); err != nil { - return trace.ConvertSystemError(err) + if os.IsNotExist(err) { + err = os.MkdirAll(dirPath, os.ModeDir|profileDirPerms) + if err != nil { + return "", trace.ConvertSystemError(err) } + } else { + return "", trace.Wrap(err) } } - return fp.Sync() + + return dirPath, nil +} + +type fsLocalNonSessionKeyStore struct { + // log holds the structured logger. + log *logrus.Entry + + // KeyDir is the directory where all keys are stored. + KeyDir string +} + +// dirFor returns the path to the session keys for a given host. The value +// for fs.KeyDir is typically "~/.tsh", sessionKeyDir is typically "keys", +// and proxyHost typically has values like "proxy.example.com". +func (fs *fsLocalNonSessionKeyStore) dirFor(proxyHost string) string { + return filepath.Join(fs.KeyDir, sessionKeyDir, proxyHost) } // GetCertsPEM returns trusted TLS certificates of certificate authorities PEM // blocks. -func (fs *FSLocalKeyStore) GetCertsPEM(proxy string) ([][]byte, error) { - dir, err := fs.dirFor(proxy, false) - if err != nil { - return nil, trace.Wrap(err) - } +func (fs *fsLocalNonSessionKeyStore) GetCertsPEM(proxy string) ([][]byte, error) { + dir := fs.dirFor(proxy) data, err := ioutil.ReadFile(filepath.Join(dir, fileNameTLSCerts)) if err != nil { return nil, trace.Wrap(err) @@ -507,7 +577,7 @@ func (fs *FSLocalKeyStore) GetCertsPEM(proxy string) ([][]byte, error) { // GetCerts returns trusted TLS certificates of certificate authorities as // x509.CertPool. -func (fs *FSLocalKeyStore) GetCerts(proxy string) (*x509.CertPool, error) { +func (fs *fsLocalNonSessionKeyStore) GetCerts(proxy string) (*x509.CertPool, error) { blocks, err := fs.GetCertsPEM(proxy) if err != nil { return nil, trace.Wrap(err) @@ -529,49 +599,8 @@ func (fs *FSLocalKeyStore) GetCerts(proxy string) (*x509.CertPool, error) { return pool, nil } -// AddKnownHostKeys adds a new entry to 'known_hosts' file -func (fs *FSLocalKeyStore) AddKnownHostKeys(hostname string, hostKeys []ssh.PublicKey) (retErr error) { - fp, err := os.OpenFile(filepath.Join(fs.KeyDir, fileNameKnownHosts), os.O_CREATE|os.O_RDWR, 0640) - if err != nil { - return trace.ConvertSystemError(err) - } - defer utils.StoreErrorOf(fp.Close, &retErr) - // read all existing entries into a map (this removes any pre-existing dupes) - entries := make(map[string]int) - output := make([]string, 0) - scanner := bufio.NewScanner(fp) - for scanner.Scan() { - line := scanner.Text() - if _, exists := entries[line]; !exists { - output = append(output, line) - entries[line] = 1 - } - } - // add every host key to the list of entries - for i := range hostKeys { - fs.log.Debugf("Adding known host %s with key: %v", hostname, sshutils.Fingerprint(hostKeys[i])) - bytes := ssh.MarshalAuthorizedKey(hostKeys[i]) - line := strings.TrimSpace(fmt.Sprintf("%s %s", hostname, bytes)) - if _, exists := entries[line]; !exists { - output = append(output, line) - } - } - // re-create the file: - _, err = fp.Seek(0, 0) - if err != nil { - return trace.Wrap(err) - } - if err = fp.Truncate(0); err != nil { - return trace.Wrap(err) - } - for _, line := range output { - fmt.Fprintf(fp, "%s\n", line) - } - return fp.Sync() -} - // GetKnownHostKeys returns all known public keys from 'known_hosts' -func (fs *FSLocalKeyStore) GetKnownHostKeys(hostname string) ([]ssh.PublicKey, error) { +func (fs *fsLocalNonSessionKeyStore) GetKnownHostKeys(hostname string) ([]ssh.PublicKey, error) { bytes, err := ioutil.ReadFile(filepath.Join(fs.KeyDir, fileNameKnownHosts)) if err != nil { if os.IsNotExist(err) { @@ -608,52 +637,77 @@ func (fs *FSLocalKeyStore) GetKnownHostKeys(hostname string) ([]ssh.PublicKey, e return retval, nil } -// dirFor returns the path to the session keys for a given host. The value -// for fs.KeyDir is typically "~/.tsh", sessionKeyDir is typically "keys", -// and proxyHost typically has values like "proxy.example.com". -// -// If the create flag is true, the directory will be created if it does -// not exist. -func (fs *FSLocalKeyStore) dirFor(proxyHost string, create bool) (string, error) { - dirPath := filepath.Join(fs.KeyDir, sessionKeyDir, proxyHost) - - if create { - if err := os.MkdirAll(dirPath, profileDirPerms); err != nil { - fs.log.Error(err) - return "", trace.ConvertSystemError(err) +// AddKnownHostKeys adds a new entry to 'known_hosts' file +func (fs *fsLocalNonSessionKeyStore) AddKnownHostKeys(hostname string, hostKeys []ssh.PublicKey) (retErr error) { + fp, err := os.OpenFile(filepath.Join(fs.KeyDir, fileNameKnownHosts), os.O_CREATE|os.O_RDWR, 0640) + if err != nil { + return trace.ConvertSystemError(err) + } + defer utils.StoreErrorOf(fp.Close, &retErr) + // read all existing entries into a map (this removes any pre-existing dupes) + entries := make(map[string]int) + output := make([]string, 0) + scanner := bufio.NewScanner(fp) + for scanner.Scan() { + line := scanner.Text() + if _, exists := entries[line]; !exists { + output = append(output, line) + entries[line] = 1 } } - return dirPath, nil -} + // check if the scanner ran into an error + if err := scanner.Err(); err != nil { + return trace.Wrap(err) + } -// initKeysDir initializes the keystore root directory. Usually it is ~/.tsh -func initKeysDir(dirPath string) (string, error) { - var err error - // not specified? use `~/.tsh` - if dirPath == "" { - u, err := user.Current() - if err != nil { - dirPath = os.TempDir() - } else { - dirPath = u.HomeDir + // add every host key to the list of entries + for i := range hostKeys { + fs.log.Debugf("Adding known host %s with key: %v", hostname, sshutils.Fingerprint(hostKeys[i])) + bytes := ssh.MarshalAuthorizedKey(hostKeys[i]) + line := strings.TrimSpace(fmt.Sprintf("%s %s", hostname, bytes)) + if _, exists := entries[line]; !exists { + output = append(output, line) } - dirPath = filepath.Join(dirPath, defaultKeyDir) } - // create if doesn't exist: - _, err = os.Stat(dirPath) + // re-create the file: + _, err = fp.Seek(0, 0) if err != nil { - if os.IsNotExist(err) { - err = os.MkdirAll(dirPath, os.ModeDir|profileDirPerms) - if err != nil { - return "", trace.ConvertSystemError(err) + return trace.Wrap(err) + } + if err = fp.Truncate(0); err != nil { + return trace.Wrap(err) + } + for _, line := range output { + fmt.Fprintf(fp, "%s\n", line) + } + return fp.Sync() +} + +// SaveCerts saves trusted TLS certificates of certificate authorities +func (fs *fsLocalNonSessionKeyStore) SaveCerts(proxy string, cas []auth.TrustedCerts) (retErr error) { + dir := fs.dirFor(proxy) + if err := os.MkdirAll(dir, profileDirPerms); err != nil { + fs.log.Error(err) + return trace.ConvertSystemError(err) + } + + fp, err := os.OpenFile(filepath.Join(dir, fileNameTLSCerts), os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0640) + if err != nil { + return trace.ConvertSystemError(err) + } + defer utils.StoreErrorOf(fp.Close, &retErr) + for _, ca := range cas { + for _, cert := range ca.TLSCertificates { + if _, err := fp.Write(cert); err != nil { + return trace.ConvertSystemError(err) + } + if _, err := fmt.Fprintln(fp); err != nil { + return trace.ConvertSystemError(err) } - } else { - return "", trace.Wrap(err) } } - - return dirPath, nil + return fp.Sync() } // noLocalKeyStore is a LocalKeyStore representing the absence of a keystore. @@ -687,3 +741,75 @@ func (noLocalKeyStore) SaveCerts(proxy string, cas []auth.TrustedCerts) error { } func (noLocalKeyStore) GetCerts(proxy string) (*x509.CertPool, error) { return nil, errNoLocalKeyStore } func (noLocalKeyStore) GetCertsPEM(proxy string) ([][]byte, error) { return nil, errNoLocalKeyStore } + +// MemLocalKeyStore is an in-memory session keystore implementation. +type MemLocalKeyStore struct { + fsLocalNonSessionKeyStore + inMem map[keyIndex]*Key +} + +// NewMemLocalKeyStore initializes a MemLocalKeyStore, the key directory here is only used +// for storing CA certificates and known host fingerprints. +func NewMemLocalKeyStore(dirPath string) (*MemLocalKeyStore, error) { + dirPath, err := initKeysDir(dirPath) + if err != nil { + return nil, trace.Wrap(err) + } + + inMem := make(map[keyIndex]*Key) + return &MemLocalKeyStore{fsLocalNonSessionKeyStore: fsLocalNonSessionKeyStore{ + log: logrus.WithFields(logrus.Fields{ + trace.Component: teleport.ComponentKeyStore, + }), + KeyDir: dirPath, + }, inMem: inMem}, nil +} + +// AddKey writes a key to the underlying key store. +func (s *MemLocalKeyStore) AddKey(proxy string, username string, key *Key) error { + s.inMem[keyIndex{proxyHost: proxy, username: username}] = key + if key.ClusterName != "" { + s.inMem[keyIndex{proxyHost: proxy, username: username, clusterName: key.ClusterName}] = key + } + return nil +} + +// GetKey returns the session key for the given username and proxy. +func (s *MemLocalKeyStore) GetKey(proxy, username string, opts ...KeyOption) (*Key, error) { + idx := keyIndex{proxyHost: proxy, username: username} + entry, ok := s.inMem[idx] + if !ok { + return nil, trace.NotFound("key for %v not found", idx) + } + for _, o := range opts { + if err := o.getKey(s, idx, entry); err != nil { + s.log.Error(err) + return nil, trace.Wrap(err) + } + } + return entry, nil +} + +// DeleteKey removes a specific session key from a proxy. +func (s *MemLocalKeyStore) DeleteKey(proxyHost, username string, opts ...KeyOption) error { + delete(s.inMem, keyIndex{proxyHost: proxyHost, username: username}) + s.DeleteKeyOption(proxyHost, username, opts...) + return nil +} + +// DeleteKeys removes all session keys. +func (s *MemLocalKeyStore) DeleteKeys() error { + s.inMem = make(map[keyIndex]*Key) + return nil +} + +// DeleteKeyOption deletes only secrets specified by the provided key +// options keeping user's SSH/TLS certificates and private key intact. +func (s *MemLocalKeyStore) DeleteKeyOption(proxyHost, username string, opts ...KeyOption) error { + for _, o := range opts { + if err := o.deleteKey(s, keyIndex{proxyHost: proxyHost, username: username}); err != nil { + return trace.Wrap(err) + } + } + return nil +} diff --git a/lib/client/keystore_test.go b/lib/client/keystore_test.go index 116be6dfbf645..fcd7473e7b1eb 100644 --- a/lib/client/keystore_test.go +++ b/lib/client/keystore_test.go @@ -448,3 +448,54 @@ Fa6bvW5jo543NztjlKts7XYVqroMCu0sIMS7R4JGsmw3VJcnnMP2 CAPub = []byte(`ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDAGDCf6+SMJwoSvZ9tfWYs3nnkH1qZVh8P99RkE1tcqkdqpieUzZaXJFH7EKtT0f9frFP7AomzW2zEVvF0FzVFYm1qrP9WlAKOiY66UHPC6bMHmFOkl8ZuUaOQ++m3XPB+Yp2kGDSPFdQcdHi7g3o5fR3F3QiZFDhb1BS0SrOCpOhLm7iLCl6DqLVKgB0cFpJ6piEr36causkECX8dVKC8v20af/5xCqU6JDPS3rVXbT6gwEA/6s5MiLBFef3yIwoWPNVbUdMvkvCK3eglBfQut38jq03YN7pMnFts46QXjlX8/+ScHNvFXR+meFy9kydCqDWp1SY1WLpULU7mog+L ekontsevoy@turing`) ) + +func TestMemLocalKeyStore(t *testing.T) { + s, cleanup := newTest(t) + defer cleanup() + + // define some helpers + const proxy = "test.com" + const username = "test" + + // create keystore + dir := t.TempDir() + keystore, err := NewMemLocalKeyStore(dir) + require.NoError(t, err) + + // add test key + key := s.makeSignedKey(t, false) + err = keystore.AddKey(proxy, username, key) + require.NoError(t, err) + + // check that the key exists in the store + retrievedKey, err := keystore.GetKey(proxy, username) + require.NoError(t, err) + require.Equal(t, key, retrievedKey) + + // delete the key + err = keystore.DeleteKey(proxy, username) + require.NoError(t, err) + + // check that the key doesn't exist in the store + retrievedKey, err = keystore.GetKey(proxy, username) + require.Error(t, err) + require.Nil(t, retrievedKey) + + // add it again + err = keystore.AddKey(proxy, username, key) + require.NoError(t, err) + + // check for the key + retrievedKey, err = keystore.GetKey(proxy, username) + require.NoError(t, err) + require.Equal(t, key, retrievedKey) + + // delete all keys + err = keystore.DeleteKeys() + require.NoError(t, err) + + // verify it's deleted + retrievedKey, err = keystore.GetKey(proxy, username) + require.Error(t, err) + require.Nil(t, retrievedKey) +} diff --git a/lib/client/profile.go b/lib/client/profile.go index 334fdf6b2cc6d..89a75291de822 100644 --- a/lib/client/profile.go +++ b/lib/client/profile.go @@ -70,65 +70,12 @@ func (cp *Profile) Name() string { return addr } -// migrateCurrentProfile makes a best-effort attempt to migrate -// the old symlink based current-profile link to the new -// file based current-profile link. -// -// DELETE IN: 6.0 -func migrateCurrentProfile(dir string) { - link := filepath.Join(dir, CurrentProfileSymlink) - linfo, err := os.Lstat(link) - if err != nil { - return - } - if finfo, err := os.Stat(filepath.Join(dir, CurrentProfileFilename)); err == nil { - if linfo.ModTime().Before(finfo.ModTime()) { - // current-profile is as new or newer than the legacy symlink, - // no migration necessary. - return - } - } - linked, err := os.Readlink(link) - if err != nil || linked == "" { - return - } - name := strings.TrimSuffix(filepath.Base(linked), ".yaml") - if name == "" { - return - } - if err := SetCurrentProfileName(dir, name); err != nil { - return - } - - // TODO IN 5.2: Re-enable removal after verifying that nothing else - // relis on `link` (note: exact version that this happens doesn't matter - // too much, but it should happen at least one version prior to removal - // of the migration). - // - //os.Remove(link) -} - -// DELETE IN: 6.0 -func setLegacySymlink(dir string, name string) error { - link := filepath.Join(dir, CurrentProfileSymlink) - if err := os.Remove(link); err != nil && !os.IsNotExist(err) { - log.Warningf("Failed to remove legacy symlink: %v", err) - } - return trace.ConvertSystemError(os.Symlink(name+".yaml", link)) -} - // SetCurrentProfileName attempts to set the current profile name. func SetCurrentProfileName(dir string, name string) error { if dir == "" { return trace.BadParameter("cannot set current profile: missing dir") } - // set legacy symlink first so that the current-profile file will have - // a more recent modification time. - if err := setLegacySymlink(dir, name); err != nil { - log.Warningf("Failed to set legacy symlink: %v", err) - } - path := filepath.Join(dir, CurrentProfileFilename) if err := ioutil.WriteFile(path, []byte(strings.TrimSpace(name)+"\n"), 0660); err != nil { return trace.Wrap(err) @@ -141,8 +88,6 @@ func GetCurrentProfileName(dir string) (name string, err error) { if dir == "" { return "", trace.BadParameter("cannot get current profile: missing dir") } - // DELETE IN 6.0 - migrateCurrentProfile(dir) data, err := ioutil.ReadFile(filepath.Join(dir, CurrentProfileFilename)) if err != nil { diff --git a/lib/client/profile_test.go b/lib/client/profile_test.go index 44108a2a9b6da..0f5db8c0f6c3d 100644 --- a/lib/client/profile_test.go +++ b/lib/client/profile_test.go @@ -83,48 +83,3 @@ func TestProfileBasics(t *testing.T) { require.NoError(t, err) require.Equal(t, *p, *clone) } - -// TestProfileSymlinkMigration verifies that the old `profile` symlink -// is correctly migrated to the new `current-profile` file. -// -// DELETE IN: 6.0 -func TestProfileSymlinkMigration(t *testing.T) { - t.Parallel() - - dir, err := ioutil.TempDir("", "teleport") - require.NoError(t, err) - defer os.RemoveAll(dir) - - name := "some-profile" - file := filepath.Join(dir, name+".yaml") - link := filepath.Join(dir, CurrentProfileSymlink) - - // note that we don't bother to create the actual profile; this - // migration deals solely with converting the `profile` symlink - // to a `current-profile` file. - - // create old style symlink - require.NoError(t, os.Symlink(file, link)) - - // ensure that link exists - _, err = os.Lstat(link) - require.NoError(t, err) - - // load current profile name; this should automatically - // trigger the migration and return the correct name. - cn, err := GetCurrentProfileName(dir) - require.NoError(t, err) - require.Equal(t, name, cn) - - // verify that current-profile file now exists - _, err = os.Stat(filepath.Join(dir, CurrentProfileFilename)) - require.NoError(t, err) - - // forcibly remove the symlink - require.NoError(t, os.Remove(link)) - - // loading current profile should still succeed. - cn, err = GetCurrentProfileName(dir) - require.NoError(t, err) - require.Equal(t, name, cn) -} diff --git a/tool/tsh/tsh.go b/tool/tsh/tsh.go index ecbaee3327c7c..015b6467812d6 100644 --- a/tool/tsh/tsh.go +++ b/tool/tsh/tsh.go @@ -194,8 +194,13 @@ type CLIConf struct { // UseLocalSSHAgent set to false will prevent this client from attempting to // connect to the local ssh-agent (or similar) socket at $SSH_AUTH_SOCK. + // + // Deprecated in favor of `AddKeysToAgent`. UseLocalSSHAgent bool + // AddKeysToAgent specifies the behaviour of how certs are handled. + AddKeysToAgent string + // EnableEscapeSequences will scan stdin for SSH escape sequences during // command/shell execution. This also requires stdin to be an interactive // terminal. @@ -243,6 +248,7 @@ const ( // cluster. All new code should use TELEPORT_CLUSTER instead. siteEnvVar = "TELEPORT_SITE" userEnvVar = "TELEPORT_USER" + addKeysToAgentEnvVar = "TELEPORT_ADD_KEYS_TO_AGENT" useLocalSSHAgentEnvVar = "TELEPORT_USE_LOCAL_SSH_AGENT" clusterHelp = "Specify the cluster to connect" @@ -279,7 +285,9 @@ func Run(args []string, opts ...cliOption) error { app.Flag("gops-addr", "Specify gops addr to listen on").Hidden().StringVar(&cf.GopsAddr) app.Flag("skip-version-check", "Skip version checking between server and client.").BoolVar(&cf.SkipVersionCheck) app.Flag("debug", "Verbose logging to stdout").Short('d').BoolVar(&cf.Debug) - app.Flag("use-local-ssh-agent", fmt.Sprintf("Load generated SSH certificates into the local ssh-agent (specified via $SSH_AUTH_SOCK). You can also set %v environment variable. Default is true.", useLocalSSHAgentEnvVar)). + app.Flag("add-keys-to-agent", fmt.Sprintf("Controls how keys are handled. Valid values are %v.", client.AllAddKeysOptions)).Short('k').Envar(addKeysToAgentEnvVar).Default(client.AddKeysToAgentAuto).StringVar(&cf.AddKeysToAgent) + app.Flag("use-local-ssh-agent", "Deprecated in favor of the add-keys-to-agent flag."). + Hidden(). Envar(useLocalSSHAgentEnvVar). Default("true"). BoolVar(&cf.UseLocalSSHAgent) @@ -460,6 +468,10 @@ func Run(args []string, opts ...cliOption) error { return trace.Wrap(err) } + if err := client.ValidateAgentKeyOption(cf.AddKeysToAgent); err != nil { + return trace.Wrap(err) + } + // Read in cluster flag from CLI or environment. readClusterFlag(&cf, os.Getenv) @@ -1634,11 +1646,10 @@ func makeClient(cf *CLIConf, useProfileLogin bool) (*client.TeleportClient, erro // (not currently implemented) or set to 'none' to suppress browser opening entirely. c.Browser = cf.Browser - // Do not write SSH certs into the local ssh-agent if user requested it. - // - // This is specifically for gpg-agent, which doesn't support SSH - // certificates (https://dev.gnupg.org/T1756) - c.UseLocalSSHAgent = cf.UseLocalSSHAgent + c.AddKeysToAgent = cf.AddKeysToAgent + if !cf.UseLocalSSHAgent { + c.AddKeysToAgent = client.AddKeysToAgentNo + } c.EnableEscapeSequences = cf.EnableEscapeSequences