diff --git a/exec.go b/exec.go index 982520f72b8..4474ba15dac 100644 --- a/exec.go +++ b/exec.go @@ -214,9 +214,9 @@ func getProcess(context *cli.Context, bundle string) (*specs.Process, error) { } p := spec.Process p.Args = context.Args()[1:] - // override the cwd, if passed - if context.String("cwd") != "" { - p.Cwd = context.String("cwd") + // Override the cwd, if passed. + if cwd := context.String("cwd"); cwd != "" { + p.Cwd = cwd } if ap := context.String("apparmor"); ap != "" { p.ApparmorProfile = ap @@ -235,27 +235,24 @@ func getProcess(context *cli.Context, bundle string) (*specs.Process, error) { // append the passed env variables p.Env = append(p.Env, context.StringSlice("env")...) - // set the tty - p.Terminal = false - if context.IsSet("tty") { - p.Terminal = context.Bool("tty") - } + // Always set tty to false, unless explicitly enabled from CLI. + p.Terminal = context.Bool("tty") if context.IsSet("no-new-privs") { p.NoNewPrivileges = context.Bool("no-new-privs") } - // override the user, if passed - if context.String("user") != "" { - u := strings.SplitN(context.String("user"), ":", 2) + // Override the user, if passed. + if user := context.String("user"); user != "" { + u := strings.SplitN(user, ":", 2) if len(u) > 1 { gid, err := strconv.Atoi(u[1]) if err != nil { - return nil, fmt.Errorf("parsing %s as int for gid failed: %w", u[1], err) + return nil, fmt.Errorf("bad gid: %w", err) } p.User.GID = uint32(gid) } uid, err := strconv.Atoi(u[0]) if err != nil { - return nil, fmt.Errorf("parsing %s as int for uid failed: %w", u[0], err) + return nil, fmt.Errorf("bad uid: %w", err) } p.User.UID = uint32(uid) } diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index ae5d4fb46b4..f5caf317019 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -823,7 +823,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, diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index a24be276878..f32438bdbb4 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -68,8 +68,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"` @@ -440,60 +441,42 @@ func syncParentSeccomp(pipe *os.File, seccompFd *os.File) error { return readSync(pipe, procSeccompDone) } -// setupUser changes the groups, gid, and uid for the user inside the container -func setupUser(config *initConfig) 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 +// setHome tries to guess $HOME based on a user's /etc/passwd entry. +// If $HOME is already set, it does nothing. +func setHome(uid int) error { + if _, ok := os.LookupEnv("HOME"); ok { + return nil } - - var addGroups []int - if len(config.AdditionalGroups) > 0 { - addGroups, err = user.GetAdditionalGroupsPath(config.AdditionalGroups, groupPath) - if err != nil { + if u, err := user.LookupUid(uid); err == nil { + if err := os.Setenv("HOME", u.Home); err != nil { return err } } + return nil +} +// setupUser changes the groups, gid, and uid for the user inside the container. +func setupUser(config *initConfig) error { // Rather than just erroring out later in setuid(2) and setgid(2), check // that the user is mapped here. - if _, err := config.Config.HostUID(execUser.Uid); err != nil { + if _, err := config.Config.HostUID(config.UID); err != nil { return errors.New("cannot set uid to unmapped user in user namespace") } - if _, err := config.Config.HostGID(execUser.Gid); err != nil { + if _, err := config.Config.HostGID(config.GID); err != nil { return errors.New("cannot set gid to unmapped user in user namespace") } - if config.RootlessEUID { + 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 } @@ -509,32 +492,24 @@ func setupUser(config *initConfig) 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 := system.Setgid(execUser.Gid); err != nil { + if err := system.Setgid(config.GID); err != nil { return err } - if err := system.Setuid(execUser.Uid); err != nil { + if err := system.Setuid(config.UID); err != nil { return err } - - // if we didn't get HOME already, set it based on the user's HOME - if envHome := os.Getenv("HOME"); envHome == "" { - if err := os.Setenv("HOME", execUser.Home); err != nil { - return err - } - } - return nil + return setHome(config.UID) } -// 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} @@ -547,7 +522,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 } @@ -557,7 +532,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, diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index 9795964caf2..4453f7fb7c5 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -395,7 +395,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) @@ -406,13 +406,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) + } } } diff --git a/libcontainer/integration/execin_test.go b/libcontainer/integration/execin_test.go index c5c324130c6..ef455bde6e0 100644 --- a/libcontainer/integration/execin_test.go +++ b/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) + } } } diff --git a/libcontainer/process.go b/libcontainer/process.go index d2c7bfcda36..ebb601ab499 100644 --- a/libcontainer/process.go +++ b/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 diff --git a/list.go b/list.go index 615f6271cb2..997cd88173b 100644 --- a/list.go +++ b/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 diff --git a/utils_linux.go b/utils_linux.go index 0f787cb3387..8a3ac693ff4 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -46,10 +46,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, @@ -69,8 +69,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)