From 3cae850f0a4cbba59929be3e79f0537bf1769794 Mon Sep 17 00:00:00 2001 From: Jan Dubois Date: Mon, 14 Oct 2024 22:01:38 -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 | 13 +++++- examples/vmnet.yaml | 6 +-- pkg/networks/networks.TEMPLATE.yaml | 2 +- pkg/networks/sudoers.go | 10 ++--- pkg/networks/validate.go | 43 ++++--------------- .../content/en/docs/config/network/_index.md | 21 +++++++-- 6 files changed, 44 insertions(+), 51 deletions(-) diff --git a/cmd/limactl/sudoers.go b/cmd/limactl/sudoers.go index 9b54adfcdb4..87de39113e8 100644 --- a/cmd/limactl/sudoers.go +++ b/cmd/limactl/sudoers.go @@ -8,16 +8,21 @@ import ( "runtime" "github.com/lima-vm/lima/pkg/networks" + "github.com/sirupsen/logrus" "github.com/spf13/cobra" ) -func newSudoersCommand() *cobra.Command { - networksMD := "$PREFIX/share/doc/lima/docs/network.md" +var networksMD = "$PREFIX/share/doc/lima/docs/network.md" + +func init() { 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") } +} + +func newSudoersCommand() *cobra.Command { sudoersCommand := &cobra.Command{ Use: "sudoers [--check [SUDOERSFILE-TO-CHECK]]", Example: ` @@ -50,8 +55,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.", networksMD) 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/sudoers.go b/pkg/networks/sudoers.go index 08a45f59b0b..482859263ae 100644 --- a/pkg/networks/sudoers.go +++ b/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,12 @@ 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) + 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 +95,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) } diff --git a/pkg/networks/validate.go b/pkg/networks/validate.go index 6c2c1a49d35..e2133dd3f96 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()&020 != 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()&020 != 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/website/content/en/docs/config/network/_index.md b/website/content/en/docs/config/network/_index.md index 48150519a90..bb7f2030b3e 100644 --- a/website/content/en/docs/config/network/_index.md +++ b/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