hostagent: lower ssh's soft fd limit to 1024 on macOS

macOS ssh polls with select(), so the ControlMaster quits with
"poll: Invalid argument" once it holds fd 1024. The hostagent raises
the fd limit that its children inherit, and Virtualization.framework
raises it again during VM start. So about 1000 forwarded connections
kill the master, and every forward it holds dies with it. Nothing logs
why.

Run every ssh that the hostagent package's own code starts under a soft
fd limit of at most 1024. Any of them started with ControlMaster=auto
can become the master, and an ssh -O forward that cannot reach a master
at its fd limit opens its own connection and holds the forward itself,
so it needs the limit too. At the limit, accept() fails with EMFILE and
ssh retries it a second later, so new connections wait or get reset
while open ones keep working. The hostagent itself keeps the higher
limit, which the gRPC forwarder needs.

sshocker's ExecuteScript starts ssh on its own, where the limit cannot
reach it. So every guest script now runs through the hostagent's copy
of that function, which only Windows guests used before.

Signed-off-by: Jan Dubois <jan.dubois@suse.com>

pkg/hostagent/hostagent.go

@@ -640,7 +640,7 @@ func (a *HostAgent) startHostAgentRoutines(ctx context.Context) error {
 set -eux -o pipefail
 ln -sf "${SSH_AUTH_SOCK}" /run/host-services/ssh-auth.sock`
 			faDesc := "linking ssh auth socket to static location /run/host-services/ssh-auth.sock"
-			stdout, stderr, err := ssh.ExecuteScript(a.instSSHAddress, a.sshLocalPort, a.sshConfig, faScript, faDesc)
+			stdout, stderr, err := a.executeUnixScript(a.sshConfig, faScript, faDesc)
 			logrus.Debugf("stdout=%#q, stderr=%#q, err=%v", stdout, stderr, err)
 			if err != nil {
 				errs = append(errs, fmt.Errorf("stdout=%#q, stderr=%#q: %w", stdout, stderr, err))
@@ -1036,7 +1036,7 @@ func executeSSH(ctx context.Context, sshConfig *ssh.SSHConfig, sshAddress string
 		"--",
 	)
 	args = append(args, command...)
-	cmd := exec.CommandContext(ctx, sshConfig.Binary(), args...)
+	cmd := sshCommand(ctx, sshConfig.Binary(), args...)
 	if out, err := cmd.Output(); err != nil {
 		return fmt.Errorf("failed to run %v: %#q: %w", cmd.Args, string(out), err)
 	}
@@ -1145,7 +1145,7 @@ var forwardSSH = func(ctx context.Context, sshConfig *ssh.SSHConfig, sshAddress
 			panic(fmt.Errorf("invalid verb %#q", verb))
 		}
 	}
-	cmd := exec.CommandContext(ctx, sshConfig.Binary(), args...)
+	cmd := sshCommand(ctx, sshConfig.Binary(), args...)
 	logrus.Debugf("Running %#q", cmd)
 	if out, err := cmd.Output(); err != nil {
 		if verb == verbForward && strings.HasPrefix(local, "/") {
@@ -1203,7 +1203,7 @@ func (a *HostAgent) watchCloudInitProgress(ctx context.Context) {
 		fi"`,
 	)
 
-	cmd = exec.CommandContext(ctx, a.sshConfig.Binary(), args...)
+	cmd = sshCommand(ctx, a.sshConfig.Binary(), args...)
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
 		logrus.WithError(err).Warn("Failed to create stdout pipe for cloud-init monitoring")
@@ -1279,7 +1279,7 @@ func (a *HostAgent) watchCloudInitProgress(ctx context.Context) {
 			"sudo", "tail", "-n", "20", "/var/log/cloud-init-output.log",
 		)
 
-		finalCmd := exec.CommandContext(ctx, a.sshConfig.Binary(), finalArgs...)
+		finalCmd := sshCommand(ctx, a.sshConfig.Binary(), finalArgs...)
 		if finalOutput, err := finalCmd.Output(); err == nil {
 			for line := range strings.SplitSeq(string(finalOutput), "\n") {
 				if strings.TrimSpace(line) != "" {
@@ -1327,7 +1327,7 @@ func copyToHost(ctx context.Context, sshConfig *ssh.SSHConfig, sshAddress string
 	if err := os.MkdirAll(filepath.Dir(local), 0o700); err != nil {
 		return fmt.Errorf("can't create directory for local file %#q: %w", local, err)
 	}
-	cmd := exec.CommandContext(ctx, sshConfig.Binary(), args...)
+	cmd := sshCommand(ctx, sshConfig.Binary(), args...)
 	out, err := cmd.Output()
 	if err != nil {
 		var exitErr *exec.ExitError
@@ -1341,7 +1341,7 @@ func copyToHost(ctx context.Context, sshConfig *ssh.SSHConfig, sshAddress string
 				"sudo", "cat",
 				remote,
 			)
-			cmd = exec.CommandContext(ctx, sshConfig.Binary(), sudoArgs...)
+			cmd = sshCommand(ctx, sshConfig.Binary(), sudoArgs...)
 			out, err = cmd.Output()
 		}
 		if errors.As(err, &exitErr) && strings.Contains(string(exitErr.Stderr), "No such file or directory") {

pkg/hostagent/hostagent_unix.go

@@ -14,7 +14,9 @@ import (
 // Default nofile limit is too low on some system.
 // For example in the macOS standard terminal is 256.
 // It means that there are only ~240 connections available from the host to the vm.
-// That function increases the nofile limit for child processes, especially the ssh process
+// That function increases the nofile limit for child processes, especially the ssh process.
+// On macOS, sshCommand then lowers the soft limit of each ssh it starts to 1024
+// when the limit is higher.
 //
 // More about limits in go: https://go.dev/issue/46279
 func adjustNofileRlimit() {

pkg/hostagent/requirements.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"os/exec"
 	"runtime"
 	"strconv"
 	"strings"
@@ -103,7 +102,7 @@ func (a *HostAgent) waitForRequirements(ctx context.Context, label string, requi
 	return errors.Join(errs...)
 }
 
-// prefixExportParam will modify a script to be executed by ssh.ExecuteScript so that it exports
+// prefixExportParam will modify a script to be executed by waitForRequirement so that it exports
 // all the variables from /mnt/lima-cidata/param.env before invoking the actual interpreter.
 //
 //   - The script is executed in user mode, so needs to read the file using `sudo`.
@@ -123,7 +122,7 @@ func (a *HostAgent) waitForRequirements(ctx context.Context, label string, requi
 //	EOF
 //	/usr/bin/env ruby
 //
-// ssh.ExecuteScript will strip the `#!` prefix from the first line and invoke the
+// executeUnixScript will strip the `#!` prefix from the first line and invoke the
 // rest of the line as the command. The full script is then passed via STDIN. We use
 // "$(printf '…')" to be able to use \n as newline escapes, to fit everything on a
 // single line:
@@ -194,7 +193,7 @@ func (a *HostAgent) waitForRequirement(r requirement) error {
 			AdditionalArgs: sshutil.DisableControlMasterOptsFromSSHArgs(sshConfig.AdditionalArgs),
 		}
 	}
-	stdout, stderr, err := ssh.ExecuteScript(a.instSSHAddress, a.sshLocalPort, sshConfig, script, r.description)
+	stdout, stderr, err := a.executeUnixScript(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)
@@ -202,9 +201,6 @@ func (a *HostAgent) waitForRequirement(r requirement) error {
 	return nil
 }
 
-// This function is copied from sshocker's ExecuteScript, because
-// the function doesn't support Windows commands.
-// TODO: Support Windows on sshocker, and replace this function.
 func (a *HostAgent) waitForWinRequirement(r requirement) error {
 	logrus.Debugf("executing script for windows %q", r.description)
 	script := r.script
@@ -227,25 +223,42 @@ func (a *HostAgent) waitForWinRequirement(r requirement) error {
 			AdditionalArgs: sshutil.DisableControlMasterOptsFromSSHArgs(sshConfig.AdditionalArgs),
 		}
 	}
+	_, _, err := a.executeScript(sshConfig, script, script, r.description)
+	return err
+}
+
+// executeUnixScript runs script on the guest with the interpreter named on its #! line.
+func (a *HostAgent) executeUnixScript(sshConfig *ssh.SSHConfig, script, scriptName string) (stdout, stderr string, err error) {
+	interpreter, err := ssh.ParseScriptInterpreter(script)
+	if err != nil {
+		return "", "", err
+	}
+	return a.executeScript(sshConfig, interpreter, script, scriptName)
+}
+
+// executeScript runs remoteCommand on the guest with script as its stdin,
+// and returns stdout and stderr.
+//
+// scriptName is used only for readability of error strings.
+func (a *HostAgent) executeScript(sshConfig *ssh.SSHConfig, remoteCommand, script, scriptName string) (stdout, stderr string, err error) {
 	if sshConfig == nil {
-		return errors.New("got nil SSHConfig")
+		return "", "", errors.New("got nil SSHConfig")
 	}
-	sshBinary := sshConfig.Binary()
 	sshArgs := sshConfig.Args()
 	if a.sshLocalPort != 0 {
 		sshArgs = append(sshArgs, "-p", strconv.Itoa(a.sshLocalPort))
 	}
-	sshArgs = append(sshArgs, a.instSSHAddress, "--", script)
-	sshCmd := exec.CommandContext(context.Background(), sshBinary, sshArgs...)
+	sshArgs = append(sshArgs, a.instSSHAddress, "--", remoteCommand)
+	sshCmd := sshCommand(context.Background(), sshConfig.Binary(), sshArgs...)
 	sshCmd.Stdin = strings.NewReader(script)
-	var stderr bytes.Buffer
-	sshCmd.Stderr = &stderr
-	logrus.Debugf("executing ssh for script :%s %v", sshCmd.Path, sshCmd.Args)
+	var stderrBuf bytes.Buffer
+	sshCmd.Stderr = &stderrBuf
+	logrus.Debugf("executing ssh for script %q: %s %v", scriptName, sshCmd.Path, sshCmd.Args)
 	out, err := sshCmd.Output()
 	if err != nil {
-		return fmt.Errorf("failed to execute script: stdout=%q, stderr=%q: %w", string(out), stderr.String(), err)
+		return string(out), stderrBuf.String(), fmt.Errorf("failed to execute script %q: stdout=%q, stderr=%q: %w", scriptName, string(out), stderrBuf.String(), err)
 	}
-	return nil
+	return string(out), stderrBuf.String(), nil
 }
 
 type requirement struct {

pkg/hostagent/sshcommand_darwin.go

@@ -0,0 +1,24 @@
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package hostagent
+
+import (
+	"context"
+	"os/exec"
+)
+
+// sshCommand returns a command that runs ssh with its soft fd limit set to at most 1024.
+// macOS ssh polls with select(), which rejects fds >= FD_SETSIZE (1024), so an ssh
+// that is allowed more fds quits once it holds fd 1024.
+func sshCommand(ctx context.Context, sshBinary string, args ...string) *exec.Cmd {
+	// Resolve ssh with os/exec. When PATH has a relative entry such as ".", the
+	// shell would run an ssh from the working directory, but os/exec refuses.
+	sshPath, err := exec.LookPath(sshBinary)
+	cmd := exec.CommandContext(ctx, "/bin/sh", append([]string{"-c", "ulimit -S -n 1024 2>/dev/null; exec \"$0\" \"$@\"", sshPath}, args...)...)
+	// When the lookup fails, Start returns err without starting sh or ssh.
+	if err != nil {
+		cmd.Err = err
+	}
+	return cmd
+}

pkg/hostagent/sshcommand_darwin_test.go

@@ -0,0 +1,60 @@
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package hostagent
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"syscall"
+	"testing"
+
+	"github.com/lima-vm/sshocker/pkg/ssh"
+	"gotest.tools/v3/assert"
+
+	"github.com/lima-vm/lima/v2/pkg/limatype"
+)
+
+func TestSSHCommandCapsNofile(t *testing.T) {
+	adjustNofileRlimit()
+	var limit syscall.Rlimit
+	assert.NilError(t, syscall.Getrlimit(syscall.RLIMIT_NOFILE, &limit))
+	out, err := sshCommand(t.Context(), "/bin/sh", "-c", "ulimit -S -n").Output()
+	assert.NilError(t, err)
+	assert.Equal(t, strings.TrimSpace(string(out)), strconv.FormatUint(min(limit.Max, 1024), 10))
+}
+
+func TestSSHCommandIgnoresSSHInWorkingDirectory(t *testing.T) {
+	dir := t.TempDir()
+	assert.NilError(t, os.WriteFile(filepath.Join(dir, "ssh"), []byte("#!/bin/sh\n"), 0o755))
+	t.Chdir(dir)
+	t.Setenv("PATH", ".")
+	assert.ErrorIs(t, sshCommand(t.Context(), "ssh").Run(), exec.ErrDot)
+}
+
+// fakeSSHCheckingNofile raises the fd limit as the hostagent does, then puts an ssh
+// on PATH that fails, printing its soft fd limit, unless that limit is at most 1024.
+func fakeSSHCheckingNofile(t *testing.T) {
+	adjustNofileRlimit()
+	dir := t.TempDir()
+	script := `#!/bin/sh
+n=$(ulimit -S -n)
+[ "$n" != unlimited ] && [ "$n" -le 1024 ] || { echo "soft fd limit $n"; exit 1; }
+`
+	assert.NilError(t, os.WriteFile(filepath.Join(dir, "ssh"), []byte(script), 0o755))
+	t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH"))
+}
+
+func TestWaitForRequirementCapsNofile(t *testing.T) {
+	fakeSSHCheckingNofile(t)
+	a := &HostAgent{instConfig: &limatype.LimaYAML{OS: new(limatype.LINUX)}, sshConfig: &ssh.SSHConfig{}}
+	assert.NilError(t, a.waitForRequirement(requirement{description: "fd limit", script: "#!/bin/sh\n"}))
+}
+
+func TestForwardSSHCapsNofile(t *testing.T) {
+	fakeSSHCheckingNofile(t)
+	assert.NilError(t, forwardSSH(t.Context(), &ssh.SSHConfig{}, "127.0.0.1", 22, "127.0.0.1:1", "127.0.0.1:2", verbForward, false))
+}

pkg/hostagent/sshcommand_others.go

@@ -0,0 +1,15 @@
+//go:build !darwin
+
+// SPDX-FileCopyrightText: Copyright The Lima Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package hostagent
+
+import (
+	"context"
+	"os/exec"
+)
+
+func sshCommand(ctx context.Context, sshBinary string, args ...string) *exec.Cmd {
+	return exec.CommandContext(ctx, sshBinary, args...)
+}