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]>
lima-vm · Oct 15, 2024 · daaaa0f · daaaa0f
1 parent 713e1cd
commit daaaa0f
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 54 deletions.
diff --git a/cmd/limactl/sudoers.go b/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,
@@ -52,6 +47,7 @@ func sudoersAction(cmd *cobra.Command, args []string) error {
 	}
 	// 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")

diff --git a/examples/vmnet.yaml b/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.

diff --git a/pkg/networks/networks.TEMPLATE.yaml b/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

diff --git a/pkg/networks/validate.go b/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)

diff --git a/pkg/store/filenames/filenames.go b/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

diff --git a/website/content/en/docs/config/network/_index.md b/website/content/en/docs/config/network/_index.md
@@ -69,10 +69,23 @@ 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 as root from source to /opt/socket_vmnet
+# using instructions on https://github.com/lima-vm/socket_vmnet
+# This assumes that Xcode Command Line Tools are already installed
+git clone https://github.com/lima-vm/socket_vmnet
+cd socket_vmnet
+# Change "v1.1.5" to the actual latest release in https://github.com/lima-vm/socket_vmnet/releases
+git checkout v1.1.5
+make
+sudo make PREFIX=/opt/socket_vmnet install.bin
 
 # Set up the sudoers file for launching socket_vmnet from Lima
 limactl sudoers >etc_sudoers.d_lima
@@ -83,6 +96,9 @@ sudo install -o root etc_sudoers.d_lima /etc/sudoers.d/lima
 >
 > Lima before v0.12 used `vde_vmnet` for managing the networks.
 > `vde_vmnet` is no longer supported.
+>
+> Lima v0.14.0 and later used to also accept `socket_vmnet` installations if they were
+> owned by the `admin` user. Starting with v1.0.0 only `root` ownership is acceptable.
 
 The networks are defined in `$LIMA_HOME/_config/networks.yaml`. If this file doesn't already exist, it will be created with these default
 settings:
@@ -145,7 +161,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 +176,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