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 <[email protected]>

apply reviews

Signed-off-by: Norio Nomura <[email protected]>

Author: norio-nomura

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
 }

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)

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)
+		})
+	}
+}