Refine public key usage when remote

* Move all public key handling into one AuthMethod. Prioritize ssh-agent
  keys over identity files.
* Cache server connection when tunneling, saves one RoundTrip on ssh
  handshake

Signed-off-by: Jhon Honce <jhonce@redhat.com>

4 files changed, 114 insertions, 29 deletions

diff --git a/cmd/podman/system/connection/add.go b/cmd/podman/system/connection/add.go
index 57e747451..da5f652c8 100644
--- a/cmd/podman/system/connection/add.go
+++ b/cmd/podman/system/connection/add.go
@@ -168,19 +168,17 @@ func getUserInfo(uri *url.URL) (*url.Userinfo, error) {
 }
 
 func getUDS(cmd *cobra.Command, uri *url.URL) (string, error) {
-	var authMethods []ssh.AuthMethod
-	passwd, set := uri.User.Password()
-	if set {
-		authMethods = append(authMethods, ssh.Password(passwd))
-	}
+	var signers []ssh.Signer
 
+	passwd, passwdSet := uri.User.Password()
 	if cmd.Flags().Changed("identity") {
 		value := cmd.Flag("identity").Value.String()
-		auth, err := terminal.PublicKey(value, []byte(passwd))
+		s, err := terminal.PublicKey(value, []byte(passwd))
 		if err != nil {
 			return "", errors.Wrapf(err, "failed to read identity %q", value)
 		}
-		authMethods = append(authMethods, auth)
+		signers = append(signers, s)
+		logrus.Debugf("SSH Ident Key %q %s %s", value, ssh.FingerprintSHA256(s.PublicKey()), s.PublicKey().Type())
 	}
 
 	if sock, found := os.LookupEnv("SSH_AUTH_SOCK"); found {
@@ -190,16 +188,51 @@ func getUDS(cmd *cobra.Command, uri *url.URL) (string, error) {
 		if err != nil {
 			return "", err
 		}
-		a := agent.NewClient(c)
-		authMethods = append(authMethods, ssh.PublicKeysCallback(a.Signers))
-	}
-
-	if len(authMethods) == 0 {
-		pass, err := terminal.ReadPassword(fmt.Sprintf("%s's login password:", uri.User.Username()))
+		agentSigners, err := agent.NewClient(c).Signers()
 		if err != nil {
 			return "", err
 		}
-		authMethods = append(authMethods, ssh.Password(string(pass)))
+
+		signers = append(signers, agentSigners...)
+
+		if logrus.IsLevelEnabled(logrus.DebugLevel) {
+			for _, s := range agentSigners {
+				logrus.Debugf("SSH Agent Key %s %s", ssh.FingerprintSHA256(s.PublicKey()), s.PublicKey().Type())
+			}
+		}
+	}
+
+	var authMethods []ssh.AuthMethod
+	if len(signers) > 0 {
+		var dedup = make(map[string]ssh.Signer)
+		// Dedup signers based on fingerprint, ssh-agent keys override CONTAINER_SSHKEY
+		for _, s := range signers {
+			fp := ssh.FingerprintSHA256(s.PublicKey())
+			if _, found := dedup[fp]; found {
+				logrus.Debugf("Dedup SSH Key %s %s", ssh.FingerprintSHA256(s.PublicKey()), s.PublicKey().Type())
+			}
+			dedup[fp] = s
+		}
+
+		var uniq []ssh.Signer
+		for _, s := range dedup {
+			uniq = append(uniq, s)
+		}
+
+		authMethods = append(authMethods, ssh.PublicKeysCallback(func() ([]ssh.Signer, error) {
+			return uniq, nil
+		}))
+	}
+
+	if passwdSet {
+		authMethods = append(authMethods, ssh.Password(passwd))
+	}
+
+	if len(authMethods) == 0 {
+		authMethods = append(authMethods, ssh.PasswordCallback(func() (string, error) {
+			pass, err := terminal.ReadPassword(fmt.Sprintf("%s's login password:", uri.User.Username()))
+			return string(pass), err
+		}))
 	}
 
 	cfg := &ssh.ClientConfig{
diff --git a/pkg/bindings/connection.go b/pkg/bindings/connection.go
index f2cb3147c..7b26037eb 100644
--- a/pkg/bindings/connection.go
+++ b/pkg/bindings/connection.go
@@ -182,30 +182,65 @@ func pingNewConnection(ctx context.Context) error {
 func sshClient(_url *url.URL, secure bool, passPhrase string, identity string) (Connection, error) {
 	// if you modify the authmethods or their conditionals, you will also need to make similar
 	// changes in the client (currently cmd/podman/system/connection/add getUDS).
-	authMethods := []ssh.AuthMethod{}
+
+	var signers []ssh.Signer // order Signers are appended to this list determines which key is presented to server
+
 	if len(identity) > 0 {
-		auth, err := terminal.PublicKey(identity, []byte(passPhrase))
+		s, err := terminal.PublicKey(identity, []byte(passPhrase))
 		if err != nil {
 			return Connection{}, errors.Wrapf(err, "failed to parse identity %q", identity)
 		}
-		logrus.Debugf("public key signer enabled for identity %q", identity)
-		authMethods = append(authMethods, auth)
+
+		signers = append(signers, s)
+		logrus.Debugf("SSH Ident Key %q %s %s", identity, ssh.FingerprintSHA256(s.PublicKey()), s.PublicKey().Type())
 	}
 
 	if sock, found := os.LookupEnv("SSH_AUTH_SOCK"); found {
-		logrus.Debugf("Found SSH_AUTH_SOCK %q, ssh-agent signer enabled", sock)
+		logrus.Debugf("Found SSH_AUTH_SOCK %q, ssh-agent signer(s) enabled", sock)
 
 		c, err := net.Dial("unix", sock)
 		if err != nil {
 			return Connection{}, err
 		}
-		a := agent.NewClient(c)
-		authMethods = append(authMethods, ssh.PublicKeysCallback(a.Signers))
+
+		agentSigners, err := agent.NewClient(c).Signers()
+		if err != nil {
+			return Connection{}, err
+		}
+		signers = append(signers, agentSigners...)
+
+		if logrus.IsLevelEnabled(logrus.DebugLevel) {
+			for _, s := range agentSigners {
+				logrus.Debugf("SSH Agent Key %s %s", ssh.FingerprintSHA256(s.PublicKey()), s.PublicKey().Type())
+			}
+		}
+	}
+
+	var authMethods []ssh.AuthMethod
+	if len(signers) > 0 {
+		var dedup = make(map[string]ssh.Signer)
+		// Dedup signers based on fingerprint, ssh-agent keys override CONTAINER_SSHKEY
+		for _, s := range signers {
+			fp := ssh.FingerprintSHA256(s.PublicKey())
+			if _, found := dedup[fp]; found {
+				logrus.Debugf("Dedup SSH Key %s %s", ssh.FingerprintSHA256(s.PublicKey()), s.PublicKey().Type())
+			}
+			dedup[fp] = s
+		}
+
+		var uniq []ssh.Signer
+		for _, s := range dedup {
+			uniq = append(uniq, s)
+		}
+		authMethods = append(authMethods, ssh.PublicKeysCallback(func() ([]ssh.Signer, error) {
+			return uniq, nil
+		}))
 	}
 
 	if pw, found := _url.User.Password(); found {
 		authMethods = append(authMethods, ssh.Password(pw))
 	}
+
 	if len(authMethods) == 0 {
 		callback := func() (string, error) {
 			pass, err := terminal.ReadPassword("Login password:")
diff --git a/pkg/domain/infra/runtime_tunnel.go b/pkg/domain/infra/runtime_tunnel.go
index 6c85e837e..3fddf577c 100644
--- a/pkg/domain/infra/runtime_tunnel.go
+++ b/pkg/domain/infra/runtime_tunnel.go
@@ -5,18 +5,38 @@ package infra
 import (
 	"context"
 	"fmt"
+	"sync"
 
 	"github.com/containers/podman/v2/pkg/bindings"
 	"github.com/containers/podman/v2/pkg/domain/entities"
 	"github.com/containers/podman/v2/pkg/domain/infra/tunnel"
 )
 
+var (
+	connectionMutex = &sync.Mutex{}
+	connection      *context.Context
+)
+
+func newConnection(uri string, identity string) (context.Context, error) {
+	connectionMutex.Lock()
+	defer connectionMutex.Unlock()
+
+	if connection == nil {
+		ctx, err := bindings.NewConnectionWithIdentity(context.Background(), uri, identity)
+		if err != nil {
+			return ctx, err
+		}
+		connection = &ctx
+	}
+	return *connection, nil
+}
+
 func NewContainerEngine(facts *entities.PodmanConfig) (entities.ContainerEngine, error) {
 	switch facts.EngineMode {
 	case entities.ABIMode:
 		return nil, fmt.Errorf("direct runtime not supported")
 	case entities.TunnelMode:
-		ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.URI, facts.Identity)
+		ctx, err := newConnection(facts.URI, facts.Identity)
 		return &tunnel.ContainerEngine{ClientCxt: ctx}, err
 	}
 	return nil, fmt.Errorf("runtime mode '%v' is not supported", facts.EngineMode)
@@ -28,7 +48,7 @@ func NewImageEngine(facts *entities.PodmanConfig) (entities.ImageEngine, error)
 	case entities.ABIMode:
 		return nil, fmt.Errorf("direct image runtime not supported")
 	case entities.TunnelMode:
-		ctx, err := bindings.NewConnectionWithIdentity(context.Background(), facts.URI, facts.Identity)
+		ctx, err := newConnection(facts.URI, facts.Identity)
 		return &tunnel.ImageEngine{ClientCxt: ctx}, err
 	}
 	return nil, fmt.Errorf("runtime mode '%v' is not supported", facts.EngineMode)
diff --git a/pkg/terminal/util.go b/pkg/terminal/util.go
index 169bec2af..231b47974 100644
--- a/pkg/terminal/util.go
+++ b/pkg/terminal/util.go
@@ -61,7 +61,7 @@ func ReadPassword(prompt string) (pw []byte, err error) {
 	}
 }
 
-func PublicKey(path string, passphrase []byte) (ssh.AuthMethod, error) {
+func PublicKey(path string, passphrase []byte) (ssh.Signer, error) {
 	key, err := ioutil.ReadFile(path)
 	if err != nil {
 		return nil, err
@@ -75,12 +75,9 @@ func PublicKey(path string, passphrase []byte) (ssh.AuthMethod, error) {
 		if len(passphrase) == 0 {
 			passphrase = ReadPassphrase()
 		}
-		signer, err = ssh.ParsePrivateKeyWithPassphrase(key, passphrase)
-		if err != nil {
-			return nil, err
-		}
+		return ssh.ParsePrivateKeyWithPassphrase(key, passphrase)
 	}
-	return ssh.PublicKeys(signer), nil
+	return signer, nil
 }
 
 func ReadPassphrase() []byte {