libpod: fix handling of additional gids in exec

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 <gscrivan@redhat.com>

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)
-		if err != nil {
-			return nil, fmt.Errorf("looking up supplemental groups for container %s exec session %s: %w", c.ID(), sessionID, err)
-		}
-	}
+	// The additional groups must always contain the user's primary group.
+	sgids := []uint32{uint32(execUser.Gid)}
 
-	// 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