Nixpkgs: 17.03's nixStable can't be used to build 17.03

[dtzWill]

Issue description

Looks like the latest Nix (1.11.10) has security improvements that limit various permissions/ownership actions during a build.

While some of these have been fixed (https://github.com/NixOS/nixpkgs-channels/commit/ad413fdb584736e7db1dc7a17bd6d5384de95904), many remain.

Being able to build packages from source, instead of using the binary cache, is a pretty important requirement for the release channel!

Examples (incomplete list, only one of my builders is using the latest "nix"):

• [x] at
• [x] cron
• [x] logcheck
• [x] logkeys
• [x] udevil
• [x] uucp
• [x] super
• [x] xconq

Steps to reproduce

• nix-build '<nixos>' -A at --check
• error:

install flags: install SHELL=/nix/store/fi3mbd2ml4pbgzyasrlnp0wyy6qi48fh-bash-4.4-p5/bin/bash    
/nix/store/k8dyhz9hrjydl23ny3131am6cws1rc07-coreutils-8.26/bin/install -c -m 755 -d /nix/store/2s9vbmdr7h5mmw5yqf6s78mva0n8mx60-at-3.1.16/bin
/nix/store/k8dyhz9hrjydl23ny3131am6cws1rc07-coreutils-8.26/bin/install -c -m 755 -d /nix/store/2s9vbmdr7h5mmw5yqf6s78mva0n8mx60-at-3.1.16/sbin
/nix/store/k8dyhz9hrjydl23ny3131am6cws1rc07-coreutils-8.26/bin/install -c -m 755 -d /nix/store/2s9vbmdr7h5mmw5yqf6s78mva0n8mx60-at-3.1.16/doc
/nix/store/k8dyhz9hrjydl23ny3131am6cws1rc07-coreutils-8.26/bin/install -c -m 755 -d /nix/store/2s9vbmdr7h5mmw5yqf6s78mva0n8mx60-at-3.1.16/doc/at
/nix/store/k8dyhz9hrjydl23ny3131am6cws1rc07-coreutils-8.26/bin/install -c -m 6755 -s at /nix/store/2s9vbmdr7h5mmw5yqf6s78mva0n8mx60-at-3.1.16/bin
/nix/store/k8dyhz9hrjydl23ny3131am6cws1rc07-coreutils-8.26/bin/install: cannot change permissions of '/nix/store/2s9vbmdr7h5mmw5yqf6s78mva0n8mx60-at-3.1.16/bin/at': Operation not permitted
make: *** [Makefile:98: install] Error 1

Technical details

• System: (NixOS: nixos-version, Ubuntu/Fedora: lsb_release -a, ...)
  17.03.1316.412b0a17aa (Gorilla)
• Nix version: (run nix-env --version)
  nix-env (Nix) 1.11.10
• Nixpkgs version: (run nix-instantiate --eval '<nixpkgs>' -A lib.nixpkgsVersion)
  17.03.1316.412b0a17aa
• Sandboxing enabled: (run grep build-use-sandbox /etc/nix/nix.conf)
  build-use-sandbox = true

Solutions

• Address the failing packages, as has been done already with openssl and others.
• Revert the Nix version bump (if these issues can't be resolved in a timely fashion, but I'm optimistic)
• Throw Hydra-powered builder goodness at rebuilding the channel from scratch
  
  
  
  • Possibly automate this when a Nix version change is introduced (?)
  
  

---

List:

• [x] coreutils
• [x] [ddccontrol](https://hydra.nixos.org/build/54539760/nixlog/1)
• [x] firejail
• [x] irods
• [x] leafnode
• [x] libcgroup
• [x] libutemper
• [x] kbdlight
• [x] mailman
• [x] mailutils
• [x] mcron
• [x] mtr
• [x] netselect
• [x] [qtsvg](https://hydra.nixos.org/build/54540217/nixlog/1)
• [x] slock
• [x] thttpd
• [x] wcslib

[lheckemann]

~Even util-linux fails (mount wants to be setuid).~

EDIT: never mind, I see that's in the fixes. I just wasn't building the latest nixpkgs version

[joachifm]

A few more from release-17.03:

• xsokoban
• unnethack
• torque
• rewritefs
• rssh
• hiawatha

[vcunat]

Yes, these security fixes caused possibly dozens of failures. I mainly fixed the channel blockers, except for ova #25901 (which isn't exactly the same problem, I think). A possible general workaround for much of this would be to create wrappers for chmod and install that disregard those fancy bits, but it might be in the end better to simply patch all the individual packages.

[Mic92]

Can the seccomp filter in nix mask unallowed bits instead of let the process crash?

[Mic92]

SECCOMP_RET_TRACE + ptrace could do the job: http://www.alfonsobeato.net/c/filter-and-modify-system-calls-with-seccomp-and-ptrace/ The only downside is, that it is architecture dependent. Proot requires quite some code to get this working.

[vcunat]

seccomp itself can probably do it, but not on Darwin (as Eelco wrote), which is why I was thinking of the wrappers. (Though parsing the arguments in all edge cases probably wouldn't be trivial in the wrappers.) Still, I don't think the Darwin sandbox disallows the bits ATM :-)

[lheckemann]

See the security advisory:

On macOS, the restriction is implemented using the existing sandbox
mechanism, which now uses a minimal "allow all except the creation of
setuid/setgid binaries" profile when regular sandboxing is disabled.

[LnL7]

On darwin builds will now always run in a sandbox that disallows setuid, but that will just silently ignore it instead of killing the process IIRC.

[dtzWill]

Thanks @lheckemann I somehow missed the security advisory, which explains the urgency for fixing these issues despite the fallout. :+1:

@vcunat patching the individual packages seems the way to go, especially if the number of package listed in this issue are at all indicative.

[dtzWill]

(Will stop cc'ing this and move to a new PR fixing mentioned packages)

[edolstra]

@LnL7 IIRC, the Darwin sandbox causes chmod +s to fail with EPERM, just as on Linux. (We don't kill the process on Linux, though seccomp does allow that.)

[dtzWill]

All packages mentioned in this issue have been fixed in #26628. I need to get to other things, but if someone has a more complete list (hydra evaluation comparison, maybe?) that'd be useful so they can get handled as well.

[vcunat]

• [x] [sudo](http://hydra.nixos.org/build/54580770)

[dtzWill]

@vcunat done!

[grahamc]

Is this issue resolved at this point, or are there more to do?

[vcunat]

Well, let's close until we find more of them.

[vcunat]

Actually, the fixes need picking to 17.03. I'd wait for a couple of days before doing that, even though the changes seem relatively safe.

[Mic92]

• [x] glusterfs: fixed in ecc34cbdb3f04855f7140b96d3631f0f2a4bd9ae on master
• [ ] [qtsvg](https://hydra.nixos.org/build/54540217/nixlog/1) (only on darwin)
• [ ] [ddccontrol](https://hydra.nixos.org/build/54539760/nixlog/1)

[vcunat]

There's been a stdenv rebuild on staging, so more are creeping up:

• [x] libcgroup
• [x] libutemper
• [ ] irods
• [x] firejail
• [x] mailman
• [x] mailutils
• [x] mtr
• [x] netselect
• [x] mcron
• [x] wcslib

There's probably some more, as there was a transient fail of go build, drowning the real failures. Let me merge master to staging to clear errors that we've fixed already.

[dtzWill]

Thanks @vcunat! That would be great.

Updating the issue with a combined list, added thttpd which I stumbled upon.

[splack]

Looks like coreutils-8.27 fails to build due to the install tests too due to this.

FAIL: tests/install/install-C
=============================

ginstall: the --compare (-C) option is ignored when you specify a mode with non-permission bits
ginstall: cannot change permissions of 'b': Operation not permitted
ginstall: the --compare (-C) option is ignored when you specify a mode with non-permission bits
ginstall: cannot change permissions of 'b': Operation not permitted
ginstall: options --compare (-C) and --preserve-timestamps are mutually exclusive
Try 'ginstall --help' for more information.
ginstall: options --compare (-C) and --strip are mutually exclusive
Try 'ginstall --help' for more information.
FAIL tests/install/install-C.sh (exit status: 1)

[dtzWill]

Fixing some of these, WIP:

https://github.com/dtzWill/nixpkgs/tree/fix/perms-fallout-misc-2

[joachifm]

@dtzWill have you looked at the fixes which are already in staging (coreutils tests for example).

[dtzWill]

Couldn't reproduce mtr, and I think the qtsvg will need a separate issue--due to scope but also for testing reasons.

I did not know things were being fixed on staging, thanks for letting me know! I'll take a look.

[dtzWill]

@joachifm hmm it looks there are some fixes on master that aren't on staging (yet?), and AFAICT from a quick look only coreutils has been fixed on staging but not master. LMK if you know of any others, I'll look a bit further myself :).

[0xABAB]

rssh is broken due to a suid issue. I did see rssh in one of the lists, but are you also aware of that issue?

[dtzWill]

@0xABAB rssh should have been fixed with https://github.com/NixOS/nixpkgs/commit/7eaa7adf4e7f3e32be85dcf3bd34ffd6b4cf23be which is now on master. I'm not sure what plans there are, or might be, regarding cherry-picking these onto the 17.03 branch.

[vcunat]

That PR is pushed to 17.03 now, too.

[dtzWill]

Awesome, TYVM for taking care of that! :+1:

[0xABAB]

I actually had an idea regarding the whole sticky bit thing, which you might consider to be gross, but we could compile chmod in such a way that it does a noop for all the things that we don't want it to do. That would eliminate a ton of 'sed' commands.

For cp a similar thing could be done. The disadvantage is that a huge combination of these things would confuse someone reading and executing the code on their machine, although we could also add a debug mode in which it would print out something like Not setting sticky bit here, because of Nix security issue documented at <url>.

The assumption here is that tools like chmod and cp don't change a lot and patching a single branch is unlikely to break.

[dtzWill]

I'm uncomfortable patching our "core" utilities to do non-standard things silently. Giving them a name like "fake-suid-coreutils" or something would help IMO but I think addressing these in all of our derivations is the right answer, even if it's a bit of a chore and noisy. Let's see if you folks agree with my reasoning:

Every instance that is patched is a divergence from the build system's normal behavior, and for that reason deserves an explicit mention in the derivation expression IMO. Just how patches aren't taken from a global patch lookup table (usually :P), the explicit and local specification of what is being changed is IMHO more important than refactoring all these into a magical coreutils that hides these changes.

While all the changes made as part of this issue /appear/ to be nothing but tedious fixes, every one of them signifies that the resulting output will not behave as the upstream author intended regarding special permission bits or application of user/groups that don't exist. Many of these had to be dealt with elsewhere, perhaps after a confused user wondered why a packaged piece of software didn't act as expected (since a suid wrapper wasn't created for it, maybe).

Basically I think making these fail immediately is actually a good thing as it forces someone to sit down and determine how to handle this behavior. Silently squashing the problems seems appealing (elegant even) for the set of software we already support since for the most part we've already addressed how to handle them re:wrappers and whatnot, but for new software that won't be the case. Furthermore mentioning it in the derivation has an important locality property that helps when updating packages.

Thoughts? :)

[grahamc]

Fully agreed, @dtzWill.

[vcunat]

I considered that approach, but only as wrappers used exclusively during nix builds (i.e. injected by stdenv) and not in the user-visible utilities. One advantage would be portability, unlike seccomp, and we wouldn't need to patch individual packages (but that part is mostly done now :).

[0xABAB]

@vcunat The injected via stdenv approach was also how I envisioned it.

Let's say I have a program named sui which on Debian would have the suid bit set in /usr/bin/sui. How can I replicate the same end user behaviour for a user named bob on NixOS who could just execute sui in their shell before this change? (I can think of some terrible hacks, but I just would like to know how this was envisioned.)

[vcunat]

On NixOS we do this via the setuid wrappers, residing in a tmpfs.

[lheckemann]

As @vcunat said, the setuid wrappers provide this on NixOS through the security.wrappers option.

setuid is deliberately not supported by Nix itself because it would more or less break one of nix's loveliest practical advantages — unprivileged package management — or come with a great deal of security headaches. I don't think there's a recommended approach for non-NixOS systems but I'd suggest using sudo to gain the necessary privileges.

[globin]

AFAICS all occurences mentioned here have been fixed => closing

[dtzWill]

Agreed, thanks!

[00vareladavid]

Hello, I am new to nixpkgs. Installing ripgrep-0.5.2 failed for me when build-use-chroot = true is set in my nix.conf. I am running nix on debian stretch. I am pretty sure this issue belongs here, but let me know if I am wrong or any more info is needed!

[lheckemann]

Hi @david-varela , is this specifically because it's trying to install binaries with setuid? I don't think ripgrep should ever need setuid. If it's not doing so, it's not this issue and you should open a new one.

[00vareladavid]

@lheckemann, you are correct. Sorry about that, I had a temporary mind lapse from lack of sleep. I'll dig around a little more and open a seperate issue if necessary, thanks