NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.8k stars 13.91k forks source link

nixos: linux file capabilities #6768

Closed nmikhailov closed 7 years ago

nmikhailov commented 9 years ago

Hello,

TLDR: Currently there is no way to set capabilities for a program.

After reading #6235, I decided that it will be nice to add nixos module for wireshark. Although for many programs and wireshark in particular, its recommended to use granular linux capabilities in favor to more dangerous setuid technique(and I think that we all can agree that additional proxies like setuidWrapper only add more problems security-wise).

It turns out that its impossible to do something like setuidWrapper for capabilities, because of capability inheritance though execv call(eg. you will also need to set capabilities on executed file). So as I see it, capabilities should be set directly in nixstore(which is something that is not currently possible i guess?) and that means that nix daemon should have be able to set capabilities and users should not be able to install packages(at least not special ones). And I guess that should be applied to setuid binaries too, because that situation is not fundamentally different. And that brings up even more problems, eg who should manage groups now(eg i want dumpcap to be executed only by group)?

That being said, I dislike both approaches and i am looking forward to see what can be done about it. Thanks

joachifm commented 9 years ago

For services there is serviceConfig.CapabilityBoundingSet and AppArmor has capability rules. If you only care about ad-hoc control of capabilities for specific executables, firejail has a capability filter feature, which I'm sure you could quite easily replicate in a custom setuid C wrapper. In the end, though, I think a rule-based MAC is the only solution which can address all your points (e.g., user/group capabilities).

I'm afraid I don't quite understand your points about writing stuff to the nix store, but I'm sure others will be able to comment on that.

lucabrunox commented 9 years ago

What's the problem with setuid wrappers? Binaries get copied to the wrapper directory from the nix store, so you can set the needed bits.

nmikhailov commented 9 years ago

@joachifm well, CapabilityBoundingSet systemd option is nice, but it is only limited to services and it's purpose is a little bit different, it's used to limit process capabilities while running it as root, but I need to gain capabilities, while running it as user. I was not aware of firejail and it seems nice tool but it looks like overkill for such simple thing as setting capabilities for a binary. And firejail needs setuid itself.

Let me show it by example: ping binary can be run either with suid or with CAP_NET_RAW set. In former way, it will be runned unprivileged and won't be able to damage host system. Setting capability is what most linux distros do and it is very trivial - you just run setcap CAP_NET_RAW /bin/ping and it adds capability info to the file. It is easy, clean and out-of-the-box solution opposing to something like firejail which probably has much larger codebase and complexity and introduces new point of failure.

Regarding nixstore - I meant that from some point of view, it would be nice to have suid/capabilities in nix store, because then someone who is not using NixOS or is not using NixOS module could benefit from it.(eg run wireshark flawlessly). But that in turn introduces a whole new set of problems.

@lethalman Well, there is no problem, but it doesn't work with capabilities and introduces a new point of failure security-wise. Don't get me wrong, there are a lot of check there, but with all respect I can't say if that is enough.

Anyway, my initial point was that it's impossible to have capabilities on file. As @joachifm showed it is kinda possible with tools like firejail, but they do it other way around - they drop root privileges and are IMO overkill. But that also means that probably it is possible to make something like current setuid wrapper, but that drops privileges before execv. But I personally think that it is worse than simple and clean traditional way.

joachifm commented 9 years ago

@nslqq FWIW I agree that setcap would be cool and clearly superior to setuid where applicable.

lucabrunox commented 9 years ago

@nslqqq I don't get why it doesn't work if you copy the file and set the caps.

nmikhailov commented 9 years ago

@lethalman Do you mean without setuidWrapper thing? Because setuidWrapper doesn't copy binary, it copies special program that execv's it. Well, copying binary should work i guess... Now I wonder why setuidWrapper doesn't do that? And it seems less agile in terms of having multiple versions of same package and other neat nix features.

lucabrunox commented 9 years ago

You can still create a wrapper with execve and set the caps to be inherited, no? I just fear problems security-wise of having setuid or caps in the nix store.

lucabrunox commented 9 years ago

Oh I guess the file in the nix store must have inheritable caps set :( So execve wrapper with effective+inheritable caps, and the nix store program with inheritable caps.... as far as I understand.

nmikhailov commented 9 years ago

Yes, that's correct On Mar 12, 2015 8:12 PM, "lethalman" notifications@github.com wrote:

Oh I guess the file in the nix store must have inheritable caps set :( So execve wrapper with effective+inheritable caps, and the nix store program with inheritable caps.... as far as I understand.

— Reply to this email directly or view it on GitHub https://github.com/NixOS/nixpkgs/issues/6768#issuecomment-78486479.

nmikhailov commented 9 years ago

There definetly will be security problems if we just let everyone set caps in nix store, I think you can effectively have suid analog with capabilities. On Mar 12, 2015 8:00 PM, "lethalman" notifications@github.com wrote:

You can still create a wrapper with execve and set the caps to be inherited, no?

— Reply to this email directly or view it on GitHub https://github.com/NixOS/nixpkgs/issues/6768#issuecomment-78484317.

nmikhailov commented 9 years ago

@lethalman and for that matter, why we can't just copy binaries without wrapper? I wonder if those symlinks to nixstore can be changed without running nixos-rebuild? Because we can always recopy right ones in activation scripts.

lucabrunox commented 9 years ago

@nslqqq that's what I suggested, copying the binary itself. The problem will be with those programs requiring the binary to be in the same directory of other binaries ecc., i.e. in their original directory. So the setuid wrapper doing execve is more general.

But I think it's ok for a good number of executables to be copied elsewhere the nix store. If it works, just go ahead for wireshark.

nmikhailov commented 9 years ago

@lethalman I wonder if there are such programs. Well ok, copying will work in some cases(I just tested with dumpcap, it works) but it looks even worse and hacky to me, because now we copy potentially large binaries out of nixstore where they belong and loose all nix features. And it doesn't work without nixos.

lucabrunox commented 9 years ago

Yes there are such programs. And yes it's certainly ugly, perhaps @edolstra has a better solution.

On Thu, Mar 12, 2015 at 7:18 PM, Nikita Mikhailov notifications@github.com wrote:

@lethalman https://github.com/lethalman I wonder if there are such programs. Well ok, copying will work in some cases(I just tested with dumpcap, it works) but it looks even worse and hacky to me, because now we copy potentially large binaries out of nixstore where they belong and loose all nix features. And it doesn't work without nixos.

— Reply to this email directly or view it on GitHub https://github.com/NixOS/nixpkgs/issues/6768#issuecomment-78554089.

NixOS Linux http://nixos.org

7c6f434c commented 9 years ago

Oh I guess the file in the nix store must have inheritable caps set :( So execve wrapper with effective+inheritable caps, and the nix store program with inheritable caps....

Inheritable capabilities seem to be acceptable for sometimes having in store, compared to other options...

Maybe make all this handling a config option.

lucabrunox commented 9 years ago

@7c6f434c that's true, but you still have to create the execve wrapper with effective caps. Not sure if it's worth the complication if you can just copy the whole binary.

7c6f434c commented 9 years ago

After my experience with TeXLive binaries I don't trust copying binaries...

nmikhailov commented 9 years ago

@7c6f434c Won't it cause problems with insufficient privileges in nix-daemon, copy-closure stuff and binary caches?

7c6f434c commented 9 years ago

We already have optional things requiring root access, like chroot builds.

lucabrunox commented 9 years ago

As a side note, if chroots require root I think this can be changed to use CLONE_NEWUSER and not require root anymore.

On Sat, Mar 14, 2015 at 3:35 PM, Michael Raskin notifications@github.com wrote:

We already have optional things requiring root access, like chroot builds.

— Reply to this email directly or view it on GitHub https://github.com/NixOS/nixpkgs/issues/6768#issuecomment-80510789.

NixOS Linux http://nixos.org

7c6f434c commented 9 years ago

What about getting write access to store (our bind mounts prevent even root processes from store writes, special bind remounts are used to give builders write access).

I guess we also need UID NS for build user isolation.

nmikhailov commented 9 years ago

Ok, so to summarize, I see these options:

  1. Add something like suid wrapper which will be suid'ed and will drop privileges and then execv required binary.

    pro

    • Relatively easy to add.

    con

    • Ugly - works only on NixOS, doesn't support multiple versions and other nix features.
    • Adds wrappers and unneeded potentially exploitable code for what is used to be very simple.
  2. Copy binaries to suid path and set capabilities there.

    pro

    • Very easy to implement.

    con

    • Ugly - works only on NixOS, doesn't support multiple versions and other nix features.
    • Probably won't for in some cases where apps have additional directory structure requirements.
  3. Add support for setting Inheritable capabilities in nix store, NAR, etc. This should not affect security of nix-store. This will still need a wrapper, but it will be

    con

    • Ugly - works only on NixOS, doesn't support multiple versions and other nix features.
    • Relativly difficult, requires some work in Nix and NixOS
  4. Add support for setting extended attributes/setuid in nix store, NAR, etc. This will also require an access model, which will restrict unprivileged users from importing privileged packages in nixstore. This proposal is somewhat relative to https://github.com/NixOS/nix/pull/329 in terms of extending nixstore capabilities.

    pro

    • cleanest solution - no ugly hacks like /var/setuid-wrappers, etc.
    • privileged apps will benefit from nix features(multiple versions, etc.)
    • privileged apps won't require nixos modules or/and nixos at all.
    • this will make possible to use PaX/Grsecurity features and improve security as requested in https://github.com/NixOS/nix/issues/185

    con

    • hard to implement
    • will require significant nixstore complication
    • requres @edolstra's blessing. And personally, I don't think such large thing could be merged with current type of management(no offense).
    • requires non-trivial policy management inside Nix (thanks @7c6f434c)

Personally, I prefer 4th solution as it is clean and nix-friendly. Also I think it was bad decision to leave this kind of stuff out of whitepaper.

7c6f434c commented 9 years ago
  1. Add support for setting extended attributes/setuid in nix store, NAR, etc. This will also require an access model, which will restrict unprivileged users from importing privileged packages in nixstore. This proposal is somewhat relative to https://github.com/NixOS/nix/pull/329 in terms of extending nixstore capabilities.

pro

  • cleanest solution - no ugly hacks like /var/setuid-wrappers, etc.
  • privileged apps will benefit from nix features(multiple versions, etc.)
  • privileged apps won't require nixos modules or/and nixos at all.
  • this will make possible to use PaX/Grsecurity features and improve security as requested in https://github.com/NixOS/nix/issues/185

con

  • hard to implement
  • will require significant nixstore complication
  • requres @edolstra's blessing. And personally, I don't think such large thing could be merged with current type of management(no offense).

Also it requires non-trivial policy management inside Nix. Also, it means that Nix has to behave very differently on Linux and on FreeBSD.

nmikhailov commented 9 years ago

@7c6f434c I agree about policy management. About different underlying systems: I thought that it will be left to package maintainer, eg. on systems where capabilities are not available suid will be set instead.

ts468 commented 9 years ago

What is the current status? How could I help?

Mathnerd314 commented 9 years ago

Recently, ambient capabilities were merged: https://github.com/torvalds/linux/commit/58319057b7847667f0c9585b9de0e8932b0fdb08. This makes it easy to implement a setcap wrapper; we have pE' = pA' = pA = fP after the wrapper sets PR_CAP_AMBIENT. So the activation script can just set the capabilities directly on the wrapper. Unfortunately, Linux 4.3 is a few months away.

domenkozar commented 8 years ago

We have now kernel 4.4 ;)

ixmatus commented 8 years ago

A setcap wrapper is something we need, has there been any movement by anyone on this? How can I help or get started if someone else hasn't already?

ixmatus commented 8 years ago

I've built the setcap-wrapper functionality and system, is there interest a pull request for this?

ixmatus commented 8 years ago

Here is my branch, I'm preparing a Pull Request but won't submit until I get a successful build on Ubuntu as-per the PR template.

https://github.com/awakenetworks/nixpkgs/tree/parnell/setcap-wrappers