Lix has broken seccomp rules blocking creating setuid files in the builder #207

Open
opened 2024-04-02 05:15:35 +00:00 by jade · 2 comments
Owner

These rules caused this issue: https://github.com/NixOS/nixpkgs/issues/300635

Amusingly, this issue is caused by the policy being so broken that it kept getting bypassed by accident until it accidentally got properly hit. The rules block chmod, fchmod, and fchmodat from creating files with setuid mode. However, it blatantly misses, at least open(2), openat(2), fchmodat2(2), and is thus at best useless.

Its purported purpose per 6cc6c15a2d50d0021d7242e9806ed6d54538de17:

This prevents builders from setting the S_ISUID or S_ISGID bits,
preventing users from using a nixbld* user to create a setuid/setgid
binary to interfere with subsequent builds under the same nixbld* uid.

As far as I can tell, there is zero security purpose to stopping setuid files being created from within the builder, at least assuming that they are not visible from outside, unless perhaps for build-users-group = while running as root (an insecure configuration that should not be used). The permissions will be stripped when the daemon finishes the build and puts it in its proper place.

(normal users can make setuid files on Linux and let other users become them)

It does smell like this region of the code is likely to be susceptible to actual security bugs and really needs an audit and some serious cleanup though...

Proposed and hopefully more reasonable fix: bind mount the nix store nosuid and delete the seccomp rule. Though this would still potentially be susceptible to other sandbox paths leaking setuid binaries. But erm. the rules simply do not work as it is today.

These rules caused this issue: https://github.com/NixOS/nixpkgs/issues/300635 Amusingly, this issue is caused by the policy being *so broken* that it kept getting bypassed by accident until it accidentally got properly hit. The rules block chmod, fchmod, and fchmodat from creating files with setuid mode. However, it blatantly misses, *at least* open(2), openat(2), fchmodat2(2), and is thus at best useless. Its purported purpose per 6cc6c15a2d50d0021d7242e9806ed6d54538de17: > This prevents builders from setting the S_ISUID or S_ISGID bits, > preventing users from using a nixbld* user to create a setuid/setgid > binary to interfere with subsequent builds under the same nixbld* uid. As far as I can tell, there is zero security purpose to stopping setuid files being created from within the builder, *at least assuming that they are not visible from outside*, unless perhaps for `build-users-group =` while running as root (an insecure configuration that should not be used). The permissions will be stripped when the daemon finishes the build and puts it in its proper place. (normal users can make setuid files on Linux and let other users become them) It does smell like this region of the code is likely to be susceptible to *actual* security bugs and really needs an audit and some serious cleanup though... Proposed and hopefully more reasonable fix: bind mount the nix store nosuid and delete the seccomp rule. Though this would still potentially be susceptible to other sandbox paths leaking setuid binaries. But erm. the rules simply do not work as it is today.
jade added the
stability
bug
labels 2024-04-02 05:15:35 +00:00
Member

Unfortunately nosuid doesn't prevent creating setuid files, only treating them as setuid during execution.

If we actually want to prevent creating setuid files we either need to have an allow-list seccomp rule or create a BPF LSM. Allow-list seccomp is kind of a pain because of differences in syscall support between architectures, and BPF LSM is a pain because it uses BPF and it's an LSM.

Unfortunately nosuid doesn't prevent creating setuid files, only treating them as setuid during execution. If we actually want to prevent creating setuid files we either need to have an allow-list seccomp rule or create a BPF LSM. Allow-list seccomp is kind of a pain because of differences in syscall support between architectures, and BPF LSM is a pain because it uses BPF and it's an LSM.
Author
Owner
related: https://gerrit.lix.systems/c/lix/+/919
jade added the
Area/store
label 2024-05-10 18:12:01 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: lix-project/lix#207
No description provided.