From 737454f68864671d05161c2d718d7978d079e941 Mon Sep 17 00:00:00 2001
From: Kimmo Lehto <klehto@mirantis.com>
Date: Thu, 7 Sep 2023 13:44:15 +0300
Subject: [PATCH] Fix ssh keypath defaulting

Signed-off-by: Kimmo Lehto <klehto@mirantis.com>
---
 ssh.go       | 28 +++++++++++++++++++++-------
 test/test.sh | 11 +++++++++++
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/ssh.go b/ssh.go
index cb76679b..2105e895 100644
--- a/ssh.go
+++ b/ssh.go
@@ -49,7 +49,7 @@ type PasswordCallback func() (secret string, err error)
 
 var (
 	authMethodCache   = sync.Map{}
-	defaultKeypaths   = []string{"~/.ssh/id_rsa", "~/.ssh/identity", "~/.ssh/id_dsa"}
+	defaultKeypaths   = []string{"~/.ssh/id_rsa", "~/.ssh/identity", "~/.ssh/id_dsa", "~/.ssh/id_ecdsa", "~/.ssh/id_ed25519"}
 	dummyhostKeyPaths []string
 	globalOnce        sync.Once
 	knownHostsMU      sync.Mutex
@@ -114,7 +114,17 @@ func expandAndValidatePath(path string) (string, error) {
 
 func (c *SSH) keypathsFromConfig() []string {
 	log.Tracef("%s: trying to get a keyfile path from ssh config", c)
-	if idf := c.getConfigAll("IdentityFile"); len(idf) > 0 {
+	idf := c.getConfigAll("IdentityFile")
+	if len(idf) == 1 && idf[0] == "~/.ssh/identity" {
+		log.Tracef("%s: using default identity file paths %+v", c, defaultKeypaths)
+		// https://github.com/kevinburke/ssh_config/blob/master/config.go#L254 says:
+		// TODO: IdentityFile has multiple default values that we should return
+		// To work around this, if we get ~/.ssh/identity, the hard coded list of known defaults are used instead
+		idf = []string{}
+		idf = append(idf, defaultKeypaths...)
+	}
+
+	if len(idf) > 0 {
 		log.Tracef("%s: detected %d identity file paths from ssh config: %v", c, len(idf), idf)
 		return idf
 	}
@@ -125,11 +135,19 @@ func (c *SSH) keypathsFromConfig() []string {
 func (c *SSH) initGlobalDefaults() {
 	log.Tracef("discovering global default keypaths")
 	dummyHostIdentityFiles := SSHConfigGetAll(hopefullyNonexistentHost, "IdentityFile")
+	if len(dummyHostIdentityFiles) == 1 && dummyHostIdentityFiles[0] == "~/.ssh/identity" {
+		// https://github.com/kevinburke/ssh_config/blob/master/config.go#L254 says:
+		// TODO: IdentityFile has multiple default values that we should return
+		// To work around this, if we get ~/.ssh/identity, the hard coded list of known defaults are used instead
+		dummyHostIdentityFiles = []string{}
+		dummyHostIdentityFiles = append(dummyHostIdentityFiles, defaultKeypaths...)
+	}
 	for _, keyPath := range dummyHostIdentityFiles {
-		if expanded, err := expandAndValidatePath(keyPath); err != nil {
+		if expanded, err := expandAndValidatePath(keyPath); err == nil {
 			dummyhostKeyPaths = append(dummyhostKeyPaths, expanded)
 		}
 	}
+	log.Tracef("global default keypaths from ssh config: %+v", dummyhostKeyPaths)
 }
 
 func findUniq(a, b []string) (string, bool) {
@@ -163,10 +181,6 @@ func (c *SSH) SetDefaults() {
 		c.KeyPath = nil
 
 		paths := c.keypathsFromConfig()
-		if len(paths) == 0 {
-			// no paths found in ssh config either, use defaults
-			paths = append(paths, defaultKeypaths...)
-		}
 
 		for _, p := range paths {
 			expanded, err := expandAndValidatePath(p)
diff --git a/test/test.sh b/test/test.sh
index aca48153..9e6b83c6 100755
--- a/test/test.sh
+++ b/test/test.sh
@@ -170,6 +170,17 @@ rig_test_key_from_path() {
   RET=$exit_code
 }
 
+rig_test_key_from_default_location() {
+  color_echo "- Testing keypath from default location"
+  make create-host
+  mv .ssh/identity .ssh/id_ecdsa
+  set +e
+  HOME=$(pwd) ./rigtest -host 127.0.0.1:$(ssh_port node0) -user root
+  local exit_code=$?
+  set -e
+  RET=$exit_code
+}
+
 rig_test_protected_key_from_path() {
   color_echo "- Testing regular keypath to encrypted key, two hosts"
   make create-host KEY_PASSPHRASE=testPhrase REPLICAS=2