Improve ssh config handling

internal/tunnel/config.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/alebeck/boring/internal/log"
 	"github.com/alebeck/boring/internal/paths"
-	"github.com/kevinburke/ssh_config"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/agent"
 	"golang.org/x/crypto/ssh/knownhosts"
@@ -41,7 +40,9 @@ func (t *Tunnel) makeRunConfig() error {
 	rc := runConfig{}
 
 	// Fill in rc's values based on ssh config
-	rc.parseSSHConf(t.Host)
+	if err := rc.parseSSHConf(t.Host); err != nil {
+		return fmt.Errorf("could not parse SSH config: %v", err)
+	}
 
 	// Override values which were manually set by user
 	if t.User != "" {
@@ -82,29 +83,29 @@ func (t *Tunnel) makeRunConfig() error {
 	return nil
 }
 
-func (rc *runConfig) parseSSHConf(alias string) {
-	rc.identityFiles = ssh_config.GetAll(alias, "IdentityFile")
+func (rc *runConfig) parseSSHConf(alias string) error {
+	c, err := makeSSHConfig()
+	if err != nil {
+		return err
+	}
 
-	hosts := strings.Split(ssh_config.Get(alias, "GlobalKnownHostsFile"), " ")
-	hosts = append(hosts, getAllMulti(alias, "UserKnownHostsFile", " ")...)
-	rc.knownHostsFiles = hosts
+	rc.identityFiles = c.GetAll(alias, "IdentityFile")
 
-	rc.ciphers = getAllMulti(alias, "Ciphers", ",")
-	rc.macs = getAllMulti(alias, "MACs", ",")
-	rc.hostKeyAlgos = getAllMulti(alias, "HostKeyAlgorithms", ",")
-	rc.kexAlgos = getAllMulti(alias, "KexAlgorithms", ",")
+	hosts := c.GetAll(alias, "GlobalKnownHostsFile")
+	hosts = append(hosts, c.GetAll(alias, "UserKnownHostsFile")...)
+	for _, h := range hosts {
+		rc.knownHostsFiles = append(rc.knownHostsFiles, strings.Split(h, " ")...)
+	}
 
-	rc.user = ssh_config.Get(alias, "User")
-	rc.port, _ = strconv.Atoi(ssh_config.Get(alias, "Port"))
-	rc.hostName = ssh_config.Get(alias, "HostName")
-}
+	rc.ciphers = split(c.Get(alias, "Ciphers"))
+	rc.macs = split(c.Get(alias, "MACs"))
+	rc.hostKeyAlgos = split(c.Get(alias, "HostKeyAlgorithms"))
+	rc.kexAlgos = split(c.Get(alias, "KexAlgorithms"))
 
-func getAllMulti(alias, key, sep string) []string {
-	var vs []string
-	for _, v := range ssh_config.GetAll(alias, key) {
-		vs = append(vs, strings.Split(v, sep)...)
-	}
-	return vs
+	rc.user = c.Get(alias, "User")
+	rc.port, _ = strconv.Atoi(c.Get(alias, "Port"))
+	rc.hostName = c.Get(alias, "HostName")
+	return nil
 }
 
 func validate(rc *runConfig) error {
@@ -150,6 +151,7 @@ func (rc *runConfig) makeClientConfig() error {
 			return fmt.Errorf("no key files found.")
 		}
 	}
+	log.Debugf("Trying %d key file(s)", len(signers))
 
 	var hosts []string
 	for _, k := range rc.knownHostsFiles {
@@ -216,3 +218,7 @@ func parseAddr(addr string, allowShort bool) (string, string, error) {
 	// it's a unix socket address
 	return addr, "unix", nil
 }
+
+func split(s string) []string {
+	return strings.Split(s, ",")
+}

internal/tunnel/ssh_config.go

@@ -0,0 +1,108 @@
+package tunnel
+
+import (
+	"os"
+
+	"github.com/alebeck/boring/internal/paths"
+	"github.com/kevinburke/ssh_config"
+)
+
+var (
+	userConfigPath   = paths.ReplaceTilde("~/.ssh/config")
+	systemConfigPath = "/etc/ssh/ssh_config"
+)
+
+// sshConfig (and the following functions) are a thin wrapper around
+// excellent kevinburke/ssh_config, with the intend to make the
+// UserConfig (here sshConfig) object accessible. We need this so we
+// can reload the config at every tunnel opening, and not just once
+// at application start.
+type sshConfig struct {
+	userConfig   *ssh_config.Config
+	systemConfig *ssh_config.Config
+}
+
+func makeSSHConfig() (*sshConfig, error) {
+	uc, err := parse(userConfigPath)
+	if err != nil {
+		return nil, err
+	}
+	sc, err := parse(systemConfigPath)
+	if err != nil {
+		return nil, err
+	}
+	c := &sshConfig{userConfig: uc, systemConfig: sc}
+	return c, nil
+}
+
+// (c) kevinburke/ssh_config
+func (c *sshConfig) Get(alias, key string) string {
+	val, err := findVal(c.userConfig, alias, key)
+	if err != nil || val != "" {
+		return val
+	}
+	val2, err2 := findVal(c.systemConfig, alias, key)
+	if err2 != nil || val2 != "" {
+		return val2
+	}
+	return ssh_config.Default(key)
+}
+
+// (c) kevinburke/ssh_config
+func (c *sshConfig) GetAll(alias, key string) []string {
+	val, err := findAll(c.userConfig, alias, key)
+	if err != nil || val != nil {
+		return val
+	}
+	val2, err2 := findAll(c.systemConfig, alias, key)
+	if err2 != nil || val2 != nil {
+		return val2
+	}
+	if key == "IdentityFile" {
+		// The original implementation returned outdated SSH1 identites
+		// file, so we instead return nothing if no files were specified.
+		// We would return the SSH2 default files, but boring has no way
+		// to determine whether those are specified or default ones, and
+		// we only include agent signers in case no files are specified.
+		return []string{}
+	}
+	if def := ssh_config.Default(key); def != "" {
+		return []string{def}
+	}
+	return []string{}
+}
+
+// (c) kevinburke/ssh_config
+func findVal(c *ssh_config.Config, alias, key string) (string, error) {
+	if c == nil {
+		return "", nil
+	}
+	val, err := c.Get(alias, key)
+	if err != nil || val == "" {
+		return "", err
+	}
+	return val, nil
+}
+
+// (c) kevinburke/ssh_config
+func findAll(c *ssh_config.Config, alias, key string) ([]string, error) {
+	if c == nil {
+		return nil, nil
+	}
+	return c.GetAll(alias, key)
+}
+
+func parse(path string) (*ssh_config.Config, error) {
+	f, err := os.Open(path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil, nil
+		}
+		return nil, err
+	}
+	c, err := ssh_config.Decode(f)
+	if err != nil {
+		return nil, err
+	}
+	return c, nil
+}