Commit Graph

99 Commits

Author SHA1 Message Date
Arian van Putten
8afba669e2 nixos/wrappers: use normal mount for /run/wrappers
We want to get rid of specialFileSystems / earlyMountScript eventually and
there is no need to run this before systemd anymore now that
the wrappers themselves are set up in a systemd unit since https://github.com/NixOS/nixpkgs/pull/263203

Also this is needed to make soft-reboot work. We want to make sure
that we remount /run/wrappers with the nosuid bit removed on soft-reboot
but because @earlyMountScript@ happens in initrd, this wouldn't happen
2024-07-20 16:07:43 +02:00
stuebinm
6afb255d97 nixos: remove all uses of lib.mdDoc
these changes were generated with nixq 0.0.2, by running

  nixq ">> lib.mdDoc[remove] Argument[keep]" --batchmode nixos/**.nix
  nixq ">> mdDoc[remove] Argument[keep]" --batchmode nixos/**.nix
  nixq ">> Inherit >> mdDoc[remove]" --batchmode nixos/**.nix

two mentions of the mdDoc function remain in nixos/, both of which
are inside of comments.

Since lib.mdDoc is already defined as just id, this commit is a no-op as
far as Nix (and the built manual) is concerned.
2024-04-13 10:07:35 -07:00
edef
b4c9840652 nixos/modules/security/wrappers: limit argv0 to 512 bytes
This mitigates CVE-2023-6246, crucially without a mass-rebuild.

Change-Id: I762a0d489ade88dafd3775d54a09f555dc8c2527
2024-02-01 18:16:55 +00:00
nikstur
d0014a531e nixos/wrappers: order service after sysusers service 2023-12-29 03:41:45 +01:00
nikstur
c9569af3e0
Merge pull request #271326 from philiptaron/shutdown.target
treewide: depend on `shutdown.target` if `DefaultDependencies=no` in almost every case
2023-12-27 08:33:26 +01:00
Sandro Jäckel
35ca689119
nixos/wrapper: add basename of the wrapped program to the wrappers name to easily identify it
Also fix the comment with test instructions
2023-12-24 20:36:12 +01:00
Philip Taron
a7a5b2eca1
nixos/suid-sgid-wrappers: ensure correct ordering w.r.t. shutdown.target 2023-11-30 15:03:56 -08:00
nikstur
f827f7ad7b nixos/wrappers: replace activationScript
Create the wrappers via a separate systemd service.
2023-10-24 23:51:37 +02:00
edef
89e45f23db nixos/modules/security/wrappers: drop dead code 2023-10-11 08:49:32 +00:00
Ben Wolsieffer
b6876d5c86
nixos/security/wrappers: don't force PIE hardening (#259509)
PIE causes problems with static binaries on ARM (see 76552e9). It is
enabled by default on other platforms anyway when musl is used, so we
don't need to specify it manually.
2023-10-10 10:13:29 +02:00
edef
09325d24b6 nixos/security/wrappers: use musl rather than glibc and explicitly unset insecure env vars
This mitigates CVE-2023-4911, crucially without a mass-rebuild.

We drop insecure environment variables explicitly, including
glibc-specific ones, since musl doesn't do this by default.

Change-Id: I591a817e6d4575243937d9ccab51c23a96bed6f9
2023-10-05 22:04:05 +00:00
Robert Obryk
c64bbd4466 nixos/security/wrappers: remove all the assertions about readlink(/proc/self/exe)
Given that we are no longer inspecting the target of the /proc/self/exe
symlink, stop asserting that it has any properties. Remove the plumbing
for wrappersDir, which is no longer used.

Asserting that the binary is located in the specific place is no longer
necessary, because we don't rely on that location being writable only by
privileged entities (we used to rely on that when assuming that
readlink(/proc/self/exe) will continue to point at us and when assuming
that the `.real` file can be trusted).

Assertions about lack of write bits on the file were
IMO meaningless since inception: ignoring the Linux's refusal to honor
S[UG]ID bits on files-writeable-by-others, if someone could have
modified the wrapper in a way that preserved the capability or S?ID
bits, they could just remove this check.

Assertions about effective UID were IMO just harmful: if we were
executed without elevation, the caller would expect the result that
would cause in a wrapperless distro: the targets gets executed without
elevation. Due to lack of elevation, that cannot be used to abuse
privileges that the elevation would give.

This change partially fixes #98863 for S[UG]ID wrappers. The issue for
capability wrappers remains.
2023-08-27 14:10:38 +02:00
Robert Obryk
e3550208de nixos/security/wrappers: read capabilities off /proc/self/exe directly
/proc/self/exe is a "fake" symlink. When it's opened, it always opens
the actual file that was execve()d in this process, even if the file was
deleted or renamed; if the file is no longer accessible from the current
chroot/mount namespace it will at the very worst fail and never open the
wrong file. Thus, we can make a much simpler argument that we're reading
capabilities off the correct file after this change (and that argument
doesn't rely on things such as protected_hardlinks being enabled, or no
users being able to write to /run/wrappers, or the verification that the
path readlink returns starts with /run/wrappers/).
2023-08-27 14:10:38 +02:00
Robert Obryk
1bdbc0b0fe nixos/security/wrappers: stop using .real files
Before this change it was crucial that nonprivileged users are unable to
create hardlinks to SUID wrappers, lest they be able to provide a
different `.real` file alongside. That was ensured by not providing a
location writable to them in the /run/wrappers tmpfs, (unless
disabled) by the fs.protected_hardlinks=1 sysctl, and by the explicit
own-path check in the wrapper. After this change, ensuring
that property is no longer important, and the check is most likely
redundant.

The simplification of expectations of the wrapper will make it
easier to remove some of the assertions in the wrapper (which currently
cause the wrapper to fail in no_new_privs environments, instead of
executing the target with non-elevated privileges).

Note that wrappers had to be copied (not symlinked) into /run/wrappers
due to the SUID/capability bits, and they couldn't be hard/softlinks of
each other due to those bits potentially differing. Thus, this change
doesn't increase the amount of memory used by /run/wrappers.

This change removes part of the test that is obsoleted by the removal of
`.real` files.
2023-08-27 14:10:36 +02:00
Robert Obryk
44fde723be nixos/security/wrappers: generate a separate and more complete apparmor policy fragment for each wrapper
This change includes some stuff (e.g. reading of the `.real` file,
execution of the wrapper's target) that belongs to the apparmor policy
of the wrapper. This necessitates making them distinct for each wrapper.
The main reason for this change is as a preparation for making each
wrapper be a distinct binary.
2023-08-27 14:10:07 +02:00
Pierre Bourdon
4428f3a79a
Revert "nixos/security/wrappers: simplifications and a fix for #98863" 2023-08-24 08:35:11 +02:00
Robert Obryk
ff204ca32b nixos/security/wrappers: remove all the assertions about readlink(/proc/self/exe)
Given that we are no longer inspecting the target of the /proc/self/exe
symlink, stop asserting that it has any properties. Remove the plumbing
for wrappersDir, which is no longer used.

Asserting that the binary is located in the specific place is no longer
necessary, because we don't rely on that location being writable only by
privileged entities (we used to rely on that when assuming that
readlink(/proc/self/exe) will continue to point at us and when assuming
that the `.real` file can be trusted).

Assertions about lack of write bits on the file were
IMO meaningless since inception: ignoring the Linux's refusal to honor
S[UG]ID bits on files-writeable-by-others, if someone could have
modified the wrapper in a way that preserved the capability or S?ID
bits, they could just remove this check.

Assertions about effective UID were IMO just harmful: if we were
executed without elevation, the caller would expect the result that
would cause in a wrapperless distro: the targets gets executed without
elevation. Due to lack of elevation, that cannot be used to abuse
privileges that the elevation would give.

This change partially fixes #98863 for S[UG]ID wrappers. The issue for
capability wrappers remains.
2023-08-16 11:33:22 +02:00
Robert Obryk
11ca4dcbb8 nixos/security/wrappers: read capabilities off /proc/self/exe directly
/proc/self/exe is a "fake" symlink. When it's opened, it always opens
the actual file that was execve()d in this process, even if the file was
deleted or renamed; if the file is no longer accessible from the current
chroot/mount namespace it will at the very worst fail and never open the
wrong file. Thus, we can make a much simpler argument that we're reading
capabilities off the correct file after this change (and that argument
doesn't rely on things such as protected_hardlinks being enabled, or no
users being able to write to /run/wrappers, or the verification that the
path readlink returns starts with /run/wrappers/).
2023-08-16 11:33:22 +02:00
Robert Obryk
ec36e0218f nixos/security/wrappers: stop using .real files
Before this change it was crucial that nonprivileged users are unable to
create hardlinks to SUID wrappers, lest they be able to provide a
different `.real` file alongside. That was ensured by not providing a
location writable to them in the /run/wrappers tmpfs, (unless
disabled) by the fs.protected_hardlinks=1 sysctl, and by the explicit
own-path check in the wrapper. After this change, ensuring
that property is no longer important, and the check is most likely
redundant.

The simplification of expectations of the wrapper will make it
easier to remove some of the assertions in the wrapper (which currently
cause the wrapper to fail in no_new_privs environments, instead of
executing the target with non-elevated privileges).

Note that wrappers had to be copied (not symlinked) into /run/wrappers
due to the SUID/capability bits, and they couldn't be hard/softlinks of
each other due to those bits potentially differing. Thus, this change
doesn't increase the amount of memory used by /run/wrappers.
2023-08-16 11:33:22 +02:00
Guillaume Girol
0e4b8a05b2 nixos/wrappers: allow setuid and setgid wrappers to run in user namespaces
In user namespaces where an unprivileged user is mapped as root and root
is unmapped, setuid bits have no effect. However setuid root
executables like mount are still usable *in the namespace* as the user
already has the required privileges. This commit detects the situation
where the wrapper gained no privileges that the parent process did not
already have and in this case does less sanity checking. In short there
is no need to be picky since the parent already can execute the foo.real
executable themselves.

Details:
man 7 user_namespaces:
   Set-user-ID and set-group-ID programs
       When a process inside a user namespace executes a set-user-ID
       (set-group-ID) program, the process's effective user (group) ID
       inside the namespace is changed to whatever value is mapped for
       the user (group) ID of the file.  However, if either the user or
       the group ID of the file has no mapping inside the namespace, the
       set-user-ID (set-group-ID) bit is silently ignored: the new
       program is executed, but the process's effective user (group) ID
       is left unchanged.  (This mirrors the semantics of executing a
       set-user-ID or set-group-ID program that resides on a filesystem
       that was mounted with the MS_NOSUID flag, as described in
       mount(2).)

The effect of the setuid bit is that the real user id is preserved and
the effective and set user ids are changed to the owner of the wrapper.
We detect that no privilege was gained by checking that euid == suid
== ruid. In this case we stop checking that euid == owner of the
wrapper file.

As a reminder here are the values of euid, ruid, suid, stat.st_uid and
stat.st_mode & S_ISUID in various cases when running a setuid 42 executable as user 1000:

Normal case:
ruid=1000 euid=42 suid=42
setuid=2048, st_uid=42

nosuid mount:
ruid=1000 euid=1000 suid=1000
setuid=2048, st_uid=42

inside unshare -rm:
ruid=0 euid=0 suid=0
setuid=2048, st_uid=65534

inside unshare -rm, on a suid mount:
ruid=0 euid=0 suid=0
setuid=2048, st_uid=65534
2023-08-09 12:00:00 +00:00
Robert Hensing
2e2f0d28ea nixos: Use checks instead of extraDependencies
... as appropriate.

This drops a few unnecessary store paths from the system closure.
2023-05-11 21:18:38 +02:00
figsoda
6bb0dbf91f nixos: fix typos 2022-12-17 19:31:14 -05:00
Jason Yundt
17352e8995 nixos/security/wrappers: clarify required format for capabilities
Before this change, the description for
security.wrappers.<name>.capabilities made it seem like you could just
string together the names of capabilities like this:

  capabilities = "CAP_SETUID,CAP_SETGID";

In reality, each item in the list must be a full-on capability clause:

  capabilities = "CAP_SETUID=ep,CAP_SETGID+i";
2022-09-11 16:36:58 +02:00
pennae
722b99bc0e nixos/*: convert options with admonitions to MD
rendering changes only slightly, most changes are in spacing.
2022-08-31 16:36:16 +02:00
pennae
9547123258 nixos/*: convert internal option descriptions to MD
we'll have to do it eventually, may as well be now.
2022-08-31 16:32:54 +02:00
pennae
ef176dcf7e nixos/*: automatically convert option descriptions
conversions were done using https://github.com/pennae/nix-doc-munge
using (probably) rev f34e145 running

    nix-doc-munge nixos/**/*.nix
    nix-doc-munge --import nixos/**/*.nix

the tool ensures that only changes that could affect the generated
manual *but don't* are committed, other changes require manual review
and are discarded.
2022-08-31 16:32:53 +02:00
Robert Hensing
d00583540b
Merge pull request #184368 from DieracDelta/jr/wrappers-run-size-option
nixos/security: add size option to /run/wrappers
2022-08-14 19:13:17 +02:00
Justin Restivo
82640adbf0 nixos/security: add size option to /run/wrappers 2022-08-14 07:31:37 -07:00
pennae
423545fe48 nixos/*: normalize manpage references to single-line form
now nix-doc-munge will not introduce whitespace changes when it replaces
manpage references with the MD equivalent.

no change to the manpage, changes to the HTML manual are whitespace only.
2022-08-05 18:34:50 +02:00
pennae
2e751c0772 treewide: automatically md-convert option descriptions
the conversion procedure is simple:

 - find all things that look like options, ie calls to either `mkOption`
   or `lib.mkOption` that take an attrset. remember the attrset as the
   option
 - for all options, find a `description` attribute who's value is not a
   call to `mdDoc` or `lib.mdDoc`
 - textually convert the entire value of the attribute to MD with a few
   simple regexes (the set from mdize-module.sh)
 - if the change produced a change in the manual output, discard
 - if the change kept the manual unchanged, add some text to the
   description to make sure we've actually found an option. if the
   manual changes this time, keep the converted description

this procedure converts 80% of nixos options to markdown. around 2000
options remain to be inspected, but most of those fail the "does not
change the manual output check": currently the MD conversion process
does not faithfully convert docbook tags like <code> and <package>, so
any option using such tags will not be converted at all.
2022-07-30 15:16:34 +02:00
Mario Rodas
cc73dc83b3
Revert "nixos/security/wrappers: use an assertion for the existence check" 2022-06-12 15:38:05 -05:00
Naïm Favier
39a56c7696
nixos/security/wrappers: use an assertion for the existence check
A simpler implementation of 7d8b303e3f
that uses an assertion instead of a derivation.

`pathHasContext` seems a bit better than `hasPrefix storeDir` because it
avoids a string comparison, and catches nonsense like
`"foo${pkgs.hello}bar"`.
2022-06-11 23:22:03 +02:00
Linus Heckemann
7c035dbb75
Merge pull request #156822 from xfix/wrapper-assert-argc-at-least-one
nixos/wrappers: require argc to be at least one
2022-05-16 18:52:51 +02:00
Ivan Kozik
f18cc2cf02 nixos/security/wrappers: chown user:group instead of user.group to fix warnings from coreutils 9.1
activating the configuration...
setting up /etc...
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.messagebus’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
chown: warning: '.' should be ':': ‘root.root’
reloading user units for root...
2022-05-05 22:05:18 +00:00
Alyssa Ross
1176525f87 treewide: remove obsolete kernel version checks
We don't support Linux kernels older than 4.4 in Nixpkgs.
2022-02-19 21:09:19 +00:00
Konrad Borowski
2a6a3d2c47 nixos/wrappers: require argc to be at least one
setuid applications were exploited in the past with an empty
argv, such as pkexec using CVE-2021-4034.
2022-01-28 12:26:20 +01:00
Konrad Borowski
1009d6e79e nixos/wrappers: create a new assert macro that always asserts
C's assert macro only works when NDEBUG is undefined. Previously
NDEBUG was undefined incorrectly which meant that the assert
macros in wrapper.c did not work.
2022-01-28 12:26:19 +01:00
Julien Moutinho
0e5611e0be security/wrappers: remove C compiler from the nixos/security.wrappers AppArmor profile 2021-12-29 16:26:57 +01:00
Naïm Favier
2ddc335e6f
nixos/doc: clean up defaults and examples 2021-10-04 12:47:20 +02:00
rnhmjoj
fedd7cd690
nixos: explicitely set security.wrappers ownership
This is slightly more verbose and inconvenient, but it forces you
to think about what the wrapper ownership and permissions will be.
2021-09-13 13:48:13 +02:00
rnhmjoj
27dcb04cde
nixos/security/wrappers: remove WRAPPER_PATH
This appears to be a leftover from 628e6a83.
2021-09-13 13:48:13 +02:00
rnhmjoj
936e8eaf41
nixos/security/wrappers: fix shell quoting 2021-09-13 13:48:12 +02:00
rnhmjoj
7d8b303e3f
nixos/security/wrappers: check that sources exist
Add a shell script that checks if the paths of all wrapped programs
actually exist to catch mistakes. This only checks for Nix store paths,
which are always expected to exist at build time.
2021-09-13 10:38:04 +02:00
rnhmjoj
22004f7e8f
nixos/security/wrappers: use fixed defaults
To keep backward compatibility and have a typing would require making
all options null by default, adding a defaultText containing the actual
value, write the default value logic based on `!= null` and replacing
the nulls laters. This pretty much defeats the point of having used
a submodule type.
2021-09-12 21:43:25 +02:00
rnhmjoj
904f68fb0f
nixos/security/wrappers: make well-typed
The security.wrappers option is morally a set of submodules but it's
actually (un)typed as a generic attribute set. This is bad for several
reasons:

1. Some of the "submodule" option are not document;
2. the default values are not documented and are chosen based on
   somewhat bizarre rules (issue #23217);
3. It's not possible to override an existing wrapper due to the
   dumb types.attrs.merge strategy;
4. It's easy to make mistakes that will go unnoticed, which is
   really bad given the sensitivity of this module (issue #47839).

This makes the option a proper set of submodule and add strict types and
descriptions to every sub-option. Considering it's not yet clear if the
way the default values are picked is intended, this reproduces the current
behavior, but it's now documented explicitly.
2021-09-12 21:43:03 +02:00
Julien Moutinho
05d334cfe2 Revert "Revert "apparmor: fix and improve the service""
This reverts commit 420f89ceb2.
2021-04-23 07:17:55 +02:00
Jörg Thalheim
dbd05a5289
Update nixos/modules/security/wrappers/wrapper.nix
Co-authored-by: Cole Helbling <cole.e.helbling@outlook.com>
2021-01-14 09:00:34 +00:00
Jörg Thalheim
eadffd9154
nixos/wrappers: fix applying capabilities
With libcap 2.41 the output of cap_to_text changed, also the original
author of code hoped that this would never happen.
To counter this now the security-wrapper only relies on the syscall
ABI, which is more stable and robust than string parsing. If new
breakages occur this will be more obvious because version numbers will
be incremented.
Furthermore all errors no make execution explicitly fail instead of
hiding errors behind debug environment variables and the code style was
more consistent with no goto fail; goto fail; vulnerabilities (https://gotofail.com/)
2021-01-14 08:46:57 +01:00
Graham Christensen
bc49a0815a
utillinux: rename to util-linux 2020-11-24 12:42:06 -05:00
Vladimír Čunát
420f89ceb2
Revert "apparmor: fix and improve the service"
This reverts commit fb6d63f3fd.

I really hope this finally fixes #99236: evaluation on Hydra.
This time I really did check basically the same commit on Hydra:
https://hydra.nixos.org/eval/1618011

Right now I don't have energy to find what exactly is wrong in the
commit, and it doesn't seem important in comparison to nixos-unstable
channel being stuck on a commit over one week old.
2020-10-07 12:22:18 +02:00