Skip to content

Commit 06ddf4f

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 06ddf4f

File tree

3 files changed

+156
-1
lines changed

3 files changed

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

0 commit comments

Comments
 (0)