Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

accept-flake-config allows running commands as root #9649

Open
9999years opened this issue Dec 21, 2023 · 18 comments
Open

accept-flake-config allows running commands as root #9649

9999years opened this issue Dec 21, 2023 · 18 comments
Labels

Comments

@9999years
Copy link
Contributor

Describe the bug

With the --accept-flake-config option or accept-flake-config = true in nix.conf, any flake build (nix build, nix develop, nix run, etc.) gets root access.

Steps To Reproduce

$ git clone https://github.com/9999years/accept-flake-config-demo.git
$ cd accept-flake-config-demo
$ nix build --accept-flake-config --print-build-logs
my-cool-and-normal-derivation (post)> root

Demo here: https://github.com/9999years/accept-flake-config-demo/

Expected behavior

I expected accept-flake-config might do something like use untrusted caches, but "root access" is pretty extreme, and the manual doesn't say "enabling this setting is equivalent to giving root access to any flake you interact with" when it describes accept-flake-config.

nix-env --version output

nix-env (Nix) 2.18.1

Additional context

I don't think this is a bug per-se, in that the behavior is intended, but I don't think this should be allowed in the general case, and even if it is the manual should make it a lot clearer how dangerous this setting is.

It might be nice to have accept-flake-config take a list of settings instead, so that (e.g.) accept-flake-config = allow-import-from-derivation substituters would allow flakes to set the allow-import-from-derivation or substituters options, but not builders or post-build-hook.

Priorities

Add 👍 to issues you find important.

@9999years 9999years added the bug label Dec 21, 2023
@lf-
Copy link
Member

lf- commented Dec 21, 2023

The design flaws around this resulted in the compromise of nix-ci.com: https://twitter.com/puckipedia/status/1693927716326703441

@lf-
Copy link
Member

lf- commented Dec 21, 2023

This was also the subject of a capture the flag security challenge I authored: https://github.com/ubcctf/maple-ctf-2023-public/blob/main/misc/flakey-ci/solve/flake.nix

@SuperSandro2000
Copy link
Member

This requires that the user is a trusted-user, right? By default this only root. So the documentation should be updated to more clear reflect, that this setting must be used very careful.

@9999years
Copy link
Contributor Author

This requires that the user is a trusted-user, right?

Part of the issue is that it's not entirely clear what features being in trusted-users is required for. But the big one is trusted-substituters -- I suspect that many Nix users have added themselves to trusted-users just to use a binary cache (and it's pretty much standard practice for Nix projects to have a binary cache).

Also, the warning on the trusted-users option is a little misleading:

Adding a user to trusted-users is essentially equivalent to giving that user root access to the system.

I read this and thought "well, my user already has root access, so that's fine", but trusted-users is actually like enabling passwordless sudo, which is less fine. The only example the warning gives is this:

For example, the user can access or replace store path contents that are critical for system security.

Well, I run macOS, so there aren't any /nix/store paths that are critical for system security, but trusted-users allows more than that I believe. (Not sure exactly what though, does anyone have a good privilege escalation example?)

I've also noticed that the documentation for post-build-hook does not say that it can be set by Flakes:

This option is only settable in the global nix.conf, or on the command line by trusted users.

@SuperSandro2000
Copy link
Member

see https://nixos.org/manual/nix/stable/command-ref/new-cli/nix3-flake.html?highlight=nixConfig#flake-format for what is allowed in nixConfig inside a flake.nix.

I think the first warning in the docker documentation regarding the docker group is very similar to the issue at hand https://docs.docker.com/engine/install/linux-postinstall/

Also, the warning on the trusted-users option is a little misleading:

But isn't that describing this issue, even if it isn't very detailed?

@9999years
Copy link
Contributor Author

But isn't that describing this issue, even if it isn't very detailed?

Yes, the lack of detail is the issue. Technically it's correct, but it doesn't do a good job of communicating the implications. "Root access" and "passwordless root access" are quite different in my threat model.

@9999years
Copy link
Contributor Author

I think the first warning in the docker documentation regarding the docker group is very similar to the issue at hand https://docs.docker.com/engine/install/linux-postinstall/

Yeah, that's a great description -- it's detailed and clearly outlines how privilege escalation can be performed (by sharing filesystems between the host and containers) and the consequences of that access (containers can write to the host filesystem). We should have a page like that for the Nix daemon and trusted-users and similar.

@evanrelf
Copy link

"Root access" and "literally being root" are meaningfully different things. We use sudo to become root temporarily, because having access 24/7 is bad.

This trusted-users + post-build-hook thing is like being root.

@thufschmitt
Copy link
Member

This trusted-users + post-build-hook thing is like being root.

It is indeed (not just post-build-hook FWIW, there are a bunch of other settings that allow you to run arbitrary code as root, including substituters).

Agreed that it should be flagged out explicitly in the documentation. Would any one care to do that?

Beyond just that, it would also be possible to mitigate this a bit by:

  1. Not running nix-daemon as root. Allow gc-ing with a rootless daemon #5380 fixes the only fundamental blocker, but there's still a bit of work before being able to streamline the rootless setup.
  2. Restrict the accesses of the daemon by hardening the systemd/launchd unit. This should be doable right now at a fairly low cost, although it would also have a limited impact (in particular the gc still requires read access to roughly the whole filesystem)

@brainrake
Copy link

brainrake commented Dec 22, 2023

With your user in trusted-users:
function sudo2 () { NIX_CONFIG="post-build-hook = $@" nix build nixpkgs#hello --rebuild --print-build-logs; }
Brand new sudo, now without password!
credit @aciceri @9999years

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/help-needed-with-nix-trusted-users-behaviour/8369/4

@Majiir
Copy link

Majiir commented Dec 25, 2023

Related: NixOS/nixpkgs#231408

@cumber
Copy link

cumber commented Jan 2, 2024

It might be nice to have accept-flake-config take a list of settings instead, so that (e.g.) accept-flake-config = allow-import-from-derivation substituters would allow flakes to set the allow-import-from-derivation or substituters options, but not builders or post-build-hook.

Yes, anytime something needs more privileges than it would have by default it's really nice to have a range of options rather than a single "remove the checks; allow everything". Without that it's really hard to have features like this not turn into major vulnerabilities.

But of course it's also hard to break privileges up into separate ones and be really sure that one sub-privilege can't possibly be exploited to break out into the other ones, and if you can't guarantee that then security-wise we're no better off than with the single "allow everything" sledgehammer.

@thufschmitt
Copy link
Member

@cumber that could make sense, but even substituters can trivially give you root access because ssh-ng://localhost?remote-program=/some-program will trigger the daemon to happily execute bash -c /some-program (as root). So it's really hard to make this split useful atm since substituters is probably the most widely used option in that context

@cumber
Copy link

cumber commented Jan 8, 2024

@cumber that could make sense, but even substituters can trivially give you root access because ssh-ng://localhost?remote-program=/some-program will trigger the daemon to happily execute bash -c /some-program (as root). So it's really hard to make this split useful atm since substituters is probably the most widely used option in that context

Yes, that's exactly the sort of thing I was thinking about when I said splitting privileges is hard to do. It'd need to be even more fine grained, and allow you to restrict the kind of substituters values you'll accept. If that ends up with you configuring a concrete whitelist, you might as well just be setting the system-wide substituters and not bothering to accept the flake config.

Though TBF, a configuration that said "I'll accept flake config adding to substituters any of the values I've whitelisted in trusted-substituters" might be reasonably nice. Flakes could use that own substituters without making you use them for everything, but still be restricted to a set you've said you'll trust.

@avnik
Copy link

avnik commented Jan 16, 2024

Could we specify/use non-root user for invoke remote builders and post-build-hook?

@sellout
Copy link
Contributor

sellout commented Jul 8, 2024

I found this issue specifically because I thought setting accept-flake-config = true seemed dangerous, but the manual didn’t mention anything about it. Definitely some are likely to have to use the flag it a few times, then just think it’s a good idea to throw it in their nix.conf. There should be some pretty serious warnings around it.

I also like @9999years suggestion “to have accept-flake-config take a list of settings instead”. I was hoping to come across something indicating that it wasn’t strictly boolean (like sandbox = "relaxed") but didn’t know what exactly I was looking for. I suppose if there is some “safe” subset (which I think wouldn’t include substituters or trusted-public-keys) those would just be available by default, so listing individual options sounds like the way to go.

It might also be nice (although complicated and partial) if there was some standardized tagging of nix.conf options that indicated the vulnerability level of different settings (e.g., )

([…] there are a bunch of other settings that allow you to run arbitrary code as root, including substituters).

Agreed that it should be flagged out explicitly in the documentation.

I’m not sure exactly what severities there are, and of course there are always undiscovered vulnerabilities, but indicating in the documentation the ones we are aware of seems quite important. We can have a standardized set of tags that consistently link to detailed descriptions of what they mean, ranging from “no known security issues” to “gives everyone on the Internet root on your machine”. And a general disclaimer like “the descriptions attempt to describe the dangers of each setting, but it is necessarily incomplete and only intended as one aspect of your considerations on what to enable” could be helpful (IANAL, obviously).

Would any one care to do that?

I would be happy to try to update the nix.conf documentation with a first swipe at something like this, but not sure where to start – I created #11066, since I couldn’t find anything that already existed. If you think that issue is important, please give it some 👍 so I know to spend time on it. (But also, if someone else wants to do it, you’re probably better positioned than I am to make it happen). Please also comment on that issue with links to other issues or discussions that point out various known vulnerabilities (preserving those links in the documentation seems important).

I’m thinking a single manual page that describes the security issues for each setting in detail, and a small high-vis tag on each setting in the nix.conf page, which links to that section of the security issue page. Making it easy to see and understand the known issues while also not getting in the way of general setting documentation.

Documenting this also helps people figure out where we might be able to make improvements by splitting up the functionality into smaller pieces (again, like @9999years suggested re: accepting a list of specific settings for accept-flake-config).

@lf-
Copy link
Member

lf- commented Jul 8, 2024

FYI Lix has changes to make accept-flake-config loudly tell you it's a bad idea everywhere in the documentation and the output, short of printing a warning to output that it's a bad idea.

most recently: https://gerrit.lix.systems/c/lix/+/1541

Alrefai added a commit to Alrefai/dotfiles that referenced this issue Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests