SELinux, incorrect labelling from derivation #546

Open
opened 2024-10-14 08:06:26 +00:00 by soupglasses · 5 comments

Describe the bug

A clear and concise description of what the bug is.

Given the simple example of generating a systemd service file:

stdenv.mkDerivation {
    name = "example-target";
    phases = [ "buildPhase" "installPhase" ];

    buildPhase = ''
      mkdir -p $out/lib/systemd/system
    '';

    installPhase = ''
      echo -e "[Unit]\n\n" > $out/lib/systemd/system/example.target
    '';
}

Building it will generate a file with incorrect labels compared to what is described in SELinux (see below).

$ ls -lhaZ /nix/store/...-example-target/lib/systemd/system/example.target
-r--r--r--. 3 root root system_u:object_r:default_t:s0 9 Jan  1  1970 /nix/store/...-example-target/lib/systemd/system/example.target

Running restorecon -v against the path will show that this is incorrectly created.

sudo /sbin/restorecon -v /nix/store/...-example-target/lib/systemd/system/example.target
Relabeled /nix/store/...-example-target/lib/systemd/system/example.target from system_u:object_r:default_t:s0 to system_u:object_r:systemd_unit_file_t:s0

Steps To Reproduce

  1. Run nix repl and load nixpkgs with :l <nixpkgs>
  2. Write out the above derivation with either a name or prefixing it with :b
  3. Check out the built file with ls -lhaZ, see its incorrect compared to what is expected.
  4. Alternatively, use restorecon -v .../path and see if it relabels the file or not.

Expected behavior

It will label files correctly as described by SELinux.

nix --version output

nix (Lix, like Nix) 2.91.0

Additional context

The current SELinux file context against /nix/store.

$ sudo /sbin/semanage fcontext -l | grep /nix/store
/nix/store/[^/]+/etc(/.*)?                         all files          system_u:object_r:etc_t:s0 
/nix/store/[^/]+/lib(/.*)?                         all files          system_u:object_r:lib_t:s0 
/nix/store/[^/]+/lib/systemd/system(/.*)?          all files          system_u:object_r:systemd_unit_file_t:s0 
/nix/store/[^/]+/man(/.*)?                         all files          system_u:object_r:man_t:s0 
/nix/store/[^/]+/s?bin(/.*)?                       all files          system_u:object_r:bin_t:s0 
/nix/store/[^/]+/share(/.*)?                       all files          system_u:object_r:usr_t:s0 
## Describe the bug A clear and concise description of what the bug is. Given the simple example of generating a systemd service file: ```nix stdenv.mkDerivation { name = "example-target"; phases = [ "buildPhase" "installPhase" ]; buildPhase = '' mkdir -p $out/lib/systemd/system ''; installPhase = '' echo -e "[Unit]\n\n" > $out/lib/systemd/system/example.target ''; } ``` Building it will generate a file with incorrect labels compared to what is described in SELinux (see below). ``` $ ls -lhaZ /nix/store/...-example-target/lib/systemd/system/example.target -r--r--r--. 3 root root system_u:object_r:default_t:s0 9 Jan 1 1970 /nix/store/...-example-target/lib/systemd/system/example.target ``` Running `restorecon -v` against the path will show that this is incorrectly created. ``` sudo /sbin/restorecon -v /nix/store/...-example-target/lib/systemd/system/example.target Relabeled /nix/store/...-example-target/lib/systemd/system/example.target from system_u:object_r:default_t:s0 to system_u:object_r:systemd_unit_file_t:s0 ``` ## Steps To Reproduce 1. Run `nix repl` and load nixpkgs with `:l <nixpkgs>` 2. Write out the above derivation with either a name or prefixing it with `:b` 3. Check out the built file with `ls -lhaZ`, see its incorrect compared to what is expected. 4. Alternatively, use `restorecon -v .../path` and see if it relabels the file or not. ## Expected behavior It will label files correctly as described by SELinux. ## `nix --version` output `nix (Lix, like Nix) 2.91.0` ## Additional context The current SELinux file context against /nix/store. ```bash $ sudo /sbin/semanage fcontext -l | grep /nix/store /nix/store/[^/]+/etc(/.*)? all files system_u:object_r:etc_t:s0 /nix/store/[^/]+/lib(/.*)? all files system_u:object_r:lib_t:s0 /nix/store/[^/]+/lib/systemd/system(/.*)? all files system_u:object_r:systemd_unit_file_t:s0 /nix/store/[^/]+/man(/.*)? all files system_u:object_r:man_t:s0 /nix/store/[^/]+/s?bin(/.*)? all files system_u:object_r:bin_t:s0 /nix/store/[^/]+/share(/.*)? all files system_u:object_r:usr_t:s0 ```
soupglasses added the
bug
label 2024-10-14 08:06:26 +00:00
Owner

okay so, the thing is, nix (both lix and cppnix) doesn't do anything to the selinux metadata as far as I'm aware; nix is completely selinux unaware.

so i pose the question: what should nix be doing in this situation, given that the derivation builder itself isn't causing the correct xattrs to be placed? (or perhaps it is getting the xattrs wiped, which would be our fault entirely and is certainly a bug!)

further question:

  • should it do the same thing for substitution?
  • do we expect nix to store the selinux context in packages? (this is likely infeasible in practice because it would require nar changes but i want to know if it's expected for good selinux interop)
    • if so, is doing so for unprivileged users a security bug? (if so, nix must not do this for you)
  • do we consider it safe to restore context of arbitrary store paths that might be generated by malicious users?
okay so, the thing is, nix (both lix and cppnix) doesn't do anything to the selinux metadata as far as I'm aware; nix is completely selinux unaware. so i pose the question: what *should* nix be doing in this situation, given that the derivation builder itself isn't causing the correct xattrs to be placed? (or perhaps it is getting the xattrs wiped, which would be our fault entirely and is certainly a bug!) further question: - should it do the same thing for substitution? - do we expect nix to store the selinux context in packages? (this is likely infeasible in practice because it would require nar changes but i want to know if it's expected for good selinux interop) - if so, is doing so for unprivileged users a security bug? (if so, nix must not do this for you) - do we consider it safe to restore context of arbitrary store paths that might be generated by malicious users?
Owner

my guess is that the selinux stuff on your system was put there by the installer as, quite frankly, laziness to avoid implementing it properly in nix and thus it's very much only partially applied and only to the paths that matter to installing nix originally. thus I'm wondering how it should actually work if it were done properly.

my guess is that the selinux stuff on your system was put there by the installer as, quite frankly, laziness to avoid implementing it properly in nix and thus it's very much only partially applied and only to the paths that matter to installing nix originally. thus I'm wondering how it should actually work if it were done properly.
Author

For context, I hit this issue while looking into the support of System-Manager, which pushes service files to SystemD of a SELinux enabled distro, so we cannot just have unconfied types (from my understanding). https://github.com/numtide/system-manager/issues/115

If this is even feasable to support inside Nix/Lix or System-Manager I do not know.

The NixOS workgroup had some patches for SELinux to allow a quite simple link between /nix/store and /.

There's also some work here https://github.com/NixOS/nix/pull/2670/files from a previous attempt.

For context, I hit this issue while looking into the support of System-Manager, which pushes service files to SystemD of a SELinux enabled distro, so we cannot just have unconfied types (from my understanding). https://github.com/numtide/system-manager/issues/115 If this is even feasable to support inside Nix/Lix or System-Manager I do not know. The [NixOS workgroup](https://nixos.wiki/wiki/Workgroup:SELinux) had some patches for SELinux to allow a quite simple link between /nix/store and /. There's also some work here https://github.com/NixOS/nix/pull/2670/files from a previous attempt.
Owner

well, i am not really convinced in the cleverness in matching special fs paths in derivation outputs but you might be able to achieve what you want using the existing post-build-hook functionality in lix to apply restorecon (though that would only work on builds, not substitutes, which isn't ideal at all).

without someone with selinux expertise to provide guidance as to the reasonable way to integrate selinux with nix, a system that will likely have a lot of complexities to integrate with (sandbox and such; it's a package manager and a docker like thing), i don't think this is likely to happen. but if you can find someone to provide that expertise and maintain it, likely it could be done?

i am also very unsure as to how much scope this feature has: how much of selinux needs to be supported in lix to work properly? i have an idea as to why restorecon was required to begin with (the directory didn't have a label to inherit, probably), but if that's the problem, fixing it requires doing something equivalent to restorecon on every store path, which is doable.

well, i am not really convinced in the cleverness in matching special fs paths in derivation outputs but you might be able to achieve what you want using the existing post-build-hook functionality in lix to apply restorecon (though that would only work on builds, not substitutes, which isn't ideal at all). without someone with selinux expertise to provide guidance as to the reasonable way to integrate selinux with nix, a system that will likely have a lot of complexities to integrate with (sandbox and such; it's a package manager *and* a docker like thing), i don't think this is likely to happen. but if you can find someone to provide that expertise and maintain it, likely it could be done? i am also very unsure as to how much scope this feature has: how much of selinux *needs* to be supported in lix to work properly? i have an idea as to why restorecon was required to begin with (the directory didn't have a label to inherit, probably), but if that's the problem, fixing it requires doing something equivalent to restorecon on every store path, which is doable.
Owner

looking through the discussion on 2670 and the other issue about SELinux support, it seems like the reaction to "we need to have extra metadata in the store" is "let the installer deal with just the specific paths in question", which leads to the exact state you arrived at.

i also don't think it's really an acceptable end state of support; it leaves nix upgrade-nix busted and of course system-manager is also going to be busted, since the restorecon stuff is only applied manually to just the store paths you care about.

whether we are going to commit resources to fixing this is another question entirely imo. though i have not polled the core team, i haven't heard it brought up. there is the concern raised by the others that this is some weird sidecar metadata that the nix system just does not natively support and can get out of sync and that is real. but it already gets badly out of sync today with the installer hacks, so i am not convinced by that argument.

per Eelco:

fundamental problem: it attaches labels to files in the Nix store, which is mutable. It's like having setuid binaries in the store.

this is very much accurate. it's weird metadata that the store does not natively manage, and it will get desynced. it is quite gross that the labels are not actually transmitted through the system and that they are applied based on file paths.

looking through the discussion on 2670 and the other issue about SELinux support, it seems like the reaction to "we need to have extra metadata in the store" is "let the installer deal with just the specific paths in question", which leads to the exact state you arrived at. i also don't think it's really an acceptable end state of support; it leaves nix upgrade-nix busted and of course system-manager is also going to be busted, since the restorecon stuff is only applied manually to just the store paths you care about. whether we are going to commit resources to fixing this is another question entirely imo. though i have not polled the core team, i haven't heard it brought up. there *is* the concern raised by the others that this is some weird sidecar metadata that the nix system just does not natively support and can get out of sync and that *is* real. but it already gets badly out of sync today with the installer hacks, so i am not convinced by that argument. per Eelco: > fundamental problem: it attaches labels to files in the Nix store, which is mutable. It's like having setuid binaries in the store. this is very much accurate. it's weird metadata that the store does not natively manage, and it will get desynced. it is quite gross that the labels are not actually transmitted through the system and that they are applied based on file paths.
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#546
No description provided.