core: do not enforce PrivateTmp with DynamicUser if /tmp and /var/tmp are already private tmpfs

[bluca]

DynamicUser= enables PrivateTmp= implicitly to avoid files owned by reusable uids
leaking into the host. Configuring TemporaryFileSystem=/tmp/ /var/ also ensures
this, so allow it as an alternative, since it has less impactful semantics with
respect to PrivateTmp=yes, which links the mount namespace to the host's /tmp/
instead.

[github-actions]

Note

We had successfully released a new major release. We are no longer in a development freeze phase.
We will try our best to get back to your PR as soon as possible. Thank you for your patience.

[yuwata]

Should we also check InaccessiblePaths=, BindReadOnlyPaths= ??

[bluca]

Should we also check InaccessiblePaths=, BindReadOnlyPaths= ??

Not sure, but if so then it's a pre-existing issue as we don't do any enforcement in PrivateTmp=yes either, so it can be handled in a separate PR if needed:

$ sudo systemd-run --wait --quiet -p DynamicUser=yes -p BindPaths=/tmp/ touch /tmp/foo
$ ls -l /tmp/foo
-rw-r--r-- 1 63013 63013 0 May 10 09:54 foo

[poettering]

that said, maybe a simple fix is to not imply PrivateTmp= in case of DynamicUser=1 but instead imply TemporaryFileSystem=/tmp /var/tmp?

wouldn't that be better and simpler?

i mean, there definitely is value in keeping dynamic uids/gids local to file systems that are destroyed when the service is dead, hence changing the logic liek that would make sense to me.

[bluca]

Ok I've applied this change, it's slightly backward incompatible unlike the original version but if that's what it takes

[poettering]

or well, i generally think that things such as PrivateTmp=1 as high-level knobs are the way to go, and they should abstract what happens in the bg a bit.

so i think we should just make sure it has a different effect in case DynamicUser=1 is used, i.e. make it act like TemporaryFs=/tmp + Bind=/var/tmp:/tmp

[bluca]

or well, i generally think that things such as PrivateTmp=1 as high-level knobs are the way to go, and they should abstract what happens in the bg a bit.

so i think we should just make sure it has a different effect in case DynamicUser=1 is used, i.e. make it act like TemporaryFs=/tmp + Bind=/var/tmp:/tmp

This is logically equivalent to your comment above - we do want to enforce that private tmpfses are used with DynamicUser=yes, so the PrivateTmp knob becomes essentially meaningless in that case - whether it's enabled or disabled, same result applies

I am not making /tmp an alias of /var/tmp as that changes too much the existing setup, where they are different directories, we can't just change that from one version to another as it is most likely going to break things, DynamicUser is used by hundreds of units in Debian alone, plus it's the default for portables