util-lib: rework /tmp and /var/tmp handling code

Beef up the existing var_tmp() call, rename it to var_tmp_dir() and add a
matching tmp_dir() call (the former looks for the place for /var/tmp, the
latter for /tmp).

Both calls check $TMPDIR, $TEMP, $TMP, following the algorithm Python3 uses.
All dirs are validated before use. secure_getenv() is used in order to limite
exposure in suid binaries.

This also ports a couple of users over to these new APIs.

The var_tmp() return parameter is changed from an allocated buffer the caller
will own to a const string either pointing into environ[], or into a static
const buffer. Given that environ[] is mostly considered constant (and this is
exposed in the very well-known getenv() call), this should be OK behaviour and
allows us to avoid memory allocations in most cases.

Note that $TMPDIR and friends override both /var/tmp and /tmp usage if set.

7 files changed, 126 insertions, 61 deletions

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index f183de4999..6114bf3315 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -1161,8 +1161,8 @@ int tempfn_random_child(const char *p, const char *extra, char **ret) {
         char *t, *x;
         uint64_t u;
         unsigned i;
+        int r;
 
-        assert(p);
         assert(ret);
 
         /* Turns this:
@@ -1171,6 +1171,12 @@ int tempfn_random_child(const char *p, const char *extra, char **ret) {
          *         /foo/bar/waldo/.#<extra>3c2b6219aa75d7d0
          */
 
+        if (!p) {
+                r = tmp_dir(&p);
+                if (r < 0)
+                        return r;
+        }
+
         if (!extra)
                 extra = "";
 
@@ -1257,10 +1263,13 @@ int fputs_with_space(FILE *f, const char *s, const char *separator, bool *space)
 
 int open_tmpfile_unlinkable(const char *directory, int flags) {
         char *p;
-        int fd;
+        int fd, r;
 
-        if (!directory)
-                directory = "/tmp";
+        if (!directory) {
+                r = tmp_dir(&directory);
+                if (r < 0)
+                        return r;
+        }
 
         /* Returns an unlinked temporary file that cannot be linked into the file system anymore */
 
diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c
index f0c6f3265e..ce87257bc1 100644
--- a/src/basic/fs-util.c
+++ b/src/basic/fs-util.c
@@ -496,34 +496,94 @@ int get_files_in_directory(const char *path, char ***list) {
         return n;
 }
 
-int var_tmp(char **ret) {
-        const char *tmp_dir = NULL;
-        const char *env_tmp_dir = NULL;
-        char *c = NULL;
-        int r;
+static int getenv_tmp_dir(const char **ret_path) {
+        const char *n;
+        int r, ret = 0;
 
-        assert(ret);
+        assert(ret_path);
 
-        env_tmp_dir = getenv("TMPDIR");
-        if (env_tmp_dir != NULL) {
-                r = is_dir(env_tmp_dir, true);
-                if (r < 0 && r != -ENOENT)
-                        return r;
-                if (r > 0)
-                        tmp_dir = env_tmp_dir;
+        /* We use the same order of environment variables python uses in tempfile.gettempdir():
+         * https://docs.python.org/3/library/tempfile.html#tempfile.gettempdir */
+        FOREACH_STRING(n, "TMPDIR", "TEMP", "TMP") {
+                const char *e;
+
+                e = secure_getenv(n);
+                if (!e)
+                        continue;
+                if (!path_is_absolute(e)) {
+                        r = -ENOTDIR;
+                        goto next;
+                }
+                if (!path_is_safe(e)) {
+                        r = -EPERM;
+                        goto next;
+                }
+
+                r = is_dir(e, true);
+                if (r < 0)
+                        goto next;
+                if (r == 0) {
+                        r = -ENOTDIR;
+                        goto next;
+                }
+
+                *ret_path = e;
+                return 1;
+
+        next:
+                /* Remember first error, to make this more debuggable */
+                if (ret >= 0)
+                        ret = r;
         }
 
-        if (!tmp_dir)
-                tmp_dir = "/var/tmp";
+        if (ret < 0)
+                return ret;
 
-        c = strdup(tmp_dir);
-        if (!c)
-                return -ENOMEM;
-        *ret = c;
+        *ret_path = NULL;
+        return ret;
+}
 
+static int tmp_dir_internal(const char *def, const char **ret) {
+        const char *e;
+        int r, k;
+
+        assert(def);
+        assert(ret);
+
+        r = getenv_tmp_dir(&e);
+        if (r > 0) {
+                *ret = e;
+                return 0;
+        }
+
+        k = is_dir(def, true);
+        if (k == 0)
+                k = -ENOTDIR;
+        if (k < 0)
+                return r < 0 ? r : k;
+
+        *ret = def;
         return 0;
 }
 
+int var_tmp_dir(const char **ret) {
+
+        /* Returns the location for "larger" temporary files, that is backed by physical storage if available, and thus
+         * even might survive a boot: /var/tmp. If $TMPDIR (or related environment variables) are set, its value is
+         * returned preferably however. Note that both this function and tmp_dir() below are affected by $TMPDIR,
+         * making it a variable that overrides all temporary file storage locations. */
+
+        return tmp_dir_internal("/var/tmp", ret);
+}
+
+int tmp_dir(const char **ret) {
+
+        /* Similar to var_tmp_dir() above, but returns the location for "smaller" temporary files, which is usually
+         * backed by an in-memory file system: /tmp. */
+
+        return tmp_dir_internal("/tmp", ret);
+}
+
 int inotify_add_watch_fd(int fd, int what, uint32_t mask) {
         char path[strlen("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1];
         int r;
diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h
index 075e5942b1..2c3b9a1c74 100644
--- a/src/basic/fs-util.h
+++ b/src/basic/fs-util.h
@@ -61,7 +61,8 @@ int mkfifo_atomic(const char *path, mode_t mode);
 
 int get_files_in_directory(const char *path, char ***list);
 
-int var_tmp(char **ret);
+int tmp_dir(const char **ret);
+int var_tmp_dir(const char **ret);
 
 #define INOTIFY_EVENT_MAX (sizeof(struct inotify_event) + NAME_MAX + 1)
 
diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c
index 27b1e0fb3f..bbf8793e57 100644
--- a/src/coredump/coredumpctl.c
+++ b/src/coredump/coredumpctl.c
@@ -30,6 +30,7 @@
 #include "compress.h"
 #include "fd-util.h"
 #include "fileio.h"
+#include "fs-util.h"
 #include "journal-internal.h"
 #include "log.h"
 #include "macro.h"
@@ -609,7 +610,13 @@ static int save_core(sd_journal *j, int fd, char **path, bool *unlink_temp) {
                 char *temp = NULL;
 
                 if (fd < 0) {
-                        temp = strdup("/var/tmp/coredump-XXXXXX");
+                        const char *vt;
+
+                        r = var_tmp_dir(&vt);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to acquire temporary directory path: %m");
+
+                        temp = strjoin(vt, "/coredump-XXXXXX", NULL);
                         if (!temp)
                                 return log_oom();
 
diff --git a/src/journal/journal-verify.c b/src/journal/journal-verify.c
index f61f158e8a..4105abfccc 100644
--- a/src/journal/journal-verify.c
+++ b/src/journal/journal-verify.c
@@ -826,7 +826,7 @@ int journal_file_verify(
         int data_fd = -1, entry_fd = -1, entry_array_fd = -1;
         unsigned i;
         bool found_last = false;
-        _cleanup_free_ char *tmp_dir = NULL;
+        const char *tmp_dir = NULL;
 
 #ifdef HAVE_GCRYPT
         uint64_t last_tag = 0;
@@ -846,7 +846,7 @@ int journal_file_verify(
         } else if (f->seal)
                 return -ENOKEY;
 
-        r = var_tmp(&tmp_dir);
+        r = var_tmp_dir(&tmp_dir);
         if (r < 0) {
                 log_error_errno(r, "Failed to determine temporary directory: %m");
                 goto fail;
diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c
index 1923e8b971..5e2462cba2 100644
--- a/src/machine/machined-dbus.c
+++ b/src/machine/machined-dbus.c
@@ -954,7 +954,7 @@ static int method_clean_pool(sd_bus_message *message, void *userdata, sd_bus_err
         /* Create a temporary file we can dump information about deleted images into. We use a temporary file for this
          * instead of a pipe or so, since this might grow quit large in theory and we don't want to process this
          * continuously */
-        result_fd = open_tmpfile_unlinkable("/tmp/", O_RDWR|O_CLOEXEC);
+        result_fd = open_tmpfile_unlinkable(NULL, O_RDWR|O_CLOEXEC);
         if (result_fd < 0)
                 return -errno;
 
diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c
index e0c040f39b..93eec3ef9c 100644
--- a/src/test/test-fs-util.c
+++ b/src/test/test-fs-util.c
@@ -83,47 +83,35 @@ static void test_get_files_in_directory(void) {
 }
 
 static void test_var_tmp(void) {
-        char *tmp_dir = NULL;
-        char *tmpdir_backup = NULL;
-        const char *default_var_tmp = NULL;
-        const char *var_name;
-        bool do_overwrite = true;
-
-        default_var_tmp = "/var/tmp";
-        var_name = "TMPDIR";
-
-        if (getenv(var_name) != NULL) {
-                tmpdir_backup = strdup(getenv(var_name));
-                assert_se(tmpdir_backup != NULL);
-        }
-
-        unsetenv(var_name);
+        _cleanup_free_ char *tmpdir_backup = NULL;
+        const char *tmp_dir = NULL, *t;
 
-        var_tmp(&tmp_dir);
-        assert_se(!strcmp(tmp_dir, default_var_tmp));
-
-        free(tmp_dir);
+        t = getenv("TMPDIR");
+        if (t) {
+                tmpdir_backup = strdup(t);
+                assert_se(tmpdir_backup);
+        }
 
-        setenv(var_name, "/tmp", do_overwrite);
-        assert_se(!strcmp(getenv(var_name), "/tmp"));
+        assert(unsetenv("TMPDIR") >= 0);
 
-        var_tmp(&tmp_dir);
-        assert_se(!strcmp(tmp_dir, "/tmp"));
+        assert_se(var_tmp_dir(&tmp_dir) >= 0);
+        assert_se(streq(tmp_dir, "/var/tmp"));
 
-        free(tmp_dir);
+        assert_se(setenv("TMPDIR", "/tmp", true) >= 0);
+        assert_se(streq(getenv("TMPDIR"), "/tmp"));
 
-        setenv(var_name, "/88_does_not_exist_88", do_overwrite);
-        assert_se(!strcmp(getenv(var_name), "/88_does_not_exist_88"));
+        assert_se(var_tmp_dir(&tmp_dir) >= 0);
+        assert_se(streq(tmp_dir, "/tmp"));
 
-        var_tmp(&tmp_dir);
-        assert_se(!strcmp(tmp_dir, default_var_tmp));
+        assert_se(setenv("TMPDIR", "/88_does_not_exist_88", true) >= 0);
+        assert_se(streq(getenv("TMPDIR"), "/88_does_not_exist_88"));
 
-        free(tmp_dir);
+        assert_se(var_tmp_dir(&tmp_dir) >= 0);
+        assert_se(streq(tmp_dir, "/var/tmp"));
 
-        if (tmpdir_backup != NULL)  {
-                setenv(var_name, tmpdir_backup, do_overwrite);
-                assert_se(!strcmp(getenv(var_name), tmpdir_backup));
-                free(tmpdir_backup);
+        if (tmpdir_backup)  {
+                assert_se(setenv("TMPDIR", tmpdir_backup, true) >= 0);
+                assert_se(streq(getenv("TMPDIR"), tmpdir_backup));
         }
 }