networks.Validate() requires that socket_vmnet is owned by root

This restriction was weakened by #1220
to only require the file and directories to be owned by the admin user, but
that configuration is not secure.

If users are willing to run an insecure configuration, then they can always
enable password-less sudo, which does not need a sudoers file at all.

Signed-off-by: Jan Dubois <[email protected]>

Author: jandubois

cmd/limactl/sudoers.go

@@ -3,21 +3,16 @@ package main
 import (
 	"errors"
 	"fmt"
-	"os"
-	"path/filepath"
 	"runtime"
 
 	"github.com/lima-vm/lima/pkg/networks"
+	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
+const networksURL = "https://lima-vm.io/docs/config/network/#socket_vmnet"
+
 func newSudoersCommand() *cobra.Command {
-	networksMD := "$PREFIX/share/doc/lima/docs/network.md"
-	if exe, err := os.Executable(); err == nil {
-		binDir := filepath.Dir(exe)
-		prefixDir := filepath.Dir(binDir)
-		networksMD = filepath.Join(prefixDir, "share/doc/lima/docs/network.md")
-	}
 	sudoersCommand := &cobra.Command{
 		Use: "sudoers [--check [SUDOERSFILE-TO-CHECK]]",
 		Example: `
@@ -31,7 +26,7 @@ $ limactl sudoers --check /etc/sudoers.d/lima
 		Long: fmt.Sprintf(`Generate the content of the /etc/sudoers.d/lima file for enabling vmnet.framework support.
 The content is written to stdout, NOT to the file.
 This command must not run as the root user.
-See %s for the usage.`, networksMD),
+See %s for the usage.`, networksURL),
 		Args:    WrapArgsError(cobra.MaximumNArgs(1)),
 		RunE:    sudoersAction,
 		GroupID: advancedCommand,
@@ -50,8 +45,12 @@ func sudoersAction(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
+	if err := nwCfg.PasswordLessSudo(); err == nil {
+		return errors.New("sudoers file is not needed because sudo doesn't require a password")
+	}
 	// Make sure the current network configuration is secure
 	if err := nwCfg.Validate(); err != nil {
+		logrus.Infof("Please check %s for more information.", networksURL)
 		return err
 	}
 	check, err := cmd.Flags().GetBool("check")

examples/vmnet.yaml

@@ -1,10 +1,6 @@
 # A template to enable vmnet.framework.
 
-# Usage:
-#   brew install socket_vmnet
-#   limactl sudoers >etc_sudoers.d_lima
-#   sudo install -o root etc_sudoers.d_lima /etc/sudoers.d/lima
-#   limactl start template://vmnet
+# Install socket_vmnet: https://lima-vm.io/docs/config/network/#socket_vmnet
 
 # This template requires Lima v0.7.0 or later.
 # Older versions of Lima were using a different syntax for supporting vmnet.framework.

pkg/networks/networks.TEMPLATE.yaml

@@ -1,6 +1,6 @@
 # Path to socket_vmnet executable. Because socket_vmnet is invoked via sudo it should be
 # installed where only root can modify/replace it. This means also none of the
-# parent directories should be writable by the user.
+# parent directories can be writable by the user.
 #
 # The varRun directory also must not be writable by the user because it will
 # include the socket_vmnet pid file. Those will be terminated via sudo, so replacing

pkg/networks/sudoers.go

@@ -53,7 +53,7 @@ func Sudoers() (string, error) {
 	return sb.String(), nil
 }
 
-func (c *Config) passwordLessSudo() error {
+func (c *Config) PasswordLessSudo() error {
 	// Flush cached sudo password
 	cmd := exec.Command("sudo", "-k")
 	if err := cmd.Run(); err != nil {
@@ -81,12 +81,13 @@ func (c *Config) passwordLessSudo() error {
 
 func (c *Config) VerifySudoAccess(sudoersFile string) error {
 	if sudoersFile == "" {
-		err := c.passwordLessSudo()
+		err := c.PasswordLessSudo()
 		if err == nil {
 			logrus.Debug("sudo doesn't seem to require a password")
 			return nil
 		}
-		return fmt.Errorf("passwordLessSudo error: %w", err)
+		//nolint:revive // error-strings
+		return fmt.Errorf("PasswordLessSudo error: %w", err)
 	}
 	hint := fmt.Sprintf("run `%s sudoers >etc_sudoers.d_lima && sudo install -o root etc_sudoers.d_lima %q`)",
 		os.Args[0], sudoersFile)
@@ -95,12 +96,12 @@ func (c *Config) VerifySudoAccess(sudoersFile string) error {
 		// Default networks.yaml specifies /etc/sudoers.d/lima file. Don't throw an error when the
 		// file doesn't exist, as long as password-less sudo still works.
 		if errors.Is(err, os.ErrNotExist) {
-			err = c.passwordLessSudo()
+			err = c.PasswordLessSudo()
 			if err == nil {
 				logrus.Debugf("%q does not exist, but sudo doesn't seem to require a password", sudoersFile)
 				return nil
 			}
-			logrus.Debugf("%q does not exist; passwordLessSudo error: %s", sudoersFile, err)
+			logrus.Debugf("%q does not exist; PasswordLessSudo error: %s", sudoersFile, err)
 		}
 		return fmt.Errorf("can't read %q: %w: (Hint: %s)", sudoersFile, err, hint)
 	}

pkg/networks/validate.go

@@ -5,11 +5,9 @@ import (
 	"fmt"
 	"io/fs"
 	"os"
-	"os/user"
 	"path/filepath"
 	"reflect"
 	"runtime"
-	"strconv"
 	"strings"
 
 	"github.com/lima-vm/lima/pkg/osutil"
@@ -100,49 +98,24 @@ func validatePath(path string, allowDaemonGroupWritable bool) error {
 	if err != nil {
 		return err
 	}
-	adminGroup, err := user.LookupGroup("admin")
-	if err != nil {
-		return err
-	}
-	adminGid, err := strconv.Atoi(adminGroup.Gid)
-	if err != nil {
-		return err
-	}
-	owner, err := user.LookupId(strconv.Itoa(int(stat.Uid)))
-	if err != nil {
-		return err
-	}
-	ownerIsAdmin := owner.Uid == "0"
-	if !ownerIsAdmin {
-		ownerGroupIDs, err := owner.GroupIds()
-		if err != nil {
-			return err
-		}
-		for _, g := range ownerGroupIDs {
-			if g == adminGroup.Gid {
-				ownerIsAdmin = true
-				break
-			}
-		}
-	}
-	if !ownerIsAdmin {
-		return fmt.Errorf(`%s %q owner %dis not an admin`, file, path, stat.Uid)
+	if stat.Uid != root.Uid {
+		return fmt.Errorf(`%s %q is not owned by %q (uid: %d), but by uid %d`, file, path, root.User, root.Uid, stat.Uid)
 	}
 	if allowDaemonGroupWritable {
 		daemon, err := osutil.LookupUser("daemon")
 		if err != nil {
 			return err
 		}
-		if fi.Mode()&0o20 != 0 && stat.Gid != root.Gid && stat.Gid != uint32(adminGid) && stat.Gid != daemon.Gid {
-			return fmt.Errorf(`%s %q is group-writable and group %d is not one of [wheel, admin, daemon]`,
-				file, path, stat.Gid)
+		if fi.Mode()&0o20 != 0 && stat.Gid != root.Gid && stat.Gid != daemon.Gid {
+			return fmt.Errorf(`%s %q is group-writable and group is neither %q (gid: %d) nor %q (gid: %d), but is gid: %d`,
+				file, path, root.User, root.Gid, daemon.User, daemon.Gid, stat.Gid)
 		}
 		if fi.Mode().IsDir() && fi.Mode()&1 == 0 && (fi.Mode()&0o010 == 0 || stat.Gid != daemon.Gid) {
 			return fmt.Errorf(`%s %q is not executable by the %q (gid: %d)" group`, file, path, daemon.User, daemon.Gid)
 		}
-	} else if fi.Mode()&0o20 != 0 && stat.Gid != root.Gid && stat.Gid != uint32(adminGid) {
-		return fmt.Errorf(`%s %q is group-writable and group %d is not one of [wheel, admin]`,
-			file, path, stat.Gid)
+	} else if fi.Mode()&0o20 != 0 && stat.Gid != root.Gid {
+		return fmt.Errorf(`%s %q is group-writable and group is not %q (gid: %d), but is gid: %d`,
+			file, path, root.User, root.Gid, stat.Gid)
 	}
 	if fi.Mode()&0o02 != 0 {
 		return fmt.Errorf("%s %q is world-writable", file, path)

pkg/store/filenames/filenames.go

@@ -1,7 +1,7 @@
 // Package filenames defines the names of the files that appear under an instance dir
 // or inside the config directory.
 //
-// See docs/internal.md .
+// See https://lima-vm.io/docs/dev/internals/
 package filenames
 
 // Instance names starting with an underscore are reserved for lima internal usage

website/content/en/docs/config/network/_index.md

@@ -69,10 +69,17 @@ The configuration steps are different for each network type:
 #### Managed (192.168.105.0/24)
 
 [`socket_vmnet`](https://github.com/lima-vm/socket_vmnet) is required for adding another guest IP that is accessible from the host and other guests.
+It must be installed according to the instruction provided on https://github.com/lima-vm/socket_vmnet.
+
+Note that installation using Homebrew is not secure and not recommended by the Lima project.
+Homebrew installation will only work with Lima if password-less `sudo` is enabled for the current user.
+The `limactl sudoers` command requires that `socket_vmnet` is installed into a secure path only
+writable by `root` and will reject `socket_vmnet` installed by Homebrew into a user-writable location.
 
 ```bash
-# Install socket_vmnet
-brew install socket_vmnet
+# Install socket_vmnet via MacPorts or from source
+# using instructions on https://github.com/lima-vm/socket_vmnet
+…
 
 # Set up the sudoers file for launching socket_vmnet from Lima
 limactl sudoers >etc_sudoers.d_lima
@@ -145,7 +152,7 @@ limactl start --network=lima:shared
 ```yaml
 networks:
   # Lima can manage the socket_vmnet daemon for networks defined in $LIMA_HOME/_config/networks.yaml automatically.
-  # The socket_vmnet binary must be installed into a secure location only alterable by the admin.
+  # The socket_vmnet binary must be installed into a secure location only alterable by the "root" user.
   # - lima: shared
   #   # MAC address of the instance; lima will pick one based on the instance name,
   #   # so DHCP assigned ip addresses should remain constant over instance restarts.
@@ -160,6 +167,14 @@ The network daemon is started automatically when the first instance referencing
 and will stop automatically once the last instance has stopped. Daemon logs will be stored in the
 `$LIMA_HOME/_networks` directory.
 
+Since the commands to start and stop the `socket_vmnet` daemon requires root, the user either must
+have password-less `sudo` enabled, or add the required commands to a `sudoers` file. This can
+be done via:
+
+```shell
+limactl sudoers | sudo tee /etc/sudoers.d/lima
+```
+
 The IP address is automatically assigned by macOS's bootpd.
 If the IP address is not assigned, try the following commands:
 ```bash