Nearly all services assume coreutils is present / systemd-confinement doesn't

[ArdaXi]

Describe the bug I just tried to enable confinement for a service, and it broke because it had a bare ln line in ExecStartPre. systemd-confinement only looks at the closure of the Exec lines and does not include the service's path.

This could be solved in two ways, either systemd-confinement also takes the service's Path into account, or services are more explicit about relying on coreutils. When I had a look in the repo, I found a grand total of two services that call ${pkgs.coreutils}/bin/ln rather than just ln, and this is probably also extended to more binaries. However, confinement.fullUnit specifically exists so the default does not pull in coreutils.

To Reproduce Steps to reproduce the behavior:

1. Pick any service that relies on coreutils being in the path, I started with Grafana
2. Enable hardening for it:

   systemd.services.grafana = {
   confinement.enable = true;
   serviceConfig.BindPaths = ["/var/lib/grafana"];
   };

3. Activate and discover Grafana will no longer start:

   grafana-pre-start[6753]: /nix/store/65y806k0bs348cadw040qzga6jcd433c-unit-script-grafana-pre-start/bin/grafana-pre-start: line 2: ln: command not found

4. Add coreutils:

   systemd.services.grafana = {
   confinement = {
   enable = true;
   packages = [ pkgs.coreutils ];
   };
   serviceConfig.BindPaths = ["/var/lib/grafana"];
   };

5. See Grafana start fine

Expected behavior Service starts without needing to add coreutils.

Additional context I realise that coreutils has been a part of the default path for services for quite a long time and services are written with this in mind. I realise that actually changing this list is unlikely to happen, especially in the short term. I would however like to ask whether services should be written with this list in mind, or whether they should be explicit in their dependencies.

https://github.com/NixOS/nixpkgs/blob/df18d77f1ea455909d0799edfa2e425aa04faf14/nixos/modules/system/boot/systemd.nix#L255-L262

I guess my question boils down to this: Should users wanting to enable confinement themselves be expected to look at the unit file and add dependencies from the above list manually, risking breakage if a service is updated to use a listed dependency, should they when in doubt just enable fullUnit (contradicting the docs) or should services directly reference packages from this list so they'll be included in the Exec closures?

Notify maintainers @aszlig

[aszlig]

systemd.services.grafana = {
  confinement.enable = true;
  serviceConfig.BindPaths = ["/var/lib/grafana"];
};

Btw. you can also use StateDirectory:

systemd.services.grafana = {
  confinement.enable = true;
  serviceConfig.StateDirectory = "grafana";
};

I guess my question boils down to this: Should users wanting to enable confinement themselves be expected to look at the unit file and add dependencies from the above list manually, risking breakage if a service is updated to use a listed dependency, should they when in doubt just enable fullUnit (contradicting the docs) or should services directly reference packages from this list so they'll be included in the Exec closures?

This is something I pointed out in the initial pull request, which is why the documentation has the following note:

The store paths listed in path are not included in the closure as well as paths from other options except those listed above.

So yes, this is deliberate and the services either need to directly reference ${pkgs.coreutils} or have coreutils in confinement.packages.

In theory, we could also add path to confinement.packages by default, but it would only add to the options needed in order to have the most strict level of confinement. For example right now if you want it to be a strict as possible you need to set mode = "chroot-only" and binSh = null. Including path by default would add another mkForce on confinement.packages.

Of course, this is how I usually approach sandboxing services, which not only involves confinement but also other things such as SystemCallFilter, PrivateNetwork, etc... and I go from the most restrictive configuration to "just enough things allowed so the application works". I also tend to sometimes go through a few hoops to make sure the application in question uses the full store path rather than rely on PATH or ugly wrappers, but of course YMMV.

If people commonly expect that path is included in confinement.packages we could change the default, but given that it will be painful in terms of backwards-compatibility (eg. existing services using the module silently get new store paths in the chroot) I'd rather see very convincing arguments why this would be a good default.

[ArdaXi]

If people commonly expect that path is included in confinement.packages we could change the default, but given that it will be painful in terms of backwards-compatibility (eg. existing services using the module silently get new store paths in the chroot) I'd rather see very convincing arguments why this would be a good default.

I agree. I'd expect the risk to be low, seeing as that list of default paths hasn't really changed in the past 9 years, but it's a valid point. I would really prefer it if services were explicit about their dependencies. That's… rather impactful though, which is why I wanted to create this issue.

I'm not entirely sure to what extent those hardcoded extra paths are actually necessary, except that now services of course depend on it and it's going to be difficult to change. I do think it would be a good idea if services listed their dependencies explicitly. Perhaps this should be an RFC instead? Not entirely up-to-date on the process any more...

[stale[bot]]

I marked this as stale due to inactivity. → More info