Skip to content

user: fail closed when supplementary groups cannot be set - #288

Open
carrerasdarren-cell wants to merge 1 commit into
google:masterfrom
carrerasdarren-cell:fix/fail-closed-setgroups
Open

user: fail closed when supplementary groups cannot be set#288
carrerasdarren-cell wants to merge 1 commit into
google:masterfrom
carrerasdarren-cell:fix/fail-closed-setgroups

Conversation

@carrerasdarren-cell

@carrerasdarren-cell carrerasdarren-cell commented Jul 21, 2026

Copy link
Copy Markdown

Summary

  • fail sandbox setup when supplementary groups cannot be configured
  • preserve the expected EPERM path only when the parent deliberately denied setgroups(2) and no supplementary groups remain
  • add regressions for unexpected failure, inherited groups, and the clean expected-EPERM case

Security rationale

setresgid() and setresuid() do not clear supplementary groups. If setgroups(2) fails and nsjail continues, a jailed command can retain group-based access held by its launcher.

The original change made unexpected setgroups(2) failures fatal, while preserving the EPERM that follows an intentional deny write to /proc/<pid>/setgroups in an unprivileged user namespace. Further testing found that this expected-error path also needs to verify the resulting credentials: CLONE_NEWUSER inherits supplementary groups, and deny prevents the child from clearing them.

With nsjail launched as UID/GID 1000 and inherited supplementary GID 1234, the current PR baseline wrote deny, received EPERM, and still executed the jailed command. /proc/self/status confirmed one supplementary group remained:

Groups: 65534

The namespace renders the unmapped GID as the overflow value, but the credential still carries the host group membership. This can preserve access to group-readable host files exposed to the jail.

The updated patch shares the exact parent-side setgroups-deny predicate with the child. Every unexpected failure stops before execve; after the expected EPERM, it calls getgroups(0, nullptr) and proceeds only if the supplementary-group count is zero.

Verification

  • native ARM64 Linux Docker build with the project's -Werror flags
  • current PR baseline with inherited GID 1234: exited 0 and retained one supplementary group
  • patched build with the same launch: exited 255 before execve
  • patched build with no inherited supplementary groups: exited 0
  • strace -e inject=setgroups:error=EPERM without the intentional deny path: exited 255
  • all three new Makefile regression cases passed
  • basic true/false and local seccomp configuration tests passed
  • git diff --check

The broader integration target then reached its existing pasta networking test and stopped because the minimal test container did not include the optional pasta executable.

An expected EPERM after denying setgroups in a new user namespace can still leave inherited supplementary groups attached to the child. Query the remaining group count and abort before exec unless the credential state is actually clean.

Add regression coverage for both an inherited-group failure and a clean no-groups launch.
@carrerasdarren-cell
carrerasdarren-cell force-pushed the fix/fail-closed-setgroups branch from baf7f7f to 93c3cad Compare July 24, 2026 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant