Skip to content

Commit dce9054

Browse files
norio-nomuranorio-nomura
committed
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]>
1 parent 03031f0 commit dce9054

File tree

3 files changed

+153
-1
lines changed

3 files changed

+153
-1
lines changed

‎pkg/hostagent/requirements.go‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/sirupsen/logrus"
1414

1515
"github.com/lima-vm/lima/v2/pkg/limatype"
16+
"github.com/lima-vm/lima/v2/pkg/sshutil"
1617
)
1718

1819
func (a *HostAgent) waitForRequirements(label string, requirements []requirement) error {
@@ -101,7 +102,15 @@ func (a *HostAgent) waitForRequirement(r requirement) error {
101102
if err != nil {
102103
return err
103104
}
104-
stdout, stderr, err := ssh.ExecuteScript(a.instSSHAddress, a.sshLocalPort, a.sshConfig, script, r.description)
105+
sshConfig := a.sshConfig
106+
if r.noMaster {
107+
sshConfig = &ssh.SSHConfig{
108+
ConfigFile: sshConfig.ConfigFile,
109+
Persist: false,
110+
AdditionalArgs: sshutil.DisableControlMasterOptsFromSSHArgs(sshConfig.AdditionalArgs),
111+
}
112+
}
113+
stdout, stderr, err := ssh.ExecuteScript(a.instSSHAddress, a.sshLocalPort, sshConfig, script, r.description)
105114
logrus.Debugf("stdout=%q, stderr=%q, err=%v", stdout, stderr, err)
106115
if err != nil {
107116
return fmt.Errorf("stdout=%q, stderr=%q: %w", stdout, stderr, err)
@@ -114,6 +123,7 @@ type requirement struct {
114123
script string
115124
debugHint string
116125
fatal bool
126+
noMaster bool
117127
}
118128

119129
func (a *HostAgent) essentialRequirements() []requirement {
@@ -128,8 +138,17 @@ true
128138
Make sure that the YAML field "ssh.localPort" is not used by other processes on the host.
129139
If any private key under ~/.ssh is protected with a passphrase, you need to have ssh-agent to be running.
130140
`,
141+
noMaster: true,
131142
})
143+
startControlMasterReq := requirement{
144+
description: "Explicitly start ssh ControlMaster",
145+
script: `#!/bin/bash
146+
true
147+
`,
148+
debugHint: `The persistent ssh ControlMaster should be started immediately.`,
149+
}
132150
if *a.instConfig.Plain {
151+
req = append(req, startControlMasterReq)
133152
return req
134153
}
135154
req = append(req,
@@ -147,6 +166,7 @@ fi
147166
Terminating the session will break the persistent SSH tunnel, so
148167
it must not be created until the session reset is done.
149168
`,
169+
noMaster: true,
150170
})
151171

152172
if *a.instConfig.MountType == limatype.REVSSHFS && len(a.instConfig.Mounts) > 0 {
@@ -176,6 +196,8 @@ fi
176196
`,
177197
debugHint: `Append "user_allow_other" to /etc/fuse.conf (/etc/fuse3.conf) in the guest`,
178198
})
199+
} else {
200+
req = append(req, startControlMasterReq)
179201
}
180202
return req
181203
}

‎pkg/sshutil/sshutil.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,36 @@ func identityFileEntry(ctx context.Context, privateKeyPath string) (string, erro
295295
return fmt.Sprintf(`IdentityFile="%s"`, privateKeyPath), nil
296296
}
297297

298+
// DisableControlMasterOptsFromSSHArgs returns ssh args that disable ControlMaster, ControlPath, and ControlPersist.
299+
func DisableControlMasterOptsFromSSHArgs(sshArgs []string) []string {
300+
argsForOverridingConfigFile := []string{
301+
"-o", "ControlMaster=no",
302+
"-o", "ControlPath=none",
303+
"-o", "ControlPersist=no",
304+
}
305+
return slices.Concat(argsForOverridingConfigFile, removeOptsFromSSHArgs(sshArgs, "ControlMaster", "ControlPath", "ControlPersist"))
306+
}
307+
308+
func removeOptsFromSSHArgs(sshArgs []string, removeOpts ...string) []string {
309+
res := make([]string, 0, len(sshArgs))
310+
isOpt := false
311+
for _, arg := range sshArgs {
312+
if isOpt {
313+
isOpt = false
314+
if !slices.ContainsFunc(removeOpts, func(opt string) bool {
315+
return strings.HasPrefix(arg, opt)
316+
}) {
317+
res = append(res, "-o", arg)
318+
}
319+
} else if arg == "-o" {
320+
isOpt = true
321+
} else {
322+
res = append(res, arg)
323+
}
324+
}
325+
return res
326+
}
327+
298328
// SSHOpts adds the following options to CommonOptions: User, ControlMaster, ControlPath, ControlPersist.
299329
func SSHOpts(ctx context.Context, sshExe SSHExe, instDir, username string, useDotSSH, forwardAgent, forwardX11, forwardX11Trusted bool) ([]string, error) {
300330
controlSock := filepath.Join(instDir, filenames.SSHSock)

‎pkg/sshutil/sshutil_test.go‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,103 @@ func Test_detectValidPublicKey(t *testing.T) {
5353
assert.Check(t, !detectValidPublicKey("arbitrary content"))
5454
assert.Check(t, !detectValidPublicKey(""))
5555
}
56+
57+
func Test_DisableControlMasterOptsFromSSHArgs(t *testing.T) {
58+
tests := []struct {
59+
name string
60+
sshArgs []string
61+
want []string
62+
}{
63+
{
64+
name: "no ControlMaster options",
65+
sshArgs: []string{
66+
"-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null",
67+
},
68+
want: []string{
69+
"-o", "ControlMaster=no",
70+
"-o", "ControlPath=none",
71+
"-o", "ControlPersist=no",
72+
"-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null",
73+
},
74+
},
75+
{
76+
name: "ControlMaster=yes",
77+
sshArgs: []string{
78+
"-o", "ControlMaster=yes", "-o", "UserKnownHostsFile=/dev/null",
79+
},
80+
want: []string{
81+
"-o", "ControlMaster=no",
82+
"-o", "ControlPath=none",
83+
"-o", "ControlPersist=no",
84+
"-o", "UserKnownHostsFile=/dev/null",
85+
},
86+
},
87+
{
88+
name: "ControlMaster=auto",
89+
sshArgs: []string{
90+
"-o", "ControlMaster=auto", "-o", "UserKnownHostsFile=/dev/null",
91+
},
92+
want: []string{
93+
"-o", "ControlMaster=no",
94+
"-o", "ControlPath=none",
95+
"-o", "ControlPersist=no",
96+
"-o", "UserKnownHostsFile=/dev/null",
97+
},
98+
},
99+
{
100+
name: "ControlMaster=auto with ControlPath",
101+
sshArgs: []string{
102+
"-o", "ControlMaster=auto", "-o", "ControlPath=/tmp/ssh-%r@%h:%p", "-o", "UserKnownHostsFile=/dev/null",
103+
},
104+
want: []string{
105+
"-o", "ControlMaster=no",
106+
"-o", "ControlPath=none",
107+
"-o", "ControlPersist=no",
108+
"-o", "UserKnownHostsFile=/dev/null",
109+
},
110+
},
111+
{
112+
name: "ControlPath only",
113+
sshArgs: []string{
114+
"-o", "ControlPath=/tmp/ssh-%r@%h:%p", "-o", "UserKnownHostsFile=/dev/null",
115+
},
116+
want: []string{
117+
"-o", "ControlMaster=no",
118+
"-o", "ControlPath=none",
119+
"-o", "ControlPersist=no",
120+
"-o", "UserKnownHostsFile=/dev/null",
121+
},
122+
},
123+
{
124+
name: "ControlMaster=no",
125+
sshArgs: []string{
126+
"-o", "ControlMaster=no", "-o", "UserKnownHostsFile=/dev/null",
127+
},
128+
want: []string{
129+
"-o", "ControlMaster=no",
130+
"-o", "ControlPath=none",
131+
"-o", "ControlPersist=no",
132+
"-o", "UserKnownHostsFile=/dev/null",
133+
},
134+
},
135+
{
136+
name: "ControlMaster=auto with other options",
137+
sshArgs: []string{
138+
"-o", "ControlMaster=auto",
139+
"-o", "StrictHostKeyChecking=no",
140+
"-o", "UserKnownHostsFile=/dev/null",
141+
},
142+
want: []string{
143+
"-o", "ControlMaster=no",
144+
"-o", "ControlPath=none",
145+
"-o", "ControlPersist=no",
146+
"-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null",
147+
},
148+
},
149+
}
150+
for _, tt := range tests {
151+
t.Run(tt.name, func(t *testing.T) {
152+
assert.DeepEqual(t, DisableControlMasterOptsFromSSHArgs(tt.sshArgs), tt.want)
153+
})
154+
}
155+
}

0 commit comments

Comments
 (0)