Make PrivateTmp dirs also inaccessible from the outside

Currently, PrivateTmp=yes means that the service cannot see the /tmp
shared by rest of the system and is isolated from other services using
PrivateTmp, but users can access and modify /tmp as seen by the
service.

Move the private /tmp and /var/tmp directories into a 0077-mode
directory. This way unpriviledged users on the system cannot see (or
modify) /tmp as seen by the service.

5 files changed, 54 insertions, 47 deletions

diff --git a/src/core/execute.c b/src/core/execute.c
index 18e25fa6e6..bc876a3292 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -40,6 +40,7 @@
 #include <sys/poll.h>
 #include <linux/seccomp-bpf.h>
 #include <glob.h>
+#include <libgen.h>
 
 #ifdef HAVE_PAM
 #include <security/pam_appl.h>
@@ -1551,19 +1552,22 @@ void exec_context_init(ExecContext *c) {
 }
 
 void exec_context_tmp_dirs_done(ExecContext *c) {
-        assert(c);
+        char* dirs[] = {c->tmp_dir ? c->tmp_dir : c->var_tmp_dir,
+                        c->tmp_dir ? c->var_tmp_dir : NULL,
+                        NULL};
+        char **dirp;
 
-        if (c->tmp_dir) {
-                rm_rf_dangerous(c->tmp_dir, false, true, false);
-                free(c->tmp_dir);
-                c->tmp_dir = NULL;
-        }
+        for(dirp = dirs; *dirp; dirp++) {
+                char *dir;
+                rm_rf_dangerous(*dirp, false, true, false);
 
-        if (c->var_tmp_dir) {
-                rm_rf_dangerous(c->var_tmp_dir, false, true, false);
-                free(c->var_tmp_dir);
-                c->var_tmp_dir = NULL;
+                dir = dirname(*dirp);
+                rmdir(dir);
+
+                free(*dirp);
         }
+
+        c->tmp_dir = c->var_tmp_dir = NULL;
 }
 
 void exec_context_done(ExecContext *c, bool reloading_or_reexecuting) {
diff --git a/src/core/namespace.c b/src/core/namespace.c
index ceeed2e1ae..972380abc0 100644
--- a/src/core/namespace.c
+++ b/src/core/namespace.c
@@ -183,22 +183,20 @@ int setup_tmpdirs(char **tmp_dir,
         assert(tmp_dir);
         assert(var_tmp_dir);
 
-        r = create_tmp_dir(tmp_dir_template, 0000, true, tmp_dir);
+        r = create_tmp_dir(tmp_dir_template, tmp_dir);
         if (r < 0)
-                goto fail2;
+                return r;
 
-        r = create_tmp_dir(var_tmp_dir_template, 0000, true, var_tmp_dir);
-        if (r < 0)
-                goto fail1;
-
-        return 0;
+        r = create_tmp_dir(var_tmp_dir_template, var_tmp_dir);
+        if (r == 0)
+                return 0;
 
-fail1:
+        /* failure */
         rmdir(*tmp_dir);
+        rmdir(tmp_dir_template);
         free(*tmp_dir);
         *tmp_dir = NULL;
 
-fail2:
         return r;
 }
 
diff --git a/src/shared/util.c b/src/shared/util.c
index 34c5330838..bdef9f0431 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -5683,46 +5683,49 @@ int search_and_fopen_nulstr(const char *path, const char *mode, const char *sear
         return search_and_fopen_internal(path, mode, s, _f);
 }
 
-int create_tmp_dir(char template[], mode_t mask, bool need_sticky, char** dir_name) {
+int create_tmp_dir(char template[], char** dir_name) {
         int r = 0;
-        char *d = NULL;
-        bool remove = false;
+        char *d, *dt;
         mode_t _cleanup_umask_ u;
 
         assert(dir_name);
 
-        u = umask(mask);
+        u = umask(0077);
         d = mkdtemp(template);
         if (!d) {
-                r = -errno;
-                log_debug("Can't create directory");
-                goto fail;
+                log_error("Can't create directory %s: %m", template);
+                return -errno;
         }
 
-        remove = true;
-
-        log_debug("Created temporary directory : %s", template);
-
-        d = strdup(template);
-        if (!d) {
+        dt = strjoin(d, "/tmp", NULL);
+        if (!dt) {
                 r = log_oom();
-                goto fail;
+                goto fail2;
         }
 
-        if (need_sticky) {
-                r = chmod(template, 0777 | S_ISVTX);
-                if (r < 0) {
-                        r = -errno;
-                        goto fail;
-                }
-                log_debug("Setting sticky bit on : %s", template);
+        umask(0000);
+        r = mkdir(dt, 0777);
+        if (r) {
+                log_error("Can't create directory %s: %m", dt);
+                r = -errno;
+                goto fail1;
+        }
+        log_debug("Created temporary directory %s", dt);
+
+        r = chmod(dt, 0777 | S_ISVTX);
+        if (r < 0) {
+                log_error("Failed to chmod %s: %m", dt);
+                r = -errno;
+                goto fail1;
         }
+        log_debug("Set sticky bit on %s", dt);
 
-        *dir_name = d;
+        *dir_name = dt;
 
         return 0;
-fail:
-        if (remove)
-                rmdir(template);
+fail1:
+        rmdir(dt);
+fail2:
+        rmdir(template);
         return r;
 }
diff --git a/src/shared/util.h b/src/shared/util.h
index 4be0b61773..7c032b6b62 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -579,7 +579,7 @@ int on_ac_power(void);
 
 int search_and_fopen(const char *path, const char *mode, const char **search, FILE **_f);
 int search_and_fopen_nulstr(const char *path, const char *mode, const char *search, FILE **_f);
-int create_tmp_dir(char template[], mode_t mask, bool need_sticky, char** dir_name);
+int create_tmp_dir(char template[], char** dir_name);
 
 #define FOREACH_LINE(line, f, on_error)                         \
         for (;;)                                                \
diff --git a/tmpfiles.d/tmp.conf b/tmpfiles.d/tmp.conf
index ef5a9f0f2c..3b534a1f75 100644
--- a/tmpfiles.d/tmp.conf
+++ b/tmpfiles.d/tmp.conf
@@ -12,5 +12,7 @@ d /tmp 1777 root root 10d
 d /var/tmp 1777 root root 30d
 
 # Exclude namespace mountpoints created with PrivateTmp=yes
-X /tmp/systemd-private-*
-X /var/tmp/systemd-private-*
+x /tmp/systemd-private-*
+x /var/tmp/systemd-private-*
+X /tmp/systemd-private-*/tmp
+X /var/tmp/systemd-private-*/tmp