summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatthew Heon <matthew.heon@pm.me>2020-02-27 16:08:29 -0500
committerMatthew Heon <matthew.heon@pm.me>2020-02-28 11:32:56 -0500
commitb41c864d569357a102ee2335a4947e59e5e2b08a (patch)
tree8be00fe617420b47ec2960f540890baaa92c32ca
parent25d29f959a16a881066727481cbbc0bd2d18c784 (diff)
downloadpodman-b41c864d569357a102ee2335a4947e59e5e2b08a.tar.gz
podman-b41c864d569357a102ee2335a4947e59e5e2b08a.tar.bz2
podman-b41c864d569357a102ee2335a4947e59e5e2b08a.zip
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 <matthew.heon@pm.me>
-rw-r--r--libpod/container_api.go5
-rw-r--r--libpod/container_internal_linux.go5
-rw-r--r--libpod/oci_conmon_linux.go25
-rw-r--r--test/e2e/exec_test.go24
4 files changed, 49 insertions, 10 deletions
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),
diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go
index ed4eb3335..ab806f683 100644
--- a/test/e2e/exec_test.go
+++ b/test/e2e/exec_test.go
@@ -1,6 +1,7 @@
package integration
import (
+ "fmt"
"os"
"strings"
@@ -244,4 +245,27 @@ var _ = Describe("Podman exec", func() {
Expect(session.ExitCode()).To(Equal(0))
})
+ It("podman exec preserves --group-add groups", func() {
+ groupName := "group1"
+ gid := "4444"
+ ctrName1 := "ctr1"
+ ctr1 := podmanTest.Podman([]string{"run", "-ti", "--name", ctrName1, fedoraMinimal, "groupadd", "-g", gid, groupName})
+ ctr1.WaitWithDefaultTimeout()
+ Expect(ctr1.ExitCode()).To(Equal(0))
+
+ imgName := "img1"
+ commit := podmanTest.Podman([]string{"commit", ctrName1, imgName})
+ commit.WaitWithDefaultTimeout()
+ Expect(commit.ExitCode()).To(Equal(0))
+
+ ctrName2 := "ctr2"
+ ctr2 := podmanTest.Podman([]string{"run", "-d", "--name", ctrName2, "--group-add", groupName, imgName, "sleep", "300"})
+ ctr2.WaitWithDefaultTimeout()
+ Expect(ctr2.ExitCode()).To(Equal(0))
+
+ exec := podmanTest.Podman([]string{"exec", "-ti", ctrName2, "id"})
+ exec.WaitWithDefaultTimeout()
+ Expect(exec.ExitCode()).To(Equal(0))
+ Expect(strings.Contains(exec.OutputToString(), fmt.Sprintf("%s(%s)", gid, groupName))).To(BeTrue())
+ })
})