tmp leaves temporary files of called programs¶

Labels: enhancement, bug, fixed / solved / done

jsmeix opened issue at 2024-02-28 13:43:¶

Since we set in default.conf TMPDIR to /var/tmp (if unset) via

export TMPDIR="${TMPDIR-/var/tmp}"

programs that are called by ReaR use by default /var/tmp
for their temporary files.

Programs may not carefully clean up their temporary files
or may leave temporary files in case of program aborts
when programs (falsely) rely on the system default /tmp
that usually gets automatically cleaned up relatively fast
e.g. via something like systemd-tmpfiles-clean.service
or at least via reboot when /tmp is a tmpfs in RAM.

In contast to /var/tmp for programs that are called by ReaR
ReaR cleanes up its own temporary stuff in BUILD_DIR
carefully and completely via the function
cleanup_build_area_and_end_program()
in lib/_input-output-functions.sh

Perhaps it would be good when /usr/sbin/rear
sets TMPDIR to $TMP_DIR (TMP_DIR=$BUILD_DIR/tmp)
if TMPDIR was initially unset when 'rear' was called
so that programs that are called by ReaR use
by default $BUILD_DIR/tmp for their temporary files?

An addedum regarding confidentiality,
here in particular about confidentiality of temporary files
from programs that are called by ReaR:

Currently we do in /usr/sbin/rear

BUILD_DIR="$( mktemp -d -t rear.XXXXXXXXXXXXXXX || Error "Could not create build area '$BUILD_DIR'" )"
ROOTFS_DIR=$BUILD_DIR/rootfs
TMP_DIR=$BUILD_DIR/tmp
mkdir -p $ROOTFS_DIR || Error "Could not create $ROOTFS_DIR"
mkdir -p $TMP_DIR || Error "Could not create $TMP_DIR"

without any special 'umask' or 'chmod' for confidentiality
so we rely on sufficiently safe 'mktemp -d' behaviour

'-d'
...
The directory will have read, write, and search permissions
for the current user, but no permissions for the group or others

as described in
https://www.gnu.org/software/coreutils/manual/html_node/mktemp-invocation.html#mktemp-invocation

So when programs that are called by ReaR use $BUILD_DIR/tmp
their temporary files are sufficiently safe via 'mktemp -d'
against unwanted access by non-root users.

jsmeix commented at 2024-02-28 13:45:¶

@pcahyna
please have a look here (as time permits).
I really appreciate your feedback here.

pcahyna commented at 2024-02-28 13:52:¶

@jsmeix that's an interesting point. After reading it, I have only one remark (I may find other stuff later):

Perhaps it would be good when /usr/sbin/rear
sets TMPDIR to $TMP_DIR (TMP_DIR=$BUILD_DIR/tmp)
if TMPDIR was initially unset when 'rear' was called
so that programs that are called by ReaR use
by default $BUILD_DIR/tmp for their temporary files?

Why only "if TMPDIR was initially unset when 'rear' was called" ? This condition complicates the whole logic, and is also unhelpful. If we were to do this change, I would do it unconditionally. If an user sets and exports TMPDIR=/some/big/file/system before calling ReaR, it would be nice to do the automatic cleanup for them as well, just as we would do for the default /var/tmp.

jsmeix commented at 2024-02-28 14:09:¶

@pcahyna
you already helped me a lot with your first remark!

I falsely thought when we set in default.conf TMPDIR
to /var/tmp only if TMPDIR was unset that then I also
need to set TMPDIR to $TMP_DIR only if TMPDIR was unset
when 'rear' was called.

When it is better to set TMPDIR to $TMP_DIR in any case
implementation is much simpler and straightforward for me!

jsmeix commented at 2024-02-28 14:37:¶

@pcahyna
because I am looking right now at usr/sbin/rear

# Save the current value to detect changes.
saved_tmpdir="${TMPDIR-}"

I am wondering why you use ${TMPDIR-}
and not simply $TMPDIR because
TMPDIR is set in default.conf if it was unset
and default.conf was already sourced before.

pcahyna commented at 2024-02-28 14:42:¶

@jsmeix I have not realized this. Thank you for pointing this out, but I would not want to rely on such long-range interactions anyway.

jsmeix commented at 2024-02-28 14:56:¶

@pcahyna
"sorry" for delivering bad news but tons of things in ReaR
depend on long-and-even-longer-range interactions ;-)

jsmeix commented at 2024-03-15 08:50:¶

Meanwhile while implementing it via
https://github.com/rear/rear/pull/3168
I came to the conclusion that it is better
(at least for now to be on the safe side for the initial implementation)
to set TMPDIR to $TMP_DIR (TMP_DIR=$BUILD_DIR/tmp)
only if TMPDIR was initially unset when 'rear' was called.

See my thoughts
https://github.com/rear/rear/pull/3168#issuecomment-1997014841
and
https://github.com/rear/rear/pull/3168#issuecomment-1997057144
and my comment in the code
https://github.com/rear/rear/pull/3168/commits/7e278f3329fe52edaa9527dfde1816e74ea5cd5a

# We set TMPDIR to ReaR's TMP_DIR only when TMPDIR was not set
# before ReaR was launched by the user via export TMPDIR="..."
# to provide final power to the user so that a specific TMPDIR can be specified
# e.g. for certain third-party backup tools that may need a specific TMPDIR

Therein "provide final power to the user"
is the crucial part which outweighs (at least for me)
https://github.com/rear/rear/issues/3167#issuecomment-1969031794

pcahyna commented at 2024-03-15 11:27:¶

@jsmeix setting TMPDIR to a subdirectory of the user-specified TMPDIR should not be a problem because the temporary files will still be under a directory that the user has specified. The user should not assume what file or directory structure under the TMPDIR that they have specified will the tool use. After all, even now, we don't put temporary files directly into TMPDIR, but under $TMPDIR/rear.XXXXXX and possibly some directories in between, and it has not been a problem.

Changing TMPDIR to something that would not be under the user-specified TMPDIR would certainly be against "final power to the user", but that's not what you are doing here.

Your examples of third-party backup tools show that when such a tool needs a specific TMPDIR, it does not obey what the user has specified - the code specific to the tool must set TMPDIR to the "right" value itself, in any case.

jsmeix commented at 2024-03-15 12:52:¶

@pcahyna
thank you for your explanation!
I will think about it over the weekend.
My current gut feeling is that you are right.

jsmeix commented at 2024-03-26 12:28:¶

With https://github.com/rear/rear/pull/3168 merged
this issue should be reasonably solved.

What is missing is proper handling of TMPDIR in RECOVERY_MODE
in particular for the "chroot /mnt/local" case, see
https://github.com/rear/rear/pull/3168#issuecomment-1988974933
and
https://github.com/rear/rear/pull/3168#issuecomment-1991347031

[Export of Github issue for rear/rear.]

#3167 Issue closed: default TMPDIR /var/tmp leaves temporary files of called programs¶