chroot: skip setgroups when namespace denies it by ajoshua2004 · Pull Request #6961 · podman-container-tools/buildah

ajoshua2004 · 2026-07-13T16:58:19Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

In a single-ID user namespace whose /proc/self/setgroups is "deny", any setgroups() call returns EPERM, so every RUN step under --isolation chroot fails. The kernel has permanently frozen the supplementary group set in that case, so the call can neither succeed nor be needed, and is skipped. The read is split into Linux/BSD helpers since /proc/self/setgroups does not exist on FreeBSD.

This keeps the primary-GID force-add in configureUIDGID rather than removing it, leaving the protection intact

Adds a regression test that a RUN step succeeds in a setgroups-denied namespace, and a test that the skip does not widen access: a probe file owned by a group the build namespace cannot map (surfacing as overflow gid 65534, present in the process's groups list) is still denied, while a legitimately-held group is readable.

This builds on the patch michelemodolo proposed in #6947, extended to gate on the setgroups-denied state, split across the Linux/BSD paths, with regression and no leak tests added.

How to verify it

Two new Linux only tests in tests/chroot.bats
bats ./tests/chroot.bats

"chroot RUN succeeds when setgroups is denied" — builds with a RUN step
inside a setgroups-denied namespace (unshare --user --map-root-user) and
asserts it reaches the RUN step instead of failing with EPERM.
"chroot RUN does not honor unmapped supplemental group when setgroups is
denied" — a probe file owned by a group the build's single-ID namespace
can't map surfaces inside as overflow gid 65534 and appears in the process's
groups list, yet the read is still denied, while a legitimately-held group
reads fine. Confirms skipping setgroups() doesn't widen access.

Which issue(s) this PR fixes:

Fixes #6947

Special notes for your reviewer:

Chose to gate the setgroups() call on the deny state rather than dropping the
primary-GID force-add in configureUIDGID (the issue's alternative), since that
force-add is the fix for rhbz#2121453 and removing it risks reopening it.

Does this PR introduce a user-facing change?

Fixed chroot isolation RUN steps failing with "error setting supplemental groups list: operation not permitted" in user namespaces where setgroups is denied.

nalind · 2026-07-14T18:13:49Z

+    -v $probe:/probe:ro,z "$ctr" -- \
+    sh -c 'cat /proc/self/status | grep ^Groups; cat /probe/nonmember_allowed && ! cat /probe/member_denied'
+  expect_output --substring "ok_nonmember"
+}


This doesn't appear to be verifying that the "setgroups" privilege is being denied to buildah run.

nalind · 2026-07-14T18:37:47Z

+  " || skip "user namespace could not map the gid for the leak probe"
+
+  echo "FROM $baseimage" > $context/Dockerfile
+  echo 'RUN cat /proc/self/status | grep Groups; cat /probe/allowed_leak && ! cat /probe/denied_leak' >> $context/Dockerfile


Don't need to pipe output from cat through grep when grep can read things directly.
Not sure what we're supposed to do with the displayed Groups list - is the test expecting one or more values to be there, or not in there?

nalind · 2026-07-14T18:38:22Z

+  ctr="$output"
+  run_buildah run --isolation chroot ${storageopts} --user 1000:${okgid} \
+    -v $probe:/probe:ro,z "$ctr" -- \
+    sh -c 'cat /proc/self/status | grep ^Groups; cat /probe/nonmember_allowed && ! cat /probe/member_denied'


Don't need to pipe output from cat through grep when grep can read things directly.
Not sure what we're supposed to do with the displayed Groups list - is the test expecting one or more values to be there, or not in there?

nalind

A couple of nits, and I don't think I understand what a couple of the tests are looking for when examining the supplemental groups list inside of a user namespace that doesn't have the subordinate IDs mapped into it.

nalind · 2026-07-14T21:25:12Z

+  # build's single-ID namespace (maps only gid 0) will NOT map -- so inside the
+  # build it surfaces as overflow (65534): a supplemental group the process
+  # appears to hold but that must not count for access.
+  leakgid=4242


Nit: this should check that ${leakgid} will be within the range of GID mappings being set up below.

nalind · 2026-07-14T21:37:18Z

+  # chgrp inside a namespace that maps our real subgid range, so no sudo needed.
+  # allowed_leak -> gid 0 here (our primary); denied_leak -> the leak gid that
+  # the build won't map.
+  unshare --user --map-root-user --map-groups=${substart},0,${subsize} --mount sh -c "


If unshare is actually mapping the first ID from that range to 0 as the --map-groups flag is requesting, then these two files are going to be owned by GIDs in the user's subordinate gid range, neither of which I think is expected to be mapped into the namespace in which the script is run below, so I would expect attempts to read from the two files to either both succeed or both fail.
The --map-root-user flag is documented as being equivalent to --map-user=0 --map-group=0, so unshare is effectively being given two flags that specify that different GIDs to map to 0. What actually happens?

nalind · 2026-07-14T22:09:11Z

+  # the unmapped gid has to actually show up as overflow in the RUN process's
+  # groups list -- otherwise "denied" below would just mean it was never a
+  # member, and the test wouldn't be checking anything
+  expect_output --substring "Groups:	65534"


Which unmapped GID does this correspond to outside of the namespace that was created using unshare? I don't think I understand what its presence in this list is expected to indicate.

nalind · 2026-07-14T22:18:16Z

+  expect_output --substring "setgroups:deny"
+  # overflow gid must appear in supplemental groups -- otherwise nonmember_allowed
+  # would not be checking anything (same as the build leak test above).
+  expect_output --substring "Groups:	65534"


I don't think I understand what this check and its comment are saying. Is this supposed to correspond to ${notmine}? If not, to what?

nalind · 2026-07-14T22:24:35Z

+  # sudo needed. map uid 1 so chown 1:... is valid alongside map-root-user.
+  # chown to uid 1 so the RUN process (uid 0, without DAC override in this user
+  # namespace) is not the owner and group/other bits are actually consulted.
+  unshare --user --map-root-user --map-users=${subuidstart},1,1 --map-groups=${substart},0,${subsize} --mount sh -c "


--map-groupsing to a range starting at 0 should conflict with --map-root-user, which implies --map-group=0. What is actually happening?

nalind

LGTM with a couple of nits that wouldn't be blockers.

In a single-ID user namespace whose /proc/self/setgroups is "deny", any setgroups() call returns EPERM, so every RUN step under --isolation chroot fails. The kernel has permanently frozen the supplementary group set in that case, so the call can neither succeed nor be needed, and is skipped. The read is split into Linux/BSD helpers since /proc/self/setgroups does not exist on FreeBSD. This keeps the primary-GID force-add in configureUIDGID rather than removing it, leaving the protection intact Adds a regression test that a RUN step succeeds in a setgroups-denied namespace, and a test that the skip does not widen access: a probe file owned by a group the build namespace cannot map (surfacing as overflow gid 65534, present in the process's group list) is still denied, while a legitimately-held group is readable. This builds on the patch michelemodolo proposed in podman-container-tools#6947, extended to gate on the setgroups-denied state, split across the Linux/BSD paths, with regression and no leak tests added. Closes podman-container-tools#6947 Signed-off-by: Joshua Arrevillaga <2004jarevillaga@gmail.com>

TomSweeneyRedHat · 2026-07-23T22:40:38Z

LGTM

TomSweeneyRedHat · 2026-07-23T22:40:44Z

/lgtm

ajoshua2004 force-pushed the chroot_run branch from 0216d59 to 1b69636 Compare July 13, 2026 17:39

nalind reviewed Jul 13, 2026

View reviewed changes

ajoshua2004 force-pushed the chroot_run branch from 1b69636 to 7086e48 Compare July 14, 2026 14:32

nalind reviewed Jul 14, 2026

View reviewed changes

ajoshua2004 force-pushed the chroot_run branch from 7086e48 to f8c629f Compare July 14, 2026 21:02

nalind reviewed Jul 14, 2026

View reviewed changes

ajoshua2004 force-pushed the chroot_run branch from f8c629f to 05d4a18 Compare July 15, 2026 21:20

nalind approved these changes Jul 21, 2026

View reviewed changes

Comment thread chroot/run_common.go

Comment thread tests/chroot.bats

ajoshua2004 force-pushed the chroot_run branch from 05d4a18 to a5eb306 Compare July 22, 2026 17:21

TomSweeneyRedHat merged commit 18bf8e3 into podman-container-tools:main Jul 23, 2026
54 of 58 checks passed

locker95 mentioned this pull request Jul 25, 2026

docs: fix pull --policy value from newer to ifnewer #6994

Closed

3 tasks

Uh oh!

Conversation

ajoshua2004 commented Jul 13, 2026

What type of PR is this?

What this PR does / why we need it:

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nalind left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nalind left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

TomSweeneyRedHat commented Jul 23, 2026

Uh oh!

TomSweeneyRedHat commented Jul 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants