From 81ec9baba1d6a5410f52a5f49c573f85b9260309 Mon Sep 17 00:00:00 2001 From: Norio Nomura Date: Thu, 25 Sep 2025 11:24:42 +0900 Subject: [PATCH] pkg/hostagent: Disable `ControlMaster` options on executing SSH until the 2nd essential requirement is satisfied debugHint in 2nd essential requirement says: > The boot sequence will terminate any existing user session after updating > /etc/environment to make sure the session includes the new values. > Terminating the session will break the persistent SSH tunnel, so > it must not be created until the session reset is done. This explicitly disables `ControlMaster` options when executing SSH until the second essential requirement is satisfied. Also, if there are two essential requirements, a requirement to explicitly enable ControlMaster will be added. Signed-off-by: Norio Nomura apply reviews Signed-off-by: Norio Nomura --- pkg/hostagent/requirements.go | 24 +++++++++- pkg/sshutil/sshutil.go | 30 +++++++++++++ pkg/sshutil/sshutil_test.go | 84 +++++++++++++++++++++++++++++++++++ 3 files changed, 137 insertions(+), 1 deletion(-) diff --git a/pkg/hostagent/requirements.go b/pkg/hostagent/requirements.go index 148d6785b67..a0d8b1705b1 100644 --- a/pkg/hostagent/requirements.go +++ b/pkg/hostagent/requirements.go @@ -13,6 +13,7 @@ import ( "github.com/sirupsen/logrus" "github.com/lima-vm/lima/v2/pkg/limatype" + "github.com/lima-vm/lima/v2/pkg/sshutil" ) func (a *HostAgent) waitForRequirements(label string, requirements []requirement) error { @@ -101,7 +102,15 @@ func (a *HostAgent) waitForRequirement(r requirement) error { if err != nil { return err } - stdout, stderr, err := ssh.ExecuteScript(a.instSSHAddress, a.sshLocalPort, a.sshConfig, script, r.description) + sshConfig := a.sshConfig + if r.noMaster { + sshConfig = &ssh.SSHConfig{ + ConfigFile: sshConfig.ConfigFile, + Persist: false, + AdditionalArgs: sshutil.DisableControlMasterOptsFromSSHArgs(sshConfig.AdditionalArgs), + } + } + stdout, stderr, err := ssh.ExecuteScript(a.instSSHAddress, a.sshLocalPort, sshConfig, script, r.description) logrus.Debugf("stdout=%q, stderr=%q, err=%v", stdout, stderr, err) if err != nil { return fmt.Errorf("stdout=%q, stderr=%q: %w", stdout, stderr, err) @@ -114,6 +123,7 @@ type requirement struct { script string debugHint string fatal bool + noMaster bool } func (a *HostAgent) essentialRequirements() []requirement { @@ -128,8 +138,17 @@ true Make sure that the YAML field "ssh.localPort" is not used by other processes on the host. If any private key under ~/.ssh is protected with a passphrase, you need to have ssh-agent to be running. `, + noMaster: true, }) + startControlMasterReq := requirement{ + description: "Explicitly start ssh ControlMaster", + script: `#!/bin/bash +true +`, + debugHint: `The persistent ssh ControlMaster should be started immediately.`, + } if *a.instConfig.Plain { + req = append(req, startControlMasterReq) return req } req = append(req, @@ -147,6 +166,7 @@ fi Terminating the session will break the persistent SSH tunnel, so it must not be created until the session reset is done. `, + noMaster: true, }) if *a.instConfig.MountType == limatype.REVSSHFS && len(a.instConfig.Mounts) > 0 { @@ -176,6 +196,8 @@ fi `, debugHint: `Append "user_allow_other" to /etc/fuse.conf (/etc/fuse3.conf) in the guest`, }) + } else { + req = append(req, startControlMasterReq) } return req } diff --git a/pkg/sshutil/sshutil.go b/pkg/sshutil/sshutil.go index 00b6984fbd8..f2b12665fa8 100644 --- a/pkg/sshutil/sshutil.go +++ b/pkg/sshutil/sshutil.go @@ -295,6 +295,36 @@ func identityFileEntry(ctx context.Context, privateKeyPath string) (string, erro return fmt.Sprintf(`IdentityFile="%s"`, privateKeyPath), nil } +// DisableControlMasterOptsFromSSHArgs returns ssh args that disable ControlMaster, ControlPath, and ControlPersist. +func DisableControlMasterOptsFromSSHArgs(sshArgs []string) []string { + argsForOverridingConfigFile := []string{ + "-o", "ControlMaster=no", + "-o", "ControlPath=none", + "-o", "ControlPersist=no", + } + return slices.Concat(argsForOverridingConfigFile, removeOptsFromSSHArgs(sshArgs, "ControlMaster", "ControlPath", "ControlPersist")) +} + +func removeOptsFromSSHArgs(sshArgs []string, removeOpts ...string) []string { + res := make([]string, 0, len(sshArgs)) + isOpt := false + for _, arg := range sshArgs { + if isOpt { + isOpt = false + if !slices.ContainsFunc(removeOpts, func(opt string) bool { + return strings.HasPrefix(arg, opt) + }) { + res = append(res, "-o", arg) + } + } else if arg == "-o" { + isOpt = true + } else { + res = append(res, arg) + } + } + return res +} + // SSHOpts adds the following options to CommonOptions: User, ControlMaster, ControlPath, ControlPersist. func SSHOpts(ctx context.Context, sshExe SSHExe, instDir, username string, useDotSSH, forwardAgent, forwardX11, forwardX11Trusted bool) ([]string, error) { controlSock := filepath.Join(instDir, filenames.SSHSock) diff --git a/pkg/sshutil/sshutil_test.go b/pkg/sshutil/sshutil_test.go index da879784b4d..b7fe84b30b5 100644 --- a/pkg/sshutil/sshutil_test.go +++ b/pkg/sshutil/sshutil_test.go @@ -53,3 +53,87 @@ func Test_detectValidPublicKey(t *testing.T) { assert.Check(t, !detectValidPublicKey("arbitrary content")) assert.Check(t, !detectValidPublicKey("")) } + +func Test_DisableControlMasterOptsFromSSHArgs(t *testing.T) { + tests := []struct { + name string + sshArgs []string + want []string + }{ + { + name: "no ControlMaster options", + sshArgs: []string{ + "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", + }, + want: []string{ + "-o", "ControlMaster=no", "-o", "ControlPath=none", "-o", "ControlPersist=no", + "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", + }, + }, + { + name: "ControlMaster=yes", + sshArgs: []string{ + "-o", "ControlMaster=yes", "-o", "UserKnownHostsFile=/dev/null", + }, + want: []string{ + "-o", "ControlMaster=no", "-o", "ControlPath=none", "-o", "ControlPersist=no", + "-o", "UserKnownHostsFile=/dev/null", + }, + }, + { + name: "ControlMaster=auto", + sshArgs: []string{ + "-o", "ControlMaster=auto", "-o", "UserKnownHostsFile=/dev/null", + }, + want: []string{ + "-o", "ControlMaster=no", "-o", "ControlPath=none", "-o", "ControlPersist=no", + "-o", "UserKnownHostsFile=/dev/null", + }, + }, + { + name: "ControlMaster=auto with ControlPath", + sshArgs: []string{ + "-o", "ControlMaster=auto", "-o", "ControlPath=/tmp/ssh-%r@%h:%p", "-o", "UserKnownHostsFile=/dev/null", + }, + want: []string{ + "-o", "ControlMaster=no", "-o", "ControlPath=none", "-o", "ControlPersist=no", + "-o", "UserKnownHostsFile=/dev/null", + }, + }, + { + name: "ControlPath only", + sshArgs: []string{ + "-o", "ControlPath=/tmp/ssh-%r@%h:%p", "-o", "UserKnownHostsFile=/dev/null", + }, + want: []string{ + "-o", "ControlMaster=no", "-o", "ControlPath=none", "-o", "ControlPersist=no", + "-o", "UserKnownHostsFile=/dev/null", + }, + }, + { + name: "ControlMaster=no", + sshArgs: []string{ + "-o", "ControlMaster=no", "-o", "UserKnownHostsFile=/dev/null", + }, + want: []string{ + "-o", "ControlMaster=no", "-o", "ControlPath=none", "-o", "ControlPersist=no", + "-o", "UserKnownHostsFile=/dev/null", + }, + }, + { + name: "ControlMaster=auto with other options", + sshArgs: []string{ + "-o", "ControlMaster=auto", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", + }, + want: []string{ + "-o", "ControlMaster=no", "-o", "ControlPath=none", "-o", "ControlPersist=no", + "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.DeepEqual(t, DisableControlMasterOptsFromSSHArgs(tt.sshArgs), tt.want) + }) + } +}