From b41c864d569357a102ee2335a4947e59e5e2b08a Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Thu, 27 Feb 2020 16:08:29 -0500 Subject: Ensure that exec sessions inherit supplemental groups This corrects a regression from Podman 1.4.x where container exec sessions inherited supplemental groups from the container, iff the exec session did not specify a user. Signed-off-by: Matthew Heon --- libpod/container_api.go | 5 ----- libpod/container_internal_linux.go | 5 ++++- libpod/oci_conmon_linux.go | 25 +++++++++++++++++++++---- 3 files changed, 25 insertions(+), 10 deletions(-) (limited to 'libpod') diff --git a/libpod/container_api.go b/libpod/container_api.go index d612341bc..dabbe27dc 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -270,11 +270,6 @@ func (c *Container) Exec(tty, privileged bool, env map[string]string, cmd []stri } }() - // if the user is empty, we should inherit the user that the container is currently running with - if user == "" { - user = c.config.User - } - opts := new(ExecOptions) opts.Cmd = cmd opts.CapAdd = capList diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 739026264..63968918c 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -330,7 +330,10 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { // Add addition groups if c.config.GroupAdd is not empty if len(c.config.Groups) > 0 { - gids, _ := lookup.GetContainerGroups(c.config.Groups, c.state.Mountpoint, nil) + gids, err := lookup.GetContainerGroups(c.config.Groups, c.state.Mountpoint, overrides) + if err != nil { + return nil, errors.Wrapf(err, "error looking up supplemental groups for container %s", c.ID()) + } for _, gid := range gids { g.AddProcessAdditionalGid(gid) } diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go index 07d38693f..800f89603 100644 --- a/libpod/oci_conmon_linux.go +++ b/libpod/oci_conmon_linux.go @@ -1252,18 +1252,35 @@ func prepareProcessExec(c *Container, cmd, env []string, tty bool, cwd, user, se } + var addGroups []string + var sgids []uint32 + + // if the user is empty, we should inherit the user that the container is currently running with + if user == "" { + user = c.config.User + addGroups = c.config.Groups + } + overrides := c.getUserOverrides() execUser, err := lookup.GetUserGroupInfo(c.state.Mountpoint, user, overrides) if err != nil { return nil, err } + if len(addGroups) > 0 { + sgids, err = lookup.GetContainerGroups(addGroups, c.state.Mountpoint, overrides) + if err != nil { + return nil, errors.Wrapf(err, "error looking up supplemental groups for container %s exec session %s", c.ID(), sessionID) + } + } + // If user was set, look it up in the container to get a UID to use on // the host - if user != "" { - sgids := make([]uint32, 0, len(execUser.Sgids)) - for _, sgid := range execUser.Sgids { - sgids = append(sgids, uint32(sgid)) + if user != "" || len(sgids) > 0 { + if user != "" { + for _, sgid := range execUser.Sgids { + sgids = append(sgids, uint32(sgid)) + } } processUser := spec.User{ UID: uint32(execUser.Uid), -- cgit v1.2.3-54-g00ecf