Merge pull request #3999 from kolyshkin/uidgid

Remove /etc/passwd and /etc/group parsing on runc run/exec

CHANGELOG.md

@@ -9,6 +9,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### libcontainer API
  * `configs.CommandHook` struct has changed, Command is now a pointer.
    Also, `configs.NewCommandHook` now accepts a `*Command`. (#4325)
+ * The `Process` struct has `User` string field replaced with numeric
+   `UID` and `GID` fields, and `AdditionalGroups` changed its type from
+   `[]string` to `[]int`. Essentially, resolution of user and group
+   names to IDs is no longer performed by libcontainer, so if a libcontainer
+   user previously relied on this feature, now they have to convert names to
+   IDs before calling libcontainer; it is recommended to use Go package
+   github.com/moby/sys/user for that. (#3999)
 
 ## [1.2.0] - 2024-10-22
 

libcontainer/container_linux.go

@@ -302,6 +302,13 @@ func (c *Container) start(process *Process) (retErr error) {
 	if c.config.Cgroups.Resources.SkipDevices {
 		return errors.New("can't start container with SkipDevices set")
 	}
+
+	if c.config.RootlessEUID && len(process.AdditionalGroups) > 0 {
+		// We cannot set any additional groups in a rootless container
+		// and thus we bail if the user asked us to do so.
+		return errors.New("cannot set any additional groups in a rootless container")
+	}
+
 	if process.Init {
 		if c.initProcessStartTime != 0 {
 			return errors.New("container already has init process")
@@ -686,7 +693,8 @@ func (c *Container) newInitConfig(process *Process) *initConfig {
 		Config:           c.config,
 		Args:             process.Args,
 		Env:              process.Env,
-		User:             process.User,
+		UID:              process.UID,
+		GID:              process.GID,
 		AdditionalGroups: process.AdditionalGroups,
 		Cwd:              process.Cwd,
 		Capabilities:     process.Capabilities,

libcontainer/env.go

@@ -6,20 +6,23 @@ import (
 	"os"
 	"slices"
 	"strings"
+
+	"github.com/moby/sys/user"
+	"github.com/sirupsen/logrus"
 )
 
 // prepareEnv processes a list of environment variables, preparing it
 // for direct consumption by unix.Exec. In particular, it:
 //   - validates each variable is in the NAME=VALUE format and
 //     contains no \0 (nil) bytes;
 //   - removes any duplicates (keeping only the last value for each key)
-//   - sets PATH for the current process, if found in the list.
+//   - sets PATH for the current process, if found in the list;
+//   - adds HOME to returned environment, if not found in the list.
 //
-// It returns the deduplicated environment, a flag telling whether HOME
-// is present in the input, and an error.
-func prepareEnv(env []string) ([]string, bool, error) {
+// Returns the prepared environment.
+func prepareEnv(env []string, uid int) ([]string, error) {
 	if env == nil {
-		return nil, false, nil
+		return nil, nil
 	}
 	// Deduplication code based on dedupEnv from Go 1.22 os/exec.
 
@@ -31,29 +34,55 @@ func prepareEnv(env []string) ([]string, bool, error) {
 		kv := env[n-1]
 		i := strings.IndexByte(kv, '=')
 		if i == -1 {
-			return nil, false, errors.New("invalid environment variable: missing '='")
+			return nil, errors.New("invalid environment variable: missing '='")
 		}
 		if i == 0 {
-			return nil, false, errors.New("invalid environment variable: name cannot be empty")
+			return nil, errors.New("invalid environment variable: name cannot be empty")
 		}
 		key := kv[:i]
 		if saw[key] { // Duplicate.
 			continue
 		}
 		saw[key] = true
 		if strings.IndexByte(kv, 0) >= 0 {
-			return nil, false, fmt.Errorf("invalid environment variable %q: contains nul byte (\\x00)", key)
+			return nil, fmt.Errorf("invalid environment variable %q: contains nul byte (\\x00)", key)
 		}
 		if key == "PATH" {
 			// Needs to be set as it is used for binary lookup.
 			if err := os.Setenv("PATH", kv[i+1:]); err != nil {
-				return nil, false, err
+				return nil, err
 			}
 		}
 		out = append(out, kv)
 	}
 	// Restore the original order.
 	slices.Reverse(out)
 
-	return out, saw["HOME"], nil
+	// If HOME is not found in env, get it from container's /etc/passwd and add.
+	if !saw["HOME"] {
+		home, err := getUserHome(uid)
+		if err != nil {
+			// For backward compatibility, don't return an error, but merely log it.
+			logrus.WithError(err).Debugf("HOME not set in process.env, and getting UID %d homedir failed", uid)
+		}
+
+		out = append(out, "HOME="+home)
+	}
+
+	return out, nil
+}
+
+func getUserHome(uid int) (string, error) {
+	const defaultHome = "/" // Default value, return this with any error.
+
+	u, err := user.LookupUid(uid)
+	if err != nil {
+		// ErrNoPasswdEntries is kinda expected as any UID can be specified.
+		if errors.Is(err, user.ErrNoPasswdEntries) {
+			err = nil
+		}
+		return defaultHome, err
+	}
+
+	return u.Home, nil
 }

libcontainer/env_test.go

@@ -1,25 +1,37 @@
 package libcontainer
 
 import (
+	"os/user"
 	"slices"
+	"strconv"
 	"testing"
 )
 
-func TestPrepareEnvDedup(t *testing.T) {
+func TestPrepareEnv(t *testing.T) {
+	u, err := user.Current()
+	if err != nil {
+		t.Fatal(err)
+	}
+	home := "HOME=" + u.HomeDir
+	uid, err := strconv.Atoi(u.Uid)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	tests := []struct {
 		env, wantEnv []string
 	}{
 		{
 			env:     []string{},
-			wantEnv: []string{},
+			wantEnv: []string{home},
 		},
 		{
-			env:     []string{"HOME=/root", "FOO=bar"},
-			wantEnv: []string{"HOME=/root", "FOO=bar"},
+			env:     []string{"HOME=/whoo", "FOO=bar"},
+			wantEnv: []string{"HOME=/whoo", "FOO=bar"},
 		},
 		{
 			env:     []string{"A=a", "A=b", "A=c"},
-			wantEnv: []string{"A=c"},
+			wantEnv: []string{"A=c", home},
 		},
 		{
 			env:     []string{"TERM=vt100", "HOME=/home/one", "HOME=/home/two", "TERM=xterm", "HOME=/home/three", "FOO=bar"},
@@ -28,7 +40,7 @@ func TestPrepareEnvDedup(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		env, _, err := prepareEnv(tc.env)
+		env, err := prepareEnv(tc.env, uid)
 		if err != nil {
 			t.Error(err)
 			continue

libcontainer/init_linux.go

@@ -14,7 +14,6 @@ import (
 	"syscall"
 
 	"github.com/containerd/console"
-	"github.com/moby/sys/user"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/sirupsen/logrus"
 	"github.com/vishvananda/netlink"
@@ -57,8 +56,9 @@ type initConfig struct {
 	ProcessLabel     string                `json:"process_label"`
 	AppArmorProfile  string                `json:"apparmor_profile"`
 	NoNewPrivileges  bool                  `json:"no_new_privileges"`
-	User             string                `json:"user"`
-	AdditionalGroups []string              `json:"additional_groups"`
+	UID              int                   `json:"uid"`
+	GID              int                   `json:"gid"`
+	AdditionalGroups []int                 `json:"additional_groups"`
 	Config           *configs.Config       `json:"config"`
 	Networks         []*network            `json:"network"`
 	PassedFilesCount int                   `json:"passed_files_count"`
@@ -208,7 +208,7 @@ func startInitialization() (retErr error) {
 }
 
 func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSocket, pidfdSocket, fifoFile, logPipe *os.File) error {
-	env, homeSet, err := prepareEnv(config.Env)
+	env, err := prepareEnv(config.Env, config.UID)
 	if err != nil {
 		return err
 	}
@@ -226,7 +226,6 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
 			pidfdSocket:   pidfdSocket,
 			config:        config,
 			logPipe:       logPipe,
-			addHome:       !homeSet,
 		}
 		return i.Init()
 	case initStandard:
@@ -238,7 +237,6 @@ func containerInit(t initType, config *initConfig, pipe *syncSocket, consoleSock
 			config:        config,
 			fifoFile:      fifoFile,
 			logPipe:       logPipe,
-			addHome:       !homeSet,
 		}
 		return i.Init()
 	}
@@ -274,7 +272,7 @@ func verifyCwd() error {
 // finalizeNamespace drops the caps, sets the correct user
 // and working dir, and closes any leaked file descriptors
 // before executing the command inside the namespace.
-func finalizeNamespace(config *initConfig, addHome bool) error {
+func finalizeNamespace(config *initConfig) error {
 	// Ensure that all unwanted fds we may have accidentally
 	// inherited are marked close-on-exec so they stay out of the
 	// container
@@ -320,7 +318,7 @@ func finalizeNamespace(config *initConfig, addHome bool) error {
 	if err := system.SetKeepCaps(); err != nil {
 		return fmt.Errorf("unable to set keep caps: %w", err)
 	}
-	if err := setupUser(config, addHome); err != nil {
+	if err := setupUser(config); err != nil {
 		return fmt.Errorf("unable to setup user: %w", err)
 	}
 	// Change working directory AFTER the user has been set up, if we haven't done it yet.
@@ -438,52 +436,11 @@ func syncParentSeccomp(pipe *syncSocket, seccompFd int) error {
 	return readSync(pipe, procSeccompDone)
 }
 
-// setupUser changes the groups, gid, and uid for the user inside the container,
-// and appends user's HOME to config.Env if addHome is true.
-func setupUser(config *initConfig, addHome bool) error {
-	// Set up defaults.
-	defaultExecUser := user.ExecUser{
-		Uid:  0,
-		Gid:  0,
-		Home: "/",
-	}
-
-	passwdPath, err := user.GetPasswdPath()
-	if err != nil {
-		return err
-	}
-
-	groupPath, err := user.GetGroupPath()
-	if err != nil {
-		return err
-	}
-
-	execUser, err := user.GetExecUserPath(config.User, &defaultExecUser, passwdPath, groupPath)
-	if err != nil {
-		return err
-	}
-
-	var addGroups []int
-	if len(config.AdditionalGroups) > 0 {
-		addGroups, err = user.GetAdditionalGroupsPath(config.AdditionalGroups, groupPath)
-		if err != nil {
-			return err
-		}
-	}
-
-	if config.RootlessEUID {
-		// We cannot set any additional groups in a rootless container and thus
-		// we bail if the user asked us to do so. TODO: We currently can't do
-		// this check earlier, but if libcontainer.Process.User was typesafe
-		// this might work.
-		if len(addGroups) > 0 {
-			return errors.New("cannot set any additional groups in a rootless container")
-		}
-	}
-
+// setupUser changes the groups, gid, and uid for the user inside the container.
+func setupUser(config *initConfig) error {
 	// Before we change to the container's user make sure that the processes
 	// STDIO is correctly owned by the user that we are switching to.
-	if err := fixStdioPermissions(execUser); err != nil {
+	if err := fixStdioPermissions(config.UID); err != nil {
 		return err
 	}
 
@@ -502,36 +459,30 @@ func setupUser(config *initConfig, addHome bool) error {
 	allowSupGroups := !config.RootlessEUID && string(bytes.TrimSpace(setgroups)) != "deny"
 
 	if allowSupGroups {
-		suppGroups := append(execUser.Sgids, addGroups...)
-		if err := unix.Setgroups(suppGroups); err != nil {
+		if err := unix.Setgroups(config.AdditionalGroups); err != nil {
 			return &os.SyscallError{Syscall: "setgroups", Err: err}
 		}
 	}
 
-	if err := unix.Setgid(execUser.Gid); err != nil {
+	if err := unix.Setgid(config.GID); err != nil {
 		if err == unix.EINVAL {
-			return fmt.Errorf("cannot setgid to unmapped gid %d in user namespace", execUser.Gid)
+			return fmt.Errorf("cannot setgid to unmapped gid %d in user namespace", config.GID)
 		}
 		return err
 	}
-	if err := unix.Setuid(execUser.Uid); err != nil {
+	if err := unix.Setuid(config.UID); err != nil {
 		if err == unix.EINVAL {
-			return fmt.Errorf("cannot setuid to unmapped uid %d in user namespace", execUser.Uid)
+			return fmt.Errorf("cannot setuid to unmapped uid %d in user namespace", config.UID)
 		}
 		return err
 	}
-
-	// If we didn't get HOME already, set it based on the user's HOME.
-	if addHome {
-		config.Env = append(config.Env, "HOME="+execUser.Home)
-	}
 	return nil
 }
 
-// fixStdioPermissions fixes the permissions of PID 1's STDIO within the container to the specified user.
+// fixStdioPermissions fixes the permissions of PID 1's STDIO within the container to the specified uid.
 // The ownership needs to match because it is created outside of the container and needs to be
 // localized.
-func fixStdioPermissions(u *user.ExecUser) error {
+func fixStdioPermissions(uid int) error {
 	var null unix.Stat_t
 	if err := unix.Stat("/dev/null", &null); err != nil {
 		return &os.PathError{Op: "stat", Path: "/dev/null", Err: err}
@@ -544,7 +495,7 @@ func fixStdioPermissions(u *user.ExecUser) error {
 
 		// Skip chown if uid is already the one we want or any of the STDIO descriptors
 		// were redirected to /dev/null.
-		if int(s.Uid) == u.Uid || s.Rdev == null.Rdev {
+		if int(s.Uid) == uid || s.Rdev == null.Rdev {
 			continue
 		}
 
@@ -554,7 +505,7 @@ func fixStdioPermissions(u *user.ExecUser) error {
 		// that users expect to be able to actually use their console. Without
 		// this code, you couldn't effectively run as a non-root user inside a
 		// container and also have a console set up.
-		if err := file.Chown(u.Uid, int(s.Gid)); err != nil {
+		if err := file.Chown(uid, int(s.Gid)); err != nil {
 			// If we've hit an EINVAL then s.Gid isn't mapped in the user
 			// namespace. If we've hit an EPERM then the inode's current owner
 			// is not mapped in our user namespace (in particular,

libcontainer/integration/exec_test.go

@@ -399,7 +399,7 @@ func TestAdditionalGroups(t *testing.T) {
 		Env:              standardEnvironment,
 		Stdin:            nil,
 		Stdout:           &stdout,
-		AdditionalGroups: []string{"plugdev", "audio"},
+		AdditionalGroups: []int{3333, 99999},
 		Init:             true,
 	}
 	err = container.Run(&pconfig)
@@ -410,13 +410,11 @@ func TestAdditionalGroups(t *testing.T) {
 
 	outputGroups := stdout.String()
 
-	// Check that the groups output has the groups that we specified
-	if !strings.Contains(outputGroups, "audio") {
-		t.Fatalf("Listed groups do not contain the audio group as expected: %v", outputGroups)
-	}
-
-	if !strings.Contains(outputGroups, "plugdev") {
-		t.Fatalf("Listed groups do not contain the plugdev group as expected: %v", outputGroups)
+	// Check that the groups output has the groups that we specified.
+	for _, gid := range pconfig.AdditionalGroups {
+		if !strings.Contains(outputGroups, strconv.Itoa(gid)) {
+			t.Errorf("Listed groups do not contain gid %d as expected: %v", gid, outputGroups)
+		}
 	}
 }