Skip to content

Comments

nixos/config/sysctl: enable common TOCTOU vulnerability mitigations#391473

Open
illdefined wants to merge 1 commit intoNixOS:masterfrom
illdefined:sysctl-toctou
Open

nixos/config/sysctl: enable common TOCTOU vulnerability mitigations#391473
illdefined wants to merge 1 commit intoNixOS:masterfrom
illdefined:sysctl-toctou

Conversation

@illdefined
Copy link
Contributor

This enables mitigations for some common time‐of‐check‐time‐of‐use (TOCTOU) race vulnerabilities by default, mostly related to use of /tmp.

cf. https://www.kernel.org/doc/Documentation/admin-guide/sysctl/fs.rst

These could instead be moved to the hardened profile, they should however be safe to enable on all systems.

I have been running all my systems with these enabled for years without running into any problems.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 20, 2025
@illdefined illdefined marked this pull request as ready for review March 20, 2025 05:55
@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 26, 2025
@06kellyjac
Copy link
Member

Seems sensible to me.

Some other sources:

fs.suid_dumpable defaults to 0 anyway but may still be worth adding just for coverage/visibility. Also covered by disabling all coredumps.

Also could do with a merge conflict resolution and maybe some release notes just to be safe.

@alyssais alyssais requested a review from a team July 3, 2025 12:22
@K900
Copy link
Contributor

K900 commented Jul 3, 2025

This feels spooky. Do we have evidence of other distros shipping this?

@06kellyjac
Copy link
Member

One of my Arch machines (I don't think I've changed anything):

fs.protected_fifos = 1
fs.protected_hardlinks = 1
fs.protected_regular = 1
fs.protected_symlinks = 1
fs.suid_dumpable = 2

My current NixOS output gives the same (even though I'm pretty sure I've hardened some other stuff)

Documentation for /proc/sys/fs/
FYI https://sysctl-explorer.net/fs/suid_dumpable/ documents everything as +1.
proudier/sysctl-explorer#4

Even if it doesn't ship as default for any distro the links above show those settings in many hardening guides and would fit in the hardened profile.

@illdefined
Copy link
Contributor Author

I just noticed that these sysctls are already being set as part of the systemd defaults: https://github.com/systemd/systemd/blob/main/sysctl.d/50-default.conf#L50

This feels spooky. Do we have evidence of other distros shipping this?

Debian & Ubuntu: https://salsa.debian.org/kernel-team/linux-base/-/blob/debian/latest/sysctl.d/50-default.conf?ref_type=heads#L50

Please note the slight variation in the default values. The value 2 extends the protection for protected_fifos and protected_regular to non-owned group-writable sticky directories, in addition to world-writable ones like /tmp.

@illdefined
Copy link
Contributor Author

I just noticed that these sysctls are already being set as part of the systemd defaults: https://github.com/systemd/systemd/blob/main/sysctl.d/50-default.conf#L50

This default configuration file is moved out of the way by the systemd package (cf. https://github.com/NixOS/nixpkgs/blob/nixos-unstable/pkgs/os-specific/linux/systemd/default.nix#L810) but then installed by the systemd coredump NixOS module: https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/system/boot/systemd/coredump.nix#L71

This means that these defaults are set if and only if systemd.coredump.enable is set.

I would prefer to always set these sysctls, irrespective of the above configuration option.

I believe that defaults of 1 for all of them should be safe, as it is the default for all major Linux distributions. This should cover common TOCTOU vulnerablities involving /tmp and /var/tmp (which usually are world‐writable sticky directories).

Defaults of 2 for fs.protected_regular and fs.protected_fifos would cover additional scenarios (group‐writable sticky directories) and might breaks things. Of those sysctls Debian and Ubuntu default fs.protected_regular to 2, which should indicate low risk.

I suggest to run all NixOS tests with this PR applied and see if anything breaks.

fs.suid_dumpable is not directly related and I’d prefer not to touch it as part of this PR. Setting it to something other than its default only makes sense in my opinion when running a service like systemd-coredump and this is already covered by the respective NixOS module.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 4, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 4, 2025
@illdefined illdefined removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 4, 2025
@nixpkgs-ci nixpkgs-ci bot added the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Jul 4, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 9.needs: reviewer This PR currently has no reviewers requested and needs attention. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants