libct: switch to numeric UID/GID/groups

This addresses the following TODO in the code (added back in 2015
by commit 845fc65e5):

> // TODO: fix libcontainer's API to better support uid/gid in a typesafe way.

Historically, libcontainer internally uses strings for user, group, and
additional (aka supplementary) groups.

Yet, runc receives those credentials as part of runtime-spec's process,
which uses integers for all of them (see [1], [2]).

What happens next is:

1. runc start/run/exec converts those credentials to strings (a User
   string containing "UID:GID", and a []string for additional GIDs) and
   passes those onto runc init.
2. runc init converts them back to int, in the most complicated way
   possible (parsing container's /etc/passwd and /etc/group).

All this conversion and, especially, parsing is totally unnecessary,
but is performed on every container exec (and start).

The only benefit of all this is, a libcontainer user could use user and
group names instead of numeric IDs (but runc itself is not using this
feature, and we don't know if there are any other users of this).

Let's remove this back and forth translation, hopefully increasing
runc exec performance.

The only remaining need to parse /etc/passwd is to set HOME environment
variable for a specified UID, in case $HOME is not explicitly set in
process.Env. This can now be done right in prepareEnv, which simplifies
the code flow a lot. Alas, we can not use standard os/user.LookupId, as
it could cache host's /etc/passwd or the current user (even with the
osusergo tag).

PS Note that the structures being changed (initConfig and Process) are
never saved to disk as JSON by runc, so there is no compatibility issue
for runc users.

Still, this is a breaking change in libcontainer, but we never promised
that libcontainer API will be stable (and there's a special package
that can handle it -- github.com/moby/sys/user). Reflect this in
CHANGELOG.

For 3998.

[1]: https://github.com/opencontainers/runtime-spec/blob/v1.0.2/config.md#posix-platform-user
[2]: https://github.com/opencontainers/runtime-spec/blob/v1.0.2/specs-go/config.go#L86

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

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

@@ -686,7 +686,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,19 @@ 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 {
+// setupUser changes the groups, gid, and uid for the user inside the container.
+func setupUser(config *initConfig) error {
+	if config.RootlessEUID && len(config.AdditionalGroups) > 0 {
 		// 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")
-		}
+		return errors.New("cannot set any additional groups in a rootless container")
 	}
 
 	// 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 +467,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 +503,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 +513,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

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)