doczkal / rpms / abrt

Forked from rpms/abrt 4 years ago
Clone

Blame SOURCES/0100-make-the-dump-directories-owned-by-root-by-default.patch

a60cd7
From 8939398b82006ba1fec4ed491339fc075f43fc7c Mon Sep 17 00:00:00 2001
a60cd7
From: Jakub Filak <jfilak@redhat.com>
a60cd7
Date: Mon, 20 Apr 2015 07:56:34 +0200
a60cd7
Subject: [ABRT PATCH] make the dump directories owned by root by default
a60cd7
a60cd7
It was discovered that the abrt event scripts create a user-readable
a60cd7
copy of a sosreport file in abrt problem directories, and include
a60cd7
excerpts of /var/log/messages selected by the user-controlled process
a60cd7
name, leading to an information disclosure.
a60cd7
a60cd7
This issue was discovered by Florian Weimer of Red Hat Product Security.
a60cd7
a60cd7
Related: #1212868
a60cd7
a60cd7
Signed-off-by: Jakub Filak <jfilak@redhat.com>
a60cd7
---
a60cd7
 src/daemon/abrt-server.c     | 34 ++++++++++++++++++++++++++++++++--
a60cd7
 src/daemon/abrt.conf         |  5 +++++
a60cd7
 src/hooks/abrt-hook-ccpp.c   | 10 +++++++---
a60cd7
 src/include/libabrt.h        |  2 ++
a60cd7
 src/lib/abrt_conf.c          |  8 ++++++++
a60cd7
 src/lib/hooklib.c            |  7 ++++++-
a60cd7
 src/plugins/abrt-dump-oops.c |  8 ++++++++
a60cd7
 src/plugins/abrt-dump-xorg.c |  8 ++++++++
a60cd7
 8 files changed, 76 insertions(+), 6 deletions(-)
a60cd7
a60cd7
diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c
a60cd7
index 307b41b..5789075 100644
a60cd7
--- a/src/daemon/abrt-server.c
a60cd7
+++ b/src/daemon/abrt-server.c
a60cd7
@@ -15,6 +15,7 @@
a60cd7
   with this program; if not, write to the Free Software Foundation, Inc.,
a60cd7
   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
a60cd7
 */
a60cd7
+#include "problem_api.h"
a60cd7
 #include "libabrt.h"
a60cd7
 
a60cd7
 /* Maximal length of backtrace. */
a60cd7
@@ -153,7 +154,36 @@ static int run_post_create(const char *dirname)
a60cd7
         error_msg("Bad problem directory name '%s', should start with: '%s'", dirname, g_settings_dump_location);
a60cd7
         return 400; /* Bad Request */
a60cd7
     }
a60cd7
-    if (!dump_dir_accessible_by_uid(dirname, client_uid))
a60cd7
+    if (g_settings_privatereports)
a60cd7
+    {
a60cd7
+        struct stat statbuf;
a60cd7
+        if (lstat(dirname, &statbuf) != 0 || !S_ISDIR(statbuf.st_mode))
a60cd7
+        {
a60cd7
+            error_msg("Path '%s' isn't directory", dirname);
a60cd7
+            return 404; /* Not Found */
a60cd7
+        }
a60cd7
+        /* Get ABRT's group gid */
a60cd7
+        struct group *gr = getgrnam("abrt");
a60cd7
+        if (!gr)
a60cd7
+        {
a60cd7
+            error_msg("Group 'abrt' does not exist");
a60cd7
+            return 500;
a60cd7
+        }
a60cd7
+        if (statbuf.st_uid != 0 || !(statbuf.st_gid == 0 || statbuf.st_gid == gr->gr_gid) || statbuf.st_mode & 07)
a60cd7
+        {
a60cd7
+            error_msg("Problem directory '%s' isn't owned by root:abrt or others are not restricted from access", dirname);
a60cd7
+            return 403;
a60cd7
+        }
a60cd7
+        struct dump_dir *dd = dd_opendir(dirname, DD_OPEN_READONLY);
a60cd7
+        const bool complete = dd && problem_dump_dir_is_complete(dd);
a60cd7
+        dd_close(dd);
a60cd7
+        if (complete)
a60cd7
+        {
a60cd7
+            error_msg("Problem directory '%s' has already been processed", dirname);
a60cd7
+            return 403;
a60cd7
+        }
a60cd7
+    }
a60cd7
+    else if (!dump_dir_accessible_by_uid(dirname, client_uid))
a60cd7
     {
a60cd7
         if (errno == ENOTDIR)
a60cd7
         {
a60cd7
@@ -377,7 +407,7 @@ static int create_problem_dir(GHashTable *problem_info, unsigned pid)
a60cd7
     /* No need to check the path length, as all variables used are limited,
a60cd7
      * and dd_create() fails if the path is too long.
a60cd7
      */
a60cd7
-    struct dump_dir *dd = dd_create(path, client_uid, DEFAULT_DUMP_DIR_MODE);
a60cd7
+    struct dump_dir *dd = dd_create(path, g_settings_privatereports ? 0 : client_uid, DEFAULT_DUMP_DIR_MODE);
a60cd7
     if (!dd)
a60cd7
     {
a60cd7
         error_msg_and_die("Error creating problem directory '%s'", path);
a60cd7
diff --git a/src/daemon/abrt.conf b/src/daemon/abrt.conf
a60cd7
index 59d1831..6c0d6b0 100644
a60cd7
--- a/src/daemon/abrt.conf
a60cd7
+++ b/src/daemon/abrt.conf
a60cd7
@@ -43,3 +43,8 @@ AutoreportingEnabled = no
a60cd7
 #                session; otherwise No.
a60cd7
 #
a60cd7
 # ShortenedReporting = yes
a60cd7
+
a60cd7
+# Disable this if you want to regular users to own the problem data colleted by
a60cd7
+# abrt.
a60cd7
+#
a60cd7
+PrivateReports = yes
a60cd7
diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c
a60cd7
index 00ae621..3a6d002 100644
a60cd7
--- a/src/hooks/abrt-hook-ccpp.c
a60cd7
+++ b/src/hooks/abrt-hook-ccpp.c
a60cd7
@@ -682,6 +682,9 @@ int main(int argc, char** argv)
a60cd7
         }
a60cd7
     }
a60cd7
 
a60cd7
+    /* If PrivateReports is on, root owns all problem directories */
a60cd7
+    const uid_t dduid = g_settings_privatereports ? 0 : fsuid;
a60cd7
+
a60cd7
     /* Open a fd to compat coredump, if requested and is possible */
a60cd7
     if (setting_MakeCompatCore && ulimit_c != 0)
a60cd7
         /* note: checks "user_pwd == NULL" inside; updates core_basename */
a60cd7
@@ -773,18 +776,19 @@ int main(int argc, char** argv)
a60cd7
         goto create_user_core;
a60cd7
     }
a60cd7
 
a60cd7
-    /* use fsuid instead of uid, so we don't expose any sensitive
a60cd7
-     * information of suided app in /var/tmp/abrt
a60cd7
+    /* use dduid (either fsuid or 0) instead of uid, so we don't expose any
a60cd7
+     * sensitive information of suided app in /var/tmp/abrt
a60cd7
      *
a60cd7
      * dd_create_skeleton() creates a new directory and leaves ownership to
a60cd7
      * the current user, hence, we have to call dd_reset_ownership() after the
a60cd7
      * directory is populated.
a60cd7
      */
a60cd7
-    dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE, /*no flags*/0);
a60cd7
+    dd = dd_create_skeleton(path, dduid, DEFAULT_DUMP_DIR_MODE, /*no flags*/0);
a60cd7
     if (dd)
a60cd7
     {
a60cd7
         char *rootdir = get_rootdir(pid);
a60cd7
 
a60cd7
+        /* This function uses fsuid only for its value. The function stores fsuid in a file name 'uid'*/
a60cd7
         dd_create_basic_files(dd, fsuid, NULL);
a60cd7
 
a60cd7
         char source_filename[sizeof("/proc/%lu/somewhat_long_name") + sizeof(long)*3];
a60cd7
diff --git a/src/include/libabrt.h b/src/include/libabrt.h
a60cd7
index 85a5a5c..0320c5b 100644
a60cd7
--- a/src/include/libabrt.h
a60cd7
+++ b/src/include/libabrt.h
a60cd7
@@ -62,6 +62,8 @@ extern bool          g_settings_autoreporting;
a60cd7
 extern char *        g_settings_autoreporting_event;
a60cd7
 #define g_settings_shortenedreporting abrt_g_settings_shortenedreporting
a60cd7
 extern bool          g_settings_shortenedreporting;
a60cd7
+#define g_settings_privatereports abrt_g_settings_privatereports
a60cd7
+extern bool          g_settings_privatereports;
a60cd7
 
a60cd7
 
a60cd7
 #define load_abrt_conf abrt_load_abrt_conf
a60cd7
diff --git a/src/lib/abrt_conf.c b/src/lib/abrt_conf.c
a60cd7
index 5eb69e2..c6aba58 100644
a60cd7
--- a/src/lib/abrt_conf.c
a60cd7
+++ b/src/lib/abrt_conf.c
a60cd7
@@ -27,6 +27,7 @@ bool          g_settings_delete_uploaded = 0;
a60cd7
 bool          g_settings_autoreporting = 0;
a60cd7
 char *        g_settings_autoreporting_event = NULL;
a60cd7
 bool          g_settings_shortenedreporting = 0;
a60cd7
+bool          g_settings_privatereports = true;
a60cd7
 
a60cd7
 void free_abrt_conf_data()
a60cd7
 {
a60cd7
@@ -102,6 +103,13 @@ static void ParseCommon(map_string_t *settings, const char *conf_filename)
a60cd7
     else
a60cd7
         g_settings_shortenedreporting = 0;
a60cd7
 
a60cd7
+    value = get_map_string_item_or_NULL(settings, "PrivateReports");
a60cd7
+    if (value)
a60cd7
+    {
a60cd7
+        g_settings_privatereports = string_to_bool(value);
a60cd7
+        remove_map_string_item(settings, "PrivateReports");
a60cd7
+    }
a60cd7
+
a60cd7
     GHashTableIter iter;
a60cd7
     const char *name;
a60cd7
     /*char *value; - already declared */
a60cd7
diff --git a/src/lib/hooklib.c b/src/lib/hooklib.c
a60cd7
index 1d45cdd..fb7750d 100644
a60cd7
--- a/src/lib/hooklib.c
a60cd7
+++ b/src/lib/hooklib.c
a60cd7
@@ -410,7 +410,12 @@ char* problem_data_save(problem_data_t *pd)
a60cd7
 {
a60cd7
     load_abrt_conf();
a60cd7
 
a60cd7
-    struct dump_dir *dd = create_dump_dir_from_problem_data(pd, g_settings_dump_location);
a60cd7
+    struct dump_dir *dd = NULL;
a60cd7
+
a60cd7
+    if (g_settings_privatereports)
a60cd7
+        dd = create_dump_dir_from_problem_data_ext(pd, g_settings_dump_location, 0);
a60cd7
+    else
a60cd7
+        dd = create_dump_dir_from_problem_data(pd, g_settings_dump_location);
a60cd7
 
a60cd7
     char *problem_id = NULL;
a60cd7
     if (dd)
a60cd7
diff --git a/src/plugins/abrt-dump-oops.c b/src/plugins/abrt-dump-oops.c
a60cd7
index 9f0dc87..05cb728 100644
a60cd7
--- a/src/plugins/abrt-dump-oops.c
a60cd7
+++ b/src/plugins/abrt-dump-oops.c
a60cd7
@@ -189,6 +189,14 @@ static unsigned create_oops_dump_dirs(GList *oops_list, unsigned oops_cnt)
a60cd7
         mode = DEFAULT_DUMP_DIR_MODE;
a60cd7
         my_euid = geteuid();
a60cd7
     }
a60cd7
+    if (g_settings_privatereports)
a60cd7
+    {
a60cd7
+        if (world_readable_dump)
a60cd7
+            log("Not going to make dump directories world readable because PrivateReports is on");
a60cd7
+
a60cd7
+        mode = DEFAULT_DUMP_DIR_MODE;
a60cd7
+        my_euid = 0;
a60cd7
+    }
a60cd7
 
a60cd7
     pid_t my_pid = getpid();
a60cd7
     unsigned idx = 0;
a60cd7
diff --git a/src/plugins/abrt-dump-xorg.c b/src/plugins/abrt-dump-xorg.c
a60cd7
index 3500629..434dc76 100644
a60cd7
--- a/src/plugins/abrt-dump-xorg.c
a60cd7
+++ b/src/plugins/abrt-dump-xorg.c
a60cd7
@@ -82,6 +82,14 @@ static void save_bt_to_dump_dir(const char *bt, const char *exe, const char *rea
a60cd7
         mode = DEFAULT_DUMP_DIR_MODE;
a60cd7
         my_euid = geteuid();
a60cd7
     }
a60cd7
+    if (g_settings_privatereports)
a60cd7
+    {
a60cd7
+        if ((g_opts & OPT_x))
a60cd7
+            log("Not going to make dump directories world readable because PrivateReports is on");
a60cd7
+
a60cd7
+        mode = DEFAULT_DUMP_DIR_MODE;
a60cd7
+        my_euid = 0;
a60cd7
+    }
a60cd7
 
a60cd7
     pid_t my_pid = getpid();
a60cd7
 
a60cd7
-- 
a60cd7
1.8.3.1
a60cd7