From 0a50cd7fc7ab5063262738caaeb3084f24ada113 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sun, 19 Jan 2025 10:01:39 +0100 Subject: [PATCH 1/4] Cache the OpenSSH version if it is needed again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Anders F Björklund --- pkg/sshutil/sshutil.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/sshutil/sshutil.go b/pkg/sshutil/sshutil.go index af141e660a6..a17ef65e9b2 100644 --- a/pkg/sshutil/sshutil.go +++ b/pkg/sshutil/sshutil.go @@ -277,18 +277,30 @@ func ParseOpenSSHVersion(version []byte) *semver.Version { return &semver.Version{} } +// sshVersions caches the parsed version of each ssh executable, if it is needed again. +var sshVersions = map[string]*semver.Version{} + func DetectOpenSSHVersion() semver.Version { var ( v semver.Version 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 { + if ver := sshVersions[path]; 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) + sshVersions[path] = &v } return v } From d3d50a959da1ef6501bf1c18f2b3af404bf9574b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 20 Jan 2025 18:03:19 +0100 Subject: [PATCH 2/4] Add the size and the mtime to the cache key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Anders F Björklund --- pkg/sshutil/sshutil.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/pkg/sshutil/sshutil.go b/pkg/sshutil/sshutil.go index a17ef65e9b2..625d1545b5c 100644 --- a/pkg/sshutil/sshutil.go +++ b/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" @@ -277,19 +278,29 @@ func ParseOpenSSHVersion(version []byte) *semver.Version { return &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 +} + // sshVersions caches the parsed version of each ssh executable, if it is needed again. -var sshVersions = map[string]*semver.Version{} +var sshVersions = map[sshExecutable]*semver.Version{} func DetectOpenSSHVersion() semver.Version { var ( v semver.Version + exe sshExecutable stderr bytes.Buffer ) path, err := exec.LookPath("ssh") if err != nil { logrus.Warnf("failed to find ssh executable: %v", err) } else { - if ver := sshVersions[path]; ver != nil { + st, _ := os.Stat(path) + exe = sshExecutable{Path: path, Size: st.Size(), ModTime: st.ModTime()} + if ver := sshVersions[exe]; ver != nil { return *ver } } @@ -300,7 +311,7 @@ func DetectOpenSSHVersion() semver.Version { } else { v = *ParseOpenSSHVersion(stderr.Bytes()) logrus.Debugf("OpenSSH version %s detected", v) - sshVersions[path] = &v + sshVersions[exe] = &v } return v } From 20a30b1910fe0bb29678b4403e533861d9e5a886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Sun, 19 Jan 2025 10:07:40 +0100 Subject: [PATCH 3/4] Respect the SSH environment variable if any MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Anders F Björklund --- cmd/limactl/copy.go | 7 ++++--- cmd/limactl/shell.go | 3 ++- cmd/limactl/show-ssh.go | 3 ++- cmd/limactl/tunnel.go | 1 + pkg/hostagent/hostagent.go | 7 ++++--- pkg/sshutil/format.go | 4 ++-- pkg/sshutil/sshutil.go | 12 ++++++------ 7 files changed, 21 insertions(+), 16 deletions(-) diff --git a/cmd/limactl/copy.go b/cmd/limactl/copy.go index 34416a46e75..4b5fc3e3cf6 100644 --- a/cmd/limactl/copy.go +++ b/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 } diff --git a/cmd/limactl/shell.go b/cmd/limactl/shell.go index bfe8d2c5cd0..1c249e88693 100644 --- a/cmd/limactl/shell.go +++ b/cmd/limactl/shell.go @@ -167,6 +167,7 @@ func shellAction(cmd *cobra.Command, args []string) error { } sshOpts, err := sshutil.SSHOpts( + arg0, inst.Dir, *inst.Config.User.Name, *inst.Config.SSH.LoadDotSSHPubKeys, @@ -188,7 +189,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" } diff --git a/cmd/limactl/show-ssh.go b/cmd/limactl/show-ssh.go index 04cbf59088a..f3867b68c83 100644 --- a/cmd/limactl/show-ssh.go +++ b/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) { diff --git a/cmd/limactl/tunnel.go b/cmd/limactl/tunnel.go index 446089c65e5..fa92d2f4d8e 100644 --- a/cmd/limactl/tunnel.go +++ b/cmd/limactl/tunnel.go @@ -106,6 +106,7 @@ func tunnelAction(cmd *cobra.Command, args []string) error { } sshOpts, err := sshutil.SSHOpts( + arg0, inst.Dir, *inst.Config.User.Name, *inst.Config.SSH.LoadDotSSHPubKeys, diff --git a/pkg/hostagent/hostagent.go b/pkg/hostagent/hostagent.go index 0be8761fc0d..396401a4896 100644 --- a/pkg/hostagent/hostagent.go +++ b/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), diff --git a/pkg/sshutil/format.go b/pkg/sshutil/format.go index e027257b48b..772b4857f64 100644 --- a/pkg/sshutil/format.go +++ b/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)) } diff --git a/pkg/sshutil/sshutil.go b/pkg/sshutil/sshutil.go index 625d1545b5c..7be1be87979 100644 --- a/pkg/sshutil/sshutil.go +++ b/pkg/sshutil/sshutil.go @@ -127,7 +127,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 @@ -195,7 +195,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 @@ -224,12 +224,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 } @@ -288,13 +288,13 @@ type sshExecutable struct { // sshVersions caches the parsed version of each ssh executable, if it is needed again. var sshVersions = map[sshExecutable]*semver.Version{} -func DetectOpenSSHVersion() semver.Version { +func DetectOpenSSHVersion(ssh string) semver.Version { var ( v semver.Version exe sshExecutable stderr bytes.Buffer ) - path, err := exec.LookPath("ssh") + path, err := exec.LookPath(ssh) if err != nil { logrus.Warnf("failed to find ssh executable: %v", err) } else { From c7356601d8040672c0d8425540b0a6f47e961a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Mon, 20 Jan 2025 18:15:42 +0100 Subject: [PATCH 4/4] Add mutex for the shared sshVersions map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Anders F Björklund --- pkg/sshutil/sshutil.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/sshutil/sshutil.go b/pkg/sshutil/sshutil.go index 7be1be87979..7e1e8701bf3 100644 --- a/pkg/sshutil/sshutil.go +++ b/pkg/sshutil/sshutil.go @@ -285,8 +285,11 @@ type sshExecutable struct { ModTime time.Time } -// sshVersions caches the parsed version of each ssh executable, if it is needed again. -var sshVersions = map[sshExecutable]*semver.Version{} +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 ( @@ -300,7 +303,10 @@ func DetectOpenSSHVersion(ssh string) semver.Version { } else { st, _ := os.Stat(path) exe = sshExecutable{Path: path, Size: st.Size(), ModTime: st.ModTime()} - if ver := sshVersions[exe]; ver != nil { + sshVersionsRW.RLock() + ver := sshVersions[exe] + sshVersionsRW.RUnlock() + if ver != nil { return *ver } } @@ -311,7 +317,9 @@ func DetectOpenSSHVersion(ssh string) semver.Version { } else { v = *ParseOpenSSHVersion(stderr.Bytes()) logrus.Debugf("OpenSSH version %s detected", v) + sshVersionsRW.Lock() sshVersions[exe] = &v + sshVersionsRW.Unlock() } return v }