Skip to content

Commit

Permalink
networks.Validate() requires that socket_vmnet is owned by root
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
jandubois committed Oct 15, 2024
1 parent 713e1cd commit 3cae850
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 51 deletions.
13 changes: 11 additions & 2 deletions cmd/limactl/sudoers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down Expand Up @@ -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")
Expand Down
6 changes: 1 addition & 5 deletions examples/vmnet.yaml
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/networks/networks.TEMPLATE.yaml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
10 changes: 5 additions & 5 deletions pkg/networks/sudoers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)

Check failure on line 89 in pkg/networks/sudoers.go

View workflow job for this annotation

GitHub Actions / Lints

error-strings: error strings should not be capitalized or end with punctuation or a newline (revive)
}
hint := fmt.Sprintf("run `%s sudoers >etc_sudoers.d_lima && sudo install -o root etc_sudoers.d_lima %q`)",
os.Args[0], sudoersFile)
Expand All @@ -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)
}
Expand Down
43 changes: 8 additions & 35 deletions pkg/networks/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ import (
"fmt"
"io/fs"
"os"
"os/user"
"path/filepath"
"reflect"
"runtime"
"strconv"
"strings"

"github.com/lima-vm/lima/pkg/osutil"
Expand Down Expand Up @@ -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 {

Check failure on line 109 in pkg/networks/validate.go

View workflow job for this annotation

GitHub Actions / Lints

octalLiteral: use new octal literal style, 0o20 (gocritic)
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 {

Check failure on line 116 in pkg/networks/validate.go

View workflow job for this annotation

GitHub Actions / Lints

octalLiteral: use new octal literal style, 0o20 (gocritic)
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)
Expand Down
21 changes: 18 additions & 3 deletions website/content/en/docs/config/network/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down

0 comments on commit 3cae850

Please sign in to comment.