From 9e9af4f9c076f382bc40821551beaeb68ca071cd Mon Sep 17 00:00:00 2001
From: aszlig <aszlig@nix.build>
Date: Thu, 14 Mar 2019 19:48:20 +0100
Subject: [PATCH] nixos/confinement: Allow to include the full unit

From @edolstra at [1]:

  BTW we probably should take the closure of the whole unit rather than
  just the exec commands, to handle things like Environment variables.

With this commit, there is now a "fullUnit" option, which can be enabled
to include the full closure of the service unit into the chroot.

However, I did not enable this by default, because I do disagree here
and *especially* things like environment variables or environment files
shouldn't be in the closure of the chroot.

For example if you have something like:

  { pkgs, ... }:

  {
    systemd.services.foobar = {
      serviceConfig.EnvironmentFile = ${pkgs.writeText "secrets" ''
        user=admin
        password=abcdefg
      '';
    };
  }

We really do not want the *file* to end up in the chroot, but rather
just the environment variables to be exported.

Another thing is that this makes it less predictable what actually will
end up in the chroot, because we have a "globalEnvironment" option that
will get merged in as well, so users adding stuff to that option will
also make it available in confined units.

I also added a big fat warning about that in the description of the
fullUnit option.

[1]: https://github.com/NixOS/nixpkgs/pull/57519#issuecomment-472855704

Signed-off-by: aszlig <aszlig@nix.build>
---
 .../modules/security/systemd-confinement.nix  | 27 ++++++++++++++++---
 nixos/tests/systemd-confinement.nix           | 13 +++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/nixos/modules/security/systemd-confinement.nix b/nixos/modules/security/systemd-confinement.nix
index a8367ca5eed..fc0ce020afc 100644
--- a/nixos/modules/security/systemd-confinement.nix
+++ b/nixos/modules/security/systemd-confinement.nix
@@ -21,6 +21,22 @@ in {
         '';
       };
 
+      options.confinement.fullUnit = lib.mkOption {
+        type = types.bool;
+        default = false;
+        description = ''
+          Whether to include the full closure of the systemd unit file into the
+          chroot, instead of just the dependencies for the executables.
+
+          <warning><para>While it may be tempting to just enable this option to
+          make things work quickly, please be aware that this might add paths
+          to the closure of the chroot that you didn't anticipate. It's better
+          to use <option>confinement.packages</option> to <emphasis
+          role="strong">explicitly</emphasis> add additional store paths to the
+          chroot.</para></warning>
+        '';
+      };
+
       options.confinement.packages = lib.mkOption {
         type = types.listOf (types.either types.str types.package);
         default = [];
@@ -32,7 +48,9 @@ in {
           ${lib.concatMapStringsSep ", " mkScOption [
             "ExecReload" "ExecStartPost" "ExecStartPre" "ExecStop"
             "ExecStopPost"
-          ]} and ${mkScOption "ExecStart"} options.
+          ]} and ${mkScOption "ExecStart"} options. If you want to have all the
+          dependencies of this systemd unit, you can use
+          <option>confinement.fullUnit</option>.
 
           <note><para><emphasis role="strong">Only</emphasis> the latter
           (${mkScOption "ExecStart"}) will be used if
@@ -87,7 +105,7 @@ in {
 
       config = let
         rootName = "${mkPathSafeName name}-chroot";
-        inherit (config.confinement) binSh;
+        inherit (config.confinement) binSh fullUnit;
       in lib.mkIf config.confinement.enable {
         serviceConfig = {
           RootDirectory = pkgs.runCommand rootName {} "mkdir \"$out\"";
@@ -111,7 +129,10 @@ in {
           execPkgs = lib.concatMap (opt: let
             isSet = config.serviceConfig ? ${opt};
           in lib.optional isSet config.serviceConfig.${opt}) execOpts;
-        in execPkgs ++ lib.optional (binSh != null) binSh;
+          unitAttrs = toplevelConfig.systemd.units."${name}.service";
+          allPkgs = lib.singleton (builtins.toJSON unitAttrs);
+          unitPkgs = if fullUnit then allPkgs else execPkgs;
+        in unitPkgs ++ lib.optional (binSh != null) binSh;
       };
     }));
   };
diff --git a/nixos/tests/systemd-confinement.nix b/nixos/tests/systemd-confinement.nix
index 63cb074d7ca..b7b10fb36aa 100644
--- a/nixos/tests/systemd-confinement.nix
+++ b/nixos/tests/systemd-confinement.nix
@@ -132,6 +132,19 @@ import ./make-test.nix {
           );
         '';
       }
+      { description = "check if only Exec* dependencies are included";
+        config.environment.FOOBAR = pkgs.writeText "foobar" "eek\n";
+        testScript = ''
+          $machine->succeed('test "$(chroot-exec \'cat "$FOOBAR"\')" != eek');
+        '';
+      }
+      { description = "check if all unit dependencies are included";
+        config.environment.FOOBAR = pkgs.writeText "foobar" "eek\n";
+        config.confinement.fullUnit = true;
+        testScript = ''
+          $machine->succeed('test "$(chroot-exec \'cat "$FOOBAR"\')" = eek');
+        '';
+      }
     ];
 
     options.__testSteps = lib.mkOption {