Merge pull request #25625 from giuseppe/set-additional-gids-exec

libpod: fix handling of additional gids in exec

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 {
+	// The additional groups must always contain the user's primary group.
-		sgids, err = lookup.GetContainerGroups(addGroups, c.state.Mountpoint, overrides)
+	sgids := []uint32{uint32(execUser.Gid)}
-		if err != nil {
-			return nil, fmt.Errorf("looking up supplemental groups for container %s exec session %s: %w", c.ID(), sessionID, err)
-		}
-	}
 
-	// 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))
 	}
+
+	// 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...)
+	}
+
+	// Avoid duplicates
+	slices.Sort(sgids)
+	sgids = slices.Compact(sgids)
+
 	processUser := spec.User{
 		UID:            uint32(execUser.Uid),
 		GID:            uint32(execUser.Gid),
 		AdditionalGids: sgids,
 	}
-
 	pspec.User = processUser
-	}
 
 	if c.config.Umask != "" {
 		umask, err := c.umask()

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