Merge pull request #3128 from afbjorklund/ssh-version-cache

Cache the OpenSSH version if it is needed again

cmd/limactl/copy.go

@@ -74,7 +74,8 @@ func copyAction(cmd *cobra.Command, args []string) error {
 	if recursive {
 		scpFlags = append(scpFlags, "-r")
 	}
-	legacySSH := sshutil.DetectOpenSSHVersion().LessThan(*semver.New("8.0.0"))
+	// this assumes that ssh and scp come from the same place, but scp has no -V
+	legacySSH := sshutil.DetectOpenSSHVersion("ssh").LessThan(*semver.New("8.0.0"))
 	for _, arg := range args {
 		path := strings.Split(arg, ":")
 		switch len(path) {
@@ -115,14 +116,14 @@ func copyAction(cmd *cobra.Command, args []string) error {
 		// arguments such as ControlPath.  This is preferred as we can multiplex
 		// sessions without re-authenticating (MaxSessions permitting).
 		for _, inst := range instances {
-			sshOpts, err = sshutil.SSHOpts(inst.Dir, *inst.Config.User.Name, false, false, false, false)
+			sshOpts, err = sshutil.SSHOpts("ssh", inst.Dir, *inst.Config.User.Name, false, false, false, false)
 			if err != nil {
 				return err
 			}
 		}
 	} else {
 		// Copying among multiple hosts; we can't pass in host-specific options.
-		sshOpts, err = sshutil.CommonOpts(false)
+		sshOpts, err = sshutil.CommonOpts("ssh", false)
 		if err != nil {
 			return err
 		}

cmd/limactl/shell.go

@@ -143,6 +143,7 @@ func shellAction(cmd *cobra.Command, args []string) error {
 	}
 
 	sshOpts, err := sshutil.SSHOpts(
+		arg0,
 		inst.Dir,
 		*inst.Config.User.Name,
 		*inst.Config.SSH.LoadDotSSHPubKeys,
@@ -164,7 +165,7 @@ func shellAction(cmd *cobra.Command, args []string) error {
 	logLevel := "ERROR"
 	// For versions older than OpenSSH 8.9p, LogLevel=QUIET was needed to
 	// avoid the "Shared connection to 127.0.0.1 closed." message with -t.
-	olderSSH := sshutil.DetectOpenSSHVersion().LessThan(*semver.New("8.9.0"))
+	olderSSH := sshutil.DetectOpenSSHVersion(arg0).LessThan(*semver.New("8.9.0"))
 	if olderSSH {
 		logLevel = "QUIET"
 	}

cmd/limactl/show-ssh.go

@@ -89,6 +89,7 @@ func showSSHAction(cmd *cobra.Command, args []string) error {
 	logrus.Warnf("`limactl show-ssh` is deprecated. Instead, use `ssh -F %s %s`.",
 		filepath.Join(inst.Dir, filenames.SSHConfig), inst.Hostname)
 	opts, err := sshutil.SSHOpts(
+		"ssh",
 		inst.Dir,
 		*inst.Config.User.Name,
 		*inst.Config.SSH.LoadDotSSHPubKeys,
@@ -100,7 +101,7 @@ func showSSHAction(cmd *cobra.Command, args []string) error {
 	}
 	opts = append(opts, "Hostname=127.0.0.1")
 	opts = append(opts, fmt.Sprintf("Port=%d", inst.SSHLocalPort))
-	return sshutil.Format(w, instName, format, opts)
+	return sshutil.Format(w, "ssh", instName, format, opts)
 }
 
 func showSSHBashComplete(cmd *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) {

cmd/limactl/tunnel.go

@@ -84,6 +84,7 @@ func tunnelAction(cmd *cobra.Command, args []string) error {
 	}
 
 	sshOpts, err := sshutil.SSHOpts(
+		arg0,
 		inst.Dir,
 		*inst.Config.User.Name,
 		*inst.Config.SSH.LoadDotSSHPubKeys,

pkg/hostagent/hostagent.go

@@ -143,6 +143,7 @@ func New(instName string, stdout io.Writer, signalCh chan os.Signal, opts ...Opt
 	}
 
 	sshOpts, err := sshutil.SSHOpts(
+		"ssh",
 		inst.Dir,
 		*inst.Config.User.Name,
 		*inst.Config.SSH.LoadDotSSHPubKeys,
@@ -152,7 +153,7 @@ func New(instName string, stdout io.Writer, signalCh chan os.Signal, opts ...Opt
 	if err != nil {
 		return nil, err
 	}
-	if err = writeSSHConfigFile(inst.Name, inst.Dir, inst.SSHAddress, sshLocalPort, sshOpts); err != nil {
+	if err = writeSSHConfigFile("ssh", inst.Name, inst.Dir, inst.SSHAddress, sshLocalPort, sshOpts); err != nil {
 		return nil, err
 	}
 	sshConfig := &ssh.SSHConfig{
@@ -220,7 +221,7 @@ func New(instName string, stdout io.Writer, signalCh chan os.Signal, opts ...Opt
 	return a, nil
 }
 
-func writeSSHConfigFile(instName, instDir, instSSHAddress string, sshLocalPort int, sshOpts []string) error {
+func writeSSHConfigFile(sshPath, instName, instDir, instSSHAddress string, sshLocalPort int, sshOpts []string) error {
 	if instDir == "" {
 		return fmt.Errorf("directory is unknown for the instance %q", instName)
 	}
@@ -231,7 +232,7 @@ func writeSSHConfigFile(instName, instDir, instSSHAddress string, sshLocalPort i
 `); err != nil {
 		return err
 	}
-	if err := sshutil.Format(&b, instName, sshutil.FormatConfig,
+	if err := sshutil.Format(&b, sshPath, instName, sshutil.FormatConfig,
 		append(sshOpts,
 			fmt.Sprintf("Hostname=%s", instSSHAddress),
 			fmt.Sprintf("Port=%d", sshLocalPort),

pkg/sshutil/format.go

@@ -58,11 +58,11 @@ func quoteOption(o string) string {
 }
 
 // Format formats the ssh options.
-func Format(w io.Writer, instName string, format FormatT, opts []string) error {
+func Format(w io.Writer, sshPath, instName string, format FormatT, opts []string) error {
 	fakeHostname := identifierutil.HostnameFromInstName(instName) // TODO: support customization
 	switch format {
 	case FormatCmd:
-		args := []string{"ssh"}
+		args := []string{sshPath}
 		for _, o := range opts {
 			args = append(args, "-o", quoteOption(o))
 		}

pkg/sshutil/sshutil.go

@@ -14,6 +14,7 @@ import (
 	"runtime"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/coreos/go-semver/semver"
 	"github.com/lima-vm/lima/pkg/ioutilx"
@@ -156,7 +157,7 @@ var sshInfo struct {
 //
 // The result always contains the IdentityFile option.
 // The result never contains the Port option.
-func CommonOpts(useDotSSH bool) ([]string, error) {
+func CommonOpts(sshPath string, useDotSSH bool) ([]string, error) {
 	configDir, err := dirnames.LimaConfigDir()
 	if err != nil {
 		return nil, err
@@ -224,7 +225,7 @@ func CommonOpts(useDotSSH bool) ([]string, error) {
 
 	sshInfo.Do(func() {
 		sshInfo.aesAccelerated = detectAESAcceleration()
-		sshInfo.openSSHVersion = DetectOpenSSHVersion()
+		sshInfo.openSSHVersion = DetectOpenSSHVersion(sshPath)
 	})
 
 	// Only OpenSSH version 8.1 and later support adding ciphers to the front of the default set
@@ -253,12 +254,12 @@ func CommonOpts(useDotSSH bool) ([]string, error) {
 }
 
 // SSHOpts adds the following options to CommonOptions: User, ControlMaster, ControlPath, ControlPersist.
-func SSHOpts(instDir, username string, useDotSSH, forwardAgent, forwardX11, forwardX11Trusted bool) ([]string, error) {
+func SSHOpts(sshPath, instDir, username string, useDotSSH, forwardAgent, forwardX11, forwardX11Trusted bool) ([]string, error) {
 	controlSock := filepath.Join(instDir, filenames.SSHSock)
 	if len(controlSock) >= osutil.UnixPathMax {
 		return nil, fmt.Errorf("socket path %q is too long: >= UNIX_PATH_MAX=%d", controlSock, osutil.UnixPathMax)
 	}
-	opts, err := CommonOpts(useDotSSH)
+	opts, err := CommonOpts(sshPath, useDotSSH)
 	if err != nil {
 		return nil, err
 	}
@@ -307,18 +308,48 @@ func ParseOpenSSHVersion(version []byte) *semver.Version {
 	return &semver.Version{}
 }
 
-func DetectOpenSSHVersion() semver.Version {
+// sshExecutable beyond path also records size and mtime, in the case of ssh upgrades.
+type sshExecutable struct {
+	Path    string
+	Size    int64
+	ModTime time.Time
+}
+
+var (
+	// sshVersions caches the parsed version of each ssh executable, if it is needed again.
+	sshVersions   = map[sshExecutable]*semver.Version{}
+	sshVersionsRW sync.RWMutex
+)
+
+func DetectOpenSSHVersion(ssh string) semver.Version {
 	var (
 		v      semver.Version
+		exe    sshExecutable
 		stderr bytes.Buffer
 	)
-	cmd := exec.Command("ssh", "-V")
+	path, err := exec.LookPath(ssh)
+	if err != nil {
+		logrus.Warnf("failed to find ssh executable: %v", err)
+	} else {
+		st, _ := os.Stat(path)
+		exe = sshExecutable{Path: path, Size: st.Size(), ModTime: st.ModTime()}
+		sshVersionsRW.RLock()
+		ver := sshVersions[exe]
+		sshVersionsRW.RUnlock()
+		if ver != nil {
+			return *ver
+		}
+	}
+	cmd := exec.Command(path, "-V")
 	cmd.Stderr = &stderr
 	if err := cmd.Run(); err != nil {
 		logrus.Warnf("failed to run %v: stderr=%q", cmd.Args, stderr.String())
 	} else {
 		v = *ParseOpenSSHVersion(stderr.Bytes())
 		logrus.Debugf("OpenSSH version %s detected", v)
+		sshVersionsRW.Lock()
+		sshVersions[exe] = &v
+		sshVersionsRW.Unlock()
 	}
 	return v
 }