From 7286be7e81e23cd4d8abe9e26fe991b302b9f8d7 Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@nix.build>
Date: Wed, 2 Feb 2022 13:12:49 +0100
Subject: [PATCH] nixos/systemd-confinement: Allow shipped unit file

In issue #157787 @martined wrote:

  Trying to use confinement on packages providing their systemd units
  with systemd.packages, for example mpd, fails with the following
  error:

  system-units> ln: failed to create symbolic link
  '/nix/store/...-system-units/mpd.service': File exists

  This is because systemd-confinement and mpd both provide a mpd.service
  file through systemd.packages. (mpd got updated that way recently to
  use upstream's service file)

To address this, we now place the unit file containing the bind-mounted
paths of the Nix closure into a drop-in directory instead of using the
name of a unit file directly.

This does come with the implication that the options set in the drop-in
directory won't apply if the main unit file is missing. In practice
however this should not happen for two reasons:

  * The systemd-confinement module already sets additional options via
    systemd.services and thus we should get a main unit file
  * In the unlikely event that we don't get a main unit file regardless
    of the previous point, the unit would be a no-op even if the options
    of the drop-in directory would apply

Another thing to consider is the order in which those options are
merged, since systemd loads the files from the drop-in directory in
alphabetical order. So given that we have confinement.conf and
overrides.conf, the confinement options are loaded before the NixOS
overrides.

Since we're only setting the BindReadOnlyPaths option, the order isn't
that important since all those paths are merged anyway and we still
don't lose the ability to reset the option since overrides.conf comes
afterwards.

Fixes: https://github.com/NixOS/nixpkgs/issues/157787
Signed-off-by: aszlig <aszlig@nix.build>
---
 .../modules/security/systemd-confinement.nix  |  4 +--
 nixos/tests/systemd-confinement.nix           | 29 +++++++++++++++++--
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/nixos/modules/security/systemd-confinement.nix b/nixos/modules/security/systemd-confinement.nix
index 0e3ec5af323..f3a2de3bf87 100644
--- a/nixos/modules/security/systemd-confinement.nix
+++ b/nixos/modules/security/systemd-confinement.nix
@@ -175,8 +175,8 @@ in {
       serviceName = "${name}.service";
       excludedPath = rootPaths;
     } ''
-      mkdir -p "$out/lib/systemd/system"
-      serviceFile="$out/lib/systemd/system/$serviceName"
+      mkdir -p "$out/lib/systemd/system/$serviceName.d"
+      serviceFile="$out/lib/systemd/system/$serviceName.d/confinement.conf"
 
       echo '[Service]' > "$serviceFile"
 
diff --git a/nixos/tests/systemd-confinement.nix b/nixos/tests/systemd-confinement.nix
index 8fafb11e1e8..3181af309a6 100644
--- a/nixos/tests/systemd-confinement.nix
+++ b/nixos/tests/systemd-confinement.nix
@@ -17,15 +17,19 @@ import ./make-test-python.nix {
       exit "''${ret:-1}"
     '';
 
-    mkTestStep = num: { config ? {}, testScript }: {
-      systemd.sockets."test${toString num}" = {
+    mkTestStep = num: {
+      testScript,
+      config ? {},
+      serviceName ? "test${toString num}",
+    }: {
+      systemd.sockets.${serviceName} = {
         description = "Socket for Test Service ${toString num}";
         wantedBy = [ "sockets.target" ];
         socketConfig.ListenStream = "/run/test${toString num}.sock";
         socketConfig.Accept = true;
       };
 
-      systemd.services."test${toString num}@" = {
+      systemd.services."${serviceName}@" = {
         description = "Confined Test Service ${toString num}";
         confinement = (config.confinement or {}) // { enable = true; };
         serviceConfig = (config.serviceConfig or {}) // {
@@ -135,6 +139,16 @@ import ./make-test-python.nix {
               machine.succeed('test "$(chroot-exec \'cat "$FOOBAR"\')" = eek')
         '';
       }
+      { serviceName = "shipped-unitfile";
+        config.confinement.mode = "chroot-only";
+        testScript = ''
+          with subtest("check if shipped unit file still works"):
+              machine.succeed(
+                  'chroot-exec \'kill -9 $$ 2>&1 || :\' | '
+                  'grep -q "Too many levels of symbolic links"'
+              )
+        '';
+      }
     ];
 
     options.__testSteps = lib.mkOption {
@@ -143,6 +157,15 @@ import ./make-test-python.nix {
     };
 
     config.environment.systemPackages = lib.singleton testClient;
+    config.systemd.packages = lib.singleton (pkgs.writeTextFile {
+      name = "shipped-unitfile";
+      destination = "/etc/systemd/system/shipped-unitfile@.service";
+      text = ''
+        [Service]
+        SystemCallFilter=~kill
+        SystemCallErrorNumber=ELOOP
+      '';
+    });
 
     config.users.groups.chroot-testgroup = {};
     config.users.users.chroot-testuser = {