implement rfd 18

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)
 		}

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 {

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("")