Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 10 additions & 13 deletions exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
79 changes: 27 additions & 52 deletions libcontainer/init_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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
}

Expand All @@ -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}
Expand All @@ -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
}

Expand All @@ -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,
Expand Down
14 changes: 6 additions & 8 deletions libcontainer/integration/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}
}

Expand Down
14 changes: 6 additions & 8 deletions libcontainer/integration/execin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions libcontainer/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 7 additions & 5 deletions list.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
Expand Down
15 changes: 9 additions & 6 deletions utils_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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)
Expand Down