ft: add systemd hardening helpers #288418
Conversation
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/hardening-systemd-services/17147/28 |
hauleth
force-pushed
the
ft/add-systemd-hardening-helpers
branch
from
da8220d
to
26dfa54
Compare
|
I have updated the PR with some comments from the forum - now there is |
| enable = lib.mkOption { | ||
| type = types.bool; | ||
| default = false; | ||
| description = lib.mdDoc '' | ||
| Basic restrictions for systemd services | ||
| ''; | ||
| }; | ||
|
|
||
| execOnlyNix = lib.mkOption { | ||
| type = types.bool; | ||
| default = true; | ||
| description = lib.mdDoc '' | ||
| Mark whole system as non-executable with exception for `/nix/store`. | ||
| ''; | ||
| }; |
There was a problem hiding this comment.
Better use mkEnableOption for these.
There was a problem hiding this comment.
The problem with mkEnableOption is that it has very limited support for docs. Current docs are more of a stub rather than target docs and I would like to expand them more when there will be established syntax for all these options.
There was a problem hiding this comment.
I don't see how mkEnableOption would limit your docs in any way? The description is its one and only argument, it's just prefixed by "Whether to enable ..." which applies to all of them.
| ipSockets = lib.mkOption { | ||
| type = types.bool; | ||
| default = false; | ||
| description = lib.mdDoc '' | ||
| Allow using AF_INET and AF_INET6 | ||
| ''; | ||
| }; |
There was a problem hiding this comment.
The options are a bit inconsistent. Some are positive (enable, activate, do) and others negative (disable, only, deactivate, restrict).
This should either be a negative option too (i.e. noIpSockets) or all the other options should be flipped to be positive.
Usually, I'd prefer positive options over negative options but I could see the case for negative options specifically for the case of enabling hardening.
OTOH, I think there could be merit in treating our little abstraction like a permissions system for which positive options would fit better.
There was a problem hiding this comment.
Personally I prefer these options to have meaningful names. I never really liked negated options and instead I prefer approach that is shown here - we have enable option and then we enable/disable features we do not want. So not everything will be true by default and not everything will be false by default, but rather the "inconsistent mix". But if the NixOS modules approach is to take only one of these routes, then I will oblige.
There was a problem hiding this comment.
I meant negation on another level; the actual effect on the resulting service.
Currently, the options mostly say "enable <hardening>/<restriction>". These then enable restrictions internally which has the effect of disabling the service from doing some things; if you set the option true, the service would lose the ability to do some thing.
What I meant was to turn them into "allow <restricted thing>" instead. For example: fullProc, nonNixExec, ipSockets. Internally, these would disable a set of restrictions but the effect of that is that the service would be enabled to do these things; if you set the option true, the service would gain an ability to do some thing.
Do you see what I mean?
if the NixOS modules approach is to take only one of these routes, then I will oblige.
There is no standard for this that I'm aware of.
This is just about generally designing the interface well and one part of that is consistency. Ideally we would have consistency across all of NixOS but that's a harder problem to solve. Local consistency is achievable though and we should strive for it.
hauleth
force-pushed
the
ft/add-systemd-hardening-helpers
branch
from
26dfa54
to
6cb1804
Compare
This module adds `systemd.services.<name>.harden` set of options that helps with setting basic systemd units hardening. That should help with making more services hardened and responding with better security analysis grade without too much repetition. Most of the set values are marked with `mkDefault` to allow explicit opt-out for options that aren't needed without resolving to `mkForce` or similar.
hauleth
force-pushed
the
ft/add-systemd-hardening-helpers
branch
from
6cb1804
to
3011b77
Compare
|
I fear this will get extremely complex with lots of services needing edge case configurations. This would lead to a rather unpleasant mix of As the scope of this option is extremely wide I would propose creating an RFC for discussing a proper design (before it becomes stable and much less easy to change) with considerations for already existing hardening options like I don't want to go too much into said design discussion here but I think a better approach would be to have a |
I do not get why that would be unpleasant. We cannot cover everything with just custom helpers without almost literally copying everything into
That is pretty much how it works in current PR state. |
+1 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/pre-rfc-systemd-hardening/39772/1 |
Description of changes
This module adds
systemd.services.<name>.hardenset of options that helps with setting basic systemd units hardening. That should help with making more services hardened and responding with better security analysis grade without too much repetition. Most of the set values are marked withmkDefaultto allow explicit opt-out for options that aren't needed without resolving tomkForceor similar.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 馃憤 reaction to pull requests you find important.