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,6 +6,9 @@ import (
 	"os"
 	"slices"
 	"strings"
+
+	"github.com/moby/sys/user"
+	"github.com/sirupsen/logrus"
 )
 
 // prepareEnv processes a list of environment variables, preparing it
@@ -13,13 +16,13 @@ import (
 //   - 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,10 +34,10 @@ 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.
@@ -42,12 +45,12 @@ func prepareEnv(env []string) ([]string, bool, error) {
 		}
 		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)
@@ -55,5 +58,31 @@ func prepareEnv(env []string) ([]string, bool, error) {
 	// 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)
+		}
 	}
 }
 

libcontainer/integration/execin_test.go

@@ -162,7 +162,7 @@ func TestExecInAdditionalGroups(t *testing.T) {
 		Env:              standardEnvironment,
 		Stdin:            nil,
 		Stdout:           &stdout,
-		AdditionalGroups: []string{"plugdev", "audio"},
+		AdditionalGroups: []int{4444, 87654},
 	}
 	err = container.Run(&pconfig)
 	ok(t, err)
@@ -175,13 +175,11 @@ func TestExecInAdditionalGroups(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)
+		}
 	}
 }
 

libcontainer/process.go

@@ -26,13 +26,13 @@ type Process struct {
 	// Env specifies the environment variables for the process.
 	Env []string
 
-	// User will set the uid and gid of the executing process running inside the container
+	// UID and GID of the executing process running inside the container
 	// local to the container's user and group configuration.
-	User string
+	UID, GID int
 
 	// AdditionalGroups specifies the gids that should be added to supplementary groups
 	// in addition to those that the user belongs to.
-	AdditionalGroups []string
+	AdditionalGroups []int
 
 	// Cwd will change the processes current working directory inside the container's rootfs.
 	Cwd string

libcontainer/setns_init_linux.go

@@ -25,7 +25,6 @@ type linuxSetnsInit struct {
 	pidfdSocket   *os.File
 	config        *initConfig
 	logPipe       *os.File
-	addHome       bool
 }
 
 func (l *linuxSetnsInit) getSessionRingName() string {
@@ -102,7 +101,7 @@ func (l *linuxSetnsInit) Init() error {
 			return err
 		}
 	}
-	if err := finalizeNamespace(l.config, l.addHome); err != nil {
+	if err := finalizeNamespace(l.config); err != nil {
 		return err
 	}
 	if err := apparmor.ApplyProfile(l.config.AppArmorProfile); err != nil {

libcontainer/standard_init_linux.go

@@ -27,7 +27,6 @@ type linuxStandardInit struct {
 	fifoFile      *os.File
 	logPipe       *os.File
 	config        *initConfig
-	addHome       bool
 }
 
 func (l *linuxStandardInit) getSessionRingParams() (string, uint32, uint32) {
@@ -187,7 +186,7 @@ func (l *linuxStandardInit) Init() error {
 			return err
 		}
 	}
-	if err := finalizeNamespace(l.config, l.addHome); err != nil {
+	if err := finalizeNamespace(l.config); err != nil {
 		return err
 	}
 	// finalizeNamespace can change user/group which clears the parent death

list.go

@@ -5,11 +5,12 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"os/user"
+	"strconv"
 	"syscall"
 	"text/tabwriter"
 	"time"
 
-	"github.com/moby/sys/user"
 	"github.com/opencontainers/runc/libcontainer"
 	"github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/urfave/cli"
@@ -136,9 +137,10 @@ func getContainers(context *cli.Context) ([]containerState, error) {
 		}
 		// This cast is safe on Linux.
 		uid := st.Sys().(*syscall.Stat_t).Uid
-		owner, err := user.LookupUid(int(uid))
-		if err != nil {
-			owner.Name = fmt.Sprintf("#%d", uid)
+		owner := "#" + strconv.Itoa(int(uid))
+		u, err := user.LookupId(owner[1:])
+		if err == nil {
+			owner = u.Username
 		}
 
 		container, err := libcontainer.Load(root, item.Name())
@@ -170,7 +172,7 @@ func getContainers(context *cli.Context) ([]containerState, error) {
 			Rootfs:         state.BaseState.Config.Rootfs,
 			Created:        state.BaseState.Created,
 			Annotations:    annotations,
-			Owner:          owner.Name,
+			Owner:          owner,
 		})
 	}
 	return s, nil

tests/integration/exec.bats

@@ -126,6 +126,18 @@ function teardown() {
 	runc exec --user 1000:1000 test_busybox id
 	[ "$status" -eq 0 ]
 	[[ "${output}" == "uid=1000 gid=1000"* ]]
+
+	# Check the default value of HOME ("/") is set even in case when
+	#  - HOME is not set in process.Env, and
+	#  - there is no entry in container's /etc/passwd for a given UID.
+	#
+	# NOTE this is not a standard runtime feature, but rather
+	# a historical de facto behavior we're afraid to change.
+
+	# shellcheck disable=SC2016
+	runc exec --user 1000 test_busybox sh -u -c 'echo $HOME'
+	[ "$status" -eq 0 ]
+	[[ "$output" = "/" ]]
 }
 
 # https://github.com/opencontainers/runc/issues/3674.

utils_linux.go

@@ -47,10 +47,10 @@ func getDefaultImagePath() string {
 // spec and stdio from the current process.
 func newProcess(p specs.Process) (*libcontainer.Process, error) {
 	lp := &libcontainer.Process{
-		Args: p.Args,
-		Env:  p.Env,
-		// TODO: fix libcontainer's API to better support uid/gid in a typesafe way.
-		User:            fmt.Sprintf("%d:%d", p.User.UID, p.User.GID),
+		Args:            p.Args,
+		Env:             p.Env,
+		UID:             int(p.User.UID),
+		GID:             int(p.User.GID),
 		Cwd:             p.Cwd,
 		Label:           p.SelinuxLabel,
 		NoNewPrivileges: &p.NoNewPrivileges,
@@ -72,8 +72,11 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) {
 		lp.Capabilities.Permitted = p.Capabilities.Permitted
 		lp.Capabilities.Ambient = p.Capabilities.Ambient
 	}
-	for _, gid := range p.User.AdditionalGids {
-		lp.AdditionalGroups = append(lp.AdditionalGroups, strconv.FormatUint(uint64(gid), 10))
+	if l := len(p.User.AdditionalGids); l > 0 {
+		lp.AdditionalGroups = make([]int, l)
+		for i, g := range p.User.AdditionalGids {
+			lp.AdditionalGroups[i] = int(g)
+		}
 	}
 	for _, rlimit := range p.Rlimits {
 		rl, err := createLibContainerRlimit(rlimit)