[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#947935: ITP: opentmpfiles -- standalone utility written to process systemd-style tmpfiles.d files



[resend]

On Thu, Jan 02, 2020 at 01:35:26PM +0100, Thomas Goirand wrote:
> * URL             : https://github.com/OpenRC/opentmpfiles

I had a quick look at the repo. This is not a comprehensive review,
just a quick list of the most obvious items:

opentmpfiles doesn't implement quite a few options: --root, --user,
v, q, e, Q, f+, a, A, m, x, X.
The last two are very important: without support for excludes, data loss is
likely. The first is even more important obviously to avoid mucking with
the host system unexpectedly.

opentmpfiles seems to have no support for the BSD lock protocol to
exclude directories from cleanup, again potential data loss.

Time-based cleanup also seems to be completely missing. (Systemd
relies on that to not fill up temporary partitions with stale
data. Lack of time-based cleanup becomes very problematic for
long-running systems.)

(Note that implementing cleanup in shell is hard, because files must
be opened with NO_ATIME to avoid bumping the access times and actually
preventing time-based cleanup from happening.)

I don't see any support for specifier expansion, so operation on wrong
files and bogus contents when written to files are likely.

w appends instead of replacing files.

systemd-tmpfiles automatically excludes unmounted autofs mountpoints,
to avoid needlessly triggering mounts. I see no support for that.

systemd-tmpfiles has a lot of very careful code to avoid unintended
privilege escalation. For example, when doing a chmod+chown, the
access mask is reduced first to avoid a time when a bigger privilege
mask would be set for the wrong owner. The actual chmod is done at the
end so the kernel can adjust the suid bit and other permissions
appropriately.

Similarly, systemd-tmpfiles is very careful when following paths which
go through a directory owned by an unprivileged user (e.g. if we are
to chown /run/foo/bar/passwd, where /run/foo belongs to user foo, we
should refuse operation if /run/foo/bar is a symlink to /etc. The
check for this needs to be done with TOCTOU in mind, i.e. using *at()
variants of all syscalls so that the foo user cannot substitute the
link between the time /run/foo/bar ownership is checked and the chmod
operation on /run/foo/bar/passwd.). Doing this in shell is going to be
hard. The code in opentmpfiles seems to have no notion of ownership
checks, i.e. is open to unintended privilege escalation.

In summary, treating this implementation as a drop-in replacement is
not a good idea. In particular, important missing functionality would
immediately impair operation of systemd on systems where such
substitution would be done. Also, the parts that are not implemented
are the hard parts. Even though it seems that approx. half of
functionality is available, the implementation effort of that second
half would be much grater, and the use of shell makes it a daunting
task.

Zbyszek


Reply to: