From d2dad993950704ab0887088e0a6ff7b5a7822444 Mon Sep 17 00:00:00 2001 From: Jan Dubois Date: Tue, 15 Oct 2024 12:59:43 -0700 Subject: [PATCH] networks.Validate() requires that socket_vmnet is owned by root This restriction was weakened by https://github.com/lima-vm/lima/pull/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 --- cmd/limactl/sudoers.go | 14 +++--- examples/vmnet.yaml | 6 +-- pkg/networks/networks.TEMPLATE.yaml | 2 +- pkg/networks/validate.go | 43 ++++--------------- pkg/store/filenames/filenames.go | 2 +- .../content/en/docs/config/network/_index.md | 35 +++++++++++++-- 6 files changed, 48 insertions(+), 54 deletions(-) diff --git a/cmd/limactl/sudoers.go b/cmd/limactl/sudoers.go index 9b54adfcdb4..afe4c5ed90b 100644 --- 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 index 0cdbc25476e..cd92752fe6a 100644 --- 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 index 884008787f7..b98e3db7b00 100644 --- 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 index 6c2c1a49d35..37eb3c7d433 100644 --- 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 index f07300a6674..0890156de43 100644 --- 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 index 48150519a90..1eafc4b73bd 100644 --- a/website/content/en/docs/config/network/_index.md +++ b/website/content/en/docs/config/network/_index.md @@ -69,20 +69,38 @@ 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 +less etc_sudoers.d_lima # verify that the file looks correct sudo install -o root etc_sudoers.d_lima /etc/sudoers.d/lima +rm etc_sudoers.d_lima ``` > **Note** > > 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 +163,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 +178,17 @@ 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 >etc_sudoers.d_lima +less etc_sudoers.d_lima # verify that the file looks correct +sudo install -o root etc_sudoers.d_lima /etc/sudoers.d/lima +rm 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