extend dump_dir to allow creating world-readable directory

- so far used only by kerneloops

11 files changed, 71 insertions, 34 deletions

diff --git a/src/daemon/MiddleWare.cpp b/src/daemon/MiddleWare.cpp
index 77e18b0a..9bbd2af3 100644
--- a/src/daemon/MiddleWare.cpp
+++ b/src/daemon/MiddleWare.cpp
@@ -768,15 +768,21 @@ int DeleteDebugDump(const char *dump_dir_name, long caller_uid)
         char caller_uid_str[sizeof(long) * 3 + 2];
         sprintf(caller_uid_str, "%ld", caller_uid);
 
-        char *uid = dd_load_text(dd, FILENAME_UID);
-        if (strcmp(uid, caller_uid_str) != 0)
+        char *uid = dd_load_text_ext(dd, FILENAME_UID, DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE);
+        /* we assume that the dump_dir can be handled by everyone if uid == NULL
+         * e.g: kerneloops
+        */
+        if (uid != NULL)
         {
-            char *inform_all = dd_load_text_ext(dd, FILENAME_INFORMALL, DD_FAIL_QUIETLY_ENOENT);
-            if (!string_to_bool(inform_all))
+            if (strcmp(uid, caller_uid_str) != 0)
             {
-                dd_close(dd);
-                error_msg("Dump directory '%s' can't be accessed by user with uid %ld", dump_dir_name, caller_uid);
-                return 1;
+                char *inform_all = dd_load_text_ext(dd, FILENAME_INFORMALL, DD_FAIL_QUIETLY_ENOENT);
+                if (!string_to_bool(inform_all))
+                {
+                    dd_close(dd);
+                    error_msg("Dump directory '%s' can't be accessed by user with uid %ld", dump_dir_name, caller_uid);
+                    return 1;
+                }
             }
         }
     }
diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c
index de22f427..0c45e3e8 100644
--- a/src/daemon/abrt-server.c
+++ b/src/daemon/abrt-server.c
@@ -114,7 +114,7 @@ static void create_debug_dump()
     /* No need to check the path length, as all variables used are limited, and dd_create()
        fails if the path is too long. */
 
-    struct dump_dir *dd = dd_create(path, client_uid);
+    struct dump_dir *dd = dd_create(path, client_uid, 0640);
     if (!dd)
     {
         error_msg_and_die("Error creating crash dump %s", path);
diff --git a/src/daemon/abrt.conf b/src/daemon/abrt.conf
index 7f5a9c58..460bf061 100644
--- a/src/daemon/abrt.conf
+++ b/src/daemon/abrt.conf
@@ -33,4 +33,4 @@ MaxCrashReportsSize = 1000
 # So far we support only one line here
 #
 [ LogScanners ]
-abrt-dump-oops = abrt-dump-oops -drw /var/log/messages
+abrt-dump-oops = abrt-dump-oops -drwx /var/log/messages
diff --git a/src/daemon/abrt_event.conf b/src/daemon/abrt_event.conf
index d9b87d5a..d127d38d 100644
--- a/src/daemon/abrt_event.conf
+++ b/src/daemon/abrt_event.conf
@@ -45,7 +45,8 @@ EVENT=post-create                       abrt-action-save-package-data
 
 include events.d/*.conf
 
-EVENT=post-create                       getent passwd "`cat uid`" | cut -d: -f1 >username
+# uid file doesn't exist for some problems like kerneloops
+EVENT=post-create                       if [ -e uid ]; then getent passwd "`cat uid`" | cut -d: -f1 >username; fi
 EVENT=post-create analyzer=Python       abrt-action-analyze-python
 EVENT=post-create analyzer=Kerneloops   abrt-action-analyze-oops
 
diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c
index c2ab166f..ddb5920a 100644
--- a/src/hooks/abrt-hook-ccpp.c
+++ b/src/hooks/abrt-hook-ccpp.c
@@ -494,7 +494,7 @@ int main(int argc, char** argv)
     if (path_len >= (sizeof(path) - sizeof("/"FILENAME_COREDUMP)))
         return 1;
 
-    struct dump_dir *dd = dd_create(path, uid);
+    struct dump_dir *dd = dd_create(path, uid, 0640);
     if (dd)
     {
         dd_create_basic_files(dd, uid);
diff --git a/src/include/report/dump_dir.h b/src/include/report/dump_dir.h
index a97a4f5c..026571b7 100644
--- a/src/include/report/dump_dir.h
+++ b/src/include/report/dump_dir.h
@@ -41,6 +41,8 @@ struct dump_dir {
     int locked;
     uid_t dd_uid;
     gid_t dd_gid;
+    /* mode fo saved files */
+    mode_t mode;
 };
 
 void dd_close(struct dump_dir *dd);
@@ -49,7 +51,7 @@ struct dump_dir *dd_opendir(const char *dir, int flags);
 /* Pass uid = (uid_t)-1L to disable chown'ing of newly created files
  * (IOW: if you aren't running under root):
  */
-struct dump_dir *dd_create(const char *dir, uid_t uid);
+struct dump_dir *dd_create(const char *dir, uid_t uid, mode_t mode);
 
 void dd_create_basic_files(struct dump_dir *dd, uid_t uid);
 int dd_exist(struct dump_dir *dd, const char *path);
diff --git a/src/lib/create_dump_dir.c b/src/lib/create_dump_dir.c
index cbacdab6..50893f05 100644
--- a/src/lib/create_dump_dir.c
+++ b/src/lib/create_dump_dir.c
@@ -22,7 +22,7 @@
 static struct dump_dir *try_dd_create(const char *base_dir_name, const char *dir_name)
 {
     char *path = concat_path_file(base_dir_name, dir_name);
-    struct dump_dir *dd = dd_create(path, (uid_t)-1L);
+    struct dump_dir *dd = dd_create(path, (uid_t)-1L, 0640);
     if (dd)
         dd_create_basic_files(dd, (uid_t)-1L);
     free(path);
diff --git a/src/lib/dump_dir.c b/src/lib/dump_dir.c
index 045149ac..464df5c9 100644
--- a/src/lib/dump_dir.c
+++ b/src/lib/dump_dir.c
@@ -287,6 +287,11 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
 
     dir = dd->dd_dirname = rm_trailing_slashes(dir);
 
+    struct stat stat_buf;
+    stat(dir, &stat_buf);
+    /* & 0666 should remove the executable bit */
+    dd->mode = (stat_buf.st_mode & 0666);
+
     errno = 0;
     if (dd_lock(dd, WAIT_FOR_OTHER_PROCESS_USLEEP, flags) < 0)
     {
@@ -294,7 +299,6 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
         {
             /* Directory is not writable. If it seems to be readable,
              * return "read only" dd, not NULL */
-            struct stat stat_buf;
             if (stat(dir, &stat_buf) == 0
              && S_ISDIR(stat_buf.st_mode)
              && access(dir, R_OK) == 0
@@ -355,7 +359,7 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
  *   Crashed application's User Id
  *
  * We currently have only three callers:
- * kernel oops hook: uid=0
+ * kernel oops hook: uid -> not saved, so everyone can steal and work with it
  *  this hook runs under 0:0
  * ccpp hook: uid=uid of crashed user's binary
  *  this hook runs under 0:0
@@ -365,10 +369,14 @@ struct dump_dir *dd_opendir(const char *dir, int flags)
  * Currently, we set dir's gid to passwd(uid)->pw_gid parameter, and we set uid to
  * abrt's user id. We do not allow write access to group.
  */
-struct dump_dir *dd_create(const char *dir, uid_t uid)
+struct dump_dir *dd_create(const char *dir, uid_t uid, mode_t mode)
 {
+    /* a little trick to copy read bits from file mode to exec bit of dir mode*/
+    mode_t dir_mode = mode | ((mode & 0444) >> 2);
     struct dump_dir *dd = dd_init();
 
+    dd->mode = mode;
+
     /* Unlike dd_opendir, can't use realpath: the directory doesn't exist yet,
      * realpath will always return NULL. We don't really have to:
      * dd_opendir(".") makes sense, dd_create(".") does not.
@@ -396,7 +404,7 @@ struct dump_dir *dd_create(const char *dir, uid_t uid)
      * the user to replace any file in the directory, changing security-sensitive data
      * (e.g. "uid", "analyzer", "executable")
      */
-    if (mkdir(dir, 0750) == -1)
+    if (mkdir(dir, dir_mode) == -1)
     {
         int err = errno;
         if (!created_parents && errno == ENOENT)
@@ -427,7 +435,7 @@ struct dump_dir *dd_create(const char *dir, uid_t uid)
     }
 
     /* mkdir's mode (above) can be affected by umask, fix it */
-    if (chmod(dir, 0750) == -1)
+    if (chmod(dir, dir_mode) == -1)
     {
         perror_msg("can't change mode of '%s'", dir);
         dd_close(dd);
@@ -472,10 +480,12 @@ void dd_create_basic_files(struct dump_dir *dd, uid_t uid)
     sprintf(long_str, "%lu", (long)t);
     dd_save_text(dd, FILENAME_TIME, long_str);
 
-    if (uid == (uid_t)-1)
-        uid = getuid();
-    sprintf(long_str, "%lu", (long)uid);
-    dd_save_text(dd, FILENAME_UID, long_str);
+    /* it doesn't make sense to create the uid file if uid == -1 */
+    if (uid != (uid_t)-1L)
+    {
+        sprintf(long_str, "%li", (long)uid);
+        dd_save_text(dd, FILENAME_UID, long_str);
+    }
 
     struct utsname buf;
     uname(&buf); /* never fails */
@@ -522,8 +532,8 @@ void dd_sanitize_mode_and_owner(struct dump_dir *dd)
         struct stat statbuf;
         if (lstat(full_path, &statbuf) == 0 && S_ISREG(statbuf.st_mode))
         {
-            if ((statbuf.st_mode & 0777) != 0640)
-                chmod(full_path, 0640);
+            if ((statbuf.st_mode & 0777) != dd->mode)
+                chmod(full_path, dd->mode);
             if (statbuf.st_uid != dd->dd_uid || statbuf.st_gid != dd->dd_gid)
             {
                 if (chown(full_path, dd->dd_uid, dd->dd_gid) != 0)
@@ -660,11 +670,11 @@ static char *load_text_file(const char *path, unsigned flags)
     return strbuf_free_nobuf(buf_content);
 }
 
-static bool save_binary_file(const char *path, const char* data, unsigned size, uid_t uid, gid_t gid)
+static bool save_binary_file(const char *path, const char* data, unsigned size, uid_t uid, gid_t gid, mode_t mode)
 {
-    /* "Why 0640?!" See dd_create() for security analysis */
+    /* the mode is set by the caller, see dd_create() for security analysis */
     unlink(path);
-    int fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, 0640);
+    int fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);
     if (fd < 0)
     {
         perror_msg("Can't open file '%s'", path);
@@ -717,7 +727,7 @@ void dd_save_text(struct dump_dir *dd, const char *name, const char *data)
         error_msg_and_die("dump_dir is not opened"); /* bug */
 
     char *full_path = concat_path_file(dd->dd_dirname, name);
-    save_binary_file(full_path, data, strlen(data), dd->dd_uid, dd->dd_gid);
+    save_binary_file(full_path, data, strlen(data), dd->dd_uid, dd->dd_gid, dd->mode);
     free(full_path);
 }
 
@@ -727,7 +737,7 @@ void dd_save_binary(struct dump_dir* dd, const char* name, const char* data, uns
         error_msg_and_die("dump_dir is not opened"); /* bug */
 
     char *full_path = concat_path_file(dd->dd_dirname, name);
-    save_binary_file(full_path, data, size, dd->dd_uid, dd->dd_gid);
+    save_binary_file(full_path, data, size, dd->dd_uid, dd->dd_gid, dd->mode);
     free(full_path);
 }
 
diff --git a/src/lib/steal_directory.c b/src/lib/steal_directory.c
index 6676a5fa..6991da18 100644
--- a/src/lib/steal_directory.c
+++ b/src/lib/steal_directory.c
@@ -31,7 +31,7 @@ struct dump_dir *steal_directory(const char *base_dir, const char *dump_dir_name
     char *dst_dir_name = concat_path_file(base_dir, base_name);
     while (1)
     {
-        dd_dst = dd_create(dst_dir_name, (uid_t)-1);
+        dd_dst = dd_create(dst_dir_name, (uid_t)-1, 0640);
         free(dst_dir_name);
         if (dd_dst)
             break;
diff --git a/src/plugins/abrt-dump-oops.c b/src/plugins/abrt-dump-oops.c
index f78ad23b..06f44520 100644
--- a/src/plugins/abrt-dump-oops.c
+++ b/src/plugins/abrt-dump-oops.c
@@ -25,6 +25,8 @@
 
 #define PROGNAME "abrt-dump-oops"
 
+static bool world_readable_dump = false;
+
 static void queue_oops(GList **vec, const char *data, const char *version)
 {
     char *ver_data = xasprintf("%s\n%s", version, data);
@@ -492,7 +494,16 @@ static int save_oops_to_dump_dir(GList *oops_list, unsigned oops_cnt)
 
     time_t t = time(NULL);
     const char *iso_date = iso_date_string(&t);
-    uid_t my_euid = geteuid();
+    /* dump should be readable by all if we're run with -x */
+    uid_t my_euid = (uid_t)-1L;
+    mode_t mode = 0644;
+    /* and readable only for the owner otherwise */
+    if (!world_readable_dump)
+    {
+        mode = 0644;
+        my_euid = geteuid();
+    }
+
     pid_t my_pid = getpid();
     int errors = 0;
     while (idx != 0 && --countdown != 0)
@@ -504,10 +515,10 @@ static int save_oops_to_dump_dir(GList *oops_list, unsigned oops_cnt)
         char *second_line = (char*)strchr(first_line, '\n'); /* never NULL */
         *second_line++ = '\0';
 
-        struct dump_dir *dd = dd_create(path, /*uid:*/ my_euid);
+        struct dump_dir *dd = dd_create(path, /*uid:*/ my_euid, mode);
         if (dd)
         {
-            dd_create_basic_files(dd, /*uid:*/ 0);
+            dd_create_basic_files(dd, /*uid:*/ my_euid);
             dd_save_text(dd, FILENAME_ANALYZER, "Kerneloops");
 // TODO: drop FILENAME_EXECUTABLE?
             dd_save_text(dd, FILENAME_EXECUTABLE, "kernel");
@@ -552,6 +563,7 @@ int main(int argc, char **argv)
         OPT_d = 1 << 3,
         OPT_o = 1 << 4,
         OPT_w = 1 << 5,
+        OPT_x = 1 << 6,
     };
     /* Keep enum above and order of options below in sync! */
     struct options program_options[] = {
@@ -561,6 +573,10 @@ int main(int argc, char **argv)
         OPT_BOOL('d', NULL, NULL, _("Create ABRT dump for every oops found")),
         OPT_BOOL('o', NULL, NULL, _("Print found oopses on standard output")),
         OPT_BOOL('w', NULL, NULL, _("Do not exit, watch the file for new oopses")),
+        /* oopses doesn't contain any sensitive info, and even
+         * the old koops app was showing the oopses to all users
+         */
+        OPT_BOOL('x', NULL, NULL, _("Make the dump directory world readable")),
         OPT_END()
     };
     unsigned opts = parse_opts(argc, argv, program_options, program_usage_string);
@@ -593,6 +609,8 @@ int main(int argc, char **argv)
         /* Scan dmesg (only once even with -w) */
         scan_dmesg(&oops_list);
 
+    world_readable_dump = (opts & OPT_x);
+
     int partial_line_len = 0;
     struct stat statbuf;
     int file_fd = -1;
diff --git a/src/report-python/dump_dir.c b/src/report-python/dump_dir.c
index 96a96fba..c34cc869 100644
--- a/src/report-python/dump_dir.c
+++ b/src/report-python/dump_dir.c
@@ -249,7 +249,7 @@ PyObject *p_dd_create(PyObject *module, PyObject *args)
     p_dump_dir *new_dd = PyObject_New(p_dump_dir, &p_dump_dir_type);
     if (!new_dd)
         return NULL;
-    new_dd->dd = dd_create(dir, uid);
+    new_dd->dd = dd_create(dir, uid, 0640);
     return (PyObject*)new_dd;
 }