Merge pull request #57519 (systemd-confinement)

Currently if you want to properly chroot a systemd service, you could do
it using BindReadOnlyPaths=/nix/store or use a separate derivation which
gathers the runtime closure of the service you want to chroot. The
former is the easier method and there is also a method directly offered
by systemd, called ProtectSystem, which still leaves the whole store
accessible. The latter however is a bit more involved, because you need
to bind-mount each store path of the runtime closure of the service you
want to chroot.

This can be achieved using pkgs.closureInfo and a small derivation that
packs everything into a systemd unit, which later can be added to
systemd.packages.

However, this process is a bit tedious, so the changes here implement
this in a more generic way.

Now if you want to chroot a systemd service, all you need to do is:

  {
    systemd.services.myservice = {
      description = "My Shiny Service";
      wantedBy = [ "multi-user.target" ];

      confinement.enable = true;
      serviceConfig.ExecStart = "${pkgs.myservice}/bin/myservice";
    };
  }

If more than the dependencies for the ExecStart* and ExecStop* (which
btw. also includes script and {pre,post}Start) need to be in the chroot,
it can be specified using the confinement.packages option. By default
(which uses the full-apivfs confinement mode), a user namespace is set
up as well and /proc, /sys and /dev are mounted appropriately.

In addition - and by default - a /bin/sh executable is provided, which
is useful for most programs that use the system() C library call to
execute commands via shell.

Unfortunately, there are a few limitations at the moment. The first
being that DynamicUser doesn't work in conjunction with tmpfs, because
systemd seems to ignore the TemporaryFileSystem option if DynamicUser is
enabled. I started implementing a workaround to do this, but I decided
to not include it as part of this pull request, because it needs a lot
more testing to ensure it's consistent with the behaviour without
DynamicUser.

The second limitation/issue is that RootDirectoryStartOnly doesn't work
right now, because it only affects the RootDirectory option and doesn't
include/exclude the individual bind mounts or the tmpfs.

A quirk we do have right now is that systemd tries to create a /usr
directory within the chroot, which subsequently fails. Fortunately, this
is just an ugly error and not a hard failure.

The changes also come with a changelog entry for NixOS 19.03, which is
why I asked for a vote of the NixOS 19.03 stable maintainers whether to
include it (I admit it's a bit late a few days before official release,
sorry for that):

  @samueldr:

    Via pull request comment[1]:

      +1 for backporting as this only enhances the feature set of nixos,
      and does not (at a glance) change existing behaviours.

    Via IRC:

      new feature: -1, tests +1, we're at zero, self-contained, with no
      global effects without actively using it, +1, I think it's good

  @lheckemann:

    Via pull request comment[2]:

      I'm neutral on backporting. On the one hand, as @samueldr says,
      this doesn't change any existing functionality. On the other hand,
      it's a new feature and we're well past the feature freeze, which
      AFAIU is intended so that new, potentially buggy features aren't
      introduced in the "stabilisation period". It is a cool feature
      though? :)

A few other people on IRC didn't have opposition either against late
inclusion into NixOS 19.03:

  @edolstra:  "I'm not against it"
  @infinisil: "+1 from me as well"
  @grahamc:   "IMO its up to the RMs"

So that makes +1 from @samueldr, 0 from @lheckemann, 0 from @edolstra
and +1 from @infinisil (even though he's not a release manager) and no
opposition from anyone, which is the reason why I'm merging this right
now.

I also would like to thank @infinisil, @edolstra and @danbst for their
reviews.

[1]: NixOS/nixpkgs#57519 (comment)
[2]: NixOS/nixpkgs#57519 (comment)

nixos/doc/manual/release-notes/rl-1903.xml

@@ -68,6 +68,17 @@
      <xref linkend="sec-kubernetes"/> for details.
     </para>
    </listitem>
+   <listitem>
+     <para>
+       There is now a set of <option>confinement</option> options for
+       <option>systemd.services</option>, which allows to restrict services
+       into a <citerefentry>
+        <refentrytitle>chroot</refentrytitle>
+        <manvolnum>2</manvolnum>
+      </citerefentry>ed environment that only contains the store paths from
+      the runtime closure of the service.
+     </para>
+   </listitem>
   </itemizedlist>
  </section>
 

nixos/modules/module-list.nix

@@ -172,6 +172,7 @@
   ./security/rtkit.nix
   ./security/wrappers/default.nix
   ./security/sudo.nix
+  ./security/systemd-confinement.nix
   ./services/admin/oxidized.nix
   ./services/admin/salt/master.nix
   ./services/admin/salt/minion.nix

nixos/modules/security/systemd-confinement.nix

@@ -0,0 +1,199 @@
+{ config, pkgs, lib, ... }:
+
+let
+  toplevelConfig = config;
+  inherit (lib) types;
+  inherit (import ../system/boot/systemd-lib.nix {
+    inherit config pkgs lib;
+  }) mkPathSafeName;
+in {
+  options.systemd.services = lib.mkOption {
+    type = types.attrsOf (types.submodule ({ name, config, ... }: {
+      options.confinement.enable = lib.mkOption {
+        type = types.bool;
+        default = false;
+        description = ''
+          If set, all the required runtime store paths for this service are
+          bind-mounted into a <literal>tmpfs</literal>-based <citerefentry>
+            <refentrytitle>chroot</refentrytitle>
+            <manvolnum>2</manvolnum>
+          </citerefentry>.
+        '';
+      };
+
+      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 = [];
+        description = let
+          mkScOption = optName: "<option>serviceConfig.${optName}</option>";
+        in ''
+          Additional packages or strings with context to add to the closure of
+          the chroot. By default, this includes all the packages from the
+          ${lib.concatMapStringsSep ", " mkScOption [
+            "ExecReload" "ExecStartPost" "ExecStartPre" "ExecStop"
+            "ExecStopPost"
+          ]} 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>The store paths listed in <option>path</option> are
+          <emphasis role="strong">not</emphasis> included in the closure as
+          well as paths from other options except those listed
+          above.</para></note>
+        '';
+      };
+
+      options.confinement.binSh = lib.mkOption {
+        type = types.nullOr types.path;
+        default = toplevelConfig.environment.binsh;
+        defaultText = "config.environment.binsh";
+        example = lib.literalExample "\${pkgs.dash}/bin/dash";
+        description = ''
+          The program to make available as <filename>/bin/sh</filename> inside
+          the chroot. If this is set to <literal>null</literal>, no
+          <filename>/bin/sh</filename> is provided at all.
+
+          This is useful for some applications, which for example use the
+          <citerefentry>
+            <refentrytitle>system</refentrytitle>
+            <manvolnum>3</manvolnum>
+          </citerefentry> library function to execute commands.
+        '';
+      };
+
+      options.confinement.mode = lib.mkOption {
+        type = types.enum [ "full-apivfs" "chroot-only" ];
+        default = "full-apivfs";
+        description = ''
+          The value <literal>full-apivfs</literal> (the default) sets up
+          private <filename class="directory">/dev</filename>, <filename
+          class="directory">/proc</filename>, <filename
+          class="directory">/sys</filename> and <filename
+          class="directory">/tmp</filename> file systems in a separate user
+          name space.
+
+          If this is set to <literal>chroot-only</literal>, only the file
+          system name space is set up along with the call to <citerefentry>
+            <refentrytitle>chroot</refentrytitle>
+            <manvolnum>2</manvolnum>
+          </citerefentry>.
+
+          <note><para>This doesn't cover network namespaces and is solely for
+          file system level isolation.</para></note>
+        '';
+      };
+
+      config = let
+        rootName = "${mkPathSafeName name}-chroot";
+        inherit (config.confinement) binSh fullUnit;
+        wantsAPIVFS = lib.mkDefault (config.confinement.mode == "full-apivfs");
+      in lib.mkIf config.confinement.enable {
+        serviceConfig = {
+          RootDirectory = pkgs.runCommand rootName {} "mkdir \"$out\"";
+          TemporaryFileSystem = "/";
+          PrivateMounts = lib.mkDefault true;
+
+          # https://github.com/NixOS/nixpkgs/issues/14645 is a future attempt
+          # to change some of these to default to true.
+          #
+          # If we run in chroot-only mode, having something like PrivateDevices
+          # set to true by default will mount /dev within the chroot, whereas
+          # with "chroot-only" it's expected that there are no /dev, /proc and
+          # /sys file systems available.
+          #
+          # However, if this suddenly becomes true, the attack surface will
+          # increase, so let's explicitly set these options to true/false
+          # depending on the mode.
+          MountAPIVFS = wantsAPIVFS;
+          PrivateDevices = wantsAPIVFS;
+          PrivateTmp = wantsAPIVFS;
+          PrivateUsers = wantsAPIVFS;
+          ProtectControlGroups = wantsAPIVFS;
+          ProtectKernelModules = wantsAPIVFS;
+          ProtectKernelTunables = wantsAPIVFS;
+        };
+        confinement.packages = let
+          execOpts = [
+            "ExecReload" "ExecStart" "ExecStartPost" "ExecStartPre" "ExecStop"
+            "ExecStopPost"
+          ];
+          execPkgs = lib.concatMap (opt: let
+            isSet = config.serviceConfig ? ${opt};
+          in lib.optional isSet config.serviceConfig.${opt}) execOpts;
+          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;
+      };
+    }));
+  };
+
+  config.assertions = lib.concatLists (lib.mapAttrsToList (name: cfg: let
+    whatOpt = optName: "The 'serviceConfig' option '${optName}' for"
+                    + " service '${name}' is enabled in conjunction with"
+                    + " 'confinement.enable'";
+  in lib.optionals cfg.confinement.enable [
+    { assertion = !cfg.serviceConfig.RootDirectoryStartOnly or false;
+      message = "${whatOpt "RootDirectoryStartOnly"}, but right now systemd"
+              + " doesn't support restricting bind-mounts to 'ExecStart'."
+              + " Please either define a separate service or find a way to run"
+              + " commands other than ExecStart within the chroot.";
+    }
+    { assertion = !cfg.serviceConfig.DynamicUser or false;
+      message = "${whatOpt "DynamicUser"}. Please create a dedicated user via"
+              + " the 'users.users' option instead as this combination is"
+              + " currently not supported.";
+    }
+  ]) config.systemd.services);
+
+  config.systemd.packages = lib.concatLists (lib.mapAttrsToList (name: cfg: let
+    rootPaths = let
+      contents = lib.concatStringsSep "\n" cfg.confinement.packages;
+    in pkgs.writeText "${mkPathSafeName name}-string-contexts.txt" contents;
+
+    chrootPaths = pkgs.runCommand "${mkPathSafeName name}-chroot-paths" {
+      closureInfo = pkgs.closureInfo { inherit rootPaths; };
+      serviceName = "${name}.service";
+      excludedPath = rootPaths;
+    } ''
+      mkdir -p "$out/lib/systemd/system"
+      serviceFile="$out/lib/systemd/system/$serviceName"
+
+      echo '[Service]' > "$serviceFile"
+
+      # /bin/sh is special here, because the option value could contain a
+      # symlink and we need to properly resolve it.
+      ${lib.optionalString (cfg.confinement.binSh != null) ''
+        binsh=${lib.escapeShellArg cfg.confinement.binSh}
+        realprog="$(readlink -e "$binsh")"
+        echo "BindReadOnlyPaths=$realprog:/bin/sh" >> "$serviceFile"
+      ''}
+
+      while read storePath; do
+        if [ -L "$storePath" ]; then
+          # Currently, systemd can't cope with symlinks in Bind(ReadOnly)Paths,
+          # so let's just bind-mount the target to that location.
+          echo "BindReadOnlyPaths=$(readlink -e "$storePath"):$storePath"
+        elif [ "$storePath" != "$excludedPath" ]; then
+          echo "BindReadOnlyPaths=$storePath"
+        fi
+      done < "$closureInfo/store-paths" >> "$serviceFile"
+    '';
+  in lib.optional cfg.confinement.enable chrootPaths) config.systemd.services);
+}

nixos/modules/system/boot/systemd-lib.nix

@@ -9,12 +9,11 @@ in rec {
 
   shellEscape = s: (replaceChars [ "\\" ] [ "\\\\" ] s);
 
+  mkPathSafeName = lib.replaceChars ["@" ":" "\\" "[" "]"] ["-" "-" "-" "" ""];
+
   makeUnit = name: unit:
-    let
-      pathSafeName = lib.replaceChars ["@" ":" "\\" "[" "]"] ["-" "-" "-" "" ""] name;
-    in
     if unit.enable then
-      pkgs.runCommand "unit-${pathSafeName}"
+      pkgs.runCommand "unit-${mkPathSafeName name}"
         { preferLocalBuild = true;
           allowSubstitutes = false;
           inherit (unit) text;
@@ -24,7 +23,7 @@ in rec {
           echo -n "$text" > $out/${shellEscape name}
         ''
     else
-      pkgs.runCommand "unit-${pathSafeName}-disabled"
+      pkgs.runCommand "unit-${mkPathSafeName name}-disabled"
         { preferLocalBuild = true;
           allowSubstitutes = false;
         }

nixos/tests/all-tests.nix

@@ -221,6 +221,7 @@ in
   switchTest = handleTest ./switch-test.nix {};
   syncthing-relay = handleTest ./syncthing-relay.nix {};
   systemd = handleTest ./systemd.nix {};
+  systemd-confinement = handleTest ./systemd-confinement.nix {};
   taskserver = handleTest ./taskserver.nix {};
   telegraf = handleTest ./telegraf.nix {};
   tomcat = handleTest ./tomcat.nix {};