about summary refs log tree commit diff
path: root/nixos/tests
diff options
context:
space:
mode:
authoraszlig <aszlig@nix.build>2024-05-06 14:50:15 +0200
committeraszlig <aszlig@nix.build>2024-05-13 00:40:40 +0200
commit0a9cecc35a651a020f66a4cc2a8333e33558650d (patch)
tree0b87cf46bb37379459c088144374984e90259525 /nixos/tests
parent27f36b5e57ca0eab08728eac58b12c3b3ca67c33 (diff)
nixos/systemd-confinement: Make / read-only
Our more thorough parametrised tests uncovered that with the changes for
supporting DynamicUser, we now have the situation that for static users
the root directory within the confined environment is now writable for
the user in question.

This is obviously not what we want and I'd consider that a regression.
However while discussing this with @ju1m and my suggestion being to
set TemporaryFileSystem to "/" (as we had previously), they had an even
better idea[1]:

> The goal is to deny write access to / to non-root users,
>
>   * TemporaryFileSystem=/ gives us that through the ownership of / by
>     root (instead of the service's user inherited from
>     RuntimeDirectory=).
>   * ProtectSystem=strict gives us that by mounting / read-only (while
>     keeping its ownership to the service's user).
>
> To avoid the incompatibilities of TemporaryFileSystem=/ mentioned
> above, I suggest to mount / read-only in all cases with
> ReadOnlyPaths = [ "+/" ]:
>
>   ...
>
> I guess this would require at least two changes to the current tests:
>
>   1. to no longer expect root to be able to write to some paths (like
>      /bin) (at least not without first remounting / in read-write
>      mode).
>   2. to no longer expect non-root users to fail to write to certain
>      paths with a "permission denied" error code, but with a
>      "read-only file system" error code.

I like the solution with ReadOnlyPaths even more because it further
reduces the attack surface if the user is root. In chroot-only mode this
is especially useful, since if there are no other bind-mounted paths
involved in the unit configuration, the whole file system within the
confined environment is read-only.

[1]: https://github.com/NixOS/nixpkgs/pull/289593#discussion_r1586794215

Signed-off-by: aszlig <aszlig@nix.build>
Diffstat (limited to 'nixos/tests')
-rw-r--r--nixos/tests/systemd-confinement/default.nix15
1 files changed, 7 insertions, 8 deletions
diff --git a/nixos/tests/systemd-confinement/default.nix b/nixos/tests/systemd-confinement/default.nix
index 44a1f9ca195bb..15d442d476b08 100644
--- a/nixos/tests/systemd-confinement/default.nix
+++ b/nixos/tests/systemd-confinement/default.nix
@@ -87,11 +87,11 @@ import ../make-test-python.nix {
           assert os.getgid() == 0
 
           assert_permissions({
-            'bin': Accessibility.WRITABLE,
-            'nix': Accessibility.WRITABLE,
-            'run': Accessibility.WRITABLE,
+            'bin': Accessibility.READABLE,
+            'nix': Accessibility.READABLE,
+            'run': Accessibility.READABLE,
             ${lib.optionalString privateTmp "'tmp': Accessibility.STICKY,"}
-            ${lib.optionalString privateTmp "'var': Accessibility.WRITABLE,"}
+            ${lib.optionalString privateTmp "'var': Accessibility.READABLE,"}
             ${lib.optionalString privateTmp "'var/tmp': Accessibility.STICKY,"}
           })
         '' else ''
@@ -120,8 +120,8 @@ import ../make-test-python.nix {
           assert os.getgid() == 0
 
           assert_permissions({
-            'bin': Accessibility.WRITABLE,
-            'nix': Accessibility.WRITABLE,
+            'bin': Accessibility.READABLE,
+            'nix': Accessibility.READABLE,
             ${lib.optionalString privateTmp "'tmp': Accessibility.STICKY,"}
             'run': Accessibility.WRITABLE,
 
@@ -129,7 +129,7 @@ import ../make-test-python.nix {
             'sys': Accessibility.SPECIAL,
             'dev': Accessibility.WRITABLE,
 
-            ${lib.optionalString privateTmp "'var': Accessibility.WRITABLE,"}
+            ${lib.optionalString privateTmp "'var': Accessibility.READABLE,"}
             ${lib.optionalString privateTmp "'var/tmp': Accessibility.STICKY,"}
           })
         '' else ''
@@ -144,7 +144,6 @@ import ../make-test-python.nix {
 
             'proc': Accessibility.SPECIAL,
             'sys': Accessibility.SPECIAL,
-
             'dev': Accessibility.SPECIAL,
             'dev/shm': Accessibility.STICKY,
             'dev/mqueue': Accessibility.STICKY,