From 51ca839c1478943232fe4cc6daec072ff79e2ae5 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Wed, 19 Mar 2025 11:14:36 +0100 Subject: [PATCH] libpod: fix handling of additional gids in exec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit change the behavior to match what Docker does. Docker always adds the specified additional gids, no matter the user specified to exec. Instead the additional gids read from the /etc/group file are added only when there is not an explicit group specified in the exec userspec. ➜ docker run -d --name container-with-groups --group-add mail --group-add news --group-add cron --group-add ftp --rm alpine top c4190928097f64cabb83af7cac6ec10041a9e74de359433dfd3e5b9d8a7dce1a ➜ docker exec container-with-groups id -G 0 1 2 3 4 6 10 11 12 13 16 20 21 26 27 ➜ docker exec --user root container-with-groups id -G 0 1 2 3 4 6 10 11 12 13 16 20 21 26 27 ➜ docker exec --user nobody container-with-groups id -G 65534 12 13 16 21 ➜ docker exec --user nobody:nobody container-with-groups id -G 65534 12 13 16 21 ➜ docker exec --user root:root container-with-groups id -G 0 12 13 16 21 ➜ docker exec --user root:root container-with-groups id -G 0 12 13 16 21 Closes: https://github.com/containers/podman/issues/25610 Signed-off-by: Giuseppe Scrivano --- libpod/oci_conmon_exec_common.go | 40 +++++++++++------------ test/system/075-exec.bats | 56 ++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 20 deletions(-) diff --git a/libpod/oci_conmon_exec_common.go b/libpod/oci_conmon_exec_common.go index ca917ff1a3..a65e22fd79 100644 --- a/libpod/oci_conmon_exec_common.go +++ b/libpod/oci_conmon_exec_common.go @@ -9,6 +9,7 @@ import ( "os" "os/exec" "path/filepath" + "slices" "strconv" "strings" "syscall" @@ -703,15 +704,11 @@ func (c *Container) prepareProcessExec(options *ExecOptions, env []string, sessi pspec.Cwd = options.Cwd } - var addGroups []string - var sgids []uint32 - // if the user is empty, we should inherit the user that the container is currently running with user := options.User if user == "" { logrus.Debugf("Set user to %s", c.config.User) user = c.config.User - addGroups = c.config.Groups } overrides := c.getUserOverrides() @@ -720,29 +717,32 @@ func (c *Container) prepareProcessExec(options *ExecOptions, env []string, sessi return nil, err } - if len(addGroups) > 0 { - sgids, err = lookup.GetContainerGroups(addGroups, c.state.Mountpoint, overrides) + // The additional groups must always contain the user's primary group. + sgids := []uint32{uint32(execUser.Gid)} + + for _, sgid := range execUser.Sgids { + sgids = append(sgids, uint32(sgid)) + } + + // Always add the groups added through --group-add, no matter the exec UID:GID. + if len(c.config.Groups) > 0 { + additionalSgids, err := lookup.GetContainerGroups(c.config.Groups, c.state.Mountpoint, overrides) if err != nil { return nil, fmt.Errorf("looking up supplemental groups for container %s exec session %s: %w", c.ID(), sessionID, err) } + sgids = append(sgids, additionalSgids...) } - // If user was set, look it up in the container to get a UID to use on - // the host - if user != "" || len(sgids) > 0 { - if user != "" { - for _, sgid := range execUser.Sgids { - sgids = append(sgids, uint32(sgid)) - } - } - processUser := spec.User{ - UID: uint32(execUser.Uid), - GID: uint32(execUser.Gid), - AdditionalGids: sgids, - } + // Avoid duplicates + slices.Sort(sgids) + sgids = slices.Compact(sgids) - pspec.User = processUser + processUser := spec.User{ + UID: uint32(execUser.Uid), + GID: uint32(execUser.Gid), + AdditionalGids: sgids, } + pspec.User = processUser if c.config.Umask != "" { umask, err := c.umask() diff --git a/test/system/075-exec.bats b/test/system/075-exec.bats index 9faa6a7e2b..16ef5c63cc 100644 --- a/test/system/075-exec.bats +++ b/test/system/075-exec.bats @@ -253,4 +253,60 @@ load helpers run_podman rm -f -t0 $cid } +# bats test_tags=ci:parallel +@test "podman exec - additional groups" { + run_podman run -d $IMAGE top + cid="$output" + + run_podman exec $cid id -g nobody + nobody_id="$output" + + run_podman exec $cid grep -h ^Groups: /proc/1/status /proc/self/status + assert "${lines[0]}" = "${lines[1]}" "must have the same additional groups" + + run_podman exec --user root $cid grep -h ^Groups: /proc/1/status /proc/self/status + assert "${lines[0]}" = "${lines[1]}" "must have the same additional groups" + + run_podman exec --user root:root $cid id -G + assert "${output}" = "0" "must have only 0 gid" + + run_podman exec --user nobody $cid id -G + assert "${output}" = "${nobody_id}" "must have only nobody gid" + + run_podman exec --user nobody:nobody $cid id -G + assert "${output}" = "${nobody_id}" "must have only nobody gid" + + run_podman rm -f -t0 $cid + + # Now test with --group-add + + run_podman run --group-add 1,2,3,4,5,6,7,8,9,10 -d $IMAGE top + cid="$output" + + run_podman exec $cid grep -h ^Groups: /proc/1/status /proc/self/status + assert "${lines[0]}" = "${lines[1]}" "must have the same additional groups" + + run_podman exec --user 0 $cid grep -h ^Groups: /proc/1/status /proc/self/status + assert "${lines[0]}" = "${lines[1]}" "must have the same additional groups" + + run_podman exec --user root $cid grep -h ^Groups: /proc/1/status /proc/self/status + assert "${lines[0]}" = "${lines[1]}" "must have the same additional groups" + + run_podman exec --user root:root $cid id -G + assert "$output" = "0 1 2 3 4 5 6 7 8 9 10" "must have only the explicit groups added and 0" + + run_podman exec --user 0:0 $cid id -G + assert "$output" = "0 1 2 3 4 5 6 7 8 9 10" "must have only the explicit groups added and 0" + + run_podman exec --user nobody $cid id -G + assert "$output" = "$nobody_id 1 2 3 4 5 6 7 8 9 10" "must have only the explicit groups added and nobody" + + run_podman exec --user nobody:nobody $cid id -G + assert "$output" = "$nobody_id 1 2 3 4 5 6 7 8 9 10" "must have only the explicit groups added and nobody" + + run_podman exec --user root:nobody $cid id -G + assert "$output" = "$nobody_id 1 2 3 4 5 6 7 8 9 10" "must have only the explicit groups added and 0" + + run_podman rm -f -t0 $cid +} # vim: filetype=sh