lix-project/lix

Fork 31

Lix allows configuration which makes everyone inside the sandbox effectively root #242

New issue

Open

opened 2024-04-19 20:07:22 +00:00 by nrabulinski · 6 comments

nrabulinski commented

2024-04-19 20:07:22 +00:00

Member

Describe the bug

Well, not a bug exactly but currently Lix allows the user to implicitly set the build user group to root. This used to be disallowed, but because it broke RPM builds someone decided to ifdef it out 16 years ago :^) 98968fbb63 just for the code to finally be completely removed a bit over a year ago 479c011784

Steps To Reproduce

Add build-users-group = to nix config
Restart the daemon
Enjoy root in the sandbox

Expected behavior

Nix should always default to using the nixbld group, or at least should do so when it's running as root

`nix --version` output

All nix versions since circa 2008

Additional context

IMO we don't necessarily need to disallow setting build-users-group = root, even though there may be hardly any reason to do so, but defaulting to the group of the user who's running the process is bad

## Describe the bug Well, not a bug exactly but currently Lix allows the user to *implicitly* set the build user group to root. This used to be disallowed, but because it broke RPM builds someone decided to ifdef it out 16 years ago :^) https://github.com/NixOS/nix/commit/98968fbb63a1a049b2439bfc2a7d53e5b51471e3 just for the code to finally be completely removed a bit over a year ago https://github.com/NixOS/nix/commit/479c0117840a5dc710019db006c5940b29d98dcc ## Steps To Reproduce 1. Add `build-users-group =` to nix config 2. Restart the daemon 3. Enjoy root in the sandbox ## Expected behavior Nix should always default to using the `nixbld` group, or at least should do so when it's running as root ## `nix --version` output All nix versions since circa 2008 ## Additional context IMO we don't necessarily need to disallow setting `build-users-group = root`, even though there may be hardly any reason to do so, but defaulting to the group of the user who's running the process is bad

nrabulinski added the

bug

label 2024-04-19 20:07:22 +00:00

jade commented

2024-04-19 20:51:59 +00:00

Owner

I think we should probably disallow setting build-users-group = root and let whoever (probably nobody) be grumpy about it breaking their special use case that we probably should not support to begin with.

Also nice CTF challenge :P https://jade.fyi/blog/reproducible-pwning-writeup/

I think we should probably disallow setting `build-users-group = root` and let whoever (probably nobody) be grumpy about it breaking their special use case that we probably should not support to begin with. Also nice CTF challenge :P https://jade.fyi/blog/reproducible-pwning-writeup/

nrabulinski commented

2024-04-19 21:21:22 +00:00

Author

Member

@jade wants to kill NixOS CTF challenges this is so sad (jk next year we'll have a Nix 0day dw)

lunaphied commented

2024-04-20 19:52:25 +00:00

Owner

Wouldn't this affect single-user Lix installations?

nrabulinski commented

2024-04-20 21:39:51 +00:00

Author

Member

It shouldn't since the worst offender here is "if build-users-group is set to empty, default to running user's group" which is a bad default. And as a bonus disallowing it to be root since putting root inside the sandbox is universally a bad idea

It shouldn't since the worst offender here is "if `build-users-group` is *set* to empty, default to running user's group" which is a bad default. And as a bonus disallowing it to be `root` since putting root inside the sandbox is universally a bad idea

jade added the

E/easy

E/help wanted

labels 2024-05-03 22:51:43 +00:00

jade commented

2024-05-03 22:53:36 +00:00

Owner

Yeah, this seems reasonable as a resolution. Action items:

Restore the checking code that got commented out due to RedHat builder
Fail if the group is set to empty explicitly (maybe? thoughts?)

Yeah, this seems reasonable as a resolution. Action items: - Restore the checking code that got commented out due to RedHat builder - Fail if the group is *set* to empty explicitly (maybe? thoughts?)

qyriad added the

Area/store

label 2024-05-06 00:54:00 +00:00

71rd commented

2024-08-12 17:48:55 +00:00

The treatment of empty values seems like a great way to open another can of worms and probably warrants another issue to maybe fix option parsing and not parse an empty value as distinct from an unset option. And for the parse instead either reject configs containing empty values, or when an option has an empty value either ignoring the line or unsetting the value of that option to default.

When using multi-user mode the daemon should probably just always fail to build and complain when the effective build-users-group is root. This still allows building as root in single-user mode, but it might be worth it to add an option that needs to be set explicitly to allow running single-user mode as root, and try to get users to instead to use the nix daemon with dedicated build-users The treatment of empty values seems like a great way to open another can of worms and probably warrants another issue to maybe fix option parsing and not parse an empty value as distinct from an unset option. And for the parse instead either reject configs containing empty values, or when an option has an empty value either ignoring the line or unsetting the value of that option to default.

No milestone

No project

No assignees

4 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#242

No description provided.

Lix allows configuration which makes everyone inside the sandbox effectively root #242

Describe the bug

Steps To Reproduce

Expected behavior

nix --version output

Additional context

`nix --version` output