Fix setuid iptables DOCKER-USER rules on legacy backends - #3353
Open
jbemmel wants to merge 2 commits into
Open
Conversation
iptables ≥ 1.8.8 exits 111 when ruid != euid; run those CLIs with matching root UIDs in the child and strip LD_* so setuid clab_admins hosts can install/remove mgmt external-access rules. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
This can probably be simplified to not pass any env variables at all |
jbemmel
marked this pull request as draft
August 24, 2026 00:57
Use an empty child environment instead of filtering LD_*, and drop the separate exec.go so the iptables spawn path stays in one file. Co-authored-by: Cursor <cursoragent@cursor.com>
jbemmel
marked this pull request as ready for review
August 24, 2026 01:00
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is broken
Containerlab can be installed setuid-root for clab_admins. In that mode the process keeps the invoking user’s real UID (ruid) and elevates only the effective UID (euid=0). Privilege work runs as root; later file ownership under $HOME still follows the real user.
For mgmt external-access, if the nftables path isn’t usable (e.g. Docker on iptables-legacy, DOCKER-USER only in legacy iptables), containerlab shells out to iptables / ip6tables to add/remove DOCKER-USER ACCEPT rules for the mgmt bridge.
From iptables 1.8.8, libxtables refuses to run when getuid() != geteuid(). It loads match/target plugins via dlopen and trusts env like LD_*, so running under a setuid parent is treated as unsafe. It exits 111 with little/no message. Containerlab logged iptables check error: exit status 111 and skipped the rules — so external access failed for setuid users. sudo containerlab worked because both UIDs are 0.
What this PR changes
Only the iptables CLI spawn path:
Matching root UIDs in the child — via SysProcAttr.Credential so the child gets Uid: 0 (Linux setuid(0) as euid-root sets real/effective/saved). Parent keeps ruid=user.
Strip LD_* from the child’s env so loader overrides don’t ride along once UIDs match.
Nftables is untouched