Merge pull request #2063 from keszybz/issue-1977-2

journal: clean up permission setting and acl adjustements on user journals

8 files changed, 149 insertions, 56 deletions

diff --git a/.gitignore b/.gitignore
index 98eb29d657..c400fea684 100644
--- a/.gitignore
+++ b/.gitignore
@@ -133,6 +133,7 @@
 /systemd-vconsole-setup
 /tags
 /test-acd
+/test-acl-util
 /test-af-list
 /test-architecture
 /test-arphrd-list
diff --git a/Makefile.am b/Makefile.am
index db206ae13e..cecd139e4f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1499,6 +1499,11 @@ tests += \
 	test-dns-domain \
 	test-install-root
 
+if HAVE_ACL
+tests += \
+	test-acl-util
+endif
+
 EXTRA_DIST += \
 	test/a.service \
 	test/basic.target \
@@ -1846,6 +1851,12 @@ test_install_root_SOURCES = \
 test_install_root_LDADD = \
 	libshared.la
 
+test_acl_util_SOURCES = \
+	src/test/test-acl-util.c
+
+test_acl_util_LDADD = \
+	libshared.la
+
 test_namespace_LDADD = \
 	libcore.la
 
diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index 7d11a568aa..97c5820c75 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -204,58 +204,19 @@ static int determine_space(Server *s, bool verbose, bool patch_min_use, uint64_t
         return determine_space_for(s, metrics, path, name, verbose, patch_min_use, available, limit);
 }
 
-void server_fix_perms(Server *s, JournalFile *f, uid_t uid) {
-        int r;
+static void server_add_acls(JournalFile *f, uid_t uid) {
 #ifdef HAVE_ACL
-        _cleanup_(acl_freep) acl_t acl = NULL;
-        acl_entry_t entry;
-        acl_permset_t permset;
+        int r;
 #endif
-
         assert(f);
 
-        r = fchmod(f->fd, 0640);
-        if (r < 0)
-                log_warning_errno(errno, "Failed to fix access mode on %s, ignoring: %m", f->path);
-
 #ifdef HAVE_ACL
         if (uid <= SYSTEM_UID_MAX)
                 return;
 
-        acl = acl_get_fd(f->fd);
-        if (!acl) {
-                log_warning_errno(errno, "Failed to read ACL on %s, ignoring: %m", f->path);
-                return;
-        }
-
-        r = acl_find_uid(acl, uid, &entry);
-        if (r <= 0) {
-
-                if (acl_create_entry(&acl, &entry) < 0 ||
-                    acl_set_tag_type(entry, ACL_USER) < 0 ||
-                    acl_set_qualifier(entry, &uid) < 0) {
-                        log_warning_errno(errno, "Failed to patch ACL on %s, ignoring: %m", f->path);
-                        return;
-                }
-        }
-
-        /* We do not recalculate the mask unconditionally here,
-         * so that the fchmod() mask above stays intact. */
-        if (acl_get_permset(entry, &permset) < 0 ||
-            acl_add_perm(permset, ACL_READ) < 0) {
-                log_warning_errno(errno, "Failed to patch ACL on %s, ignoring: %m", f->path);
-                return;
-        }
-
-        r = calc_acl_mask_if_needed(&acl);
-        if (r < 0) {
-                log_warning_errno(r, "Failed to patch ACL on %s, ignoring: %m", f->path);
-                return;
-        }
-
-        if (acl_set_fd(f->fd, acl) < 0)
-                log_warning_errno(errno, "Failed to set ACL on %s, ignoring: %m", f->path);
-
+        r = add_acls_for_user(f->fd, uid);
+        if (r < 0)
+                log_warning_errno(r, "Failed to set ACL on %s, ignoring: %m", f->path);
 #endif
 }
 
@@ -301,7 +262,7 @@ static JournalFile* find_journal(Server *s, uid_t uid) {
         if (r < 0)
                 return s->system_journal;
 
-        server_fix_perms(s, f, uid);
+        server_add_acls(f, uid);
 
         r = ordered_hashmap_put(s->user_journals, UID_TO_PTR(uid), f);
         if (r < 0) {
@@ -332,7 +293,7 @@ static int do_rotate(
                 else
                         log_error_errno(r, "Failed to create new %s journal: %m", name);
         else
-                server_fix_perms(s, *f, uid);
+                server_add_acls(*f, uid);
 
         return r;
 }
@@ -971,7 +932,7 @@ static int system_journal_open(Server *s, bool flush_requested) {
                 fn = strjoina(fn, "/system.journal");
                 r = journal_file_open_reliably(fn, O_RDWR|O_CREAT, 0640, s->compress, s->seal, &s->system_metrics, s->mmap, NULL, &s->system_journal);
                 if (r >= 0) {
-                        server_fix_perms(s, s->system_journal, 0);
+                        server_add_acls(s->system_journal, 0);
                         (void) determine_space_for(s, &s->system_metrics, "/var/log/journal/", "System journal", true, true, NULL, NULL);
                 } else if (r < 0) {
                         if (r != -ENOENT && r != -EROFS)
@@ -1015,7 +976,7 @@ static int system_journal_open(Server *s, bool flush_requested) {
                 }
 
                 if (s->runtime_journal) {
-                        server_fix_perms(s, s->runtime_journal, 0);
+                        server_add_acls(s->runtime_journal, 0);
                         (void) determine_space_for(s, &s->runtime_metrics, "/run/log/journal/", "Runtime journal", true, true, NULL, NULL);
                 }
         }
diff --git a/src/journal/journald-server.h b/src/journal/journald-server.h
index dcc21bb7c3..1822765228 100644
--- a/src/journal/journald-server.h
+++ b/src/journal/journald-server.h
@@ -174,7 +174,6 @@ int config_parse_split_mode(const char *unit, const char *filename, unsigned lin
 const char *split_mode_to_string(SplitMode s) _const_;
 SplitMode split_mode_from_string(const char *s) _pure_;
 
-void server_fix_perms(Server *s, JournalFile *f, uid_t uid);
 int server_init(Server *s);
 void server_done(Server *s);
 void server_sync(Server *s);
diff --git a/src/shared/acl-util.c b/src/shared/acl-util.c
index 35f2e1b67d..b4028564c2 100644
--- a/src/shared/acl-util.c
+++ b/src/shared/acl-util.c
@@ -71,6 +71,7 @@ int acl_find_uid(acl_t acl, uid_t uid, acl_entry_t *entry) {
 int calc_acl_mask_if_needed(acl_t *acl_p) {
         acl_entry_t i;
         int r;
+        bool need = false;
 
         assert(acl_p);
 
@@ -85,17 +86,16 @@ int calc_acl_mask_if_needed(acl_t *acl_p) {
                 if (tag == ACL_MASK)
                         return 0;
 
-                if (IN_SET(tag, ACL_USER, ACL_GROUP)) {
-                        if (acl_calc_mask(acl_p) < 0)
-                                return -errno;
-
-                        return 1;
-                }
+                if (IN_SET(tag, ACL_USER, ACL_GROUP))
+                        need = true;
         }
         if (r < 0)
                 return -errno;
 
-        return 0;
+        if (need && acl_calc_mask(acl_p) < 0)
+                return -errno;
+
+        return need;
 }
 
 int add_base_acls_if_needed(acl_t *acl_p, const char *path) {
@@ -398,3 +398,34 @@ int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *acl) {
         old = NULL;
         return 0;
 }
+
+int add_acls_for_user(int fd, uid_t uid) {
+        _cleanup_(acl_freep) acl_t acl = NULL;
+        acl_entry_t entry;
+        acl_permset_t permset;
+        int r;
+
+        acl = acl_get_fd(fd);
+        if (!acl)
+                return -errno;
+
+        r = acl_find_uid(acl, uid, &entry);
+        if (r <= 0) {
+                if (acl_create_entry(&acl, &entry) < 0 ||
+                    acl_set_tag_type(entry, ACL_USER) < 0 ||
+                    acl_set_qualifier(entry, &uid) < 0)
+                        return -errno;
+        }
+
+        /* We do not recalculate the mask unconditionally here,
+         * so that the fchmod() mask above stays intact. */
+        if (acl_get_permset(entry, &permset) < 0 ||
+            acl_add_perm(permset, ACL_READ) < 0)
+                return -errno;
+
+        r = calc_acl_mask_if_needed(&acl);
+        if (r < 0)
+                return r;
+
+        return acl_set_fd(fd, acl);
+}
diff --git a/src/shared/acl-util.h b/src/shared/acl-util.h
index 256a6a5900..1d7f45e2a8 100644
--- a/src/shared/acl-util.h
+++ b/src/shared/acl-util.h
@@ -35,6 +35,7 @@ int add_base_acls_if_needed(acl_t *acl_p, const char *path);
 int acl_search_groups(const char* path, char ***ret_groups);
 int parse_acl(const char *text, acl_t *acl_access, acl_t *acl_default, bool want_mask);
 int acls_for_file(const char *path, acl_type_t type, acl_t new, acl_t *acl);
+int add_acls_for_user(int fd, uid_t uid);
 
 /* acl_free takes multiple argument types.
  * Multiple cleanup functions are necessary. */
diff --git a/src/test/test-acl-util.c b/src/test/test-acl-util.c
new file mode 100644
index 0000000000..bdea04d7f0
--- /dev/null
+++ b/src/test/test-acl-util.c
@@ -0,0 +1,85 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2015 Zbigniew Jędrzejewski-Szmek
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "acl-util.h"
+#include "fd-util.h"
+#include "fileio.h"
+#include "string-util.h"
+#include "user-util.h"
+
+static void test_add_acls_for_user(void) {
+        char fn[] = "/tmp/test-empty.XXXXXX";
+        _cleanup_close_ int fd = -1;
+        char *cmd;
+        uid_t uid;
+        int r;
+
+        fd = mkostemp_safe(fn, O_RDWR|O_CLOEXEC);
+        assert_se(fd >= 0);
+
+        /* Use the mode that user journal files use */
+        assert(fchmod(fd, 0640) == 0);
+
+        cmd = strjoina("ls -l ", fn);
+        assert_se(system(cmd) == 0);
+
+        cmd = strjoina("getfacl -p ", fn);
+        assert_se(system(cmd) == 0);
+
+        if (getuid() == 0) {
+                const char *nobody = "nobody";
+                r = get_user_creds(&nobody, &uid, NULL, NULL, NULL);
+                if (r < 0)
+                        uid = 0;
+        } else
+                uid = getuid();
+
+        r = add_acls_for_user(fd, uid);
+        assert_se(r >= 0);
+
+        cmd = strjoina("ls -l ", fn);
+        assert_se(system(cmd) == 0);
+
+        cmd = strjoina("getfacl -p ", fn);
+        assert_se(system(cmd) == 0);
+
+        /* set the acls again */
+
+        r = add_acls_for_user(fd, uid);
+        assert_se(r >= 0);
+
+        cmd = strjoina("ls -l ", fn);
+        assert_se(system(cmd) == 0);
+
+        cmd = strjoina("getfacl -p ", fn);
+        assert_se(system(cmd) == 0);
+
+        unlink(fn);
+}
+
+int main(int argc, char **argv) {
+        test_add_acls_for_user();
+}
diff --git a/tmpfiles.d/systemd.conf.m4 b/tmpfiles.d/systemd.conf.m4
index d9d51af929..0575408dbe 100644
--- a/tmpfiles.d/systemd.conf.m4
+++ b/tmpfiles.d/systemd.conf.m4
@@ -33,9 +33,13 @@ A+ /run/log/journal/%m - - - - group:adm:r-x,group:wheel:r-x
 
 z /var/log/journal 2755 root systemd-journal - -
 z /var/log/journal/%m 2755 root systemd-journal - -
+z /var/log/journal/%m/system.journal 0640 root systemd-journal - -
 m4_ifdef(`HAVE_ACL',``
+a+ /var/log/journal    - - - - d:group:adm:r-x,d:group:wheel:r-x
+a+ /var/log/journal    - - - - group:adm:r-x,group:wheel:r-x
 a+ /var/log/journal/%m - - - - d:group:adm:r-x,d:group:wheel:r-x
 a+ /var/log/journal/%m - - - - group:adm:r-x,group:wheel:r-x
+a+ /var/log/journal/%m/system.journal - - - - group:adm:r--,group:wheel:r--
 '')m4_dnl
 
 d /var/lib/systemd 0755 root root -