-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
nixos/acme: harden systemd units #123258
nixos/acme: harden systemd units #123258
Conversation
nixos/modules/security/acme.nix
Outdated
RestrictRealtime = true; | ||
RestrictSUIDSGID = true; | ||
SystemCallArchitectures = "native"; | ||
SystemCallFilter = "@system-service ~@resources ~@ipc ~@keyring ~@resources ~@setuid"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the most controversial part honestly.
As I understand it, in a mixed allow/deny list the evaluation goes from left to right and adds and removes things in order. So I add @system-service
, then remove @resources
, @ipc
, etc. from it.
If you specify both types of this option (i.e. allow-listing and deny-listing), the first encountered will take precedence and will dictate the default action (termination or approval of a system call). Then the next occurrences of this option will add or delete the listed system calls from the set of the filtered system calls, depending of its type and the default action. (For example, if you have started with an allow list rule for read() and write(), and right after it add a deny list rule for write(), then write() will be removed from the set.)
Honestly I'm now not sure, why systemd-analyze security
still shows I allow @privileged
. I'm not worried about @privileged
, because we actually allow @chown
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional to have ~@resources
there twice? Also how does this affect a call to systemctl (for example to restart another service as configured in security.acme.certs.<cert>.postRun
)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional to have ~@resources there twice?
Indeed unintentional, thanks for spotting this.
Also how does this affect a call to systemctl (for example to restart another service as configured in security.acme.certs..postRun)?
This is what is in @resources
, so I think not at all.
❯ systemd-analyze syscall-filter @resources
@resources
# Alter resource settings
ioprio_set
mbind
migrate_pages
move_pages
nice
sched_setaffinity
sched_setattr
sched_setparam
sched_setscheduler
set_mempolicy
setpriority
setrlimit
Unless you meant the syscall filter in general. I think @default
, which is part of @system-service
takes care of that:
❯ systemd-analyze syscall-filter @default
@default
# System calls that are always permitted
brk
cacheflush
clock_getres
clock_getres_time64
clock_gettime
clock_gettime64
clock_nanosleep
clock_nanosleep_time64
execve
exit
exit_group
futex
futex_time64
get_robust_list
get_thread_area
getegid
getegid32
geteuid
geteuid32
getgid
getgid32
getgroups
getgroups32
getpgid
getpgrp
getpid
getppid
getresgid
getresgid32
getresuid
getresuid32
getrlimit
getsid
gettid
gettimeofday
getuid
getuid32
membarrier
mmap
mmap2
munmap
nanosleep
pause
prlimit64
restart_syscall
rseq
rt_sigreturn
sched_yield
set_robust_list
set_thread_area
set_tid_address
set_tls
sigreturn
time
ugetrlimit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the duplicate @resources
deny entry.
Type = "oneshot"; | ||
User = "acme"; | ||
Group = mkDefault "acme"; | ||
UMask = 0022; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✗ UMask= Files created by service are world-readable by default 0.1
I'm not sure, but can't we default to 0027
(640, 750) instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I've done that in https://github.com/NixOS/nixpkgs/pull/121750/files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly, this interferes with access to http-01 challenges.
[Sun May 16 16:42:35.722162 2021] [core:error] [pid 3269:tid 140107940099648] (13)Permission denied: [client 192.168.1.1:51694] AH00035: access to /.well-known/acme-challenge/YjIqXHTp3AifLwWilpIr2bOwrNaMqBtlkfMXeaGGXhA denied (filesystem path '/var/lib/acme/acme-challenge/.well-known') because search permissions are missing on a component of the path
[Sun May 16 16:42:37.718537 2021] [core:error] [pid 3269:tid 140107906528832] (13)Permission denied: [client 192.168.1.1:51698] AH00035: access to /.well-known/acme-challenge/YjIqXHTp3AifLwWilpIr2bOwrNaMqBtlkfMXeaGGXhA denied (filesystem path '/var/lib/acme/acme-challenge/.well-known') because search permissions are missing on a component of the path
[Sun May 16 16:42:37.719453 2021] [core:error] [pid 3269:tid 140107898136128] (13)Permission denied: [client 192.168.1.1:51700] AH00035: access to /.well-known/acme-challenge/YjIqXHTp3AifLwWilpIr2bOwrNaMqBtlkfMXeaGGXhA denied (filesystem path '/var/lib/acme/acme-challenge/.well-known') because search permissions are missing on a component of the path
[Sun May 16 16:42:40.271658 2021] [core:error] [pid 3269:tid 140107789030976] (13)Permission denied: [client 192.168.1.1:51706] AH00035: access to /.well-known/acme-challenge/5MYKzt-kQKMOzuxQ1y_tF_Fzz29w1v-S8VCYKZ4nL_E denied (filesystem path '/var/lib/acme/acme-challenge/.well-known') because search permissions are missing on a component of the path
[Sun May 16 16:42:42.274052 2021] [core:error] [pid 3271:tid 140107940099648] (13)Permission denied: [client 192.168.1.1:51710] AH00035: access to /.well-known/acme-challenge/5MYKzt-kQKMOzuxQ1y_tF_Fzz29w1v-S8VCYKZ4nL_E denied (filesystem path '/var/lib/acme/acme-challenge/.well-known') because search permissions are missing on a component of the path
[Sun May 16 16:42:43.275959 2021] [core:error] [pid 3270:tid 140107889743424] (13)Permission denied: [client 192.168.1.1:51714] AH00035: access to /.well-known/acme-challenge/5MYKzt-kQKMOzuxQ1y_tF_Fzz29w1v-S8VCYKZ4nL_E denied (filesystem path '/var/lib/acme/acme-challenge/.well-known') because search permissions are missing on a component of the path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah also #106603 which I almost forgot about!
I'm getting a warning on my system after applying these changes:
Is the syntax correct? I don't know what it should be personally. |
Hm, so it seems that is not the way to remove individual syscall groups after all. I'll be checking up the docs … again. |
@m1cr0man I've updated the I also added
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m1cr0man I've updated the
SystemCallFilter
and added some comments.I also added
ProtectSystem = "strict";
, so now paths you want to access need to be added toReadOnlyPaths
orReadWritePaths
. Does that sound reasonable to you?
Yeah so I've just done some testing with this. We prefix the postRun scripting with "+" which from the docs means it ignores most of the hardening being imposed here.
I modified the postRun test such that it writes to a file outside of /var/lib/acme and it was successful. This is something I would be concerned about breaking in existing setups but with this test I would be confident that they won't be affected.
Happy to see it merged now, but if you could apply this patch so that the test suite covers what I'm explaining above that would be great.
diff --git a/nixos/tests/acme.nix b/nixos/tests/acme.nix
index 6f98b0da378..edd387cae0c 100644
--- a/nixos/tests/acme.nix
+++ b/nixos/tests/acme.nix
@@ -105,9 +105,9 @@ in import ./make-test-python.nix ({ lib, ... }: {
security.acme.certs."a.example.test".keyType = "ec384";
security.acme.certs."a.example.test".postRun = ''
set -euo pipefail
- touch test
- chown root:root test
- echo testing > test
+ touch /home/test
+ chown root:root /home/test
+ echo testing > /home/test
'';
};
@@ -375,7 +375,7 @@ in import ./make-test-python.nix ({ lib, ... }: {
switch_to(webserver, "cert-change")
webserver.wait_for_unit("acme-finished-a.example.test.target")
check_connection_key_bits(client, "a.example.test", "384")
- webserver.succeed("grep testing /var/lib/acme/a.example.test/test")
+ webserver.succeed("grep testing /home/test")
with subtest("Correctly implements OCSP stapling"):
switch_to(webserver, "ocsp-stapling")
Rebased after merging #121750. |
Motivation for this change
This change applies some hopefully non-controversial hardening. Hardening is always a fickle business and I hope I didn't step on anyones usecase.
I've run through the acme test suite and it is still working 😀
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)