From fa1950198b4cd7ea9c3c04c4d0abcde7a69ff912 Mon Sep 17 00:00:00 2001 From: Matej Habrnal Date: Jun 17 2015 14:53:27 +0000 Subject: Several bug fixes Move the default dump location to /var/spool/abrt from /var/tmp/abrt and Use root for owner of all dump directories Fixes for CVE-2015-3315, CVE-2015-3142, CVE-2015-1869, CVE-2015-1870 Fixes for CVE-2015-3147, CVE-2015-3151, CVE-2015-3150, CVE-2015-3159 Resolves: #1179752 Signed-off-by: Matej Habrnal --- diff --git a/0053-abrt-hook-ccpp-minor-refactoring.patch b/0053-abrt-hook-ccpp-minor-refactoring.patch new file mode 100644 index 0000000..9d72d1f --- /dev/null +++ b/0053-abrt-hook-ccpp-minor-refactoring.patch @@ -0,0 +1,211 @@ +From 7a10fa7c19e876ea7e5109d3d71dc9bbffc70214 Mon Sep 17 00:00:00 2001 +From: Martin Milata +Date: Mon, 1 Dec 2014 11:47:55 +0100 +Subject: [PATCH] abrt-hook-ccpp: minor refactoring + +Related to #829. + +Signed-off-by: Martin Milata +--- + src/hooks/abrt-hook-ccpp.c | 79 ++++++++++++++++++++++++---------------------- + 1 file changed, 41 insertions(+), 38 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 6f471e9..3785d89 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -140,11 +140,9 @@ static off_t copyfd_sparse(int src_fd, int dst_fd1, int dst_fd2, off_t size2) + + + /* Global data */ +- + static char *user_pwd; +-static char *proc_pid_status; + static struct dump_dir *dd; +-static int user_core_fd = -1; ++ + /* + * %s - signal number + * %c - ulimit -c value +@@ -210,7 +208,7 @@ static char* get_rootdir(pid_t pid) + return malloc_readlink(buf); + } + +-static int get_fsuid(void) ++static int get_fsuid(char *proc_pid_status) + { + int real, euid, saved; + /* if we fail to parse the uid, then make it root only readable to be safe */ +@@ -434,7 +432,7 @@ static bool dump_fd_info(const char *dest_filename, char *source_filename, int s + } + + /* Like xopen, but on error, unlocks and deletes dd and user core */ +-static int create_or_die(const char *filename) ++static int create_or_die(const char *filename, int user_core_fd) + { + int fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, DEFAULT_DUMP_DIR_MODE); + if (fd >= 0) +@@ -455,6 +453,31 @@ static int create_or_die(const char *filename) + perror_msg_and_die("Can't open '%s'", filename); + } + ++static int create_user_core(int user_core_fd, pid_t pid, off_t ulimit_c) ++{ ++ if (user_core_fd >= 0) ++ { ++ off_t core_size = copyfd_size(STDIN_FILENO, user_core_fd, ulimit_c, COPYFD_SPARSE); ++ if (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 || core_size < 0) ++ { ++ /* perror first, otherwise unlink may trash errno */ ++ perror_msg("Error writing '%s'", full_core_basename); ++ xchdir(user_pwd); ++ unlink(core_basename); ++ return 1; ++ } ++ if (ulimit_c == 0 || core_size > ulimit_c) ++ { ++ xchdir(user_pwd); ++ unlink(core_basename); ++ return 1; ++ } ++ log_notice("Saved core dump of pid %lu to %s (%llu bytes)", (long)pid, full_core_basename, (long long)core_size); ++ } ++ ++ return 0; ++} ++ + int main(int argc, char** argv) + { + /* Kernel starts us with all fd's closed. +@@ -558,10 +581,10 @@ int main(int argc, char** argv) + log_notice("user_pwd:'%s'", user_pwd); + + sprintf(path, "/proc/%lu/status", (long)pid); +- proc_pid_status = xmalloc_xopen_read_close(path, /*maxsz:*/ NULL); ++ char *proc_pid_status = xmalloc_xopen_read_close(path, /*maxsz:*/ NULL); + + uid_t fsuid = uid; +- uid_t tmp_fsuid = get_fsuid(); ++ uid_t tmp_fsuid = get_fsuid(proc_pid_status); + int suid_policy = dump_suid_policy(); + if (tmp_fsuid != uid) + { +@@ -574,6 +597,7 @@ int main(int argc, char** argv) + } + + /* Open a fd to compat coredump, if requested and is possible */ ++ int user_core_fd = -1; + if (setting_MakeCompatCore && ulimit_c != 0) + /* note: checks "user_pwd == NULL" inside; updates core_basename */ + user_core_fd = open_user_core(uid, fsuid, pid, &argv[1]); +@@ -582,7 +606,7 @@ int main(int argc, char** argv) + { + /* readlink on /proc/$PID/exe failed, don't create abrt dump dir */ + error_msg("Can't read /proc/%lu/exe link", (long)pid); +- goto create_user_core; ++ return create_user_core(user_core_fd, pid, ulimit_c); + } + + const char *signame = NULL; +@@ -601,7 +625,7 @@ int main(int argc, char** argv) + //case SIGSYS : signame = "SYS" ; break; //Bad argument to routine (SVr4) + //case SIGXCPU: signame = "XCPU"; break; //CPU time limit exceeded (4.2BSD) + //case SIGXFSZ: signame = "XFSZ"; break; //File size limit exceeded (4.2BSD) +- default: goto create_user_core; // not a signal we care about ++ default: return create_user_core(user_core_fd, pid, ulimit_c); // not a signal we care about + } + + if (!daemon_is_ok()) +@@ -611,14 +635,14 @@ int main(int argc, char** argv) + "/proc/sys/kernel/core_pattern contains a stale value, " + "consider resetting it to 'core'" + ); +- goto create_user_core; ++ return create_user_core(user_core_fd, pid, ulimit_c); + } + + if (g_settings_nMaxCrashReportsSize > 0) + { + /* If free space is less than 1/4 of MaxCrashReportsSize... */ + if (low_free_space(g_settings_nMaxCrashReportsSize, g_settings_dump_location)) +- goto create_user_core; ++ return create_user_core(user_core_fd, pid, ulimit_c); + } + + /* Check /var/tmp/abrt/last-ccpp marker, do not dump repeated crashes +@@ -628,7 +652,7 @@ int main(int argc, char** argv) + if (check_recent_crash_file(path, executable)) + { + /* It is a repeating crash */ +- goto create_user_core; ++ return create_user_core(user_core_fd, pid, ulimit_c); + } + + const char *last_slash = strrchr(executable, '/'); +@@ -657,7 +681,7 @@ int main(int argc, char** argv) + g_settings_dump_location, iso_date_string(NULL), (long)pid); + if (path_len >= (sizeof(path) - sizeof("/"FILENAME_COREDUMP))) + { +- goto create_user_core; ++ return create_user_core(user_core_fd, pid, ulimit_c); + } + + /* use fsuid instead of uid, so we don't expose any sensitive +@@ -741,7 +765,7 @@ int main(int argc, char** argv) + if (src_fd_binary > 0) + { + strcpy(path + path_len, "/"FILENAME_BINARY); +- int dst_fd = create_or_die(path); ++ int dst_fd = create_or_die(path, user_core_fd); + off_t sz = copyfd_eof(src_fd_binary, dst_fd, COPYFD_SPARSE); + if (fsync(dst_fd) != 0 || close(dst_fd) != 0 || sz < 0) + { +@@ -752,7 +776,7 @@ int main(int argc, char** argv) + } + + strcpy(path + path_len, "/"FILENAME_COREDUMP); +- int abrt_core_fd = create_or_die(path); ++ int abrt_core_fd = create_or_die(path, user_core_fd); + + /* We write both coredumps at once. + * We can't write user coredump first, since it might be truncated +@@ -812,7 +836,7 @@ int main(int argc, char** argv) + if (src_fd >= 0) + { + strcpy(path + path_len, "/hs_err.log"); +- int dst_fd = create_or_die(path); ++ int dst_fd = create_or_die(path, user_core_fd); + off_t sz = copyfd_eof(src_fd, dst_fd, COPYFD_SPARSE); + if (close(dst_fd) != 0 || sz < 0) + { +@@ -856,26 +880,5 @@ int main(int argc, char** argv) + } + + /* We didn't create abrt dump, but may need to create compat coredump */ +- create_user_core: +- if (user_core_fd >= 0) +- { +- off_t core_size = copyfd_size(STDIN_FILENO, user_core_fd, ulimit_c, COPYFD_SPARSE); +- if (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 || core_size < 0) +- { +- /* perror first, otherwise unlink may trash errno */ +- perror_msg("Error writing '%s'", full_core_basename); +- xchdir(user_pwd); +- unlink(core_basename); +- return 1; +- } +- if (ulimit_c == 0 || core_size > ulimit_c) +- { +- xchdir(user_pwd); +- unlink(core_basename); +- return 1; +- } +- log_notice("Saved core dump of pid %lu to %s (%llu bytes)", (long)pid, full_core_basename, (long long)core_size); +- } +- +- return 0; ++ return create_user_core(user_core_fd, pid, ulimit_c); + } +-- +2.1.0 + diff --git a/0054-Create-core-backtrace-in-unwind-hook.patch b/0054-Create-core-backtrace-in-unwind-hook.patch new file mode 100644 index 0000000..bb8b7c0 --- /dev/null +++ b/0054-Create-core-backtrace-in-unwind-hook.patch @@ -0,0 +1,356 @@ +From 0f669af4d7180b12142a41117ae2459d6960dd31 Mon Sep 17 00:00:00 2001 +From: Martin Milata +Date: Mon, 1 Dec 2014 12:05:36 +0100 +Subject: [PATCH] Create core backtrace in unwind hook + +Related to #829. + +We need to implement #882 in order for this to work. This change +requires (yet unreleased) satyr-0.16. + +The feature is turned off by default, you need to pass +--enable-dump-time-unwind to configure in order to enable it. + +Signed-off-by: Martin Milata +--- + configure.ac | 12 ++ + doc/abrt-CCpp.conf.txt | 18 +++ + .../en-US/Automatic_Bug_Reporting_Tool_ABRT.xml | 2 +- + src/hooks/CCpp.conf | 15 +++ + src/hooks/abrt-hook-ccpp.c | 126 ++++++++++++++------- + src/hooks/abrt-install-ccpp-hook.in | 4 +- + 6 files changed, 132 insertions(+), 45 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 71d7c18..d7e0ea5 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -266,6 +266,18 @@ AC_ARG_ENABLE([addon-vmcore], + AM_CONDITIONAL(BUILD_ADDON_VMCORE, false) + [fi] + ++# Perform stack unwind on live/dying process in the core handler? ++ ++AC_ARG_ENABLE([dump-time-unwind], ++ AS_HELP_STRING([--enable-dump-time-unwind], ++ [create core stack trace while the crashed process is still in memory (default is no)]), ++ [], [enable_dump_time_unwind=no]) ++ ++[if test "$enable_native_unwinder" = "yes" -a "$enable_dump_time_unwind" = "yes"] ++[then] ++ AC_DEFINE([ENABLE_DUMP_TIME_UNWIND], [1], [Create core stacktrace while the process is still in memory.]) ++[fi] ++ + AC_SUBST(CONF_DIR) + AC_SUBST(DEFAULT_CONF_DIR) + AC_SUBST(VAR_RUN) +diff --git a/doc/abrt-CCpp.conf.txt b/doc/abrt-CCpp.conf.txt +index ad3830b..498d53d 100644 +--- a/doc/abrt-CCpp.conf.txt ++++ b/doc/abrt-CCpp.conf.txt +@@ -19,12 +19,30 @@ SaveBinaryImage = 'yes' / 'no' ...:: + Useful, for example, when _deleted binary_ segfaults. + Default is 'no'. + ++CreateCoreBacktrace = 'yes' / 'no' ...:: ++ When this option is set to 'yes', core backtrace is generated ++ from the memory image of the crashing process. Only the crash ++ thread is present in the backtrace. This feature requires ++ kernel 3.18 or newer, otherwise the core backtrace is not ++ created. ++ Default is 'yes'. ++ ++SaveFullCore = 'yes' / 'no' ...:: ++ Save full coredump? If set to 'no', coredump won't be saved ++ and you won't be able to report the crash to Bugzilla. Only ++ useful with 'CreateCoreBacktrace' set to 'yes'. Please ++ note that if this option is set to 'no' and MakeCompatCore ++ is set to 'yes', the core is still written to the current ++ directory. ++ Default is 'yes'. ++ + VerboseLog = NUM:: + Used to make the hook more verbose + + SEE ALSO + -------- + abrt.conf(5) ++abrt-action-generate-core-backtrace(1) + + AUTHORS + ------- +diff --git a/doc/deployment/en-US/Automatic_Bug_Reporting_Tool_ABRT.xml b/doc/deployment/en-US/Automatic_Bug_Reporting_Tool_ABRT.xml +index e11307a..daee375 100644 +--- a/doc/deployment/en-US/Automatic_Bug_Reporting_Tool_ABRT.xml ++++ b/doc/deployment/en-US/Automatic_Bug_Reporting_Tool_ABRT.xml +@@ -187,7 +187,7 @@ Starting abrt daemon: [ OK ] + + Please note that installing ABRT packages overwrites the /proc/sys/kernel/core_pattern file which can contain a template used to name core dump files. The content of this file will be overwritten to: + +- |/usr/libexec/abrt-hook-ccpp %s %c %p %u %g %t e ++ |/usr/libexec/abrt-hook-ccpp %s %c %p %u %g %t e %i + + + Finally, if you run ABRT in a graphical desktop environment, you can verify that the ABRT notification applet is running: +diff --git a/src/hooks/CCpp.conf b/src/hooks/CCpp.conf +index d199116..b1a0a22 100644 +--- a/src/hooks/CCpp.conf ++++ b/src/hooks/CCpp.conf +@@ -8,6 +8,21 @@ MakeCompatCore = yes + # (useful, for example, when _deleted binary_ segfaults) + SaveBinaryImage = no + ++# When this option is set to 'yes', core backtrace is generated ++# from the memory image of the crashing process. Only the crash ++# thread is present in the backtrace. This feature requires ++# kernel 3.18 or newer, otherwise the core backtrace is not ++# created. ++CreateCoreBacktrace = yes ++ ++# Save full coredump? If set to 'no', coredump won't be saved ++# and you won't be able to report the crash to Bugzilla. Only ++# useful with CreateCoreBacktrace set to 'yes'. Please ++# note that if this option is set to 'no' and MakeCompatCore ++# is set to 'yes', the core is still written to the current ++# directory. ++SaveFullCore = yes ++ + # Used for debugging the hook + #VerboseLog = 2 + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 3785d89..57c56a7 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -21,6 +21,11 @@ + #include + #include "libabrt.h" + ++#ifdef ENABLE_DUMP_TIME_UNWIND ++#include ++#include ++#endif /* ENABLE_DUMP_TIME_UNWIND */ ++ + #define DUMP_SUID_UNSAFE 1 + #define DUMP_SUID_SAFE 2 + +@@ -151,13 +156,13 @@ static struct dump_dir *dd; + * %g - gid + * %t - UNIX time of dump + * %e - executable filename +- * %h - hostname ++ * %i - crash thread tid + * %% - output one "%" + */ + /* Hook must be installed with exactly the same sequence of %c specifiers. + * Last one, %h, may be omitted (we can find it out). + */ +-static const char percent_specifiers[] = "%scpugteh"; ++static const char percent_specifiers[] = "%scpugtei"; + static char *core_basename = (char*) "core"; + /* + * Used for error messages only. +@@ -453,6 +458,24 @@ static int create_or_die(const char *filename, int user_core_fd) + perror_msg_and_die("Can't open '%s'", filename); + } + ++static void create_core_backtrace(pid_t tid, const char *executable, int signal_no, const char *dd_path) ++{ ++#ifdef ENABLE_DUMP_TIME_UNWIND ++ if (g_verbose > 1) ++ sr_debug_parser = true; ++ ++ char *error_message = NULL; ++ bool success = sr_abrt_create_core_stacktrace_from_core_hook(dd_path, tid, executable, ++ signal_no, &error_message); ++ ++ if (!success) ++ { ++ log("Failed to create core_backtrace: %s", error_message); ++ free(error_message); ++ } ++#endif /* ENABLE_DUMP_TIME_UNWIND */ ++} ++ + static int create_user_core(int user_core_fd, pid_t pid, off_t ulimit_c) + { + if (user_core_fd >= 0) +@@ -493,9 +516,9 @@ int main(int argc, char** argv) + + if (argc < 8) + { +- /* percent specifier: %s %c %p %u %g %t %e %h */ ++ /* percent specifier: %s %c %p %u %g %t %e %i */ + /* argv: [0] [1] [2] [3] [4] [5] [6] [7] [8]*/ +- error_msg_and_die("Usage: %s SIGNO CORE_SIZE_LIMIT PID UID GID TIME BINARY_NAME [HOSTNAME]", argv[0]); ++ error_msg_and_die("Usage: %s SIGNO CORE_SIZE_LIMIT PID UID GID TIME BINARY_NAME [TID]", argv[0]); + } + + /* Not needed on 2.6.30. +@@ -520,6 +543,8 @@ int main(int argc, char** argv) + /* ... and plugins/CCpp.conf */ + bool setting_MakeCompatCore; + bool setting_SaveBinaryImage; ++ bool setting_SaveFullCore; ++ bool setting_CreateCoreBacktrace; + { + map_string_t *settings = new_map_string(); + load_abrt_plugin_conf_file("CCpp.conf", settings); +@@ -528,6 +553,10 @@ int main(int argc, char** argv) + setting_MakeCompatCore = value && string_to_bool(value); + value = get_map_string_item_or_NULL(settings, "SaveBinaryImage"); + setting_SaveBinaryImage = value && string_to_bool(value); ++ value = get_map_string_item_or_NULL(settings, "SaveFullCore"); ++ setting_SaveFullCore = value ? string_to_bool(value) : true; ++ value = get_map_string_item_or_NULL(settings, "CreateCoreBacktrace"); ++ setting_CreateCoreBacktrace = value ? string_to_bool(value) : true; + value = get_map_string_item_or_NULL(settings, "VerboseLog"); + if (value) + g_verbose = xatoi_positive(value); +@@ -560,11 +589,10 @@ int main(int argc, char** argv) + free(s); + } + +- struct utsname uts; +- if (!argv[8]) /* no HOSTNAME? */ ++ pid_t tid = 0; ++ if (argv[8]) + { +- uname(&uts); +- argv[8] = uts.nodename; ++ tid = xatoi_positive(argv[8]); + } + + char path[PATH_MAX]; +@@ -769,49 +797,57 @@ int main(int argc, char** argv) + off_t sz = copyfd_eof(src_fd_binary, dst_fd, COPYFD_SPARSE); + if (fsync(dst_fd) != 0 || close(dst_fd) != 0 || sz < 0) + { +- dd_delete(dd); +- error_msg_and_die("Error saving '%s'", path); ++ dd_delete(dd); error_msg_and_die("Error saving '%s'", path); + } + close(src_fd_binary); + } + +- strcpy(path + path_len, "/"FILENAME_COREDUMP); +- int abrt_core_fd = create_or_die(path, user_core_fd); +- +- /* We write both coredumps at once. +- * We can't write user coredump first, since it might be truncated +- * and thus can't be copied and used as abrt coredump; +- * and if we write abrt coredump first and then copy it as user one, +- * then we have a race when process exits but coredump does not exist yet: +- * $ echo -e '#include\nmain(){raise(SIGSEGV);}' | gcc -o test -x c - +- * $ rm -f core*; ulimit -c unlimited; ./test; ls -l core* +- * 21631 Segmentation fault (core dumped) ./test +- * ls: cannot access core*: No such file or directory <=== BAD +- */ +- off_t core_size = copyfd_sparse(STDIN_FILENO, abrt_core_fd, user_core_fd, ulimit_c); +- if (fsync(abrt_core_fd) != 0 || close(abrt_core_fd) != 0 || core_size < 0) ++ off_t core_size = 0; ++ if (setting_SaveFullCore) + { +- unlink(path); +- dd_delete(dd); +- if (user_core_fd >= 0) ++ strcpy(path + path_len, "/"FILENAME_COREDUMP); ++ int abrt_core_fd = create_or_die(path, user_core_fd); ++ ++ /* We write both coredumps at once. ++ * We can't write user coredump first, since it might be truncated ++ * and thus can't be copied and used as abrt coredump; ++ * and if we write abrt coredump first and then copy it as user one, ++ * then we have a race when process exits but coredump does not exist yet: ++ * $ echo -e '#include\nmain(){raise(SIGSEGV);}' | gcc -o test -x c - ++ * $ rm -f core*; ulimit -c unlimited; ./test; ls -l core* ++ * 21631 Segmentation fault (core dumped) ./test ++ * ls: cannot access core*: No such file or directory <=== BAD ++ */ ++ core_size = copyfd_sparse(STDIN_FILENO, abrt_core_fd, user_core_fd, ulimit_c); ++ if (fsync(abrt_core_fd) != 0 || close(abrt_core_fd) != 0 || core_size < 0) + { ++ unlink(path); ++ dd_delete(dd); ++ if (user_core_fd >= 0) ++ { ++ xchdir(user_pwd); ++ unlink(core_basename); ++ } ++ /* copyfd_sparse logs the error including errno string, ++ * but it does not log file name */ ++ error_msg_and_die("Error writing '%s'", path); ++ } ++ if (user_core_fd >= 0 ++ /* error writing user coredump? */ ++ && (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 ++ /* user coredump is too big? */ ++ || (ulimit_c == 0 /* paranoia */ || core_size > ulimit_c) ++ ) ++ ) { ++ /* nuke it (silently) */ + xchdir(user_pwd); + unlink(core_basename); + } +- /* copyfd_sparse logs the error including errno string, +- * but it does not log file name */ +- error_msg_and_die("Error writing '%s'", path); + } +- if (user_core_fd >= 0 +- /* error writing user coredump? */ +- && (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 +- /* user coredump is too big? */ +- || (ulimit_c == 0 /* paranoia */ || core_size > ulimit_c) +- ) +- ) { +- /* nuke it (silently) */ +- xchdir(user_pwd); +- unlink(core_basename); ++ else ++ { ++ /* User core is created even if WriteFullCore is off. */ ++ create_user_core(user_core_fd, pid, ulimit_c); + } + + /* Save JVM crash log if it exists. (JVM's coredump per se +@@ -847,6 +883,10 @@ int main(int argc, char** argv) + } + } + ++ /* Perform crash-time unwind of the guilty thread. */ ++ if (tid > 0 && setting_CreateCoreBacktrace) ++ create_core_backtrace(tid, executable, signal_no, dd->dd_dirname); ++ + /* We close dumpdir before we start catering for crash storm case. + * Otherwise, delete_dump_dir's from other concurrent + * CCpp's won't be able to delete our dump (their delete_dump_dir +@@ -860,7 +900,9 @@ int main(int argc, char** argv) + strcpy(path, newpath); + free(newpath); + +- log_notice("Saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size); ++ if (core_size > 0) ++ log_notice("Saved core dump of pid %lu (%s) to %s (%llu bytes)", ++ (long)pid, executable, path, (long long)core_size); + + notify_new_path(path); + +diff --git a/src/hooks/abrt-install-ccpp-hook.in b/src/hooks/abrt-install-ccpp-hook.in +index aa01231..d4ed4a5 100755 +--- a/src/hooks/abrt-install-ccpp-hook.in ++++ b/src/hooks/abrt-install-ccpp-hook.in +@@ -11,9 +11,9 @@ SAVED_PATTERN_DIR="@VAR_RUN@/abrt" + SAVED_PATTERN_FILE="@VAR_RUN@/abrt/saved_core_pattern" + HOOK_BIN="@libexecdir@/abrt-hook-ccpp" + # Must match percent_specifiers[] order in abrt-hook-ccpp.c: +-PATTERN="|$HOOK_BIN %s %c %p %u %g %t %e" ++PATTERN="|$HOOK_BIN %s %c %p %u %g %t %e %i" + # Same, but with bogus "executable name" parameter +-PATTERN1="|$HOOK_BIN %s %c %p %u %g %t e" ++PATTERN1="|$HOOK_BIN %s %c %p %u %g %t e %i" + + # core_pipe_limit specifies how many dump_helpers can run at the same time + # 0 - means unlimited, but it's not guaranteed that /proc/ of crashing +-- +2.1.0 + diff --git a/0055-abrt-install-ccpp-hook-check-configuration.patch b/0055-abrt-install-ccpp-hook-check-configuration.patch new file mode 100644 index 0000000..7235b50 --- /dev/null +++ b/0055-abrt-install-ccpp-hook-check-configuration.patch @@ -0,0 +1,124 @@ +From 8f6a49db8997b1296eafa639920e039f87fb1989 Mon Sep 17 00:00:00 2001 +From: Martin Milata +Date: Mon, 8 Dec 2014 17:01:56 +0100 +Subject: [PATCH] abrt-install-ccpp-hook check configuration + +Check that either full coredump or core backtrace are configured to be +saved, fail init script if neither is. + +Related to #829. + +Signed-off-by: Martin Milata +--- + src/hooks/abrt-hook-ccpp.c | 63 +++++++++++++++++++++++-------------- + src/hooks/abrt-install-ccpp-hook.in | 5 +++ + 2 files changed, 44 insertions(+), 24 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 57c56a7..f8f97ad 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -501,6 +501,18 @@ static int create_user_core(int user_core_fd, pid_t pid, off_t ulimit_c) + return 0; + } + ++static int test_configuration(bool setting_SaveFullCore, bool setting_CreateCoreBacktrace) ++{ ++ if (!setting_SaveFullCore && !setting_CreateCoreBacktrace) ++ { ++ fprintf(stderr, "Both SaveFullCore and CreateCoreBacktrace are disabled - " ++ "at least one of them is needed for useful report.\n"); ++ return 1; ++ } ++ ++ return 0; ++} ++ + int main(int argc, char** argv) + { + /* Kernel starts us with all fd's closed. +@@ -510,31 +522,9 @@ int main(int argc, char** argv) + */ + int fd = xopen("/dev/null", O_RDWR); + while (fd < 2) +- fd = xdup(fd); ++ fd = xdup(fd); + if (fd > 2) +- close(fd); +- +- if (argc < 8) +- { +- /* percent specifier: %s %c %p %u %g %t %e %i */ +- /* argv: [0] [1] [2] [3] [4] [5] [6] [7] [8]*/ +- error_msg_and_die("Usage: %s SIGNO CORE_SIZE_LIMIT PID UID GID TIME BINARY_NAME [TID]", argv[0]); +- } +- +- /* Not needed on 2.6.30. +- * At least 2.6.18 has a bug where +- * argv[1] = "SIGNO CORE_SIZE_LIMIT PID ..." +- * argv[2] = "CORE_SIZE_LIMIT PID ..." +- * and so on. Fixing it: +- */ +- if (strchr(argv[1], ' ')) +- { +- int i; +- for (i = 1; argv[i]; i++) +- { +- strchrnul(argv[i], ' ')[0] = '\0'; +- } +- } ++ close(fd); + + logmode = LOGMODE_JOURNAL; + +@@ -563,6 +553,31 @@ int main(int argc, char** argv) + free_map_string(settings); + } + ++ if (argc == 2 && strcmp(argv[1], "--config-test")) ++ return test_configuration(setting_SaveFullCore, setting_CreateCoreBacktrace); ++ ++ if (argc < 8) ++ { ++ /* percent specifier: %s %c %p %u %g %t %e %i */ ++ /* argv: [0] [1] [2] [3] [4] [5] [6] [7] [8]*/ ++ error_msg_and_die("Usage: %s SIGNO CORE_SIZE_LIMIT PID UID GID TIME BINARY_NAME [TID]", argv[0]); ++ } ++ ++ /* Not needed on 2.6.30. ++ * At least 2.6.18 has a bug where ++ * argv[1] = "SIGNO CORE_SIZE_LIMIT PID ..." ++ * argv[2] = "CORE_SIZE_LIMIT PID ..." ++ * and so on. Fixing it: ++ */ ++ if (strchr(argv[1], ' ')) ++ { ++ int i; ++ for (i = 1; argv[i]; i++) ++ { ++ strchrnul(argv[i], ' ')[0] = '\0'; ++ } ++ } ++ + errno = 0; + const char* signal_str = argv[1]; + int signal_no = xatoi_positive(signal_str); +diff --git a/src/hooks/abrt-install-ccpp-hook.in b/src/hooks/abrt-install-ccpp-hook.in +index d4ed4a5..fff0a33 100755 +--- a/src/hooks/abrt-install-ccpp-hook.in ++++ b/src/hooks/abrt-install-ccpp-hook.in +@@ -31,6 +31,11 @@ CORE_PIPE_LIMIT_FILE="/proc/sys/kernel/core_pipe_limit" + CORE_PIPE_LIMIT="4" + + start() { ++ if ! $HOOK_BIN --test-config; then ++ echo "Invalid configuration." ++ exit 1 ++ fi ++ + cur=`cat "$PATTERN_FILE"` + cur_first=`printf "%s" "$cur" | sed 's/ .*//'` + +-- +2.1.0 + diff --git a/0056-ccpp-hook-move-proc-pid-utils-to-libreport.patch b/0056-ccpp-hook-move-proc-pid-utils-to-libreport.patch new file mode 100644 index 0000000..e7a80f1 --- /dev/null +++ b/0056-ccpp-hook-move-proc-pid-utils-to-libreport.patch @@ -0,0 +1,309 @@ +From 105ff10f12dce019ca2a455001239967c2e0e856 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 8 Dec 2014 16:02:11 +0100 +Subject: [PATCH] ccpp-hook: move /proc/[pid]/ utils to libreport + +Related to #548 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 202 +++++++++++++++------------------------------ + 1 file changed, 66 insertions(+), 136 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index f8f97ad..7fd9520 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -36,20 +36,6 @@ + */ + #define IGNORE_RESULT(func_call) do { if (func_call) /* nothing */; } while (0) + +-static char* malloc_readlink(const char *linkname) +-{ +- char buf[PATH_MAX + 1]; +- int len; +- +- len = readlink(linkname, buf, sizeof(buf)-1); +- if (len >= 0) +- { +- buf[len] = '\0'; +- return xstrdup(buf); +- } +- return NULL; +-} +- + /* Custom version of copyfd_xyz, + * one which is able to write into two descriptors at once. + */ +@@ -171,75 +157,6 @@ static char *core_basename = (char*) "core"; + */ + static char *full_core_basename; + +- +-static char* get_executable(pid_t pid, int *fd_p) +-{ +- char buf[sizeof("/proc/%lu/exe") + sizeof(long)*3]; +- +- sprintf(buf, "/proc/%lu/exe", (long)pid); +- if (fd_p) +- *fd_p = open(buf, O_RDONLY); /* might fail and return -1, it's ok */ +- char *executable = malloc_readlink(buf); +- if (!executable) +- return NULL; +- /* find and cut off " (deleted)" from the path */ +- char *deleted = executable + strlen(executable) - strlen(" (deleted)"); +- if (deleted > executable && strcmp(deleted, " (deleted)") == 0) +- { +- *deleted = '\0'; +- log_info("File '%s' seems to be deleted", executable); +- } +- /* find and cut off prelink suffixes from the path */ +- char *prelink = executable + strlen(executable) - strlen(".#prelink#.XXXXXX"); +- if (prelink > executable && strncmp(prelink, ".#prelink#.", strlen(".#prelink#.")) == 0) +- { +- log_info("File '%s' seems to be a prelink temporary file", executable); +- *prelink = '\0'; +- } +- return executable; +-} +- +-static char* get_cwd(pid_t pid) +-{ +- char buf[sizeof("/proc/%lu/cwd") + sizeof(long)*3]; +- sprintf(buf, "/proc/%lu/cwd", (long)pid); +- return malloc_readlink(buf); +-} +- +-static char* get_rootdir(pid_t pid) +-{ +- char buf[sizeof("/proc/%lu/root") + sizeof(long)*3]; +- sprintf(buf, "/proc/%lu/root", (long)pid); +- return malloc_readlink(buf); +-} +- +-static int get_fsuid(char *proc_pid_status) +-{ +- int real, euid, saved; +- /* if we fail to parse the uid, then make it root only readable to be safe */ +- int fs_uid = 0; +- +- char *line = proc_pid_status; /* never NULL */ +- for (;;) +- { +- if (strncmp(line, "Uid", 3) == 0) +- { +- int n = sscanf(line, "Uid:\t%d\t%d\t%d\t%d\n", &real, &euid, &saved, &fs_uid); +- if (n != 4) +- { +- perror_msg_and_die("Can't parse Uid: line"); +- } +- break; +- } +- line = strchr(line, '\n'); +- if (!line) +- break; +- line++; +- } +- +- return fs_uid; +-} +- + static int dump_suid_policy() + { + /* +@@ -400,42 +317,6 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + return user_core_fd; + } + +-static bool dump_fd_info(const char *dest_filename, char *source_filename, int source_base_ofs) +-{ +- FILE *fp = fopen(dest_filename, "w"); +- if (!fp) +- return false; +- +- unsigned fd = 0; +- while (fd <= 99999) /* paranoia check */ +- { +- sprintf(source_filename + source_base_ofs, "fd/%u", fd); +- char *name = malloc_readlink(source_filename); +- if (!name) +- break; +- fprintf(fp, "%u:%s\n", fd, name); +- free(name); +- +- sprintf(source_filename + source_base_ofs, "fdinfo/%u", fd); +- fd++; +- FILE *in = fopen(source_filename, "r"); +- if (!in) +- continue; +- char buf[128]; +- while (fgets(buf, sizeof(buf)-1, in)) +- { +- /* in case the line is not terminated, terminate it */ +- char *eol = strchrnul(buf, '\n'); +- eol[0] = '\n'; +- eol[1] = '\0'; +- fputs(buf, fp); +- } +- fclose(in); +- } +- fclose(fp); +- return true; +-} +- + /* Like xopen, but on error, unlocks and deletes dd and user core */ + static int create_or_die(const char *filename, int user_core_fd) + { +@@ -513,6 +394,34 @@ static int test_configuration(bool setting_SaveFullCore, bool setting_CreateCore + return 0; + } + ++int save_crashing_binary(pid_t pid, const char *dest_path, uid_t uid, gid_t gid) ++{ ++ char buf[sizeof("/proc/%lu/exe") + sizeof(long)*3]; ++ ++ sprintf(buf, "/proc/%lu/exe", (long)pid); ++ int src_fd_binary = open(buf, O_RDONLY); /* might fail and return -1, it's ok */ ++ if (src_fd_binary < 0) ++ { ++ log_notice("Failed to open an image of crashing binary"); ++ return 0; ++ } ++ ++ int dst_fd = open(dest_path, O_WRONLY | O_CREAT | O_TRUNC, DEFAULT_DUMP_DIR_MODE); ++ if (dst_fd < 0) ++ { ++ log_notice("Failed to create file '%s'", dest_path); ++ close(src_fd_binary); ++ return -1; ++ } ++ ++ IGNORE_RESULT(fchown(dst_fd, uid, gid)); ++ ++ off_t sz = copyfd_eof(src_fd_binary, dst_fd, COPYFD_SPARSE); ++ close(src_fd_binary); ++ ++ return fsync(dst_fd) != 0 || close(dst_fd) != 0 || sz < 0; ++} ++ + int main(int argc, char** argv) + { + /* Kernel starts us with all fd's closed. +@@ -612,8 +521,7 @@ int main(int argc, char** argv) + + char path[PATH_MAX]; + +- int src_fd_binary = -1; +- char *executable = get_executable(pid, setting_SaveBinaryImage ? &src_fd_binary : NULL); ++ char *executable = get_executable(pid); + if (executable && strstr(executable, "/abrt-hook-ccpp")) + { + error_msg_and_die("PID %lu is '%s', not dumping it to avoid recursion", +@@ -628,6 +536,9 @@ int main(int argc, char** argv) + + uid_t fsuid = uid; + uid_t tmp_fsuid = get_fsuid(proc_pid_status); ++ if (tmp_fsuid < 0) ++ perror_msg_and_die("Can't parse 'Uid: line' in /proc/%lu/status", (long)pid); ++ + int suid_policy = dump_suid_policy(); + if (tmp_fsuid != uid) + { +@@ -805,16 +716,16 @@ int main(int argc, char** argv) + + dd_save_text(dd, FILENAME_ABRT_VERSION, VERSION); + +- if (src_fd_binary > 0) ++ if (setting_SaveBinaryImage) + { + strcpy(path + path_len, "/"FILENAME_BINARY); +- int dst_fd = create_or_die(path, user_core_fd); +- off_t sz = copyfd_eof(src_fd_binary, dst_fd, COPYFD_SPARSE); +- if (fsync(dst_fd) != 0 || close(dst_fd) != 0 || sz < 0) ++ ++ if (save_crashing_binary(pid, path, dd->dd_uid, dd->dd_gid)) + { +- dd_delete(dd); error_msg_and_die("Error saving '%s'", path); ++ error_msg("Error saving '%s'", path); ++ ++ goto error_exit; + } +- close(src_fd_binary); + } + + off_t core_size = 0; +@@ -837,15 +748,12 @@ int main(int argc, char** argv) + if (fsync(abrt_core_fd) != 0 || close(abrt_core_fd) != 0 || core_size < 0) + { + unlink(path); +- dd_delete(dd); +- if (user_core_fd >= 0) +- { +- xchdir(user_pwd); +- unlink(core_basename); +- } ++ + /* copyfd_sparse logs the error including errno string, + * but it does not log file name */ +- error_msg_and_die("Error writing '%s'", path); ++ error_msg("Error writing '%s'", path); ++ ++ goto error_exit; + } + if (user_core_fd >= 0 + /* error writing user coredump? */ +@@ -865,6 +773,13 @@ int main(int argc, char** argv) + create_user_core(user_core_fd, pid, ulimit_c); + } + ++ /* User core is either written or closed */ ++ user_core_fd = -1; ++ ++ /* ++ * ! No other errors should cause removal of the user core ! ++ */ ++ + /* Save JVM crash log if it exists. (JVM's coredump per se + * is nearly useless for JVM developers) + */ +@@ -891,8 +806,9 @@ int main(int argc, char** argv) + off_t sz = copyfd_eof(src_fd, dst_fd, COPYFD_SPARSE); + if (close(dst_fd) != 0 || sz < 0) + { +- dd_delete(dd); +- error_msg_and_die("Error saving '%s'", path); ++ error_msg("Error saving '%s'", path); ++ ++ goto error_exit; + } + close(src_fd); + } +@@ -909,6 +825,8 @@ int main(int argc, char** argv) + * Classic deadlock. + */ + dd_close(dd); ++ dd = NULL; ++ + path[path_len] = '\0'; /* path now contains only directory name */ + char *newpath = xstrndup(path, path_len - (sizeof(".new")-1)); + if (rename(path, newpath) == 0) +@@ -938,4 +856,16 @@ int main(int argc, char** argv) + + /* We didn't create abrt dump, but may need to create compat coredump */ + return create_user_core(user_core_fd, pid, ulimit_c); ++ ++error_exit: ++ if (dd) ++ dd_delete(dd); ++ ++ if (user_core_fd >= 0) ++ { ++ xchdir(user_pwd); ++ unlink(core_basename); ++ } ++ ++ xfunc_die(); + } +-- +2.1.0 + diff --git a/0057-ccpp-hook-move-utility-functions-to-hooklib.patch b/0057-ccpp-hook-move-utility-functions-to-hooklib.patch new file mode 100644 index 0000000..0b547b6 --- /dev/null +++ b/0057-ccpp-hook-move-utility-functions-to-hooklib.patch @@ -0,0 +1,166 @@ +From 8e8d2edd5c5eac60b33cb36cc6012d076ddb9b13 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 11 Dec 2014 16:17:21 +0100 +Subject: [PATCH] ccpp-hook: move utility functions to hooklib + +Related to #548 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 49 ++---------------------------------------- + src/include/hooklib.h | 6 ++++++ + src/lib/hooklib.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++ + 3 files changed, 61 insertions(+), 47 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 7fd9520..37a9a74 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -26,9 +26,6 @@ + #include + #endif /* ENABLE_DUMP_TIME_UNWIND */ + +-#define DUMP_SUID_UNSAFE 1 +-#define DUMP_SUID_SAFE 2 +- + + /* I want to use -Werror, but gcc-4.4 throws a curveball: + * "warning: ignoring return value of 'ftruncate', declared with attribute warn_unused_result" +@@ -157,33 +154,6 @@ static char *core_basename = (char*) "core"; + */ + static char *full_core_basename; + +-static int dump_suid_policy() +-{ +- /* +- - values are: +- 0 - don't dump suided programs - in this case the hook is not called by kernel +- 1 - create coredump readable by fs_uid +- 2 - create coredump readable by root only +- */ +- int c; +- int suid_dump_policy = 0; +- const char *filename = "/proc/sys/fs/suid_dumpable"; +- FILE *f = fopen(filename, "r"); +- if (!f) +- { +- log("Can't open %s", filename); +- return suid_dump_policy; +- } +- +- c = fgetc(f); +- fclose(f); +- if (c != EOF) +- suid_dump_policy = c - '0'; +- +- //log("suid dump policy is: %i", suid_dump_policy); +- return suid_dump_policy; +-} +- + static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_values) + { + errno = 0; +@@ -564,23 +534,8 @@ int main(int argc, char** argv) + } + + const char *signame = NULL; +- switch (signal_no) +- { +- case SIGILL : signame = "ILL" ; break; +- case SIGFPE : signame = "FPE" ; break; +- case SIGSEGV: signame = "SEGV"; break; +- case SIGBUS : signame = "BUS" ; break; //Bus error (bad memory access) +- case SIGABRT: signame = "ABRT"; break; //usually when abort() was called +- // We have real-world reports from users who see buggy programs +- // dying with SIGTRAP, uncommented it too: +- case SIGTRAP: signame = "TRAP"; break; //Trace/breakpoint trap +- // These usually aren't caused by bugs: +- //case SIGQUIT: signame = "QUIT"; break; //Quit from keyboard +- //case SIGSYS : signame = "SYS" ; break; //Bad argument to routine (SVr4) +- //case SIGXCPU: signame = "XCPU"; break; //CPU time limit exceeded (4.2BSD) +- //case SIGXFSZ: signame = "XFSZ"; break; //File size limit exceeded (4.2BSD) +- default: return create_user_core(user_core_fd, pid, ulimit_c); // not a signal we care about +- } ++ if (!signal_is_fatal(signal_no, &signame)) ++ return create_user_core(user_core_fd, pid, ulimit_c); // not a signal we care about + + if (!daemon_is_ok()) + { +diff --git a/src/include/hooklib.h b/src/include/hooklib.h +index 4edd4ea..1ede5e4 100644 +--- a/src/include/hooklib.h ++++ b/src/include/hooklib.h +@@ -29,3 +29,9 @@ + stored data, but it's not guaranteed) + */ + char *problem_data_save(problem_data_t *pd); ++ ++#define DUMP_SUID_UNSAFE 1 ++#define DUMP_SUID_SAFE 2 ++ ++int dump_suid_policy(); ++int signal_is_fatal(int signal_no, const char **name); +diff --git a/src/lib/hooklib.c b/src/lib/hooklib.c +index 1d45cdd..2be4e80 100644 +--- a/src/lib/hooklib.c ++++ b/src/lib/hooklib.c +@@ -422,3 +422,56 @@ char* problem_data_save(problem_data_t *pd) + log_info("problem id: '%s'", problem_id); + return problem_id; + } ++ ++int dump_suid_policy() ++{ ++ /* ++ - values are: ++ 0 - don't dump suided programs - in this case the hook is not called by kernel ++ 1 - create coredump readable by fs_uid ++ 2 - create coredump readable by root only ++ */ ++ int c; ++ int suid_dump_policy = 0; ++ const char *filename = "/proc/sys/fs/suid_dumpable"; ++ FILE *f = fopen(filename, "r"); ++ if (!f) ++ { ++ log("Can't open %s", filename); ++ return suid_dump_policy; ++ } ++ ++ c = fgetc(f); ++ fclose(f); ++ if (c != EOF) ++ suid_dump_policy = c - '0'; ++ ++ //log("suid dump policy is: %i", suid_dump_policy); ++ return suid_dump_policy; ++} ++ ++int signal_is_fatal(int signal_no, const char **name) ++{ ++ const char *signame = NULL; ++ switch (signal_no) ++ { ++ case SIGILL : signame = "ILL" ; break; ++ case SIGFPE : signame = "FPE" ; break; ++ case SIGSEGV: signame = "SEGV"; break; ++ case SIGBUS : signame = "BUS" ; break; //Bus error (bad memory access) ++ case SIGABRT: signame = "ABRT"; break; //usually when abort() was called ++ // We have real-world reports from users who see buggy programs ++ // dying with SIGTRAP, uncommented it too: ++ case SIGTRAP: signame = "TRAP"; break; //Trace/breakpoint trap ++ // These usually aren't caused by bugs: ++ //case SIGQUIT: signame = "QUIT"; break; //Quit from keyboard ++ //case SIGSYS : signame = "SYS" ; break; //Bad argument to routine (SVr4) ++ //case SIGXCPU: signame = "XCPU"; break; //CPU time limit exceeded (4.2BSD) ++ //case SIGXFSZ: signame = "XFSZ"; break; //File size limit exceeded (4.2BSD) ++ } ++ ++ if (name != NULL) ++ *name = signame; ++ ++ return signame != NULL; ++} +-- +2.1.0 + diff --git a/0058-core-use-updated-dump_fd_info.patch b/0058-core-use-updated-dump_fd_info.patch new file mode 100644 index 0000000..34eb5aa --- /dev/null +++ b/0058-core-use-updated-dump_fd_info.patch @@ -0,0 +1,29 @@ +From 0d9f9bba421c8271e52b67320b161c60bcf0151d Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 21 Jan 2015 10:41:05 +0100 +Subject: [PATCH] core: use updated dump_fd_info() + +Related to abrt/libreport#320 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 37a9a74..e1d81b6 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -630,7 +630,8 @@ int main(int argc, char** argv) + IGNORE_RESULT(chown(dest_filename, dd->dd_uid, dd->dd_gid)); + + strcpy(dest_base, FILENAME_OPEN_FDS); +- if (dump_fd_info(dest_filename, source_filename, source_base_ofs)) ++ strcpy(source_filename + source_base_ofs, "fd"); ++ if (dump_fd_info(dest_filename, source_filename) == 0) + IGNORE_RESULT(chown(dest_filename, dd->dd_uid, dd->dd_gid)); + + free(dest_filename); +-- +2.1.0 + diff --git a/0060-abrtd-Don-t-allow-users-to-list-problems-by-hand.patch b/0060-abrtd-Don-t-allow-users-to-list-problems-by-hand.patch new file mode 100644 index 0000000..085d604 --- /dev/null +++ b/0060-abrtd-Don-t-allow-users-to-list-problems-by-hand.patch @@ -0,0 +1,28 @@ +From 2624beb1e91b4de9286c6e4220c63cc2157d868b Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 2 Mar 2015 13:44:51 +0100 +Subject: [PATCH] abrtd: Don't allow users to list problems "by hand" + +See commit 61f3b160f609c112728e6cf3c55076aeabb75319 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrtd.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/daemon/abrtd.c b/src/daemon/abrtd.c +index cce49eb..cf1bf1e 100644 +--- a/src/daemon/abrtd.c ++++ b/src/daemon/abrtd.c +@@ -183,7 +183,7 @@ static void sanitize_dump_dir_rights(void) + * us with thousands of bogus or malicious dumps */ + /* 07000 bits are setuid, setgit, and sticky, and they must be unset */ + /* 00777 bits are usual "rwxrwxrwx" access rights */ +- ensure_writable_dir(g_settings_dump_location, 0755, "abrt"); ++ ensure_writable_dir(g_settings_dump_location, 0751, "abrt"); + /* temp dir */ + ensure_writable_dir(VAR_RUN"/abrt", 0755, "root"); + } +-- +2.1.0 + diff --git a/0061-retrace-client-stop-failing-on-SSL2.patch b/0061-retrace-client-stop-failing-on-SSL2.patch new file mode 100644 index 0000000..7bf6485 --- /dev/null +++ b/0061-retrace-client-stop-failing-on-SSL2.patch @@ -0,0 +1,37 @@ +From 9c1e3a75c81aa498231e59b1997a2408c580b879 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 4 Feb 2014 13:03:21 +0100 +Subject: [PATCH] retrace-client: stop failing on SSL2 + +Closes rhbz#1200852 + +Signed-off-by: Jakub Filak +--- + src/plugins/https-utils.c | 9 +++++---- + 1 file changed, 5 insertions(+), 4 deletions(-) + +diff --git a/src/plugins/https-utils.c b/src/plugins/https-utils.c +index cb3c606..7a22729 100644 +--- a/src/plugins/https-utils.c ++++ b/src/plugins/https-utils.c +@@ -213,12 +213,13 @@ void ssl_connect(struct https_cfg *cfg, PRFileDesc **tcp_sock, PRFileDesc **ssl_ + error_msg_and_die(_("Failed to wrap TCP socket by SSL.")); + if (SECSuccess != SSL_OptionSet(*ssl_sock, SSL_HANDSHAKE_AS_CLIENT, PR_TRUE)) + error_msg_and_die(_("Failed to enable client handshake to SSL socket.")); +- if (SECSuccess != SSL_OptionSet(*ssl_sock, SSL_ENABLE_SSL2, PR_TRUE)) +- error_msg_and_die(_("Failed to enable client handshake to SSL socket.")); ++ // https://bugzilla.redhat.com/show_bug.cgi?id=1189952 ++ //if (SECSuccess != SSL_OptionSet(*ssl_sock, SSL_ENABLE_SSL2, PR_TRUE)) ++ // error_msg_and_die(_("Failed to enable SSL2.")); + if (SECSuccess != SSL_OptionSet(*ssl_sock, SSL_ENABLE_SSL3, PR_TRUE)) +- error_msg_and_die(_("Failed to enable client handshake to SSL socket.")); ++ error_msg_and_die(_("Failed to enable SSL3.")); + if (SECSuccess != SSL_OptionSet(*ssl_sock, SSL_ENABLE_TLS, PR_TRUE)) +- error_msg_and_die(_("Failed to enable client handshake to SSL socket.")); ++ error_msg_and_die(_("Failed to enable TLS.")); + if (SECSuccess != SSL_SetURL(*ssl_sock, cfg->url)) + error_msg_and_die(_("Failed to set URL to SSL socket.")); + +-- +2.1.0 + diff --git a/0062-dbus-add-a-new-method-GetProblemData.patch b/0062-dbus-add-a-new-method-GetProblemData.patch new file mode 100644 index 0000000..255356c --- /dev/null +++ b/0062-dbus-add-a-new-method-GetProblemData.patch @@ -0,0 +1,115 @@ +From 96a53a3d949142e1a401a0eaac1d09b30476e78c Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 16 Mar 2015 08:58:58 +0100 +Subject: [PATCH] dbus: add a new method GetProblemData + +The method returns serialized problem_data_t for a given problem id. + +The method is needed by cockpit-abrt which is supposed to have a page +showing comprehensive problem details. + +Signed-off-by: Jakub Filak +--- + src/dbus/abrt-dbus.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- + 1 file changed, 70 insertions(+), 2 deletions(-) + +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index 308a9af..cf7785d 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -49,6 +49,10 @@ static const gchar introspection_xml[] = + " " + " " + " " ++ " " ++ " " ++ " " ++ " " + " " + " " + " " +@@ -545,6 +549,68 @@ static void handle_method_call(GDBusConnection *connection, + return; + } + ++ if (g_strcmp0(method_name, "GetProblemData") == 0) ++ { ++ /* Parameter tuple is (s) */ ++ const char *problem_id; ++ ++ g_variant_get(parameters, "(&s)", &problem_id); ++ ++ int ddstat = dump_dir_stat_for_uid(problem_id, caller_uid); ++ if ((ddstat & DD_STAT_ACCESSIBLE_BY_UID) == 0 && ++ polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes) ++ { ++ log_notice("Unauthorized access : '%s'", problem_id); ++ g_dbus_method_invocation_return_dbus_error(invocation, ++ "org.freedesktop.problems.AuthFailure", ++ _("Not Authorized")); ++ return; ++ } ++ ++ struct dump_dir *dd = dd_opendir(problem_id, DD_OPEN_READONLY); ++ if (dd == NULL) ++ { ++ log_notice("Can't access the problem '%s' for reading", problem_id); ++ g_dbus_method_invocation_return_dbus_error(invocation, ++ "org.freedesktop.problems.Failure", ++ _("Can't access the problem for reading")); ++ return; ++ } ++ ++ problem_data_t *pd = create_problem_data_from_dump_dir(dd); ++ dd_close(dd); ++ ++ GVariantBuilder *response_builder = g_variant_builder_new(G_VARIANT_TYPE_ARRAY); ++ ++ GHashTableIter pd_iter; ++ char *element_name; ++ struct problem_item *element_info; ++ g_hash_table_iter_init(&pd_iter, pd); ++ while (g_hash_table_iter_next(&pd_iter, (void**)&element_name, (void**)&element_info)) ++ { ++ unsigned long size = 0; ++ if (problem_item_get_size(element_info, &size) != 0) ++ { ++ log_notice("Can't get stat of : '%s'", element_info->content); ++ continue; ++ } ++ ++ g_variant_builder_add(response_builder, "{s(its)}", ++ element_name, ++ element_info->flags, ++ size, ++ element_info->content); ++ } ++ ++ GVariant *response = g_variant_new("(a{s(its)})", response_builder); ++ g_variant_builder_unref(response_builder); ++ ++ problem_data_free(pd); ++ ++ g_dbus_method_invocation_return_value(invocation, response); ++ return; ++ } ++ + if (g_strcmp0(method_name, "SetElement") == 0) + { + const char *problem_id; +@@ -813,8 +879,10 @@ int main(int argc, char *argv[]) + * the introspection data structures - so we just build + * them from XML. + */ +- introspection_data = g_dbus_node_info_new_for_xml(introspection_xml, NULL); +- g_assert(introspection_data != NULL); ++ GError *err = NULL; ++ introspection_data = g_dbus_node_info_new_for_xml(introspection_xml, &err); ++ if (err != NULL) ++ error_msg_and_die("Invalid D-Bus interface: %s", err->message); + + owner_id = g_bus_own_name(G_BUS_TYPE_SYSTEM, + ABRT_DBUS_NAME, +-- +2.1.0 + diff --git a/0063-doc-add-documentation-for-GetProblemData.patch b/0063-doc-add-documentation-for-GetProblemData.patch new file mode 100644 index 0000000..9ac90ee --- /dev/null +++ b/0063-doc-add-documentation-for-GetProblemData.patch @@ -0,0 +1,38 @@ +From 7af709b3054bafc118d216c59b2d2c9ed49ce31c Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 16 Mar 2015 14:13:14 +0100 +Subject: [PATCH] doc: add documentation for GetProblemData + +For cockpit-abrt. + +Signed-off-by: Jakub Filak +--- + doc/problems-service/org.freedesktop.Problems.xml.in | 12 ++++++++++++ + 1 file changed, 12 insertions(+) + +diff --git a/doc/problems-service/org.freedesktop.Problems.xml.in b/doc/problems-service/org.freedesktop.Problems.xml.in +index 705b286..6fcd990 100644 +--- a/doc/problems-service/org.freedesktop.Problems.xml.in ++++ b/doc/problems-service/org.freedesktop.Problems.xml.in +@@ -253,6 +253,18 @@ for prblmid in problems.GetProblems(): + + + ++ " ++ Returns problem's data. ++ ++ ++ An identifier of problem. ++ ++ ++ ++ A dictionary where keys are element names and values are triplets (element libreport flags, element size, element contents). ++ ++ ++ + + Assures ownership of a specified problem for the caller. + +-- +2.1.0 + diff --git a/0064-applet-get-the-list-of-problems-through-D-Bus-servic.patch b/0064-applet-get-the-list-of-problems-through-D-Bus-servic.patch new file mode 100644 index 0000000..b88cc17 --- /dev/null +++ b/0064-applet-get-the-list-of-problems-through-D-Bus-servic.patch @@ -0,0 +1,98 @@ +From b6ed011ee153bb698d176019e9c2fbedeacad5fa Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 19 Mar 2015 08:35:38 +0100 +Subject: [PATCH] applet: get the list of problems through D-Bus service + +The default dump location directory is not iterable for regular users. + +I cherry-picked and merged these two commits: +57895ccd0c6289faada8e5f3327e276ffded46b5 +3484123353de0d77745d348cd371c317e9a52483 + +Signed-off-by: Jakub Filak +--- + src/applet/applet.c | 47 +++-------------------------------------------- + 1 file changed, 3 insertions(+), 44 deletions(-) + +diff --git a/src/applet/applet.c b/src/applet/applet.c +index 644da60..3198ae5 100644 +--- a/src/applet/applet.c ++++ b/src/applet/applet.c +@@ -70,7 +70,6 @@ enum + static GDBusConnection *g_system_bus; + static GtkStatusIcon *ap_status_icon; + static GtkWidget *ap_menu; +-static char **s_dirs; + static GList *g_deferred_crash_queue; + static guint g_deferred_timeout; + static int g_signal_pipe[2]; +@@ -429,29 +428,6 @@ static char *build_message(problem_info_t *pi) + return msg; + } + +-static GList *add_dirs_to_dirlist(GList *dirlist, const char *dirname) +-{ +- DIR *dir = opendir(dirname); +- if (!dir) +- return dirlist; +- +- struct dirent *dent; +- while ((dent = readdir(dir)) != NULL) +- { +- if (dot_or_dotdot(dent->d_name)) +- continue; +- char *full_name = concat_path_file(dirname, dent->d_name); +- struct stat statbuf; +- if (lstat(full_name, &statbuf) == 0 && S_ISDIR(statbuf.st_mode)) +- dirlist = g_list_prepend(dirlist, full_name); +- else +- free(full_name); +- } +- closedir(dir); +- +- return g_list_reverse(dirlist); +-} +- + /* Compares the problem directories to list saved in + * $XDG_CACHE_HOME/abrt/applet_dirlist and updates the applet_dirlist + * with updated list. +@@ -461,13 +437,9 @@ static GList *add_dirs_to_dirlist(GList *dirlist, const char *dirname) + */ + static void new_dir_exists(GList **new_dirs) + { +- GList *dirlist = NULL; +- char **pp = s_dirs; +- while (*pp) +- { +- dirlist = add_dirs_to_dirlist(dirlist, *pp); +- pp++; +- } ++ GList *dirlist = get_problems_over_dbus(/*don't authorize*/false); ++ if (dirlist == ERR_PTR) ++ return; + + const char *cachedir = g_get_user_cache_dir(); + char *dirlist_name = concat_path_file(cachedir, "abrt"); +@@ -1660,19 +1632,6 @@ int main(int argc, char** argv) + load_event_config_data(); + load_user_settings("abrt-applet"); + +- const char *default_dirs[] = { +- g_settings_dump_location, +- NULL, +- NULL, +- }; +- argv += optind; +- if (!argv[0]) +- { +- default_dirs[1] = concat_path_file(g_get_user_cache_dir(), "abrt/spool"); +- argv = (char**)default_dirs; +- } +- s_dirs = argv; +- + /* Initialize our (dbus_abrt) machinery: hook _system_ dbus to glib main loop. + * (session bus is left to be handled by libnotify, see below) */ + DBusError err; +-- +2.1.0 + diff --git a/0065-libabrt-add-new-function-fetching-full-problem-data-.patch b/0065-libabrt-add-new-function-fetching-full-problem-data-.patch new file mode 100644 index 0000000..7f5480a --- /dev/null +++ b/0065-libabrt-add-new-function-fetching-full-problem-data-.patch @@ -0,0 +1,88 @@ +From 6ba08b9f28343da5c6102833d1a5062475f09468 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 24 Mar 2015 19:03:52 +0100 +Subject: [PATCH] libabrt: add new function fetching full problem data over + DBus + +This function is required because users may not have direct file system +access to the problem data. + +Signed-off-by: Jakub Filak +--- + src/include/libabrt.h | 7 +++++++ + src/lib/problem_api_dbus.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ + 2 files changed, 51 insertions(+) + +diff --git a/src/include/libabrt.h b/src/include/libabrt.h +index 37704dd..5a019fb 100644 +--- a/src/include/libabrt.h ++++ b/src/include/libabrt.h +@@ -169,6 +169,13 @@ int delete_problem_dirs_over_dbus(const GList *problem_dir_paths); + problem_data_t *get_problem_data_dbus(const char *problem_dir_path); + + /** ++ @brief Fetches full problem data for specified problem id ++ ++ @return problem_data_t or ERR_PTR on failure ++*/ ++problem_data_t *get_full_problem_data_over_dbus(const char *problem_dir_path); ++ ++/** + @brief Fetches all problems from problem database + + @param authorize If set to true will try to fetch even problems owned by other users (will require root authorization over policy kit) +diff --git a/src/lib/problem_api_dbus.c b/src/lib/problem_api_dbus.c +index 2d77898..549175c 100644 +--- a/src/lib/problem_api_dbus.c ++++ b/src/lib/problem_api_dbus.c +@@ -183,3 +183,47 @@ GList *get_problems_over_dbus(bool authorize) + + return list; + } ++ ++problem_data_t *get_full_problem_data_over_dbus(const char *problem_dir_path) ++{ ++ INITIALIZE_LIBABRT(); ++ ++ GDBusProxy *proxy = get_dbus_proxy(); ++ if (!proxy) ++ return ERR_PTR; ++ ++ GError *error = NULL; ++ GVariant *result = g_dbus_proxy_call_sync(proxy, ++ "GetProblemData", ++ g_variant_new("(s)", problem_dir_path), ++ G_DBUS_CALL_FLAGS_NONE, ++ -1, ++ NULL, ++ &error); ++ ++ if (error) ++ { ++ error_msg(_("Can't get problem data from abrt-dbus: %s"), error->message); ++ g_error_free(error); ++ return ERR_PTR; ++ } ++ ++ GVariantIter *iter = NULL; ++ g_variant_get(result, "(a{s(its)})", &iter); ++ ++ gchar *name = NULL; ++ gint flags; ++ gulong size; ++ gchar *value = NULL; ++ ++ problem_data_t *pd = problem_data_new(); ++ while (g_variant_iter_loop(iter, "{&s(it&s)}", &name, &flags, &size, &value)) ++ problem_data_add_ext(pd, name, value, flags, size); ++ ++ problem_data_add(pd, CD_DUMPDIR, problem_dir_path, ++ CD_FLAG_TXT + CD_FLAG_ISNOTEDITABLE + CD_FLAG_LIST); ++ ++ g_variant_unref(result); ++ ++ return pd; ++} +-- +2.1.0 + diff --git a/0066-dbus-add-new-method-to-test-existence-of-an-element.patch b/0066-dbus-add-new-method-to-test-existence-of-an-element.patch new file mode 100644 index 0000000..be29afe --- /dev/null +++ b/0066-dbus-add-new-method-to-test-existence-of-an-element.patch @@ -0,0 +1,110 @@ +From 502491485b3c1c69120c1413b18f7fd55f6a94c7 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 24 Mar 2015 20:48:33 +0100 +Subject: [PATCH] dbus: add new method to test existence of an element + +It is sometimes necessary to check if some elemen exist, so this method +should be fast as much as it is possible to do this task over DBus. + +I was thinking about calling the GetInfo method with a single element +but I refused this idea as it is inherently overcomplicated and error +prone. + +Signed-off-by: Jakub Filak +--- + .../org.freedesktop.Problems.xml.in | 16 ++++++++ + src/dbus/abrt-dbus.c | 44 ++++++++++++++++++++++ + 2 files changed, 60 insertions(+) + +diff --git a/doc/problems-service/org.freedesktop.Problems.xml.in b/doc/problems-service/org.freedesktop.Problems.xml.in +index 6fcd990..2bf8c32 100644 +--- a/doc/problems-service/org.freedesktop.Problems.xml.in ++++ b/doc/problems-service/org.freedesktop.Problems.xml.in +@@ -253,6 +253,22 @@ for prblmid in problems.GetProblems(): + + + ++ ++ Checks whether the element exists. ++ ++ ++ An identifier of problem. ++ ++ ++ ++ A name of checked element. ++ ++ ++ ++ True if the element exists; otherwise false. ++ ++ ++ + " + Returns problem's data. + +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index cf7785d..4eeff41 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -49,6 +49,11 @@ static const gchar introspection_xml[] = + " " + " " + " " ++ " " ++ " " ++ " " ++ " " ++ " " + " " + " " + " " +@@ -703,6 +708,45 @@ static void handle_method_call(GDBusConnection *connection, + return; + } + ++ if (g_strcmp0(method_name, "TestElementExists") == 0) ++ { ++ const char *problem_id; ++ const char *element; ++ ++ g_variant_get(parameters, "(&s&s)", &problem_id, &element); ++ ++ ++ struct dump_dir *dd = dd_opendir(problem_id, DD_OPEN_READONLY); ++ if (!dd) ++ { ++ log_notice("Can't access the problem '%s'", problem_id); ++ g_dbus_method_invocation_return_dbus_error(invocation, ++ "org.freedesktop.problems.Failure", ++ _("Can't access the problem")); ++ return; ++ } ++ ++ int ddstat = dump_dir_stat_for_uid(problem_id, caller_uid); ++ if ((ddstat & DD_STAT_ACCESSIBLE_BY_UID) == 0 && ++ polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes) ++ { ++ dd_close(dd); ++ log_notice("Unauthorized access : '%s'", problem_id); ++ g_dbus_method_invocation_return_dbus_error(invocation, ++ "org.freedesktop.problems.AuthFailure", ++ _("Not Authorized")); ++ return; ++ } ++ ++ int ret = dd_exist(dd, element); ++ dd_close(dd); ++ ++ GVariant *response = g_variant_new("(b)", ret); ++ g_dbus_method_invocation_return_value(invocation, response); ++ ++ return; ++ } ++ + if (g_strcmp0(method_name, "DeleteProblem") == 0) + { + /* Dbus parameters are always tuples. +-- +2.1.0 + diff --git a/0067-libabrt-add-wrappers-TestElemeExists-and-GetInfo-for.patch b/0067-libabrt-add-wrappers-TestElemeExists-and-GetInfo-for.patch new file mode 100644 index 0000000..4580f42 --- /dev/null +++ b/0067-libabrt-add-wrappers-TestElemeExists-and-GetInfo-for.patch @@ -0,0 +1,129 @@ +From 96ff7e247ca0f767b0f24f109d9248101ee6baa2 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 24 Mar 2015 20:54:40 +0100 +Subject: [PATCH] libabrt: add wrappers TestElemeExists and GetInfo for one + element + +To conveniently use the DBus methods. + +Signed-off-by: Jakub Filak +--- + src/include/libabrt.h | 18 +++++++++++ + src/lib/problem_api_dbus.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++ + 2 files changed, 93 insertions(+) + +diff --git a/src/include/libabrt.h b/src/include/libabrt.h +index 5a019fb..0171cb7 100644 +--- a/src/include/libabrt.h ++++ b/src/include/libabrt.h +@@ -153,6 +153,24 @@ void koops_print_suspicious_strings_filtered(const regex_t **filterout); + int chown_dir_over_dbus(const char *problem_dir_path); + + /** ++ @brief Checks whether the given element name exists ++ ++ Might require authorization ++ ++ @return Positive number if such an element exist, 0 if doesn't and negative number if an error occurs. ++ */ ++int test_exist_over_dbus(const char *problem_id, const char *element_name); ++ ++/** ++ @ Returns value of the given element name ++ ++ Might require authorization ++ ++ @return malloced string or NULL if no such an element exists; ERR_PTR in case of any error. ++ */ ++char *load_text_over_dbus(const char *problem_id, const char *element_name); ++ ++/** + @brief Delets multiple problems specified by their id (as returned from problem_data_save) + + @param problem_dir_paths List of problem ids +diff --git a/src/lib/problem_api_dbus.c b/src/lib/problem_api_dbus.c +index 549175c..5148932 100644 +--- a/src/lib/problem_api_dbus.c ++++ b/src/lib/problem_api_dbus.c +@@ -227,3 +227,78 @@ problem_data_t *get_full_problem_data_over_dbus(const char *problem_dir_path) + + return pd; + } ++ ++int test_exist_over_dbus(const char *problem_id, const char *element_name) ++{ ++ INITIALIZE_LIBABRT(); ++ ++ GDBusProxy *proxy = get_dbus_proxy(); ++ if (!proxy) ++ return -1; ++ ++ GError *error = NULL; ++ GVariant *result = g_dbus_proxy_call_sync(proxy, ++ "TestElementExists", ++ g_variant_new("(ss)", problem_id, element_name), ++ G_DBUS_CALL_FLAGS_NONE, ++ -1, ++ NULL, ++ &error); ++ ++ if (error) ++ { ++ error_msg(_("Can't test whether the element exists over abrt-dbus: %s"), error->message); ++ g_error_free(error); ++ return -1; ++ } ++ ++ gboolean retval; ++ g_variant_get(result, "(b)", &retval); ++ g_variant_unref(result); ++ ++ return retval; ++} ++ ++char *load_text_over_dbus(const char *problem_id, const char *element_name) ++{ ++ INITIALIZE_LIBABRT(); ++ ++ GDBusProxy *proxy = get_dbus_proxy(); ++ if (!proxy) ++ return ERR_PTR; ++ ++ GVariantBuilder *builder = g_variant_builder_new(G_VARIANT_TYPE("as")); ++ g_variant_builder_add(builder, "s", element_name); ++ GVariant *params = g_variant_new("(sas)", problem_id, builder); ++ g_variant_builder_unref(builder); ++ ++ GError *error = NULL; ++ GVariant *result = g_dbus_proxy_call_sync(proxy, ++ "GetInfo", ++ params, ++ G_DBUS_CALL_FLAGS_NONE, ++ -1, ++ NULL, ++ &error); ++ ++ if (error) ++ { ++ error_msg(_("Can't get problem data from abrt-dbus: %s"), error->message); ++ g_error_free(error); ++ return ERR_PTR; ++ } ++ ++ GVariant *values = g_variant_get_child_value(result, 0); ++ g_variant_unref(result); ++ ++ char *retval = NULL; ++ if (g_variant_n_children(values) == 1) ++ { ++ GVariant *contents = g_variant_get_child_value(values, 0); ++ gchar *key; ++ g_variant_get(contents, "{&ss}", &key, &retval); ++ } ++ ++ g_variant_unref(values); ++ return retval; ++} +-- +2.1.0 + diff --git a/0068-cli-use-the-DBus-methods-for-getting-problem-informa.patch b/0068-cli-use-the-DBus-methods-for-getting-problem-informa.patch new file mode 100644 index 0000000..59a4c0c --- /dev/null +++ b/0068-cli-use-the-DBus-methods-for-getting-problem-informa.patch @@ -0,0 +1,347 @@ +From 739b52102b162209d3add0988e829f4409971a3c Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 24 Mar 2015 20:57:34 +0100 +Subject: [PATCH] cli: use the DBus methods for getting problem information + +The dump directory is no longer accessible by non-root users and we also +want to get rid of direct access to allow administrators (wheel members) +see problem data without the need to ChownProblem directory before. + +Signed-off-by: Jakub Filak +--- + src/cli/abrt-cli-core.c | 74 ++++++++++++++++++++++++------------------------- + src/cli/abrt-cli-core.h | 4 ++- + src/cli/list.c | 45 +++++++++++------------------- + src/cli/process.c | 6 +--- + src/cli/status.c | 66 +++++++++++++------------------------------ + 5 files changed, 77 insertions(+), 118 deletions(-) + +diff --git a/src/cli/abrt-cli-core.c b/src/cli/abrt-cli-core.c +index 23a74a8..77a37f7 100644 +--- a/src/cli/abrt-cli-core.c ++++ b/src/cli/abrt-cli-core.c +@@ -39,24 +39,22 @@ vector_of_problem_data_t *new_vector_of_problem_data(void) + return g_ptr_array_new_with_free_func((void (*)(void*)) &problem_data_free); + } + +-static int +-append_problem_data(struct dump_dir *dd, void *arg) ++vector_of_problem_data_t *fetch_crash_infos(void) + { +- vector_of_problem_data_t *vpd = arg; +- +- problem_data_t *problem_data = create_problem_data_from_dump_dir(dd); +- problem_data_add(problem_data, CD_DUMPDIR, dd->dd_dirname, +- CD_FLAG_TXT + CD_FLAG_ISNOTEDITABLE + CD_FLAG_LIST); +- g_ptr_array_add(vpd, problem_data); +- return 0; +-} ++ GList *problems = get_problems_over_dbus(/*don't authorize*/false); ++ if (problems == ERR_PTR) ++ return NULL; + +-vector_of_problem_data_t *fetch_crash_infos(GList *dir_list) +-{ + vector_of_problem_data_t *vpd = new_vector_of_problem_data(); + +- for (GList *li = dir_list; li; li = li->next) +- for_each_problem_in_dir(li->data, getuid(), append_problem_data, vpd); ++ for (GList *iter = problems; iter; iter = g_list_next(iter)) ++ { ++ problem_data_t *problem_data = get_full_problem_data_over_dbus((const char *)(iter->data)); ++ if (problem_data == ERR_PTR) ++ continue; ++ ++ g_ptr_array_add(vpd, problem_data); ++ } + + return vpd; + } +@@ -74,36 +72,38 @@ static bool isxdigit_str(const char *str) + return true; + } + +-struct name_resolution_param { +- const char *shortcut; +- unsigned strlen_shortcut; +- char *found_name; +-}; +- +-static int find_dir_by_hash(struct dump_dir *dd, void *arg) ++char *find_problem_by_hash(const char *hash, GList *problems) + { +- struct name_resolution_param *param = arg; +- char hash_str[SHA1_RESULT_LEN*2 + 1]; +- str_to_sha1str(hash_str, dd->dd_dirname); +- if (strncasecmp(param->shortcut, hash_str, param->strlen_shortcut) == 0) ++ unsigned hash_len = strlen(hash); ++ if (!isxdigit_str(hash) || hash_len < 5) ++ return NULL; ++ ++ char *found_name = NULL; ++ for (GList *iter = problems; iter; iter = g_list_next(iter)) + { +- if (param->found_name) +- error_msg_and_die(_("'%s' identifies more than one problem directory"), param->shortcut); +- param->found_name = xstrdup(dd->dd_dirname); ++ char hash_str[SHA1_RESULT_LEN*2 + 1]; ++ str_to_sha1str(hash_str, (const char *)(iter->data)); ++ if (strncasecmp(hash, hash_str, hash_len) == 0) ++ { ++ if (found_name) ++ error_msg_and_die(_("'%s' identifies more than one problem directory"), hash); ++ found_name = xstrdup((const char *)(iter->data)); ++ } + } +- return 0; ++ ++ return found_name; + } + + char *hash2dirname(const char *hash) + { +- unsigned hash_len = strlen(hash); +- if (!isxdigit_str(hash) || hash_len < 5) ++ /* Try loading by dirname hash */ ++ GList *problems = get_problems_over_dbus(/*don't authorize*/false); ++ if (problems == ERR_PTR) + return NULL; + +- /* Try loading by dirname hash */ +- struct name_resolution_param param = { hash, hash_len, NULL }; +- GList *dir_list = get_problem_storages(); +- for (GList *li = dir_list; li; li = li->next) +- for_each_problem_in_dir(li->data, getuid(), find_dir_by_hash, ¶m); +- return param.found_name; ++ char *found_name = find_problem_by_hash(hash, problems); ++ ++ g_list_free_full(problems, free); ++ ++ return found_name; + } +diff --git a/src/cli/abrt-cli-core.h b/src/cli/abrt-cli-core.h +index 83d0b5d..33b2ea6 100644 +--- a/src/cli/abrt-cli-core.h ++++ b/src/cli/abrt-cli-core.h +@@ -28,9 +28,11 @@ problem_data_t *get_problem_data(vector_of_problem_data_t *vector, unsigned i); + + void free_vector_of_problem_data(vector_of_problem_data_t *vector); + vector_of_problem_data_t *new_vector_of_problem_data(void); +-vector_of_problem_data_t *fetch_crash_infos(GList *dir_list); ++vector_of_problem_data_t *fetch_crash_infos(void); + + /* Returns malloced string, or NULL if not found: */ ++char *find_problem_by_hash(const char *hash, GList *problems); ++/* Returns malloced string, or NULL if not found: */ + char *hash2dirname(const char *hash); + + +diff --git a/src/cli/list.c b/src/cli/list.c +index 2eefcfe..c0c819d 100644 +--- a/src/cli/list.c ++++ b/src/cli/list.c +@@ -30,33 +30,28 @@ + * ~/.abrt/spool and /var/tmp/abrt? needs more _meditation_. + */ + +-static problem_data_t *load_problem_data(const char *dump_dir_name) ++static problem_data_t *load_problem_data(const char *problem_id) + { +- /* First, try loading by dirname */ +- int sv_logmode = logmode; +- logmode = 0; /* suppress EPERM/EACCES errors in opendir */ +- struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ DD_OPEN_READONLY); +- logmode = sv_logmode; ++ char *name2 = NULL; ++ ++ /* First, check if there is a problem with the passed id */ ++ GList *problems = get_problems_over_dbus(/*don't authorize*/false); ++ GList *item = g_list_find_custom(problems, problem_id, (GCompareFunc)strcmp); + + /* (git requires at least 5 char hash prefix, we do the same) */ +- if (!dd && errno == ENOENT) ++ if (item == NULL) + { + /* Try loading by dirname hash */ +- char *name2 = hash2dirname(dump_dir_name); +- if (name2) +- dd = dd_opendir(name2, /*flags:*/ DD_OPEN_READONLY); +- free(name2); +- } ++ name2 = find_problem_by_hash(problem_id, problems); ++ if (name2 == NULL) ++ return NULL; + +- if (!dd) +- return NULL; ++ problem_id = name2; ++ } + +- problem_data_t *problem_data = create_problem_data_from_dump_dir(dd); +- problem_data_add(problem_data, CD_DUMPDIR, dd->dd_dirname, +- CD_FLAG_TXT + CD_FLAG_ISNOTEDITABLE + CD_FLAG_LIST); +- dd_close(dd); ++ problem_data_t *problem_data = get_full_problem_data_over_dbus(problem_id); + +- return problem_data; ++ return (problem_data == ERR_PTR ? NULL : problem_data); + } + + /** Prints basic information about a crash to stdout. */ +@@ -127,7 +122,7 @@ static bool print_crash_list(vector_of_problem_data_t *crash_list, int detailed, + int cmd_list(int argc, const char **argv) + { + const char *program_usage_string = _( +- "& list [options] [DIR]..." ++ "& list [options]" + ); + + int opt_not_reported = 0; +@@ -145,15 +140,8 @@ int cmd_list(int argc, const char **argv) + }; + + parse_opts(argc, (char **)argv, program_options, program_usage_string); +- argv += optind; +- +- GList *D_list = NULL; +- while (*argv) +- D_list = g_list_append(D_list, xstrdup(*argv++)); +- if (!D_list) +- D_list = get_problem_storages(); + +- vector_of_problem_data_t *ci = fetch_crash_infos(D_list); ++ vector_of_problem_data_t *ci = fetch_crash_infos(); + + g_ptr_array_sort_with_data(ci, &cmp_problem_data, (char *) FILENAME_LAST_OCCURRENCE); + +@@ -163,7 +151,6 @@ int cmd_list(int argc, const char **argv) + print_crash_list(ci, opt_detailed, opt_not_reported, opt_since, opt_until, CD_TEXT_ATT_SIZE_BZ); + + free_vector_of_problem_data(ci); +- list_free_with_free(D_list); + + #if SUGGEST_AUTOREPORTING != 0 + load_abrt_conf(); +diff --git a/src/cli/process.c b/src/cli/process.c +index 7f4fff5..39462f9 100644 +--- a/src/cli/process.c ++++ b/src/cli/process.c +@@ -152,18 +152,14 @@ int cmd_process(int argc, const char **argv) + }; + + parse_opts(argc, (char **)argv, program_options, program_usage_string); +- argv += optind; + +- GList *D_list = get_problem_storages(); +- +- vector_of_problem_data_t *ci = fetch_crash_infos(D_list); ++ vector_of_problem_data_t *ci = fetch_crash_infos(); + + g_ptr_array_sort_with_data(ci, &cmp_problem_data, (char *) FILENAME_LAST_OCCURRENCE); + + process_crashes(ci, opt_since); + + free_vector_of_problem_data(ci); +- list_free_with_free(D_list); + + return 0; + } +diff --git a/src/cli/status.c b/src/cli/status.c +index 1de2d41..68bdd0e 100644 +--- a/src/cli/status.c ++++ b/src/cli/status.c +@@ -21,53 +21,36 @@ + #include + #include "problem_api.h" + +-struct time_range { +- unsigned count; +- unsigned long since; +-}; +- +-static int count_dir_if_newer_than(struct dump_dir *dd, void *arg) +-{ +- struct time_range *me = arg; +- +- if (dd_exist(dd, FILENAME_REPORTED_TO)) +- return 0; +- +- char *time_str = dd_load_text(dd, FILENAME_LAST_OCCURRENCE); +- long val = atol(time_str); +- free(time_str); +- if (val < me->since) +- return 0; +- +- me->count++; +- return 0; +-} +- +-static void count_problems_in_dir(gpointer data, gpointer arg) ++static unsigned int count_problem_dirs(unsigned long since) + { +- char *path = data; +- struct time_range *me = arg; ++ unsigned count = 0; + +- log_info("scanning '%s' for problems since %lu", path, me->since); ++ GList *problems = get_problems_over_dbus(/*don't authorize*/false); ++ for (GList *iter = problems; iter != NULL; iter = g_list_next(iter)) ++ { ++ const char *problem_id = (const char *)iter->data; ++ if (test_exist_over_dbus(problem_id, FILENAME_REPORTED_TO)) ++ continue; + +- for_each_problem_in_dir(path, getuid(), count_dir_if_newer_than, me); +-} ++ char *time_str = load_text_over_dbus(problem_id, FILENAME_LAST_OCCURRENCE); ++ if (time_str == NULL) ++ continue; + +-static unsigned int count_problem_dirs(GList *paths, unsigned long since) +-{ +- struct time_range me; +- me.count = 0; +- me.since = since; ++ long val = atol(time_str); ++ free(time_str); ++ if (val < since) ++ return 0; + +- g_list_foreach(paths, count_problems_in_dir, &me); ++ count++; ++ } + +- return me.count; ++ return count; + } + + int cmd_status(int argc, const char **argv) + { + const char *program_usage_string = _( +- "& status [DIR]..." ++ "& status" + ); + + int opt_bare = 0; /* must be _int_, OPT_BOOL expects that! */ +@@ -81,17 +64,8 @@ int cmd_status(int argc, const char **argv) + }; + + parse_opts(argc, (char **)argv, program_options, program_usage_string); +- argv += optind; +- +- GList *problem_dir_list = NULL; +- while (*argv) +- problem_dir_list = g_list_append(problem_dir_list, xstrdup(*argv++)); +- if (!problem_dir_list) +- problem_dir_list = get_problem_storages(); +- +- unsigned int problem_count = count_problem_dirs(problem_dir_list, opt_since); + +- list_free_with_free(problem_dir_list); ++ unsigned int problem_count = count_problem_dirs(opt_since); + + /* show only if there is at least 1 problem or user set the -v */ + if (problem_count > 0 || g_verbose > 0) +-- +2.1.0 + diff --git a/0069-cli-status-don-t-return-0-if-there-is-a-problem-olde.patch b/0069-cli-status-don-t-return-0-if-there-is-a-problem-olde.patch new file mode 100644 index 0000000..996c41a --- /dev/null +++ b/0069-cli-status-don-t-return-0-if-there-is-a-problem-olde.patch @@ -0,0 +1,49 @@ +From 491668f3d83f763a11111151a3fc1f3d78cec3ab Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 8 Apr 2015 09:47:08 +0200 +Subject: [PATCH] cli-status: don't return 0 if there is a problem older than + limit + +The loop should skip such a problem and not return from the counting +function with 0. This is an obvious bug introduced in +commit 58d8e83f58afb32db3bdfd5170e65dc0ef2d2ce0 + +Signed-off-by: Jakub Filak +--- + src/cli/status.c | 11 ++++++++++- + 1 file changed, 10 insertions(+), 1 deletion(-) + +diff --git a/src/cli/status.c b/src/cli/status.c +index 68bdd0e..a65ba05 100644 +--- a/src/cli/status.c ++++ b/src/cli/status.c +@@ -30,16 +30,25 @@ static unsigned int count_problem_dirs(unsigned long since) + { + const char *problem_id = (const char *)iter->data; + if (test_exist_over_dbus(problem_id, FILENAME_REPORTED_TO)) ++ { ++ log_debug("Not counting problem %s: already reported", problem_id); + continue; ++ } + + char *time_str = load_text_over_dbus(problem_id, FILENAME_LAST_OCCURRENCE); + if (time_str == NULL) ++ { ++ log_debug("Not counting problem %s: failed to get time element", problem_id); + continue; ++ } + + long val = atol(time_str); + free(time_str); + if (val < since) +- return 0; ++ { ++ log_debug("Not counting problem %s: older tham limit (%ld < %ld)", problem_id, val, since); ++ continue; ++ } + + count++; + } +-- +2.1.0 + diff --git a/0070-upload-validate-and-sanitize-uploaded-dump-directori.patch b/0070-upload-validate-and-sanitize-uploaded-dump-directori.patch new file mode 100644 index 0000000..0661c1f --- /dev/null +++ b/0070-upload-validate-and-sanitize-uploaded-dump-directori.patch @@ -0,0 +1,144 @@ +From a8ee7db7bdd391154918a9fef814d315f3a092b6 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 20 Apr 2015 15:15:40 +0200 +Subject: [PATCH] upload: validate and sanitize uploaded dump directories + +It was discovered that, when moving problem reports from +/var/spool/abrt-upload to /var/spool/abrt or /var/tmp/abrt, +abrt-handle-upload does not verify that the new problem directory +has appropriate permissions and does not contain symbolic links. A +crafted problem report exposes other parts of abrt to attack, and +the abrt-handle-upload script allows to overwrite arbitrary files. + +Acknowledgement: + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +Related: #1212953 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-handle-upload.in | 80 +++++++++++++++++++++++++++++++++++----- + 1 file changed, 71 insertions(+), 9 deletions(-) + +diff --git a/src/daemon/abrt-handle-upload.in b/src/daemon/abrt-handle-upload.in +index dbc4534..96e68b9 100755 +--- a/src/daemon/abrt-handle-upload.in ++++ b/src/daemon/abrt-handle-upload.in +@@ -10,6 +10,7 @@ import getopt + import tempfile + import shutil + import datetime ++import grp + + from reportclient import set_verbosity, error_msg_and_die, error_msg, log + +@@ -36,12 +37,77 @@ def init_gettext(): + + import problem + +-def write_str_to(filename, s): +- fd = os.open(filename, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, @DEFAULT_DUMP_DIR_MODE@ | stat.S_IROTH) ++def write_str_to(filename, s, uid, gid, mode): ++ fd = os.open(filename, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, mode) + if fd >= 0: ++ os.fchown(fd, uid, gid) + os.write(fd, s) + os.close(fd) + ++ ++def validate_transform_move_and_notify(uploaded_dir_path, problem_dir_path, dest=None): ++ fsuid = 0 ++ fsgid = 0 ++ ++ try: ++ gabrt = grp.getgrnam("abrt") ++ fsgid = gabrt.gr_gid ++ except KeyError as ex: ++ error_msg("Failed to get GID of 'abrt' (using 0 instead): {0}'".format(str(ex))) ++ ++ try: ++ # give the uploaded directory to 'root:abrt' or 'root:root' ++ os.chown(uploaded_dir_path, fsuid, fsgid) ++ # set the right permissions for this machine ++ # (allow the owner and the group to access problem elements, ++ # the default dump dir mode lacks x bit for both) ++ os.chmod(uploaded_dir_path, @DEFAULT_DUMP_DIR_MODE@ | stat.S_IXUSR | stat.S_IXGRP) ++ ++ # sanitize problem elements ++ for item in os.listdir(uploaded_dir_path): ++ apath = os.path.join(uploaded_dir_path, item) ++ if os.path.islink(apath): ++ # remove symbolic links ++ os.remove(apath) ++ elif os.path.isdir(apath): ++ # remove directories ++ shutil.rmtree(apath) ++ elif os.path.isfile(apath): ++ # set file ownership to 'root:abrt' or 'root:root' ++ os.chown(apath, fsuid, fsgid) ++ # set the right file permissions for this machine ++ os.chmod(apath, @DEFAULT_DUMP_DIR_MODE@) ++ else: ++ # remove things that are neither files, symlinks nor directories ++ os.remove(apath) ++ except OSError as ex: ++ error_msg("Removing uploaded dir '{0}': '{1}'".format(uploaded_dir_path, str(ex))) ++ try: ++ shutil.rmtree(uploaded_dir_path) ++ except OSError as ex2: ++ error_msg_and_die("Failed to clean up dir '{0}': '{1}'".format(uploaded_dir_path, str(ex2))) ++ return ++ ++ # overwrite remote if it exists ++ remote_path = os.path.join(uploaded_dir_path, "remote") ++ write_str_to(remote_path, "1", fsuid, fsgid, @DEFAULT_DUMP_DIR_MODE@) ++ ++ # abrtd would increment count value and abrt-server refuses to process ++ # problem directories containing 'count' element when PrivateReports is on. ++ count_path = os.path.join(uploaded_dir_path, "count") ++ if os.path.exists(count_path): ++ # overwrite remote_count if it exists ++ remote_count_path = os.path.join(uploaded_dir_path, "remote_count") ++ os.rename(count_path, remote_count_path) ++ ++ if not dest: ++ dest = problem_dir_path ++ ++ shutil.move(uploaded_dir_path, dest) ++ ++ problem.notify_new_path(problem_dir_path) ++ ++ + if __name__ == "__main__": + + # Helper: exit with cleanup +@@ -176,22 +242,18 @@ if __name__ == "__main__": + # The archive can contain either plain dump files + # or one or more complete problem data directories. + # Checking second possibility first. +- if os.path.exists(tempdir+"/analyzer") and os.path.exists(tempdir+"/time"): +- write_str_to(tempdir+"/remote", "1") +- shutil.move(tempdir, abrt_dir) +- problem.notify_new_path(abrt_dir+"/"+os.path.basename(tempdir)) ++ if (os.path.exists(tempdir+"/analyzer") or os.path.exists(tempdir+"/type")) and os.path.exists(tempdir+"/time"): ++ validate_transform_move_and_notify(tempdir, abrt_dir+"/"+os.path.basename(tempdir), dest=abrt_dir) + else: + for d in os.listdir(tempdir): + if not os.path.isdir(tempdir+"/"+d): + continue +- write_str_to(tempdir+"/"+d+"/remote", "1") + dst = abrt_dir+"/"+d + if os.path.exists(dst): + dst += "."+str(os.getpid()) + if os.path.exists(dst): + continue +- shutil.move(tempdir+"/"+d, dst) +- problem.notify_new_path(dst) ++ validate_transform_move_and_notify(tempdir+"/"+d, dst) + + die_exitcode = 0 + # This deletes working_dir (== delete_on_exit) +-- +2.1.0 + diff --git a/0071-applet-switch-to-D-Bus-methods.patch b/0071-applet-switch-to-D-Bus-methods.patch new file mode 100644 index 0000000..018b064 --- /dev/null +++ b/0071-applet-switch-to-D-Bus-methods.patch @@ -0,0 +1,362 @@ +From b1d43d73cce12980017dc5fa773d88ef72ec432b Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 8 Apr 2015 08:23:03 +0200 +Subject: [PATCH] applet: switch to D-Bus methods + +This patch is a part of our efforts to make abrt-applet independent on +the backend. + +This patch converts all data manipulation functions to D-Bus calls, so +the notifications are made of data obtained through D-Bus. + +The reporting still relies on file system access, though. + +Signed-off-by: Jakub Filak +--- + src/applet/applet.c | 130 ++++++++++++++++++++++++++------------------- + src/include/libabrt.h | 16 ++++++ + src/lib/problem_api_dbus.c | 61 +++++++++++++++------ + 3 files changed, 137 insertions(+), 70 deletions(-) + +diff --git a/src/applet/applet.c b/src/applet/applet.c +index 3198ae5..6474f6f 100644 +--- a/src/applet/applet.c ++++ b/src/applet/applet.c +@@ -170,11 +170,6 @@ static bool is_silent_shortened_reporting_enabled(void) + return get_configured_bool_or_default("SilentShortenedReporting", 0); + } + +-static bool is_notification_of_incomplete_problems_enabled(void) +-{ +- return get_configured_bool_or_default("NotifyIncompleteProblems", 0); +-} +- + /* + * Converts a NM state value stored in GVariant to boolean. + * +@@ -307,6 +302,7 @@ typedef struct problem_info { + bool reported; + bool was_announced; + bool is_writable; ++ int time; + } problem_info_t; + + static void push_to_deferred_queue(problem_info_t *pi) +@@ -319,6 +315,26 @@ static const char *problem_info_get_dir(problem_info_t *pi) + return problem_data_get_content_or_NULL(pi->problem_data, CD_DUMPDIR); + } + ++static int problem_info_get_time(problem_info_t *pi) ++{ ++ if (pi->time == -1) ++ { ++ const char *time_str = problem_data_get_content_or_NULL(pi->problem_data, FILENAME_TIME); ++ ++ if (time_str == NULL) ++ error_msg_and_die("BUG: Problem info has data without the element time"); ++ ++ pi->time = atoi(time_str); ++ } ++ ++ return pi->time; ++} ++ ++static bool problem_info_is_reported(problem_info_t *pi) ++{ ++ return problem_data_get_content_or_NULL(pi->problem_data, FILENAME_REPORTED_TO) != NULL; ++} ++ + static void problem_info_set_dir(problem_info_t *pi, const char *dir) + { + problem_data_add_text_noteditable(pi->problem_data, CD_DUMPDIR, dir); +@@ -358,6 +374,7 @@ static problem_info_t *problem_info_new(const char *dir) + { + problem_info_t *pi = xzalloc(sizeof(*pi)); + pi->problem_data = problem_data_new(); ++ pi->time = -1; + problem_info_set_dir(pi, dir); + return pi; + } +@@ -1245,6 +1262,7 @@ static void show_problem_notification(problem_info_t *pi, int flags) + static void Crash(DBusMessage* signal) + { + log_debug("Crash recorded"); ++ + DBusMessageIter in_iter; + dbus_message_iter_init(signal, &in_iter); + +@@ -1316,12 +1334,20 @@ static void Crash(DBusMessage* signal) + + + problem_info_t *pi = problem_info_new(dir); +- if (uuid != NULL && uuid[0] != '\0') +- problem_data_add_text_noteditable(pi->problem_data, FILENAME_UUID, uuid); +- if (duphash != NULL && duphash[0] != '\0') +- problem_data_add_text_noteditable(pi->problem_data, FILENAME_DUPHASH, duphash); +- if (package_name != NULL && package_name[0] != '\0') +- problem_data_add_text_noteditable(pi->problem_data, FILENAME_COMPONENT, package_name); ++ ++ static const char *elements[] = { ++ FILENAME_CMDLINE, ++ FILENAME_COUNT, ++ FILENAME_UUID, ++ FILENAME_DUPHASH, ++ FILENAME_COMPONENT, ++ FILENAME_ENVIRON, ++ FILENAME_PID, ++ NULL, ++ }; ++ ++ fill_problem_data_over_dbus(dir, elements, pi->problem_data); ++ + pi->foreign = foreign_problem; + show_problem_notification(pi, flags); + } +@@ -1558,6 +1584,20 @@ xsmp_client_connect(void) + + int main(int argc, char** argv) + { ++ static const char *elements[] = { ++ FILENAME_CMDLINE, ++ FILENAME_COUNT, ++ FILENAME_UUID, ++ FILENAME_DUPHASH, ++ FILENAME_COMPONENT, ++ FILENAME_UID, ++ FILENAME_TIME, ++ FILENAME_REPORTED_TO, ++ FILENAME_NOT_REPORTABLE, ++ NULL ++ }; ++ ++ + /* I18n */ + setlocale(LC_ALL, ""); + #if ENABLE_NLS +@@ -1675,63 +1715,45 @@ int main(int argc, char** argv) + /* Age limit = now - 3 days */ + const unsigned long min_born_time = (unsigned long)(time_before_ndays(3)); + +- const bool notify_incomplete = is_notification_of_incomplete_problems_enabled(); +- +- while (new_dirs) ++ for ( ; new_dirs != NULL; new_dirs = g_list_next(new_dirs)) + { +- struct dump_dir *dd = dd_opendir((char *)new_dirs->data, DD_OPEN_READONLY); +- if (dd == NULL) ++ const char *problem_id = (const char *)new_dirs->data; ++ problem_info_t *pi = problem_info_new(problem_id); ++ ++ if (fill_problem_data_over_dbus(problem_id, elements, pi->problem_data) != 0) + { +- log_notice("'%s' is not a dump dir - ignoring\n", (char *)new_dirs->data); +- new_dirs = g_list_next(new_dirs); ++ log_notice("'%s' is not a dump dir - ignoring\n", problem_id); ++ problem_info_free(pi); + continue; + } + +- if (dd->dd_time < min_born_time) ++ /* TODO: add a filter for only complete problems to GetProblems D-Bus method */ ++ if (!dbus_problem_is_complete(problem_id)) + { +- log_notice("Ignoring outdated problem '%s'", (char *)new_dirs->data); +- goto next; ++ log_notice("Ignoring incomplete problem '%s'", problem_id); ++ problem_info_free(pi); ++ continue; + } + +- const bool incomplete = !problem_dump_dir_is_complete(dd); +- if (!notify_incomplete && incomplete) ++ /* TODO: add a filter for max-old reported problems to GetProblems D-Bus method */ ++ if (problem_info_get_time(pi) < min_born_time) + { +- log_notice("Ignoring incomplete problem '%s'", (char *)new_dirs->data); +- goto next; ++ log_notice("Ignoring outdated problem '%s'", problem_id); ++ problem_info_free(pi); ++ continue; + } + +- if (!dd_exist(dd, FILENAME_REPORTED_TO)) +- { +- problem_info_t *pi = problem_info_new(new_dirs->data); +- const char *elements[] = {FILENAME_UUID, FILENAME_DUPHASH, FILENAME_COMPONENT, FILENAME_NOT_REPORTABLE}; +- +- for (size_t i = 0; i < sizeof(elements)/sizeof(*elements); ++i) +- { +- char * const value = dd_load_text_ext(dd, elements[i], +- DD_FAIL_QUIETLY_ENOENT | DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE); +- if (value) +- problem_data_add_text_noteditable(pi->problem_data, elements[i], value); +- free(value); +- } +- +- pi->incomplete = incomplete; +- +- /* Can't be foreign because if the problem is foreign then the +- * dd_opendir() call failed few lines above and the problem is ignored. +- * */ +- pi->foreign = false; +- +- notify_list = g_list_prepend(notify_list, pi); +- } +- else ++ /* TODO: add a filter for not-yet reported problems to GetProblems D-Bus method */ ++ if (problem_info_is_reported(pi)) + { +- log_notice("Ignoring already reported problem '%s'", (char *)new_dirs->data); ++ log_notice("Ignoring already reported problem '%s'", problem_id); ++ problem_info_free(pi); ++ continue; + } + +-next: +- dd_close(dd); +- +- new_dirs = g_list_next(new_dirs); ++ /* Can't be foreig because new_dir_exists() returns only own problems */ ++ pi->foreign = false; ++ notify_list = g_list_prepend(notify_list, pi); + } + + g_ignore_set = ignored_problems_new(concat_path_file(g_get_user_cache_dir(), "abrt/ignored_problems")); +diff --git a/src/include/libabrt.h b/src/include/libabrt.h +index 0171cb7..65d30a1 100644 +--- a/src/include/libabrt.h ++++ b/src/include/libabrt.h +@@ -162,6 +162,15 @@ int chown_dir_over_dbus(const char *problem_dir_path); + int test_exist_over_dbus(const char *problem_id, const char *element_name); + + /** ++ @brief Checks whether the problem corresponding to the given ID is complete ++ ++ Might require authorization ++ ++ @return Positive number if such the proble is complete, 0 if doesn't and negative number if an error occurs. ++ */ ++int dbus_problem_is_complete(const char *problem_id); ++ ++/** + @ Returns value of the given element name + + Might require authorization +@@ -180,6 +189,13 @@ char *load_text_over_dbus(const char *problem_id, const char *element_name); + int delete_problem_dirs_over_dbus(const GList *problem_dir_paths); + + /** ++ @brief Fetches given problem elements for specified problem id ++ ++ @return on failures returns non zero value and emits error message ++*/ ++int fill_problem_data_over_dbus(const char *problem_dir_path, const char **elements, problem_data_t *problem_data); ++ ++/** + @brief Fetches problem information for specified problem id + + @return problem_data_t or NULL on failure +diff --git a/src/lib/problem_api_dbus.c b/src/lib/problem_api_dbus.c +index 5148932..ce5c47b 100644 +--- a/src/lib/problem_api_dbus.c ++++ b/src/lib/problem_api_dbus.c +@@ -101,23 +101,21 @@ int delete_problem_dirs_over_dbus(const GList *problem_dir_paths) + return 0; + } + +-problem_data_t *get_problem_data_dbus(const char *problem_dir_path) ++int fill_problem_data_over_dbus(const char *problem_id, const char **elements, problem_data_t *problem_data) + { + INITIALIZE_LIBABRT(); + + GDBusProxy *proxy = get_dbus_proxy(); + if (!proxy) +- return NULL; ++ return -1; + +- GVariantBuilder *builder = g_variant_builder_new(G_VARIANT_TYPE("as")); +- g_variant_builder_add(builder, "s", FILENAME_TIME ); +- g_variant_builder_add(builder, "s", FILENAME_REASON ); +- g_variant_builder_add(builder, "s", FILENAME_NOT_REPORTABLE); +- g_variant_builder_add(builder, "s", FILENAME_COMPONENT ); +- g_variant_builder_add(builder, "s", FILENAME_EXECUTABLE ); +- g_variant_builder_add(builder, "s", FILENAME_REPORTED_TO ); +- GVariant *params = g_variant_new("(sas)", problem_dir_path, builder); +- g_variant_builder_unref(builder); ++ GVariantBuilder *args_builder = g_variant_builder_new(G_VARIANT_TYPE("as")); ++ ++ for (const char **iter = elements; *iter; ++iter) ++ g_variant_builder_add(args_builder, "s", *iter); ++ ++ GVariant *params = g_variant_new("(sas)", problem_id, args_builder); ++ g_variant_builder_unref(args_builder); + + GError *error = NULL; + GVariant *result = g_dbus_proxy_call_sync(proxy, +@@ -130,20 +128,46 @@ problem_data_t *get_problem_data_dbus(const char *problem_dir_path) + + if (error) + { +- error_msg(_("Can't get problem data from abrt-dbus: %s"), error->message); ++ error_msg(_("D-Bus GetInfo method call failed: %s"), error->message); + g_error_free(error); +- return NULL; ++ return -2; + } + +- problem_data_t *pd = problem_data_new(); ++ + char *key, *val; + GVariantIter *iter; + g_variant_get(result, "(a{ss})", &iter); + while (g_variant_iter_loop(iter, "{ss}", &key, &val)) ++ problem_data_add_text_noteditable(problem_data, key, val); ++ ++ g_variant_unref(result); ++ ++ return 0; ++} ++ ++problem_data_t *get_problem_data_dbus(const char *problem_dir_path) ++{ ++ INITIALIZE_LIBABRT(); ++ ++ static const char *elements[] = { ++ FILENAME_TIME, ++ FILENAME_REASON, ++ FILENAME_NOT_REPORTABLE, ++ FILENAME_COMPONENT, ++ FILENAME_EXECUTABLE, ++ FILENAME_REPORTED_TO, ++ NULL, ++ }; ++ ++ problem_data_t *pd = problem_data_new(); ++ ++ if (fill_problem_data_over_dbus(problem_dir_path, elements, pd) != 0) + { +- problem_data_add_text_noteditable(pd, key, val); ++ error_msg(_("Can't get problem data from abrt-dbus")); ++ problem_data_free(pd); ++ return NULL; + } +- g_variant_unref(result); ++ + return pd; + } + +@@ -259,6 +283,11 @@ int test_exist_over_dbus(const char *problem_id, const char *element_name) + return retval; + } + ++int dbus_problem_is_complete(const char *problem_id) ++{ ++ return test_exist_over_dbus(problem_id, FILENAME_COUNT); ++} ++ + char *load_text_over_dbus(const char *problem_id, const char *element_name) + { + INITIALIZE_LIBABRT(); +-- +2.1.0 + diff --git a/0072-cli-do-not-exit-with-segfault-if-dbus-fails.patch b/0072-cli-do-not-exit-with-segfault-if-dbus-fails.patch new file mode 100644 index 0000000..0ef921e --- /dev/null +++ b/0072-cli-do-not-exit-with-segfault-if-dbus-fails.patch @@ -0,0 +1,30 @@ +From 3d46d2a1326c5cb8a443db4e3a1b744482ddca94 Mon Sep 17 00:00:00 2001 +From: Matej Habrnal +Date: Mon, 4 May 2015 10:35:25 +0200 +Subject: [PATCH] cli: do not exit with segfault if dbus fails + +There was a segfault when we ran 'abrt-cli list' and dbus failed. + +Related to rhbz#1217901 + +Signed-off-by: Matej Habrnal +--- + src/cli/list.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/src/cli/list.c b/src/cli/list.c +index c0c819d..31d1835 100644 +--- a/src/cli/list.c ++++ b/src/cli/list.c +@@ -142,6 +142,8 @@ int cmd_list(int argc, const char **argv) + parse_opts(argc, (char **)argv, program_options, program_usage_string); + + vector_of_problem_data_t *ci = fetch_crash_infos(); ++ if (ci == NULL) ++ return 1; + + g_ptr_array_sort_with_data(ci, &cmp_problem_data, (char *) FILENAME_LAST_OCCURRENCE); + +-- +2.1.0 + diff --git a/0074-lib-add-new-kernel-taint-flags.patch b/0074-lib-add-new-kernel-taint-flags.patch new file mode 100644 index 0000000..ae67b1c --- /dev/null +++ b/0074-lib-add-new-kernel-taint-flags.patch @@ -0,0 +1,60 @@ +From 5484c326298f5cfab97553154398e805059a1756 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 5 May 2015 10:46:06 +0200 +Subject: [PATCH] lib: add new kernel taint flags + +Signed-off-by: Jakub Filak +--- + src/lib/kernel.c | 16 +++++++++++----- + 1 file changed, 11 insertions(+), 5 deletions(-) + +diff --git a/src/lib/kernel.c b/src/lib/kernel.c +index be80cbc..75c56e0 100644 +--- a/src/lib/kernel.c ++++ b/src/lib/kernel.c +@@ -608,8 +608,14 @@ char *koops_extract_version(const char *linepointer) + * 'W' - Taint on warning. + * 'C' - modules from drivers/staging are loaded. + * 'I' - Working around severe firmware bug. ++ * 'O' - Out-of-tree module has been loaded. ++ * 'E' - Unsigned module has been loaded. ++ * 'L' - A soft lockup has previously occurred. ++ * 'K' - Kernel has been live patched. ++ * ++ * Compatibility flags from older versions and downstream sources: + * 'H' - Hardware is unsupported. +- * T - Tech_preview ++ * 'T' - Tech_preview + */ + + #if 0 /* unused */ +@@ -634,7 +640,7 @@ char *kernel_tainted_short(const char *kernel_bt) + return NULL; + + tainted += strlen("Tainted: "); +- /* 13 == current count of known flags */ ++ /* 17 + 2 == current count of known flags */ + /* http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob_plain;f=kernel/panic.c;hb=HEAD */ + /* 26 the maximal sane count of flags because of alphabet limits */ + unsigned sz = 26 + 1; +@@ -677,14 +683,14 @@ static const char *const tnts_long[] = { + /* B */ "System has hit bad_page.", + /* C */ "Modules from drivers/staging are loaded.", + /* D */ "Kernel has oopsed before", +- /* E */ NULL, ++ /* E */ "Unsigned module has been loaded." + /* F */ "Module has been forcibly loaded.", + /* G */ "Proprietary module has not been loaded.", + /* H */ NULL, + /* I */ "Working around severe firmware bug.", + /* J */ NULL, +- /* K */ NULL, +- /* L */ NULL, ++ /* K */ "Kernel has been live patched.", ++ /* L */ "A soft lockup has previously occurred.", + /* M */ "System experienced a machine check exception.", + /* N */ NULL, + /* O */ "Out-of-tree module has been loaded.", +-- +2.1.0 + diff --git a/0075-a-a-s-p-d-add-new-known-interpreter-to-conf-file.patch b/0075-a-a-s-p-d-add-new-known-interpreter-to-conf-file.patch new file mode 100644 index 0000000..10bfbce --- /dev/null +++ b/0075-a-a-s-p-d-add-new-known-interpreter-to-conf-file.patch @@ -0,0 +1,29 @@ +From 4fca9db63a9674f5d64a519a7594eb0cd5649574 Mon Sep 17 00:00:00 2001 +From: Matej Habrnal +Date: Mon, 18 May 2015 08:45:48 +0200 +Subject: [PATCH] a-a-s-p-d: add new known interpreter to conf file + +There were new bugzillas opened with wrong component 'python3' because of a new +version of python (3.4). We don't want to blame the interpreters but the +running scripts. + +close #965 + +Signed-off-by: Matej Habrnal +--- + src/daemon/abrt-action-save-package-data.conf | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/daemon/abrt-action-save-package-data.conf b/src/daemon/abrt-action-save-package-data.conf +index cac3c7c..a02f5df 100644 +--- a/src/daemon/abrt-action-save-package-data.conf ++++ b/src/daemon/abrt-action-save-package-data.conf +@@ -18,4 +18,4 @@ ProcessUnpackaged = no + BlackListedPaths = /usr/share/doc/*, */example*, /usr/bin/nspluginviewer + + # interpreters names +-Interpreters = python2, python2.7, python, python3, python3.3, perl, perl5.16.2 ++Interpreters = python2, python2.7, python, python3, python3.3, python3.4, python3.5, perl, perl5.16.2 +-- +2.1.0 + diff --git a/0076-doc-update-abrt-cli-man-page.patch b/0076-doc-update-abrt-cli-man-page.patch new file mode 100644 index 0000000..54fda2f --- /dev/null +++ b/0076-doc-update-abrt-cli-man-page.patch @@ -0,0 +1,71 @@ +From 75d3d3bbc5cf7cddbac6e8c4b7c880dd76bb8d5c Mon Sep 17 00:00:00 2001 +From: Matej Habrnal +Date: Thu, 21 May 2015 11:52:35 +0200 +Subject: [PATCH] doc: update abrt-cli man page + +Related to rhbz#1179752 + +Signed-off-by: Matej Habrnal +--- + doc/abrt-cli.txt | 28 +++++++++++++++++++++------- + 1 file changed, 21 insertions(+), 7 deletions(-) + +diff --git a/doc/abrt-cli.txt b/doc/abrt-cli.txt +index cd14bc9..399b5fd 100644 +--- a/doc/abrt-cli.txt ++++ b/doc/abrt-cli.txt +@@ -7,30 +7,44 @@ abrt-cli - List, remove, print, analyze, report problems + + SYNOPSIS + -------- +-'abrt-cli' list [-vdf] [DIR]... ++'abrt-cli' list [-vn] [--detailed] [--since NUM] [--until NUM] [DIR]... + +-'abrt-cli' remove [-v] DIR... ++'abrt-cli' remove [-v] DIR... + +-'abrt-cli' report [-v] DIR... ++'abrt-cli' report [-v] [--delete] DIR... + +-'abrt-cli' info [-vd] [-s SIZE] DIR... ++'abrt-cli' info [-v] [--detailed] [-s SIZE] DIR... + +-'abrt-cli' process [-v] DIR... ++'abrt-cli' status [-vb] [--since NUM] ++ ++'abrt-cli' process [-v] [--since NUM] DIR... + + OPTIONS + ------- + -v,--verbose:: + Be more verbose. Can be given multiple times. + +--d,--detailed:: ++-b, --bare:: ++ Print only the problem count without any message ++ ++--detailed:: + Show detailed report + ++--delete:: ++ Remove PROBLEM_DIR after reporting ++ + -n,--not-reported:: + List only not-reported problems + +--s,--size SIZE: ++--size SIZE:: + Text larger than SIZE bytes will be shown abridged + ++--since NUM:: ++ Selects only problems detected after timestamp ++ ++--until NUM:: ++ Selects only the problems older than specified timestamp ++ + AUTHORS + ------- + * ABRT team +-- +2.1.0 + diff --git a/0078-turn-off-exploring-crashed-process-s-root-directorie.patch b/0078-turn-off-exploring-crashed-process-s-root-directorie.patch new file mode 100644 index 0000000..05d8c1e --- /dev/null +++ b/0078-turn-off-exploring-crashed-process-s-root-directorie.patch @@ -0,0 +1,130 @@ +From 2b1fa2caf4ef3cb253e931f53b43fc9499661da4 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 16 Apr 2015 11:12:40 +0200 +Subject: [PATCH] turn off exploring crashed process's root directories + +A local user can arrange a process root directory in way that abrt will +make a copy of file not accessible by the attacker. + +I don't want to remove the chroot code entirely because it might be +useful for non-production environments. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-action-save-package-data.c | 6 +++++- + src/daemon/abrt.conf | 16 ++++++++++++++++ + src/hooks/abrt-hook-ccpp.c | 12 +++++++++++- + src/include/libabrt.h | 2 ++ + src/lib/abrt_conf.c | 10 ++++++++++ + 5 files changed, 44 insertions(+), 2 deletions(-) + +diff --git a/src/daemon/abrt-action-save-package-data.c b/src/daemon/abrt-action-save-package-data.c +index cc86327..816e0d0 100644 +--- a/src/daemon/abrt-action-save-package-data.c ++++ b/src/daemon/abrt-action-save-package-data.c +@@ -239,7 +239,11 @@ static int SavePackageDescriptionToDebugDump(const char *dump_dir_name) + + cmdline = dd_load_text_ext(dd, FILENAME_CMDLINE, DD_FAIL_QUIETLY_ENOENT); + executable = dd_load_text(dd, FILENAME_EXECUTABLE); +- rootdir = dd_load_text_ext(dd, FILENAME_ROOTDIR, ++ ++ /* Do not implicitly query rpm database in process's root dir, if ++ * ExploreChroots is disabled. */ ++ if (g_settings_explorechroots) ++ rootdir = dd_load_text_ext(dd, FILENAME_ROOTDIR, + DD_FAIL_QUIETLY_ENOENT | DD_LOAD_TEXT_RETURN_NULL_ON_FAILURE); + + /* Close dd while we query package database. It can take some time, +diff --git a/src/daemon/abrt.conf b/src/daemon/abrt.conf +index 59d1831..02e969d 100644 +--- a/src/daemon/abrt.conf ++++ b/src/daemon/abrt.conf +@@ -43,3 +43,19 @@ AutoreportingEnabled = no + # session; otherwise No. + # + # ShortenedReporting = yes ++ ++# Enables various features exploring process's root directories if they differ ++# from the default root directory. The folowing list includes examples of ++# enabled features: ++# * query the rpm database in the process's root directory ++# * save files like /etc/os-release from the process's root directory ++# ++# This feature is disabled by default because it might be used by a local user ++# to steal your data. ++# ++# Caution: ++# ++# THIS FEATURE MIGHT BE USED BY A LOCAL USER TO STEEL YOUR DATA BY ARRANGING A ++# SPECIAL ROOT DIRECTORY IN USER MOUNT NAMESAPCE ++# ++# ExploreChroots = false +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index e1d81b6..7626a97 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -601,7 +601,17 @@ int main(int argc, char** argv) + { + char *rootdir = get_rootdir(pid); + +- dd_create_basic_files(dd, fsuid, (rootdir && strcmp(rootdir, "/") != 0) ? rootdir : NULL); ++ /* Reading data from an arbitrary root directory is not secure. */ ++ if (g_settings_explorechroots) ++ { ++ /* Yes, test 'rootdir' but use 'source_filename' because 'rootdir' can ++ * be '/' for a process with own namespace. 'source_filename' is /proc/[pid]/root. */ ++ dd_create_basic_files(dd, fsuid, (rootdir != NULL) ? rootdir : NULL); ++ } ++ else ++ { ++ dd_create_basic_files(dd, fsuid, NULL); ++ } + + char source_filename[sizeof("/proc/%lu/somewhat_long_name") + sizeof(long)*3]; + int source_base_ofs = sprintf(source_filename, "/proc/%lu/smaps", (long)pid); +diff --git a/src/include/libabrt.h b/src/include/libabrt.h +index 65d30a1..abfbc97 100644 +--- a/src/include/libabrt.h ++++ b/src/include/libabrt.h +@@ -62,6 +62,8 @@ extern bool g_settings_autoreporting; + extern char * g_settings_autoreporting_event; + #define g_settings_shortenedreporting abrt_g_settings_shortenedreporting + extern bool g_settings_shortenedreporting; ++#define g_settings_explorechroots abrt_g_settings_explorechroots ++extern bool g_settings_explorechroots; + + + #define load_abrt_conf abrt_load_abrt_conf +diff --git a/src/lib/abrt_conf.c b/src/lib/abrt_conf.c +index f7fdc6d..46ff689 100644 +--- a/src/lib/abrt_conf.c ++++ b/src/lib/abrt_conf.c +@@ -27,6 +27,7 @@ bool g_settings_delete_uploaded = 0; + bool g_settings_autoreporting = 0; + char * g_settings_autoreporting_event = NULL; + bool g_settings_shortenedreporting = 0; ++bool g_settings_explorechroots = 0; + + void free_abrt_conf_data() + { +@@ -106,6 +107,15 @@ static void ParseCommon(map_string_t *settings, const char *conf_filename) + g_settings_shortenedreporting = (desktop_env && strcasestr(desktop_env, "gnome") != NULL); + } + ++ value = get_map_string_item_or_NULL(settings, "ExploreChroots"); ++ if (value) ++ { ++ g_settings_explorechroots = string_to_bool(value); ++ remove_map_string_item(settings, "ExploreChroots"); ++ } ++ else ++ g_settings_explorechroots = false; ++ + GHashTableIter iter; + const char *name; + /*char *value; - already declared */ +-- +2.1.0 + diff --git a/0079-ccpp-fix-symlink-race-conditions.patch b/0079-ccpp-fix-symlink-race-conditions.patch new file mode 100644 index 0000000..a3a71b3 --- /dev/null +++ b/0079-ccpp-fix-symlink-race-conditions.patch @@ -0,0 +1,93 @@ +From 506b867ad956fe28efef0b80b53a26ed0258abef Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 12:14:22 +0200 +Subject: [PATCH] ccpp: fix symlink race conditions + +Fix copy & chown race conditions + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 31 +++++++++++++------------------ + 1 file changed, 13 insertions(+), 18 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 7626a97..218abac 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -309,21 +309,24 @@ static int create_or_die(const char *filename, int user_core_fd) + perror_msg_and_die("Can't open '%s'", filename); + } + +-static void create_core_backtrace(pid_t tid, const char *executable, int signal_no, const char *dd_path) ++static void create_core_backtrace(pid_t tid, const char *executable, int signal_no, struct dump_dir *dd) + { + #ifdef ENABLE_DUMP_TIME_UNWIND + if (g_verbose > 1) + sr_debug_parser = true; + + char *error_message = NULL; +- bool success = sr_abrt_create_core_stacktrace_from_core_hook(dd_path, tid, executable, +- signal_no, &error_message); ++ char *core_bt = sr_abrt_get_core_stacktrace_from_core_hook(tid, executable, ++ signal_no, &error_message); + +- if (!success) ++ if (core_bt == NULL) + { + log("Failed to create core_backtrace: %s", error_message); + free(error_message); + } ++ ++ dd_save_text(dd, FILENAME_CORE_BACKTRACE, core_bt); ++ free(core_bt); + #endif /* ENABLE_DUMP_TIME_UNWIND */ + } + +@@ -621,28 +624,20 @@ int main(int argc, char** argv) + + // Disabled for now: /proc/PID/smaps tends to be BIG, + // and not much more informative than /proc/PID/maps: +- //copy_file(source_filename, dest_filename, 0640); +- //chown(dest_filename, dd->dd_uid, dd->dd_gid); ++ // dd_copy_file(dd, FILENAME_SMAPS, source_filename); + + strcpy(source_filename + source_base_ofs, "maps"); +- strcpy(dest_base, FILENAME_MAPS); +- copy_file(source_filename, dest_filename, DEFAULT_DUMP_DIR_MODE); +- IGNORE_RESULT(chown(dest_filename, dd->dd_uid, dd->dd_gid)); ++ dd_copy_file(dd, FILENAME_MAPS, source_filename); + + strcpy(source_filename + source_base_ofs, "limits"); +- strcpy(dest_base, FILENAME_LIMITS); +- copy_file(source_filename, dest_filename, DEFAULT_DUMP_DIR_MODE); +- IGNORE_RESULT(chown(dest_filename, dd->dd_uid, dd->dd_gid)); ++ dd_copy_file(dd, FILENAME_LIMITS, source_filename); + + strcpy(source_filename + source_base_ofs, "cgroup"); +- strcpy(dest_base, FILENAME_CGROUP); +- copy_file(source_filename, dest_filename, DEFAULT_DUMP_DIR_MODE); +- IGNORE_RESULT(chown(dest_filename, dd->dd_uid, dd->dd_gid)); ++ dd_copy_file(dd, FILENAME_CGROUP, source_filename); + + strcpy(dest_base, FILENAME_OPEN_FDS); + strcpy(source_filename + source_base_ofs, "fd"); +- if (dump_fd_info(dest_filename, source_filename) == 0) +- IGNORE_RESULT(chown(dest_filename, dd->dd_uid, dd->dd_gid)); ++ dump_fd_info_ext(dest_filename, source_filename, dd->dd_uid, dd->dd_gid); + + free(dest_filename); + +@@ -782,7 +777,7 @@ int main(int argc, char** argv) + + /* Perform crash-time unwind of the guilty thread. */ + if (tid > 0 && setting_CreateCoreBacktrace) +- create_core_backtrace(tid, executable, signal_no, dd->dd_dirname); ++ create_core_backtrace(tid, executable, signal_no, dd); + + /* We close dumpdir before we start catering for crash storm case. + * Otherwise, delete_dump_dir's from other concurrent +-- +2.1.0 + diff --git a/0080-ccpp-stop-reading-hs_error.log-from-tmp.patch b/0080-ccpp-stop-reading-hs_error.log-from-tmp.patch new file mode 100644 index 0000000..ef43f4d --- /dev/null +++ b/0080-ccpp-stop-reading-hs_error.log-from-tmp.patch @@ -0,0 +1,39 @@ +From 1d09c7271eae9aee2fae62367c9a83b30a685c69 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 28 Apr 2015 14:00:18 +0200 +Subject: [PATCH] ccpp: stop reading hs_error.log from /tmp + +The file might contain anything and there is no way to verify its +contents. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 218abac..880daf6 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -741,6 +741,8 @@ int main(int argc, char** argv) + * ! No other errors should cause removal of the user core ! + */ + ++/* Because of #1211835 and #1126850 */ ++#if 0 + /* Save JVM crash log if it exists. (JVM's coredump per se + * is nearly useless for JVM developers) + */ +@@ -774,6 +776,7 @@ int main(int argc, char** argv) + close(src_fd); + } + } ++#endif + + /* Perform crash-time unwind of the guilty thread. */ + if (tid > 0 && setting_CreateCoreBacktrace) +-- +2.1.0 + diff --git a/0081-ccpp-postpone-changing-ownership-of-new-dump-directo.patch b/0081-ccpp-postpone-changing-ownership-of-new-dump-directo.patch new file mode 100644 index 0000000..6ee14ce --- /dev/null +++ b/0081-ccpp-postpone-changing-ownership-of-new-dump-directo.patch @@ -0,0 +1,52 @@ +From ccbab90e154f7917178cc1d56d8990b01ea45023 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 15:27:09 +0200 +Subject: [PATCH] ccpp: postpone changing ownership of new dump directories + +Florian Weimer : + + Currently, dd_create changes ownership of the directory immediately, + when it is still empty. This means that any operations within the + directory (which happen as the root user) can race with changes to + the directory contents by the user. If you delay changing directory + ownership until all the files have created and written, this is no + longer a problem. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 9 ++++++++- + 1 file changed, 8 insertions(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 880daf6..04889da 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -598,8 +598,12 @@ int main(int argc, char** argv) + + /* use fsuid instead of uid, so we don't expose any sensitive + * information of suided app in /var/tmp/abrt ++ * ++ * dd_create_skeleton() creates a new directory and leaves ownership to ++ * the current user, hence, we have to call dd_reset_ownership() after the ++ * directory is populated. + */ +- dd = dd_create(path, fsuid, DEFAULT_DUMP_DIR_MODE); ++ dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE); + if (dd) + { + char *rootdir = get_rootdir(pid); +@@ -782,6 +786,9 @@ int main(int argc, char** argv) + if (tid > 0 && setting_CreateCoreBacktrace) + create_core_backtrace(tid, executable, signal_no, dd); + ++ /* And finally set the right uid and gid */ ++ dd_reset_ownership(dd); ++ + /* We close dumpdir before we start catering for crash storm case. + * Otherwise, delete_dump_dir's from other concurrent + * CCpp's won't be able to delete our dump (their delete_dump_dir +-- +2.1.0 + diff --git a/0082-ccpp-create-dump-directory-without-parents.patch b/0082-ccpp-create-dump-directory-without-parents.patch new file mode 100644 index 0000000..8066745 --- /dev/null +++ b/0082-ccpp-create-dump-directory-without-parents.patch @@ -0,0 +1,31 @@ +From a691dd91f561bb32367ecab510930767871137c6 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 15 Apr 2015 17:42:59 +0200 +Subject: [PATCH] ccpp: create dump directory without parents + +This patch makes the code more robust. +This patch ensures that abrt-hook-ccpp never creates the dump location. + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 04889da..f77a23f 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -603,7 +603,7 @@ int main(int argc, char** argv) + * the current user, hence, we have to call dd_reset_ownership() after the + * directory is populated. + */ +- dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE); ++ dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE, /*no flags*/0); + if (dd) + { + char *rootdir = get_rootdir(pid); +-- +2.1.0 + diff --git a/0083-ccpp-do-not-override-existing-files-by-compat-cores.patch b/0083-ccpp-do-not-override-existing-files-by-compat-cores.patch new file mode 100644 index 0000000..1892fdf --- /dev/null +++ b/0083-ccpp-do-not-override-existing-files-by-compat-cores.patch @@ -0,0 +1,82 @@ +From 0516cbbb356ca0bfd3faf7accb36c60fc5da89d7 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 17 Apr 2015 14:36:45 +0200 +Subject: [PATCH] ccpp: do not override existing files by compat cores + +Implement all checks used in kernel's do_coredump() and require +non-relative path if suid_dumpable is 2. + +Related: #1212818 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 20 ++++++++++++++++---- + 1 file changed, 16 insertions(+), 4 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index f77a23f..b481421 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -26,6 +26,8 @@ + #include + #endif /* ENABLE_DUMP_TIME_UNWIND */ + ++static int g_user_core_flags; ++static int g_need_nonrelative; + + /* I want to use -Werror, but gcc-4.4 throws a curveball: + * "warning: ignoring return value of 'ftruncate', declared with attribute warn_unused_result" +@@ -227,7 +229,14 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + + full_core_basename = core_basename; + if (core_basename[0] != '/') ++ { ++ if (g_need_nonrelative) ++ { ++ error_msg("Current suid_dumpable policy prevents from saving core dumps according to relative core_pattern"); ++ return -1; ++ } + core_basename = concat_path_file(user_pwd, core_basename); ++ } + + /* Open (create) compat core file. + * man core: +@@ -262,19 +271,19 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + struct stat sb; + errno = 0; + /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */ +- int user_core_fd = open(core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW, 0600); /* kernel makes 0600 too */ ++ int user_core_fd = open(core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */ + xsetegid(0); + xseteuid(0); + if (user_core_fd < 0 + || fstat(user_core_fd, &sb) != 0 + || !S_ISREG(sb.st_mode) + || sb.st_nlink != 1 +- /* kernel internal dumper checks this too: if (inode->i_uid != current->fsuid) , need to mimic? */ ++ || sb.st_uid != fsuid + ) { + if (user_core_fd < 0) + perror_msg("Can't open '%s'", full_core_basename); + else +- perror_msg("'%s' is not a regular file with link count 1", full_core_basename); ++ perror_msg("'%s' is not a regular file with link count 1 owned by UID(%d)", full_core_basename, fsuid); + return -1; + } + if (ftruncate(user_core_fd, 0) != 0) { +@@ -518,8 +527,11 @@ int main(int argc, char** argv) + /* use root for suided apps unless it's explicitly set to UNSAFE */ + fsuid = 0; + if (suid_policy == DUMP_SUID_UNSAFE) +- { + fsuid = tmp_fsuid; ++ else ++ { ++ g_user_core_flags = O_EXCL; ++ g_need_nonrelative = 1; + } + } + +-- +2.1.0 + diff --git a/0084-ccpp-do-not-use-value-of-proc-PID-cwd-for-chdir.patch b/0084-ccpp-do-not-use-value-of-proc-PID-cwd-for-chdir.patch new file mode 100644 index 0000000..bdca474 --- /dev/null +++ b/0084-ccpp-do-not-use-value-of-proc-PID-cwd-for-chdir.patch @@ -0,0 +1,269 @@ +From 49279f4030225cee166362d7222e97496022331f Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 17 Apr 2015 14:40:20 +0200 +Subject: [PATCH] ccpp: do not use value of /proc/PID/cwd for chdir + +Avoid symlink resolutions. + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 116 ++++++++++++++++++++++++--------------------- + 1 file changed, 61 insertions(+), 55 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index b481421..8f3b2b0 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -131,6 +131,7 @@ static off_t copyfd_sparse(int src_fd, int dst_fd1, int dst_fd2, off_t size2) + + /* Global data */ + static char *user_pwd; ++static DIR *proc_cwd; + static struct dump_dir *dd; + + /* +@@ -149,22 +150,24 @@ static struct dump_dir *dd; + */ + static const char percent_specifiers[] = "%scpugtei"; + static char *core_basename = (char*) "core"; +-/* +- * Used for error messages only. +- * It is either the same as core_basename if it is absolute, +- * or $PWD/core_basename. +- */ +-static char *full_core_basename; ++ ++static DIR *open_cwd(pid_t pid) ++{ ++ char buf[sizeof("/proc/%lu/cwd") + sizeof(long)*3]; ++ sprintf(buf, "/proc/%lu/cwd", (long)pid); ++ ++ DIR *cwd = opendir(buf); ++ if (cwd == NULL) ++ perror_msg("Can't open process's CWD for CompatCore"); ++ ++ return cwd; ++} + + static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_values) + { +- errno = 0; +- if (user_pwd == NULL +- || chdir(user_pwd) != 0 +- ) { +- perror_msg("Can't cd to '%s'", user_pwd); ++ proc_cwd = open_cwd(pid); ++ if (proc_cwd == NULL) + return -1; +- } + + struct passwd* pw = getpwuid(uid); + gid_t gid = pw ? pw->pw_gid : uid; +@@ -227,15 +230,10 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + } + } + +- full_core_basename = core_basename; +- if (core_basename[0] != '/') ++ if (g_need_nonrelative && core_basename[0] != '/') + { +- if (g_need_nonrelative) +- { +- error_msg("Current suid_dumpable policy prevents from saving core dumps according to relative core_pattern"); +- return -1; +- } +- core_basename = concat_path_file(user_pwd, core_basename); ++ error_msg("Current suid_dumpable policy prevents from saving core dumps according to relative core_pattern"); ++ return -1; + } + + /* Open (create) compat core file. +@@ -271,7 +269,7 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + struct stat sb; + errno = 0; + /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */ +- int user_core_fd = open(core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */ ++ int user_core_fd = openat(dirfd(proc_cwd), core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */ + xsetegid(0); + xseteuid(0); + if (user_core_fd < 0 +@@ -281,15 +279,15 @@ static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_valu + || sb.st_uid != fsuid + ) { + if (user_core_fd < 0) +- perror_msg("Can't open '%s'", full_core_basename); ++ perror_msg("Can't open '%s' at '%s'", core_basename, user_pwd); + else +- perror_msg("'%s' is not a regular file with link count 1 owned by UID(%d)", full_core_basename, fsuid); ++ perror_msg("'%s' at '%s' is not a regular file with link count 1 owned by UID(%d)", core_basename, user_pwd, fsuid); + return -1; + } + if (ftruncate(user_core_fd, 0) != 0) { + /* perror first, otherwise unlink may trash errno */ +- perror_msg("Can't truncate '%s' to size 0", full_core_basename); +- unlink(core_basename); ++ perror_msg("Can't truncate '%s' at '%s' to size 0", core_basename, user_pwd); ++ unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); + return -1; + } + +@@ -310,10 +308,8 @@ static int create_or_die(const char *filename, int user_core_fd) + if (dd) + dd_delete(dd); + if (user_core_fd >= 0) +- { +- xchdir(user_pwd); +- unlink(core_basename); +- } ++ unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); ++ + errno = sv_errno; + perror_msg_and_die("Can't open '%s'", filename); + } +@@ -341,27 +337,34 @@ static void create_core_backtrace(pid_t tid, const char *executable, int signal_ + + static int create_user_core(int user_core_fd, pid_t pid, off_t ulimit_c) + { ++ int err = 1; + if (user_core_fd >= 0) + { + off_t core_size = copyfd_size(STDIN_FILENO, user_core_fd, ulimit_c, COPYFD_SPARSE); + if (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 || core_size < 0) + { + /* perror first, otherwise unlink may trash errno */ +- perror_msg("Error writing '%s'", full_core_basename); +- xchdir(user_pwd); +- unlink(core_basename); +- return 1; ++ perror_msg("Error writing '%s' at '%s'", core_basename, user_pwd); ++ unlinkat(dirfd(proc_cwd), core_basename, /*only files*/0); ++ goto finito; + } + if (ulimit_c == 0 || core_size > ulimit_c) + { +- xchdir(user_pwd); +- unlink(core_basename); +- return 1; ++ unlinkat(dirfd(proc_cwd), core_basename, /*only files*/0); ++ goto finito; + } +- log_notice("Saved core dump of pid %lu to %s (%llu bytes)", (long)pid, full_core_basename, (long long)core_size); ++ log_notice("Saved core dump of pid %lu to %s at '%s' (%llu bytes)", (long)pid, core_basename, user_pwd, (long long)core_size); + } ++ err = 0; + +- return 0; ++finito: ++ if (proc_cwd != NULL) ++ { ++ closedir(proc_cwd); ++ proc_cwd = NULL; ++ } ++ ++ return err; + } + + static int test_configuration(bool setting_SaveFullCore, bool setting_CreateCoreBacktrace) +@@ -417,6 +420,7 @@ int main(int argc, char** argv) + if (fd > 2) + close(fd); + ++ int err = 1; + logmode = LOGMODE_JOURNAL; + + /* Parse abrt.conf */ +@@ -598,7 +602,8 @@ int main(int argc, char** argv) + error_msg_and_die("Error saving '%s'", path); + } + log_notice("Saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size); +- return 0; ++ err = 0; ++ goto cleanup_and_exit; + } + + unsigned path_len = snprintf(path, sizeof(path), "%s/ccpp-%s-%lu.new", +@@ -669,6 +674,7 @@ int main(int argc, char** argv) + if (strcmp(rootdir, "/") != 0) + dd_save_text(dd, FILENAME_ROOTDIR, rootdir); + } ++ free(rootdir); + + char *reason = xasprintf("%s killed by SIG%s", + last_slash, signame ? signame : signal_str); +@@ -701,7 +707,7 @@ int main(int argc, char** argv) + { + error_msg("Error saving '%s'", path); + +- goto error_exit; ++ goto cleanup_and_exit; + } + } + +@@ -730,7 +736,7 @@ int main(int argc, char** argv) + * but it does not log file name */ + error_msg("Error writing '%s'", path); + +- goto error_exit; ++ goto cleanup_and_exit; + } + if (user_core_fd >= 0 + /* error writing user coredump? */ +@@ -740,8 +746,7 @@ int main(int argc, char** argv) + ) + ) { + /* nuke it (silently) */ +- xchdir(user_pwd); +- unlink(core_basename); ++ unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); + } + } + else +@@ -787,7 +792,7 @@ int main(int argc, char** argv) + { + error_msg("Error saving '%s'", path); + +- goto error_exit; ++ goto cleanup_and_exit; + } + close(src_fd); + } +@@ -833,22 +838,23 @@ int main(int argc, char** argv) + trim_problem_dirs(g_settings_dump_location, maxsize * (double)(1024*1024), path); + } + +- free(rootdir); +- return 0; ++ err = 0; ++ } ++ else ++ { ++ /* We didn't create abrt dump, but may need to create compat coredump */ ++ return create_user_core(user_core_fd, pid, ulimit_c); + } + +- /* We didn't create abrt dump, but may need to create compat coredump */ +- return create_user_core(user_core_fd, pid, ulimit_c); +- +-error_exit: ++cleanup_and_exit: + if (dd) + dd_delete(dd); + + if (user_core_fd >= 0) +- { +- xchdir(user_pwd); +- unlink(core_basename); +- } ++ unlinkat(dirfd(proc_cwd), core_basename, /*only files*/0); ++ ++ if (proc_cwd != NULL) ++ closedir(proc_cwd); + +- xfunc_die(); ++ return err; + } +-- +2.1.0 + diff --git a/0085-ccpp-make-saving-of-binary-more-robust.patch b/0085-ccpp-make-saving-of-binary-more-robust.patch new file mode 100644 index 0000000..c1016a3 --- /dev/null +++ b/0085-ccpp-make-saving-of-binary-more-robust.patch @@ -0,0 +1,59 @@ +From ce7d0e05de76ae5383bcdfdba3ffa8816ffb63bd Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 13 May 2015 13:05:48 +0200 +Subject: [PATCH] ccpp: make saving of binary more robust + +Do not override existing files (defend against hard | symbolic link +attacks) and use the *at functions (defend against race conditions). + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 12 +++++------- + 1 file changed, 5 insertions(+), 7 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 8f3b2b0..9d9f549 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -379,7 +379,7 @@ static int test_configuration(bool setting_SaveFullCore, bool setting_CreateCore + return 0; + } + +-int save_crashing_binary(pid_t pid, const char *dest_path, uid_t uid, gid_t gid) ++static int save_crashing_binary(pid_t pid, struct dump_dir *dd) + { + char buf[sizeof("/proc/%lu/exe") + sizeof(long)*3]; + +@@ -391,15 +391,15 @@ int save_crashing_binary(pid_t pid, const char *dest_path, uid_t uid, gid_t gid) + return 0; + } + +- int dst_fd = open(dest_path, O_WRONLY | O_CREAT | O_TRUNC, DEFAULT_DUMP_DIR_MODE); ++ int dst_fd = openat(dd->dd_fd, FILENAME_BINARY, O_WRONLY | O_CREAT | O_EXCL | O_TRUNC, DEFAULT_DUMP_DIR_MODE); + if (dst_fd < 0) + { +- log_notice("Failed to create file '%s'", dest_path); ++ log_notice("Failed to create file '"FILENAME_BINARY"' at '%s'", dd->dd_dirname); + close(src_fd_binary); + return -1; + } + +- IGNORE_RESULT(fchown(dst_fd, uid, gid)); ++ IGNORE_RESULT(fchown(dst_fd, dd->dd_uid, dd->dd_gid)); + + off_t sz = copyfd_eof(src_fd_binary, dst_fd, COPYFD_SPARSE); + close(src_fd_binary); +@@ -701,9 +701,7 @@ int main(int argc, char** argv) + + if (setting_SaveBinaryImage) + { +- strcpy(path + path_len, "/"FILENAME_BINARY); +- +- if (save_crashing_binary(pid, path, dd->dd_uid, dd->dd_gid)) ++ if (save_crashing_binary(pid, dd)) + { + error_msg("Error saving '%s'", path); + +-- +2.1.0 + diff --git a/0086-ccpp-harden-dealing-with-UID-GID.patch b/0086-ccpp-harden-dealing-with-UID-GID.patch new file mode 100644 index 0000000..caf1726 --- /dev/null +++ b/0086-ccpp-harden-dealing-with-UID-GID.patch @@ -0,0 +1,61 @@ +From e38774dea8d0a23b952a423b4e7a946f0f570149 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 17 Apr 2015 14:42:13 +0200 +Subject: [PATCH] ccpp: harden dealing with UID/GID + +* Don't fall back to UID 0 (fixed in libreport) +* Use fsgid. + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 13 +++++++------ + 1 file changed, 7 insertions(+), 6 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 9d9f549..92413e3 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -163,16 +163,13 @@ static DIR *open_cwd(pid_t pid) + return cwd; + } + +-static int open_user_core(uid_t uid, uid_t fsuid, pid_t pid, char **percent_values) ++static int open_user_core(uid_t uid, uid_t fsuid, gid_t fsgid, pid_t pid, char **percent_values) + { + proc_cwd = open_cwd(pid); + if (proc_cwd == NULL) + return -1; + +- struct passwd* pw = getpwuid(uid); +- gid_t gid = pw ? pw->pw_gid : uid; +- //log("setting uid: %i gid: %i", uid, gid); +- xsetegid(gid); ++ xsetegid(fsgid); + xseteuid(fsuid); + + if (strcmp(core_basename, "core") == 0) +@@ -525,6 +522,10 @@ int main(int argc, char** argv) + if (tmp_fsuid < 0) + perror_msg_and_die("Can't parse 'Uid: line' in /proc/%lu/status", (long)pid); + ++ const int fsgid = get_fsgid(proc_pid_status); ++ if (fsgid < 0) ++ error_msg_and_die("Can't parse 'Gid: line' in /proc/%lu/status", (long)pid); ++ + int suid_policy = dump_suid_policy(); + if (tmp_fsuid != uid) + { +@@ -543,7 +544,7 @@ int main(int argc, char** argv) + int user_core_fd = -1; + if (setting_MakeCompatCore && ulimit_c != 0) + /* note: checks "user_pwd == NULL" inside; updates core_basename */ +- user_core_fd = open_user_core(uid, fsuid, pid, &argv[1]); ++ user_core_fd = open_user_core(uid, fsuid, fsgid, pid, &argv[1]); + + if (executable == NULL) + { +-- +2.1.0 + diff --git a/0087-ccpp-check-for-overflow-in-abrt-coredump-path-creati.patch b/0087-ccpp-check-for-overflow-in-abrt-coredump-path-creati.patch new file mode 100644 index 0000000..f7b72ea --- /dev/null +++ b/0087-ccpp-check-for-overflow-in-abrt-coredump-path-creati.patch @@ -0,0 +1,30 @@ +From 3d9e235072f6d219181a12b003112d5315544649 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 17 Apr 2015 14:43:59 +0200 +Subject: [PATCH] ccpp: check for overflow in abrt coredump path creation + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 92413e3..53700e4 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -592,7 +592,9 @@ int main(int argc, char** argv) + * and maybe crash again... + * Unlike dirs, mere files are ignored by abrtd. + */ +- snprintf(path, sizeof(path), "%s/%s-coredump", g_settings_dump_location, last_slash); ++ if (snprintf(path, sizeof(path), "%s/%s-coredump", g_settings_dump_location, last_slash) >= sizeof(path)) ++ error_msg_and_die("Error saving '%s': truncated long file path", path); ++ + int abrt_core_fd = xopen3(path, O_WRONLY | O_CREAT | O_TRUNC, 0600); + off_t core_size = copyfd_eof(STDIN_FILENO, abrt_core_fd, COPYFD_SPARSE); + if (core_size < 0 || fsync(abrt_core_fd) != 0) +-- +2.1.0 + diff --git a/0088-ccpp-emulate-selinux-for-creation-of-compat-cores.patch b/0088-ccpp-emulate-selinux-for-creation-of-compat-cores.patch new file mode 100644 index 0000000..a477bef --- /dev/null +++ b/0088-ccpp-emulate-selinux-for-creation-of-compat-cores.patch @@ -0,0 +1,183 @@ +From cfc1a5da0a0f2273e0ce39da0c2fa81053ed0eaa Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 17 Apr 2015 16:06:33 +0200 +Subject: [PATCH] ccpp: emulate selinux for creation of compat cores + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +http://article.gmane.org/gmane.comp.security.selinux/21842 + +Signed-off-by: Jakub Filak +--- + configure.ac | 1 + + src/hooks/Makefile.am | 4 ++- + src/hooks/abrt-hook-ccpp.c | 85 ++++++++++++++++++++++++++++++++++++++++++++-- + 3 files changed, 86 insertions(+), 4 deletions(-) + +diff --git a/configure.ac b/configure.ac +index d7e0ea5..430f23c 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -141,6 +141,7 @@ PKG_CHECK_MODULES([POLKIT], [polkit-gobject-1]) + PKG_CHECK_MODULES([GIO], [gio-2.0]) + PKG_CHECK_MODULES([SATYR], [satyr]) + PKG_CHECK_MODULES([SYSTEMD_JOURNAL], [libsystemd-journal]) ++PKG_CHECK_MODULES([LIBSELINUX], [libselinux]) + + PKG_PROG_PKG_CONFIG + AC_ARG_WITH([systemdsystemunitdir], +diff --git a/src/hooks/Makefile.am b/src/hooks/Makefile.am +index 13702b5..ff070cf 100644 +--- a/src/hooks/Makefile.am ++++ b/src/hooks/Makefile.am +@@ -33,10 +33,12 @@ abrt_hook_ccpp_CPPFLAGS = \ + -DDEFAULT_DUMP_DIR_MODE=$(DEFAULT_DUMP_DIR_MODE) \ + $(GLIB_CFLAGS) \ + $(LIBREPORT_CFLAGS) \ ++ $(LIBSELINUX_CFLAGS) \ + -D_GNU_SOURCE + abrt_hook_ccpp_LDADD = \ + ../lib/libabrt.la \ +- $(LIBREPORT_LIBS) ++ $(LIBREPORT_LIBS) \ ++ $(LIBSELINUX_LIBS) + + # abrt-merge-pstoreoops + abrt_merge_pstoreoops_SOURCES = \ +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 53700e4..9696423 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -20,6 +20,7 @@ + */ + #include + #include "libabrt.h" ++#include + + #ifdef ENABLE_DUMP_TIME_UNWIND + #include +@@ -163,12 +164,68 @@ static DIR *open_cwd(pid_t pid) + return cwd; + } + ++/* Computes a security context of new file created by the given process with ++ * pid in the given directory represented by file descriptor. ++ * ++ * On errors returns negative number. Returns 0 if the function succeeds and ++ * computes the context and returns positive number and assigns NULL to newcon ++ * if the security context is not needed (SELinux disabled). ++ */ ++static int compute_selinux_con_for_new_file(pid_t pid, int dir_fd, security_context_t *newcon) ++{ ++ security_context_t srccon; ++ security_context_t dstcon; ++ ++ const int r = is_selinux_enabled(); ++ if (r == 0) ++ { ++ *newcon = NULL; ++ return 1; ++ } ++ else if (r == -1) ++ { ++ perror_msg("Couldn't get state of SELinux"); ++ return -1; ++ } ++ else if (r != 1) ++ error_msg_and_die("Unexpected SELinux return value: %d", r); ++ ++ ++ if (getpidcon_raw(pid, &srccon) < 0) ++ { ++ perror_msg("getpidcon_raw(%d)", pid); ++ return -1; ++ } ++ ++ if (fgetfilecon_raw(dir_fd, &dstcon) < 0) ++ { ++ perror_msg("getfilecon_raw(%s)", user_pwd); ++ return -1; ++ } ++ ++ if (security_compute_create_raw(srccon, dstcon, string_to_security_class("file"), newcon) < 0) ++ { ++ perror_msg("security_compute_create_raw(%s, %s, 'file')", srccon, dstcon); ++ return -1; ++ } ++ ++ return 0; ++} ++ + static int open_user_core(uid_t uid, uid_t fsuid, gid_t fsgid, pid_t pid, char **percent_values) + { + proc_cwd = open_cwd(pid); + if (proc_cwd == NULL) + return -1; + ++ /* http://article.gmane.org/gmane.comp.security.selinux/21842 */ ++ security_context_t newcon; ++ if (compute_selinux_con_for_new_file(pid, dirfd(proc_cwd), &newcon) < 0) ++ { ++ log_notice("Not going to create a user core due to SELinux errors"); ++ return -1; ++ } ++ + xsetegid(fsgid); + xseteuid(fsuid); + +@@ -263,10 +320,25 @@ static int open_user_core(uid_t uid, uid_t fsuid, gid_t fsgid, pid_t pid, char * + * (However, see the description of the prctl(2) PR_SET_DUMPABLE operation, + * and the description of the /proc/sys/fs/suid_dumpable file in proc(5).) + */ ++ ++ /* Set SELinux context like kernel when creating core dump file */ ++ if (newcon != NULL && setfscreatecon_raw(newcon) < 0) ++ { ++ perror_msg("setfscreatecon_raw(%s)", newcon); ++ return -1; ++ } ++ + struct stat sb; + errno = 0; + /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */ + int user_core_fd = openat(dirfd(proc_cwd), core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */ ++ ++ if (newcon != NULL && setfscreatecon_raw(NULL) < 0) ++ { ++ error_msg("setfscreatecon_raw(NULL)"); ++ goto user_core_fail; ++ } ++ + xsetegid(0); + xseteuid(0); + if (user_core_fd < 0 +@@ -279,16 +351,23 @@ static int open_user_core(uid_t uid, uid_t fsuid, gid_t fsgid, pid_t pid, char * + perror_msg("Can't open '%s' at '%s'", core_basename, user_pwd); + else + perror_msg("'%s' at '%s' is not a regular file with link count 1 owned by UID(%d)", core_basename, user_pwd, fsuid); +- return -1; ++ goto user_core_fail; + } + if (ftruncate(user_core_fd, 0) != 0) { + /* perror first, otherwise unlink may trash errno */ + perror_msg("Can't truncate '%s' at '%s' to size 0", core_basename, user_pwd); +- unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); +- return -1; ++ goto user_core_fail; + } + + return user_core_fd; ++ ++user_core_fail: ++ if (user_core_fd >= 0) ++ { ++ close(user_core_fd); ++ unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); ++ } ++ return -1; + } + + /* Like xopen, but on error, unlocks and deletes dd and user core */ +-- +2.1.0 + diff --git a/0090-ccpp-avoid-overriding-system-files-by-coredump.patch b/0090-ccpp-avoid-overriding-system-files-by-coredump.patch new file mode 100644 index 0000000..2977aa8 --- /dev/null +++ b/0090-ccpp-avoid-overriding-system-files-by-coredump.patch @@ -0,0 +1,28 @@ +From de3b8b654d4962e1fa2d7b068644beeed7b0826d Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 21 Apr 2015 07:54:17 +0200 +Subject: [PATCH] ccpp: avoid overriding system files by coredump + +Related: #1211835 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 9696423..c4ad8d1 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -373,7 +373,7 @@ user_core_fail: + /* Like xopen, but on error, unlocks and deletes dd and user core */ + static int create_or_die(const char *filename, int user_core_fd) + { +- int fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, DEFAULT_DUMP_DIR_MODE); ++ int fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, DEFAULT_DUMP_DIR_MODE); + if (fd >= 0) + { + IGNORE_RESULT(fchown(fd, dd->dd_uid, dd->dd_gid)); +-- +2.1.0 + diff --git a/0091-configure-move-the-default-dump-location-to-var-spoo.patch b/0091-configure-move-the-default-dump-location-to-var-spoo.patch new file mode 100644 index 0000000..61b5ef0 --- /dev/null +++ b/0091-configure-move-the-default-dump-location-to-var-spoo.patch @@ -0,0 +1,47 @@ +From b0f5fad5d92a8df57c9cf1e92e06d9ed60e49537 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 20 Apr 2015 08:06:09 +0200 +Subject: [PATCH] configure: move the default dump location to /var/spool + +Signed-off-by: Jakub Filak +--- + configure.ac | 4 ++-- + src/daemon/abrt.conf | 4 ++-- + 2 files changed, 4 insertions(+), 4 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 430f23c..9443c50 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -181,8 +181,8 @@ PROBLEMS_CONFIG_INTERFACES_DIR=${dbusinterfacedir} + + AC_ARG_WITH([defaultdumplocation], + AS_HELP_STRING([--with-defaultdumplocation=DIR], +- [Default dump location ('LOCALSTATEDIR/tmp/abrt')]), +- [], [with_defaultdumplocation=${localstatedir}/tmp/abrt]) ++ [Default dump location ('LOCALSTATEDIR/spool/abrt')]), ++ [], [with_defaultdumplocation=${localstatedir}/spool/abrt]) + AC_SUBST([DEFAULT_DUMP_LOCATION], [$with_defaultdumplocation]) + + AC_ARG_WITH(augeaslenslibdir, +diff --git a/src/daemon/abrt.conf b/src/daemon/abrt.conf +index 02e969d..0050c6d 100644 +--- a/src/daemon/abrt.conf ++++ b/src/daemon/abrt.conf +@@ -10,11 +10,11 @@ + MaxCrashReportsSize = 1000 + + # Specify where you want to store coredumps and all files which are needed for +-# reporting. (default:/var/tmp/abrt) ++# reporting. (default:/var/spool/abrt) + # + # Changing dump location could cause problems with SELinux. See man abrt_selinux(8). + # +-#DumpLocation = /var/tmp/abrt ++#DumpLocation = /var/spool/abrt + + # If you want to automatically clean the upload directory you have to tweak the + # selinux policy. +-- +2.1.0 + diff --git a/0093-daemon-use-libreport-s-function-checking-file-name.patch b/0093-daemon-use-libreport-s-function-checking-file-name.patch new file mode 100644 index 0000000..3a4705c --- /dev/null +++ b/0093-daemon-use-libreport-s-function-checking-file-name.patch @@ -0,0 +1,54 @@ +From 8ff7f7f65cf871b889c3a9a53cd1a432c63d2180 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 23 Apr 2015 13:12:01 +0200 +Subject: [PATCH] daemon: use libreport's function checking file name + +Move the functions to libreport because we need the same functionality +there too. + +Related: #1214451 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 18 +----------------- + 1 file changed, 1 insertion(+), 17 deletions(-) + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index 9951468..287c510 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -445,22 +445,6 @@ static int create_problem_dir(GHashTable *problem_info, unsigned pid) + exit(0); + } + +-/* Checks if a string contains only printable characters. */ +-static gboolean printable_str(const char *str) +-{ +- do { +- if ((unsigned char)(*str) < ' ' || *str == 0x7f) +- return FALSE; +- str++; +- } while (*str); +- return TRUE; +-} +- +-static gboolean is_correct_filename(const char *value) +-{ +- return printable_str(value) && !strchr(value, '/') && !strchr(value, '.'); +-} +- + static gboolean key_value_ok(gchar *key, gchar *value) + { + char *i; +@@ -479,7 +463,7 @@ static gboolean key_value_ok(gchar *key, gchar *value) + || strcmp(key, FILENAME_TYPE) == 0 + ) + { +- if (!is_correct_filename(value)) ++ if (!str_is_correct_filename(value)) + { + error_msg("Value of '%s' ('%s') is not a valid directory name", + key, value); +-- +2.1.0 + diff --git a/0094-lib-add-functions-validating-dump-dir.patch b/0094-lib-add-functions-validating-dump-dir.patch new file mode 100644 index 0000000..f1bc45e --- /dev/null +++ b/0094-lib-add-functions-validating-dump-dir.patch @@ -0,0 +1,312 @@ +From 427b1c2b9b03c1fe52edb8a129e821a9ea2e0bea Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 23 Apr 2015 14:40:18 +0200 +Subject: [PATCH] lib: add functions validating dump dir + +Move the code from abrt-server to shared library and fix the condition +validating dump dir's path. + +As of now, abrt is allowed to process only direct sub-directories of the +dump locations with appropriate mode and owner and group. + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 36 ++++++++++++-------- + src/include/libabrt.h | 9 +++++ + src/lib/hooklib.c | 77 +++++++++++++++++++++++++++++++++++++++++++ + tests/Makefile.am | 3 +- + tests/hooklib.at | 85 ++++++++++++++++++++++++++++++++++++++++++++++++ + tests/testsuite.at | 1 + + 6 files changed, 196 insertions(+), 15 deletions(-) + create mode 100644 tests/hooklib.at + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index 287c510..8c48509 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -15,6 +15,7 @@ + with this program; if not, write to the Free Software Foundation, Inc., + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ ++#include "problem_api.h" + #include "libabrt.h" + + /* Maximal length of backtrace. */ +@@ -75,20 +76,6 @@ static unsigned total_bytes_read = 0; + static uid_t client_uid = (uid_t)-1L; + + +-static bool dir_is_in_dump_location(const char *dump_dir_name) +-{ +- unsigned len = strlen(g_settings_dump_location); +- +- if (strncmp(dump_dir_name, g_settings_dump_location, len) == 0 +- && dump_dir_name[len] == '/' +- /* must not contain "/." anywhere (IOW: disallow ".." component) */ +- && !strstr(dump_dir_name + len, "/.") +- ) { +- return 1; +- } +- return 0; +-} +- + /* Remove dump dir */ + static int delete_path(const char *dump_dir_name) + { +@@ -99,6 +86,11 @@ static int delete_path(const char *dump_dir_name) + error_msg("Bad problem directory name '%s', should start with: '%s'", dump_dir_name, g_settings_dump_location); + return 400; /* Bad Request */ + } ++ if (!dir_has_correct_permissions(dump_dir_name, DD_PERM_DAEMONS)) ++ { ++ error_msg("Problem directory '%s' has wrong owner or group", dump_dir_name); ++ return 400; /* */ ++ } + if (!dump_dir_accessible_by_uid(dump_dir_name, client_uid)) + { + if (errno == ENOTDIR) +@@ -153,6 +145,22 @@ static int run_post_create(const char *dirname) + error_msg("Bad problem directory name '%s', should start with: '%s'", dirname, g_settings_dump_location); + return 400; /* Bad Request */ + } ++ if (!dir_has_correct_permissions(dirname, DD_PERM_EVENTS)) ++ { ++ error_msg("Problem directory '%s' has wrong owner or group", dirname); ++ return 400; /* */ ++ } ++ /* Check completness */ ++ { ++ struct dump_dir *dd = dd_opendir(dirname, DD_OPEN_READONLY); ++ const bool complete = dd && problem_dump_dir_is_complete(dd); ++ dd_close(dd); ++ if (complete) ++ { ++ error_msg("Problem directory '%s' has already been processed", dirname); ++ return 403; ++ } ++ } + if (!dump_dir_accessible_by_uid(dirname, client_uid)) + { + if (errno == ENOTDIR) +diff --git a/src/include/libabrt.h b/src/include/libabrt.h +index abfbc97..9de222d 100644 +--- a/src/include/libabrt.h ++++ b/src/include/libabrt.h +@@ -47,6 +47,15 @@ char *run_unstrip_n(const char *dump_dir_name, unsigned timeout_sec); + #define get_backtrace abrt_get_backtrace + char *get_backtrace(const char *dump_dir_name, unsigned timeout_sec, const char *debuginfo_dirs); + ++#define dir_is_in_dump_location abrt_dir_is_in_dump_location ++bool dir_is_in_dump_location(const char *dir_name); ++ ++enum { ++ DD_PERM_EVENTS = 1 << 0, ++ DD_PERM_DAEMONS = 1 << 1, ++}; ++#define dir_has_correct_permissions abrt_dir_has_correct_permissions ++bool dir_has_correct_permissions(const char *dir_name, int flags); + + #define g_settings_nMaxCrashReportsSize abrt_g_settings_nMaxCrashReportsSize + extern unsigned int g_settings_nMaxCrashReportsSize; +diff --git a/src/lib/hooklib.c b/src/lib/hooklib.c +index 2be4e80..c94cadf 100644 +--- a/src/lib/hooklib.c ++++ b/src/lib/hooklib.c +@@ -475,3 +475,80 @@ int signal_is_fatal(int signal_no, const char **name) + + return signame != NULL; + } ++ ++bool dir_is_in_dump_location(const char *dir_name) ++{ ++ unsigned len = strlen(g_settings_dump_location); ++ ++ /* The path must start with "g_settings_dump_location" */ ++ if (strncmp(dir_name, g_settings_dump_location, len) != 0) ++ { ++ log_debug("Bad parent directory: '%s' not in '%s'", g_settings_dump_location, dir_name); ++ return false; ++ } ++ ++ /* and must be a sub-directory of the g_settings_dump_location dir */ ++ const char *base_name = dir_name + len; ++ while (*base_name && *base_name == '/') ++ ++base_name; ++ ++ if (*(base_name - 1) != '/' || !str_is_correct_filename(base_name)) ++ { ++ log_debug("Invalid dump directory name: '%s'", base_name); ++ return false; ++ } ++ ++ /* and we are sure it is a directory */ ++ struct stat sb; ++ if (lstat(dir_name, &sb) < 0) ++ { ++ VERB2 perror_msg("stat('%s')", dir_name); ++ return errno== ENOENT; ++ } ++ ++ return S_ISDIR(sb.st_mode); ++} ++ ++bool dir_has_correct_permissions(const char *dir_name, int flags) ++{ ++ struct stat statbuf; ++ if (lstat(dir_name, &statbuf) != 0 || !S_ISDIR(statbuf.st_mode)) ++ { ++ error_msg("Path '%s' isn't directory", dir_name); ++ return false; ++ } ++ ++ /* Get ABRT's group id */ ++ struct group *gr = getgrnam("abrt"); ++ if (!gr) ++ { ++ error_msg("The group 'abrt' does not exist"); ++ return false; ++ } ++ ++ /* The group must be root or abrt. */ ++ const bool correct_group = statbuf.st_gid == 0 ++ || statbuf.st_gid == gr->gr_gid; ++ ++ /* Require owner 'root' and group 'abrt' for the event shell scripts. ++ * Because the shell scripts are vulnerable to hard link and symbolic link ++ * attacks. ++ */ ++ const bool events = statbuf.st_uid == 0 ++ && correct_group ++ && (statbuf.st_mode & S_IWGRP) == 0 ++ && (statbuf.st_mode & S_IWOTH) == 0; ++ ++ if ((flags & DD_PERM_EVENTS)) ++ return events; ++ ++ /* Be less restrictive and allow the daemos (abrtd, abrt-dbus) to work with ++ * dump directories having group 'root' or 'abrt'. ++ * ++ * 1. Chowning of dump directories switches the ownership to 'user':'abrt' ++ * 2. We want to allow users to delete their problems ++ * 3. We want to allow users to modify their data ++ * 4. The daemons are hardened agains hard link and symbolic link issues. ++ */ ++ return correct_group; ++} +diff --git a/tests/Makefile.am b/tests/Makefile.am +index 5ef08a0..416f579 100644 +--- a/tests/Makefile.am ++++ b/tests/Makefile.am +@@ -29,7 +29,8 @@ TESTSUITE_AT = \ + testsuite.at \ + pyhook.at \ + koops-parser.at \ +- ignored_problems.at ++ ignored_problems.at \ ++ hooklib.at + + EXTRA_DIST += $(TESTSUITE_AT) + TESTSUITE = $(srcdir)/testsuite +diff --git a/tests/hooklib.at b/tests/hooklib.at +new file mode 100644 +index 0000000..70631c6 +--- /dev/null ++++ b/tests/hooklib.at +@@ -0,0 +1,85 @@ ++# -*- Autotest -*- ++ ++AT_BANNER([hooklib]) ++ ++AT_TESTFUN([dir_is_in_dump_location], ++[[ ++#include "libabrt.h" ++#include ++ ++void test(char *name, bool expected) ++{ ++ if (dir_is_in_dump_location(name) != expected) ++ { ++ fprintf(stderr, "Bad: %s", name); ++ abort(); ++ } ++ ++ free(name); ++} ++ ++int main(void) ++{ ++ g_verbose = 3; ++ load_abrt_conf(); ++ ++ g_verbose = 3; ++ ++ char *name; ++ ++ assert(dir_is_in_dump_location("/") == false); ++ ++ asprintf(&name, "%s", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s..evil", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s///", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/.", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s///.", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/./", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/.///", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/..", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s///..", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/../", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/..///", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/good/../../../evil", g_settings_dump_location); ++ test(name, false); ++ ++ asprintf(&name, "%s/good..still", g_settings_dump_location); ++ test(name, true); ++ ++ asprintf(&name, "%s/good.new", g_settings_dump_location); ++ test(name, true); ++ ++ asprintf(&name, "%s/.meta", g_settings_dump_location); ++ test(name, true); ++ ++ asprintf(&name, "%s/..data", g_settings_dump_location); ++ test(name, true); ++ ++ return 0; ++} ++]]) +diff --git a/tests/testsuite.at b/tests/testsuite.at +index b8f363d..765de2a 100644 +--- a/tests/testsuite.at ++++ b/tests/testsuite.at +@@ -4,3 +4,4 @@ + m4_include([koops-parser.at]) + m4_include([pyhook.at]) + m4_include([ignored_problems.at]) ++m4_include([hooklib.at]) +-- +2.1.0 + diff --git a/0095-dbus-process-only-valid-sub-directories-of-the-dump-.patch b/0095-dbus-process-only-valid-sub-directories-of-the-dump-.patch new file mode 100644 index 0000000..a2c4264 --- /dev/null +++ b/0095-dbus-process-only-valid-sub-directories-of-the-dump-.patch @@ -0,0 +1,127 @@ +From c5cd9d25473e2fea29f7fa609c81ae821e50f118 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 23 Apr 2015 14:46:27 +0200 +Subject: [PATCH] dbus: process only valid sub-directories of the dump location + +Must have correct rights and must be a direct sub-directory of the dump +location. + +This issue was discovered by Florian Weimer of Red Hat Product Security. + +Related: #1214451 + +Signed-off-by: Jakub Filak +--- + src/dbus/abrt-dbus.c | 46 ++++++++++++++++++++++++++++++++++------------ + src/lib/problem_api.c | 7 +++++++ + 2 files changed, 41 insertions(+), 12 deletions(-) + +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index 4eeff41..585fcd7 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -141,21 +141,20 @@ static uid_t get_caller_uid(GDBusConnection *connection, GDBusMethodInvocation * + return caller_uid; + } + +-static bool allowed_problem_dir(const char *dir_name) ++bool allowed_problem_dir(const char *dir_name) + { +-//HACK HACK HACK! Disabled for now until we fix clients (abrt-gui) to not pass /home/user/.cache/abrt/spool +-#if 0 +- unsigned len = strlen(g_settings_dump_location); +- +- /* If doesn't start with "g_settings_dump_location[/]"... */ +- if (strncmp(dir_name, g_settings_dump_location, len) != 0 +- || (dir_name[len] != '/' && dir_name[len] != '\0') +- /* or contains "/." anywhere (-> might contain ".." component) */ +- || strstr(dir_name + len, "/.") +- ) { ++ if (!dir_is_in_dump_location(dir_name)) ++ { ++ error_msg("Bad problem directory name '%s', should start with: '%s'", dir_name, g_settings_dump_location); + return false; + } +-#endif ++ ++ if (!dir_has_correct_permissions(dir_name, DD_PERM_DAEMONS)) ++ { ++ error_msg("Problem directory '%s' has invalid owner, groop or mode", dir_name); ++ return false; ++ } ++ + return true; + } + +@@ -561,6 +560,12 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s)", &problem_id); + ++ if (!allowed_problem_dir(problem_id)) ++ { ++ return_InvalidProblemDir_error(invocation, problem_id); ++ return; ++ } ++ + int ddstat = dump_dir_stat_for_uid(problem_id, caller_uid); + if ((ddstat & DD_STAT_ACCESSIBLE_BY_UID) == 0 && + polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes) +@@ -624,6 +629,12 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s&s)", &problem_id, &element, &value); + ++ if (!allowed_problem_dir(problem_id)) ++ { ++ return_InvalidProblemDir_error(invocation, problem_id); ++ return; ++ } ++ + if (element == NULL || element[0] == '\0' || strlen(element) > 64) + { + log_notice("'%s' is not a valid element name of '%s'", element, problem_id); +@@ -683,6 +694,12 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s)", &problem_id, &element); + ++ if (!allowed_problem_dir(problem_id)) ++ { ++ return_InvalidProblemDir_error(invocation, problem_id); ++ return; ++ } ++ + struct dump_dir *dd = open_directory_for_modification_of_element( + invocation, caller_uid, problem_id, element); + if (!dd) +@@ -715,6 +732,11 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s)", &problem_id, &element); + ++ if (!allowed_problem_dir(problem_id)) ++ { ++ return_InvalidProblemDir_error(invocation, problem_id); ++ return; ++ } + + struct dump_dir *dd = dd_opendir(problem_id, DD_OPEN_READONLY); + if (!dd) +diff --git a/src/lib/problem_api.c b/src/lib/problem_api.c +index 07707db..86222cf 100644 +--- a/src/lib/problem_api.c ++++ b/src/lib/problem_api.c +@@ -76,6 +76,13 @@ int for_each_problem_in_dir(const char *path, + + static int add_dirname_to_GList(struct dump_dir *dd, void *arg) + { ++ if (!dir_has_correct_permissions(dd->dd_dirname, DD_PERM_DAEMONS)) ++ { ++ log("Ignoring '%s': invalid owner, group or mode", dd->dd_dirname); ++ /*Do not break*/ ++ return 0; ++ } ++ + GList **list = arg; + *list = g_list_prepend(*list, xstrdup(dd->dd_dirname)); + return 0; +-- +2.1.0 + diff --git a/0096-dbus-avoid-race-conditions-in-tests-for-dum-dir-avai.patch b/0096-dbus-avoid-race-conditions-in-tests-for-dum-dir-avai.patch new file mode 100644 index 0000000..f236c54 --- /dev/null +++ b/0096-dbus-avoid-race-conditions-in-tests-for-dum-dir-avai.patch @@ -0,0 +1,390 @@ +From 0bfb03ea68eb745172feccfc0f01b2ad13ff33bb Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Fri, 24 Apr 2015 13:48:32 +0200 +Subject: [PATCH] dbus: avoid race-conditions in tests for dum dir availability + +Florian Weimer + + dump_dir_accessible_by_uid() is fundamentally insecure because it + opens up a classic time-of-check-time-of-use race between this + function and and dd_opendir(). + +Related: #1214745 + +Signed-off-by: Jakub Filak +--- + src/dbus/abrt-dbus.c | 225 +++++++++++++++++++++----------------------------- + src/lib/problem_api.c | 21 +++-- + 2 files changed, 109 insertions(+), 137 deletions(-) + +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index 585fcd7..0f7ac2d 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -201,6 +201,69 @@ static void return_InvalidProblemDir_error(GDBusMethodInvocation *invocation, co + free(msg); + } + ++enum { ++ OPEN_FAIL_NO_REPLY = 1 << 0, ++ OPEN_AUTH_ASK = 1 << 1, ++ OPEN_AUTH_FAIL = 1 << 2, ++}; ++ ++static struct dump_dir *open_dump_directory(GDBusMethodInvocation *invocation, ++ const gchar *caller, uid_t caller_uid, const char *problem_dir, int dd_flags, int flags) ++{ ++ if (!allowed_problem_dir(problem_dir)) ++ { ++ log("UID=%d attempted to access not allowed problem directory '%s'", ++ caller_uid, problem_dir); ++ if (!(flags & OPEN_FAIL_NO_REPLY)) ++ return_InvalidProblemDir_error(invocation, problem_dir); ++ return NULL; ++ } ++ ++ struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_FD_ONLY); ++ if (dd == NULL) ++ { ++ perror_msg("can't open problem directory '%s'", problem_dir); ++ if (!(flags & OPEN_FAIL_NO_REPLY)) ++ return_InvalidProblemDir_error(invocation, problem_dir); ++ return NULL; ++ } ++ ++ if (!dd_accessible_by_uid(dd, caller_uid)) ++ { ++ if (errno == ENOTDIR) ++ { ++ log_notice("Requested directory does not exist '%s'", problem_dir); ++ if (!(flags & OPEN_FAIL_NO_REPLY)) ++ return_InvalidProblemDir_error(invocation, problem_dir); ++ dd_close(dd); ++ return NULL; ++ } ++ ++ if ( !(flags & OPEN_AUTH_ASK) ++ || polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes) ++ { ++ log_notice("not authorized"); ++ if (!(flags & OPEN_FAIL_NO_REPLY)) ++ g_dbus_method_invocation_return_dbus_error(invocation, ++ "org.freedesktop.problems.AuthFailure", ++ _("Not Authorized")); ++ dd_close(dd); ++ return NULL; ++ } ++ } ++ ++ dd = dd_fdopendir(dd, dd_flags); ++ if (dd == NULL) ++ { ++ log_notice("Can't open the problem '%s' with flags %x0", problem_dir, dd_flags); ++ if (!(flags & OPEN_FAIL_NO_REPLY)) ++ g_dbus_method_invocation_return_dbus_error(invocation, ++ "org.freedesktop.problems.Failure", ++ _("Can't open the problem")); ++ } ++ return dd; ++} ++ + /* + * Checks element's rights and does not open directory if element is protected. + * Checks problem's rights and does not open directory if user hasn't got +@@ -237,35 +300,8 @@ static struct dump_dir *open_directory_for_modification_of_element( + } + } + +- if (!dump_dir_accessible_by_uid(problem_id, caller_uid)) +- { +- if (errno == ENOTDIR) +- { +- log_notice("'%s' is not a valid problem directory", problem_id); +- return_InvalidProblemDir_error(invocation, problem_id); +- } +- else +- { +- log_notice("UID(%d) is not Authorized to access '%s'", caller_uid, problem_id); +- g_dbus_method_invocation_return_dbus_error(invocation, +- "org.freedesktop.problems.AuthFailure", +- _("Not Authorized")); +- } +- +- return NULL; +- } +- +- struct dump_dir *dd = dd_opendir(problem_id, /* flags : */ 0); +- if (!dd) +- { /* This should not happen because of the access check above */ +- log_notice("Can't access the problem '%s' for modification", problem_id); +- g_dbus_method_invocation_return_dbus_error(invocation, +- "org.freedesktop.problems.Failure", +- _("Can't access the problem for modification")); +- return NULL; +- } +- +- return dd; ++ return open_dump_directory(invocation, /*caller*/NULL, caller_uid, problem_id, /*Read/Write*/0, ++ OPEN_AUTH_FAIL); + } + + +@@ -421,7 +457,15 @@ static void handle_method_call(GDBusConnection *connection, + return; + } + +- int ddstat = dump_dir_stat_for_uid(problem_dir, caller_uid); ++ struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_FD_ONLY); ++ if (dd == NULL) ++ { ++ perror_msg("can't open problem directory '%s'", problem_dir); ++ return_InvalidProblemDir_error(invocation, problem_dir); ++ return; ++ } ++ ++ int ddstat = dd_stat_for_uid(dd, caller_uid); + if (ddstat < 0) + { + if (errno == ENOTDIR) +@@ -435,6 +479,7 @@ static void handle_method_call(GDBusConnection *connection, + + return_InvalidProblemDir_error(invocation, problem_dir); + ++ dd_close(dd); + return; + } + +@@ -442,6 +487,7 @@ static void handle_method_call(GDBusConnection *connection, + { //caller seems to be in group with access to this dir, so no action needed + log_notice("caller has access to the requested directory %s", problem_dir); + g_dbus_method_invocation_return_value(invocation, NULL); ++ dd_close(dd); + return; + } + +@@ -452,10 +498,11 @@ static void handle_method_call(GDBusConnection *connection, + g_dbus_method_invocation_return_dbus_error(invocation, + "org.freedesktop.problems.AuthFailure", + _("Not Authorized")); ++ dd_close(dd); + return; + } + +- struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES); ++ dd = dd_fdopendir(dd, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES); + if (!dd) + { + return_InvalidProblemDir_error(invocation, problem_dir); +@@ -483,37 +530,10 @@ static void handle_method_call(GDBusConnection *connection, + g_variant_get_child(parameters, 0, "&s", &problem_dir); + log_notice("problem_dir:'%s'", problem_dir); + +- if (!allowed_problem_dir(problem_dir)) +- { +- return_InvalidProblemDir_error(invocation, problem_dir); +- return; +- } +- +- if (!dump_dir_accessible_by_uid(problem_dir, caller_uid)) +- { +- if (errno == ENOTDIR) +- { +- log_notice("Requested directory does not exist '%s'", problem_dir); +- return_InvalidProblemDir_error(invocation, problem_dir); +- return; +- } +- +- if (polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes) +- { +- log_notice("not authorized"); +- g_dbus_method_invocation_return_dbus_error(invocation, +- "org.freedesktop.problems.AuthFailure", +- _("Not Authorized")); +- return; +- } +- } +- +- struct dump_dir *dd = dd_opendir(problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES); ++ struct dump_dir *dd = open_dump_directory(invocation, caller, caller_uid, ++ problem_dir, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES , OPEN_AUTH_ASK); + if (!dd) +- { +- return_InvalidProblemDir_error(invocation, problem_dir); + return; +- } + + /* Get 2nd param - vector of element names */ + GVariant *array = g_variant_get_child_value(parameters, 1); +@@ -560,32 +580,10 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s)", &problem_id); + +- if (!allowed_problem_dir(problem_id)) +- { +- return_InvalidProblemDir_error(invocation, problem_id); +- return; +- } +- +- int ddstat = dump_dir_stat_for_uid(problem_id, caller_uid); +- if ((ddstat & DD_STAT_ACCESSIBLE_BY_UID) == 0 && +- polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes) +- { +- log_notice("Unauthorized access : '%s'", problem_id); +- g_dbus_method_invocation_return_dbus_error(invocation, +- "org.freedesktop.problems.AuthFailure", +- _("Not Authorized")); +- return; +- } +- +- struct dump_dir *dd = dd_opendir(problem_id, DD_OPEN_READONLY); +- if (dd == NULL) +- { +- log_notice("Can't access the problem '%s' for reading", problem_id); +- g_dbus_method_invocation_return_dbus_error(invocation, +- "org.freedesktop.problems.Failure", +- _("Can't access the problem for reading")); ++ struct dump_dir *dd = open_dump_directory(invocation, caller, caller_uid, ++ problem_id, DD_OPEN_READONLY, OPEN_AUTH_ASK); ++ if (!dd) + return; +- } + + problem_data_t *pd = create_problem_data_from_dump_dir(dd); + dd_close(dd); +@@ -629,12 +627,6 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s&s)", &problem_id, &element, &value); + +- if (!allowed_problem_dir(problem_id)) +- { +- return_InvalidProblemDir_error(invocation, problem_id); +- return; +- } +- + if (element == NULL || element[0] == '\0' || strlen(element) > 64) + { + log_notice("'%s' is not a valid element name of '%s'", element, problem_id); +@@ -694,12 +686,6 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s)", &problem_id, &element); + +- if (!allowed_problem_dir(problem_id)) +- { +- return_InvalidProblemDir_error(invocation, problem_id); +- return; +- } +- + struct dump_dir *dd = open_directory_for_modification_of_element( + invocation, caller_uid, problem_id, element); + if (!dd) +@@ -732,33 +718,10 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s)", &problem_id, &element); + +- if (!allowed_problem_dir(problem_id)) +- { +- return_InvalidProblemDir_error(invocation, problem_id); +- return; +- } +- +- struct dump_dir *dd = dd_opendir(problem_id, DD_OPEN_READONLY); ++ struct dump_dir *dd = open_dump_directory(invocation, caller, caller_uid, ++ problem_id, DD_OPEN_READONLY, OPEN_AUTH_ASK); + if (!dd) +- { +- log_notice("Can't access the problem '%s'", problem_id); +- g_dbus_method_invocation_return_dbus_error(invocation, +- "org.freedesktop.problems.Failure", +- _("Can't access the problem")); +- return; +- } +- +- int ddstat = dump_dir_stat_for_uid(problem_id, caller_uid); +- if ((ddstat & DD_STAT_ACCESSIBLE_BY_UID) == 0 && +- polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes) +- { +- dd_close(dd); +- log_notice("Unauthorized access : '%s'", problem_id); +- g_dbus_method_invocation_return_dbus_error(invocation, +- "org.freedesktop.problems.AuthFailure", +- _("Not Authorized")); + return; +- } + + int ret = dd_exist(dd, element); + dd_close(dd); +@@ -793,20 +756,18 @@ static void handle_method_call(GDBusConnection *connection, + for (GList *l = problem_dirs; l; l = l->next) + { + const char *dir_name = (const char*)l->data; +- if (!dump_dir_accessible_by_uid(dir_name, caller_uid)) ++ ++ struct dump_dir *dd = open_dump_directory(invocation, caller, caller_uid, ++ dir_name, /*Read/Write*/0, OPEN_FAIL_NO_REPLY | OPEN_AUTH_ASK); ++ ++ if (dd) + { +- if (errno == ENOTDIR) ++ if (dd_delete(dd) != 0) + { +- log_notice("Requested directory does not exist '%s'", dir_name); +- continue; +- } +- +- if (polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes) +- { // if user didn't provide correct credentials, just move to the next dir +- continue; ++ error_msg("Failed to delete problem directory '%s'", dir_name); ++ dd_close(dd); + } + } +- delete_dump_dir(dir_name); + } + + g_dbus_method_invocation_return_value(invocation, NULL); +diff --git a/src/lib/problem_api.c b/src/lib/problem_api.c +index 86222cf..96a49fc 100644 +--- a/src/lib/problem_api.c ++++ b/src/lib/problem_api.c +@@ -46,7 +46,17 @@ int for_each_problem_in_dir(const char *path, + continue; /* skip "." and ".." */ + + char *full_name = concat_path_file(path, dent->d_name); +- if (caller_uid == -1 || dump_dir_accessible_by_uid(full_name, caller_uid)) ++ ++ struct dump_dir *dd = dd_opendir(full_name, DD_OPEN_FD_ONLY ++ | DD_FAIL_QUIETLY_ENOENT ++ | DD_FAIL_QUIETLY_EACCES); ++ if (dd == NULL) ++ { ++ VERB2 perror_msg("can't open problem directory '%s'", full_name); ++ continue; ++ } ++ ++ if (caller_uid == -1 || dd_accessible_by_uid(dd, caller_uid)) + { + /* Silently ignore *any* errors, not only EACCES. + * We saw "lock file is locked by process PID" error +@@ -55,14 +65,15 @@ int for_each_problem_in_dir(const char *path, + int sv_logmode = logmode; + /* Silently ignore errors only in the silent log level. */ + logmode = g_verbose == 0 ? 0: sv_logmode; +- struct dump_dir *dd = dd_opendir(full_name, DD_OPEN_READONLY | DD_FAIL_QUIETLY_EACCES | DD_DONT_WAIT_FOR_LOCK); ++ dd = dd_fdopendir(dd, DD_OPEN_READONLY | DD_DONT_WAIT_FOR_LOCK); + logmode = sv_logmode; + if (dd) +- { + brk = callback ? callback(dd, arg) : 0; +- dd_close(dd); +- } + } ++ ++ if (dd) ++ dd_close(dd); ++ + free(full_name); + if (brk) + break; +-- +2.1.0 + diff --git a/0097-dbus-report-invalid-element-names.patch b/0097-dbus-report-invalid-element-names.patch new file mode 100644 index 0000000..12c1b3e --- /dev/null +++ b/0097-dbus-report-invalid-element-names.patch @@ -0,0 +1,92 @@ +From 23c800077fb6e821d54080ccc5d1258f37fcd8d4 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 27 Apr 2015 07:52:00 +0200 +Subject: [PATCH] dbus: report invalid element names + +Return D-Bus error in case of invalid problem element name. + +Related: #1214451 + +Signed-off-by: Jakub Filak +--- + src/dbus/abrt-dbus.c | 35 +++++++++++++++++++++++++---------- + 1 file changed, 25 insertions(+), 10 deletions(-) + +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index 0f7ac2d..489d273 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -158,6 +158,21 @@ bool allowed_problem_dir(const char *dir_name) + return true; + } + ++bool allowed_problem_element(GDBusMethodInvocation *invocation, const char *element) ++{ ++ if (str_is_correct_filename(element)) ++ return true; ++ ++ log_notice("'%s' is not a valid element name", element); ++ char *error = xasprintf(_("'%s' is not a valid element name"), element); ++ g_dbus_method_invocation_return_dbus_error(invocation, ++ "org.freedesktop.problems.InvalidElement", ++ error); ++ ++ free(error); ++ return false; ++} ++ + static char *handle_new_problem(GVariant *problem_info, uid_t caller_uid, char **error) + { + problem_data_t *pd = problem_data_new(); +@@ -627,17 +642,8 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s&s)", &problem_id, &element, &value); + +- if (element == NULL || element[0] == '\0' || strlen(element) > 64) +- { +- log_notice("'%s' is not a valid element name of '%s'", element, problem_id); +- char *error = xasprintf(_("'%s' is not a valid element name"), element); +- g_dbus_method_invocation_return_dbus_error(invocation, +- "org.freedesktop.problems.InvalidElement", +- error); +- +- free(error); ++ if (!allowed_problem_element(invocation, element)) + return; +- } + + struct dump_dir *dd = open_directory_for_modification_of_element( + invocation, caller_uid, problem_id, element); +@@ -686,6 +692,9 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s)", &problem_id, &element); + ++ if (!allowed_problem_element(invocation, element)) ++ return; ++ + struct dump_dir *dd = open_directory_for_modification_of_element( + invocation, caller_uid, problem_id, element); + if (!dd) +@@ -718,6 +727,9 @@ static void handle_method_call(GDBusConnection *connection, + + g_variant_get(parameters, "(&s&s)", &problem_id, &element); + ++ if (!allowed_problem_element(invocation, element)) ++ return; ++ + struct dump_dir *dd = open_dump_directory(invocation, caller, caller_uid, + problem_id, DD_OPEN_READONLY, OPEN_AUTH_ASK); + if (!dd) +@@ -790,6 +802,9 @@ static void handle_method_call(GDBusConnection *connection, + g_variant_get_child(parameters, 3, "x", ×tamp_to); + g_variant_get_child(parameters, 4, "b", &all); + ++ if (!allowed_problem_element(invocation, element)) ++ return; ++ + if (all && polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") == PolkitYes) + caller_uid = 0; + +-- +2.1.0 + diff --git a/0098-a-a-i-d-t-a-cache-sanitize-arguments.patch b/0098-a-a-i-d-t-a-cache-sanitize-arguments.patch new file mode 100644 index 0000000..9ad1100 --- /dev/null +++ b/0098-a-a-i-d-t-a-cache-sanitize-arguments.patch @@ -0,0 +1,212 @@ +From c8ab547f17910391d0cd66a9c6f1917c295480db Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 29 Apr 2015 13:57:39 +0200 +Subject: [PATCH] a-a-i-d-t-a-cache: sanitize arguments + +Parse command lines arguments and use them to create new arguments for +exec(). No black listing algorithm would be safe enough. The only +allowed arguments are the following: +* v - verbose +* y - noninteractive +* repo - enable only repositories whose names match the pattern +* exact - download packages for the specified files +* ids - passed as magic proc fd path to the wrapped executable + +The wrapper opens the list of needed build ids passes /proc/self/fd/[fd] +to the wrapped process. This allows us to open the file with caller's +UID/GID in order to avoid information disclosures. + +Forbidden arguments: +* cache - allows regular users to create a user writable dump directory +* tmpdir - the same as above +* size_mb - no need to allow users to fiddle with the cache size + +Related: #1216962 + +Signed-off-by: Jakub Filak +--- + po/POTFILES.in | 1 + + .../abrt-action-install-debuginfo-to-abrt-cache.c | 137 ++++++++++++++++----- + 2 files changed, 109 insertions(+), 29 deletions(-) + +diff --git a/po/POTFILES.in b/po/POTFILES.in +index 15b879f..bd76b87 100644 +--- a/po/POTFILES.in ++++ b/po/POTFILES.in +@@ -31,6 +31,7 @@ src/plugins/abrt-action-check-oops-for-hw-error.in + src/plugins/abrt-action-generate-backtrace.c + src/plugins/abrt-action-generate-core-backtrace.c + src/plugins/abrt-action-install-debuginfo.in ++src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c + src/plugins/abrt-action-perform-ccpp-analysis.in + src/plugins/abrt-action-trim-files.c + src/plugins/abrt-action-ureport +diff --git a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c +index e0eccc0..4fa1783 100644 +--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c ++++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c +@@ -29,41 +29,120 @@ + */ + int main(int argc, char **argv) + { +- /* +- * We disallow passing of arguments which point to writable dirs +- * and other files possibly not accessible to calling user. +- * This way, the script will always use default values for these arguments. +- */ +- char **pp = argv; +- char *arg; +- while ((arg = *++pp) != NULL) ++ /* I18n */ ++ setlocale(LC_ALL, ""); ++#if ENABLE_NLS ++ bindtextdomain(PACKAGE, LOCALEDIR); ++ textdomain(PACKAGE); ++#endif ++ ++ abrt_init(argv); ++ ++ /* Can't keep these strings/structs static: _() doesn't support that */ ++ const char *program_usage_string = _( ++ "& [-y] [-i BUILD_IDS_FILE|-i -] [-e PATH[:PATH]...]\n" ++ "\t[-r REPO]\n" ++ "\n" ++ "Installs debuginfo packages for all build-ids listed in BUILD_IDS_FILE to\n" ++ "ABRT system cache." ++ ); ++ ++ enum { ++ OPT_v = 1 << 0, ++ OPT_y = 1 << 1, ++ OPT_i = 1 << 2, ++ OPT_e = 1 << 3, ++ OPT_r = 1 << 4, ++ OPT_s = 1 << 5, ++ }; ++ ++ const char *build_ids = "build_ids"; ++ const char *exact = NULL; ++ const char *repo = NULL; ++ const char *size_mb = NULL; ++ ++ struct options program_options[] = { ++ OPT__VERBOSE(&g_verbose), ++ OPT_BOOL ('y', "yes", NULL, _("Noninteractive, assume 'Yes' to all questions")), ++ OPT_STRING('i', "ids", &build_ids, "BUILD_IDS_FILE", _("- means STDIN, default: build_ids")), ++ OPT_STRING('e', "exact", &exact, "EXACT", _("Download only specified files")), ++ OPT_STRING('r', "repo", &repo, "REPO", _("Pattern to use when searching for repos, default: *debug*")), ++ OPT_STRING('s', "size_mb", &size_mb, "SIZE_MB", _("Ignored option")), ++ OPT_END() ++ }; ++ const unsigned opts = parse_opts(argc, argv, program_options, program_usage_string); ++ ++ const gid_t egid = getegid(); ++ const gid_t rgid = getgid(); ++ const uid_t euid = geteuid(); ++ const gid_t ruid = getuid(); ++ ++ /* We need to open the build ids file under the caller's UID/GID to avoid ++ * information disclosures when reading files with changed UID. ++ * Unfortunately, we cannot replace STDIN with the new fd because ABRT uses ++ * STDIN to communicate with the caller. So, the following code opens a ++ * dummy file descriptor to the build ids file and passes the new fd's proc ++ * path to the wrapped program in the ids argument. ++ * The new fd remains opened, the OS will close it for us. */ ++ char *build_ids_self_fd = NULL; ++ if (strcmp("-", build_ids) != 0) ++ { ++ if (setregid(egid, rgid) < 0) ++ perror_msg_and_die("setregid(egid, rgid)"); ++ ++ if (setreuid(euid, ruid) < 0) ++ perror_msg_and_die("setreuid(euid, ruid)"); ++ ++ const int build_ids_fd = open(build_ids, O_RDONLY); ++ ++ if (setregid(rgid, egid) < 0) ++ perror_msg_and_die("setregid(rgid, egid)"); ++ ++ if (setreuid(ruid, euid) < 0 ) ++ perror_msg_and_die("setreuid(ruid, euid)"); ++ ++ if (build_ids_fd < 0) ++ perror_msg_and_die("Failed to open file '%s'", build_ids); ++ ++ /* We are not going to free this memory. There is no place to do so. */ ++ build_ids_self_fd = xasprintf("/proc/self/fd/%d", build_ids_fd); ++ } ++ ++ /* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, --, NULL */ ++ const char *args[11]; + { +- /* Allow taking ids from stdin */ +- if (strcmp(arg, "--ids=-") == 0) +- continue; +- +- if (strncmp(arg, "--exact", 7) == 0) +- continue; +- +- if (strncmp(arg, "--cache", 7) == 0) +- error_msg_and_die("bad option %s", arg); +- if (strncmp(arg, "--tmpdir", 8) == 0) +- error_msg_and_die("bad option %s", arg); +- if (strncmp(arg, "--ids", 5) == 0) +- error_msg_and_die("bad option %s", arg); ++ const char *verbs[] = { "", "-v", "-vv", "-vvv" }; ++ unsigned i = 0; ++ args[i++] = EXECUTABLE; ++ args[i++] = "--ids"; ++ args[i++] = (build_ids_self_fd != NULL) ? build_ids_self_fd : "-"; ++ if (g_verbose > 0) ++ args[i++] = verbs[g_verbose <= 3 ? g_verbose : 3]; ++ if ((opts & OPT_y)) ++ args[i++] = "-y"; ++ if ((opts & OPT_e)) ++ { ++ args[i++] = "--exact"; ++ args[i++] = exact; ++ } ++ if ((opts & OPT_r)) ++ { ++ args[i++] = "--repo"; ++ args[i++] = repo; ++ } ++ args[i++] = "--"; ++ args[i] = NULL; + } + + /* Switch real user/group to effective ones. + * Otherwise yum library gets confused - gets EPERM (why??). + */ +- gid_t g = getegid(); + /* do setregid only if we have to, to not upset selinux needlessly */ +- if (g != getgid()) +- IGNORE_RESULT(setregid(g, g)); +- uid_t u = geteuid(); +- if (u != getuid()) ++ if (egid != rgid) ++ IGNORE_RESULT(setregid(egid, egid)); ++ if (euid != ruid) + { +- IGNORE_RESULT(setreuid(u, u)); ++ IGNORE_RESULT(setreuid(euid, euid)); + /* We are suid'ed! */ + /* Prevent malicious user from messing up with suid'ed process: */ + #if 1 +@@ -117,11 +196,11 @@ int main(int argc, char **argv) + // abrt-action-install-debuginfo doesn't fail when spawning + // abrt-action-trim-files + char path_env[] = "PATH=/usr/sbin:/sbin:/usr/bin:/bin:"BIN_DIR":"SBIN_DIR; +- if (u != 0) ++ if (euid != 0) + strcpy(path_env, "PATH=/usr/bin:/bin:"BIN_DIR); + putenv(path_env); + } + +- execvp(EXECUTABLE, argv); ++ execvp(EXECUTABLE, (char **)args); + error_msg_and_die("Can't execute %s", EXECUTABLE); + } +-- +2.1.0 + diff --git a/0099-a-a-i-d-t-a-cache-sanitize-umask.patch b/0099-a-a-i-d-t-a-cache-sanitize-umask.patch new file mode 100644 index 0000000..d032578 --- /dev/null +++ b/0099-a-a-i-d-t-a-cache-sanitize-umask.patch @@ -0,0 +1,31 @@ +From 35fe31aceb8221fd8bd8ea8d48d1bb4f0fbdf837 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 29 Apr 2015 14:13:57 +0200 +Subject: [PATCH] a-a-i-d-t-a-cache: sanitize umask + +We cannot trust anything when running suided program. + +Related: #1216962 + +Signed-off-by: Jakub Filak +--- + src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c +index 4fa1783..81b1486 100644 +--- a/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c ++++ b/src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c +@@ -199,6 +199,9 @@ int main(int argc, char **argv) + if (euid != 0) + strcpy(path_env, "PATH=/usr/bin:/bin:"BIN_DIR); + putenv(path_env); ++ ++ /* Use safe umask */ ++ umask(0022); + } + + execvp(EXECUTABLE, (char **)args); +-- +2.1.0 + diff --git a/0100-ccpp-revert-the-UID-GID-changes-if-user-core-fails.patch b/0100-ccpp-revert-the-UID-GID-changes-if-user-core-fails.patch new file mode 100644 index 0000000..f80abd2 --- /dev/null +++ b/0100-ccpp-revert-the-UID-GID-changes-if-user-core-fails.patch @@ -0,0 +1,102 @@ +From f51412944254a5abcbd1458e0b4bfa9d876e2fa9 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 4 May 2015 13:23:43 +0200 +Subject: [PATCH] ccpp: revert the UID/GID changes if user core fails + +Thanks Florian Weimer + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 58 ++++++++++++++++++++++++++++------------------ + 1 file changed, 36 insertions(+), 22 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index c4ad8d1..0448216 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -226,9 +226,6 @@ static int open_user_core(uid_t uid, uid_t fsuid, gid_t fsgid, pid_t pid, char * + return -1; + } + +- xsetegid(fsgid); +- xseteuid(fsuid); +- + if (strcmp(core_basename, "core") == 0) + { + /* Mimic "core.PID" if requested */ +@@ -321,36 +318,53 @@ static int open_user_core(uid_t uid, uid_t fsuid, gid_t fsgid, pid_t pid, char * + * and the description of the /proc/sys/fs/suid_dumpable file in proc(5).) + */ + +- /* Set SELinux context like kernel when creating core dump file */ +- if (newcon != NULL && setfscreatecon_raw(newcon) < 0) +- { +- perror_msg("setfscreatecon_raw(%s)", newcon); +- return -1; +- } ++ int user_core_fd = -1; ++ int selinux_fail = 1; + +- struct stat sb; +- errno = 0; +- /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */ +- int user_core_fd = openat(dirfd(proc_cwd), core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */ ++ /* ++ * These calls must be reverted as soon as possible. ++ */ ++ xsetegid(fsgid); ++ xseteuid(fsuid); + +- if (newcon != NULL && setfscreatecon_raw(NULL) < 0) ++ /* Set SELinux context like kernel when creating core dump file. ++ * This condition is TRUE if */ ++ if (/* SELinux is disabled */ newcon == NULL ++ || /* or the call succeeds */ setfscreatecon_raw(newcon) >= 0) + { +- error_msg("setfscreatecon_raw(NULL)"); +- goto user_core_fail; ++ /* Do not O_TRUNC: if later checks fail, we do not want to have file already modified here */ ++ user_core_fd = openat(dirfd(proc_cwd), core_basename, O_WRONLY | O_CREAT | O_NOFOLLOW | g_user_core_flags, 0600); /* kernel makes 0600 too */ ++ ++ /* Do the error check here and print the error message in order to ++ * avoid interference in 'errno' usage caused by SELinux functions */ ++ if (user_core_fd < 0) ++ perror_msg("Can't open '%s' at '%s'", core_basename, user_pwd); ++ ++ /* Fail if SELinux is enabled and the call fails */ ++ if (newcon != NULL && setfscreatecon_raw(NULL) < 0) ++ perror_msg("setfscreatecon_raw(NULL)"); ++ else ++ selinux_fail = 0; + } ++ else ++ perror_msg("setfscreatecon_raw(%s)", newcon); + ++ /* ++ * DON'T JUMP OVER THIS REVERT OF THE UID/GID CHANGES ++ */ + xsetegid(0); + xseteuid(0); +- if (user_core_fd < 0 +- || fstat(user_core_fd, &sb) != 0 ++ ++ if (user_core_fd < 0 || selinux_fail) ++ goto user_core_fail; ++ ++ struct stat sb; ++ if (fstat(user_core_fd, &sb) != 0 + || !S_ISREG(sb.st_mode) + || sb.st_nlink != 1 + || sb.st_uid != fsuid + ) { +- if (user_core_fd < 0) +- perror_msg("Can't open '%s' at '%s'", core_basename, user_pwd); +- else +- perror_msg("'%s' at '%s' is not a regular file with link count 1 owned by UID(%d)", core_basename, user_pwd, fsuid); ++ perror_msg("'%s' at '%s' is not a regular file with link count 1 owned by UID(%d)", core_basename, user_pwd, fsuid); + goto user_core_fail; + } + if (ftruncate(user_core_fd, 0) != 0) { +-- +2.1.0 + diff --git a/0101-daemon-harden-against-race-conditions-in-DELETE.patch b/0101-daemon-harden-against-race-conditions-in-DELETE.patch new file mode 100644 index 0000000..8a32f03 --- /dev/null +++ b/0101-daemon-harden-against-race-conditions-in-DELETE.patch @@ -0,0 +1,58 @@ +From f1188d8857f2c9773156890d0037296f5361b0bf Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 6 May 2015 14:04:42 +0200 +Subject: [PATCH] daemon: harden against race conditions in DELETE + +There is a race between checking dump dir accessibility and deleting it +in abrt-server. + +Related: #1214457. + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 21 +++++++++++++++++++-- + 1 file changed, 19 insertions(+), 2 deletions(-) + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index 8c48509..cfdd9b7 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -91,8 +91,16 @@ static int delete_path(const char *dump_dir_name) + error_msg("Problem directory '%s' has wrong owner or group", dump_dir_name); + return 400; /* */ + } +- if (!dump_dir_accessible_by_uid(dump_dir_name, client_uid)) ++ ++ struct dump_dir *dd = dd_opendir(dump_dir_name, DD_OPEN_FD_ONLY); ++ if (dd == NULL) ++ { ++ perror_msg("Can't open problem directory '%s'", dump_dir_name); ++ return 400; ++ } ++ if (!dd_accessible_by_uid(dd, client_uid)) + { ++ dd_close(dd); + if (errno == ENOTDIR) + { + error_msg("Path '%s' isn't problem directory", dump_dir_name); +@@ -102,7 +110,16 @@ static int delete_path(const char *dump_dir_name) + return 403; /* Forbidden */ + } + +- delete_dump_dir(dump_dir_name); ++ dd = dd_fdopendir(dd, /*flags:*/ 0); ++ if (dd) ++ { ++ if (dd_delete(dd) != 0) ++ { ++ error_msg("Failed to delete problem directory '%s'", dump_dir_name); ++ dd_close(dd); ++ return 400; ++ } ++ } + + return 0; /* success */ + } +-- +2.1.0 + diff --git a/0102-daemon-allow-only-root-user-to-trigger-the-post-crea.patch b/0102-daemon-allow-only-root-user-to-trigger-the-post-crea.patch new file mode 100644 index 0000000..ed92243 --- /dev/null +++ b/0102-daemon-allow-only-root-user-to-trigger-the-post-crea.patch @@ -0,0 +1,65 @@ +From ee698cf90dc5be666a115db6db245de6812e6ee2 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 6 May 2015 14:39:44 +0200 +Subject: [PATCH] daemon: allow only root user to trigger the post-create + +There is no reason to allow non-root users to trigger this +functionality. Regular users can create abrt problems only through +abrtd or abrt-dbus and both triggers the post-create. + +Other hooks run under root user (CCpp, Koops, VMCore, Xorg). + +Related: #1212861 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 19 ++++++++----------- + 1 file changed, 8 insertions(+), 11 deletions(-) + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index cfdd9b7..5fc4b1a 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -178,16 +178,6 @@ static int run_post_create(const char *dirname) + return 403; + } + } +- if (!dump_dir_accessible_by_uid(dirname, client_uid)) +- { +- if (errno == ENOTDIR) +- { +- error_msg("Path '%s' isn't problem directory", dirname); +- return 404; /* Not Found */ +- } +- error_msg("Problem directory '%s' can't be accessed by user with uid %ld", dirname, (long)client_uid); +- return 403; /* Forbidden */ +- } + + int child_stdout_fd; + int child_pid = spawn_event_handler_child(dirname, "post-create", &child_stdout_fd); +@@ -740,14 +730,21 @@ static int perform_http_xact(void) + /* Body received, EOF was seen. Don't let alarm to interrupt after this. */ + alarm(0); + ++ int ret = 0; + if (url_type == CREATION_NOTIFICATION) + { ++ if (client_uid != 0) ++ { ++ error_msg("UID=%ld is not authorized to trigger post-create processing", (long)client_uid); ++ ret = 403; /* Forbidden */ ++ goto out; ++ } ++ + messagebuf_data[messagebuf_len] = '\0'; + return run_post_create(messagebuf_data); + } + + /* Save problem dir */ +- int ret = 0; + unsigned pid = convert_pid(problem_info); + die_if_data_is_missing(problem_info); + +-- +2.1.0 + diff --git a/0103-daemon-dbus-allow-only-root-to-create-CCpp-Koops-vmc.patch b/0103-daemon-dbus-allow-only-root-to-create-CCpp-Koops-vmc.patch new file mode 100644 index 0000000..e0ec2cf --- /dev/null +++ b/0103-daemon-dbus-allow-only-root-to-create-CCpp-Koops-vmc.patch @@ -0,0 +1,127 @@ +From 4e85328fd73b0d61fb82b535a7d2d8b642b3f95f Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 7 May 2015 11:07:12 +0200 +Subject: [PATCH] daemon, dbus: allow only root to create CCpp, Koops, vmcore + and xorg +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Florian Weimer : + This prevents users from feeding things that are not actually + coredumps and excerpts from /proc to these analyzers. + + For example, it should not be possible to trigger a rule with + “EVENT=post-create analyzer=CCpp” using NewProblem + +Related: #1212861 + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 2 +- + src/dbus/abrt-dbus.c | 10 +++++++++- + src/include/libabrt.h | 2 ++ + src/lib/hooklib.c | 24 ++++++++++++++++++++++++ + 4 files changed, 36 insertions(+), 2 deletions(-) + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index 5fc4b1a..90339ab 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -486,7 +486,7 @@ static gboolean key_value_ok(gchar *key, gchar *value) + } + } + +- return TRUE; ++ return allowed_new_user_problem_entry(client_uid, key, value); + } + + /* Handles a message received from client over socket. */ +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index 489d273..62f331b 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -175,6 +175,7 @@ bool allowed_problem_element(GDBusMethodInvocation *invocation, const char *elem + + static char *handle_new_problem(GVariant *problem_info, uid_t caller_uid, char **error) + { ++ char *problem_id = NULL; + problem_data_t *pd = problem_data_new(); + + GVariantIter *iter; +@@ -182,6 +183,12 @@ static char *handle_new_problem(GVariant *problem_info, uid_t caller_uid, char * + gchar *key, *value; + while (g_variant_iter_loop(iter, "{ss}", &key, &value)) + { ++ if (allowed_new_user_problem_entry(caller_uid, key, value) == false) ++ { ++ *error = xasprintf("You are not allowed to create element '%s' containing '%s'", key, value); ++ goto finito; ++ } ++ + problem_data_add_text_editable(pd, key, value); + } + +@@ -196,12 +203,13 @@ static char *handle_new_problem(GVariant *problem_info, uid_t caller_uid, char * + /* At least it should generate local problem identifier UUID */ + problem_data_add_basics(pd); + +- char *problem_id = problem_data_save(pd); ++ problem_id = problem_data_save(pd); + if (problem_id) + notify_new_path(problem_id); + else if (error) + *error = xasprintf("Cannot create a new problem"); + ++finito: + problem_data_free(pd); + return problem_id; + } +diff --git a/src/include/libabrt.h b/src/include/libabrt.h +index 9de222d..5178eef 100644 +--- a/src/include/libabrt.h ++++ b/src/include/libabrt.h +@@ -56,6 +56,8 @@ enum { + }; + #define dir_has_correct_permissions abrt_dir_has_correct_permissions + bool dir_has_correct_permissions(const char *dir_name, int flags); ++#define allowed_new_user_problem_entry abrt_allowed_new_user_problem_entry ++bool allowed_new_user_problem_entry(uid_t uid, const char *name, const char *value); + + #define g_settings_nMaxCrashReportsSize abrt_g_settings_nMaxCrashReportsSize + extern unsigned int g_settings_nMaxCrashReportsSize; +diff --git a/src/lib/hooklib.c b/src/lib/hooklib.c +index c94cadf..0a8d703 100644 +--- a/src/lib/hooklib.c ++++ b/src/lib/hooklib.c +@@ -552,3 +552,27 @@ bool dir_has_correct_permissions(const char *dir_name, int flags) + */ + return correct_group; + } ++ ++bool allowed_new_user_problem_entry(uid_t uid, const char *name, const char *value) ++{ ++ /* Allow root to create everything */ ++ if (uid == 0) ++ return true; ++ ++ /* Permit non-root users to create everything except: analyzer and type */ ++ if (strcmp(name, FILENAME_ANALYZER) != 0 ++ && strcmp(name, FILENAME_TYPE) != 0 ++ /* compatibility value used in abrt-server */ ++ && strcmp(name, "basename") != 0) ++ return true; ++ ++ /* Permit non-root users to create all types except: C/C++, Koops, vmcore and xorg */ ++ if (strcmp(value, "CCpp") != 0 ++ && strcmp(value, "Kerneloops") != 0 ++ && strcmp(value, "vmcore") != 0 ++ && strcmp(value, "xorg") != 0) ++ return true; ++ ++ error_msg("Only root is permitted to create element '%s' containing '%s'", name, value); ++ return false; ++} +-- +2.1.0 + diff --git a/0104-koops-don-t-save-dmesg-if-kernel.dmesg_restrict-1.patch b/0104-koops-don-t-save-dmesg-if-kernel.dmesg_restrict-1.patch new file mode 100644 index 0000000..63cf597 --- /dev/null +++ b/0104-koops-don-t-save-dmesg-if-kernel.dmesg_restrict-1.patch @@ -0,0 +1,44 @@ +From 31e54ec8ea43d246e745a3a58fc5f07bfce4dfa0 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 18 May 2015 08:43:29 +0200 +Subject: [PATCH] koops: don't save dmesg if kernel.dmesg_restrict=1 + +Also don't run abrt-action-check-oops-for-hw-error if the file dmesg +does not exist as the program searches MCE related strings there. + +Related: rhbz#1128400 +Requires: rhbz#1227661 + +Signed-off-by: Jakub Filak +--- + src/plugins/koops_event.conf | 15 +++++++-------- + 1 file changed, 7 insertions(+), 8 deletions(-) + +diff --git a/src/plugins/koops_event.conf b/src/plugins/koops_event.conf +index eea1055..0e413a8 100644 +--- a/src/plugins/koops_event.conf ++++ b/src/plugins/koops_event.conf +@@ -1,13 +1,12 @@ + # Analyze + EVENT=post-create analyzer=Kerneloops +- # >> instead of > is due to bugzilla.redhat.com/show_bug.cgi?id=854266 +- # 'dmesg' file is required by check-oops-for-hw-error +- dmesg >>dmesg +- # Do not fail the event (->do not delete problem dir) +- # if check-oops-for-hw-error exits nonzero: +- { +- abrt-action-check-oops-for-hw-error || true +- } && ++ # Honor dmesg_restrict -> bugzilla.redhat.com/1128400 ++ if [ "$(cat /proc/sys/kernel/dmesg_restrict)" == "0" ]; then ++ # >> instead of > is due to bugzilla.redhat.com/854266 ++ # 'dmesg' file is required by check-oops-for-hw-error ++ dmesg >>dmesg ++ abrt-action-check-oops-for-hw-error ++ fi + { + # run abrt-action-analyze-oops only if check-hw-error didn't create the + # required files +-- +2.1.0 + diff --git a/0105-ccpp-include-the-system-logs-only-with-root-s-coredu.patch b/0105-ccpp-include-the-system-logs-only-with-root-s-coredu.patch new file mode 100644 index 0000000..8c4e1fb --- /dev/null +++ b/0105-ccpp-include-the-system-logs-only-with-root-s-coredu.patch @@ -0,0 +1,47 @@ +From 52b7072c2c821fcf7d132967a03a2086d4621069 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 18 May 2015 09:34:57 +0200 +Subject: [PATCH] ccpp: include the system logs only with root's coredumps + +Search for suspicious lines in 'journalctl' only if uid == 0. A problem +of the type CCpp can be created only by root so no user can trick abrt +to run 'post-create' on a malicious problem directory with uid == 0. + +Related: rhbz#1212868 + +Signed-off-by: Jakub Filak +--- + src/plugins/ccpp_event.conf | 10 +++++++++- + 1 file changed, 9 insertions(+), 1 deletion(-) + +diff --git a/src/plugins/ccpp_event.conf b/src/plugins/ccpp_event.conf +index 15bb18c..809c3b7 100644 +--- a/src/plugins/ccpp_event.conf ++++ b/src/plugins/ccpp_event.conf +@@ -33,14 +33,22 @@ EVENT=post-create analyzer=CCpp + journalctl --system -n1 >/dev/null + if [ $? -ne 0 ]; + then ++ # Remove the exit below if you don't mind sharing data from the ++ # system logs with unprivileged users -> bugzilla.redhat.com/1212868 ++ exit 0 + # It's not an error if /var/log/messages isn't readable: + test -f /var/log/messages || exit 0 + test -r /var/log/messages || exit 0 + log=`grep -F -e "$base_executable" /var/log/messages | tail -99` + else + uid=`cat uid` && ++ ( ++ # Remove the line below if you don't mind sharing data from the ++ # system logs with unprivileged users -> bugzilla.redhat.com/1212868 ++ [ "$uid" -ne 0 ] && exit 0 + log="[System Logs]:\n" && +- log=$log`journalctl -b --since=-3m --system -n 99 _COMM="$base_executable"` && ++ log=$log`journalctl -b --since=-3m --system -n 99 _COMM="$base_executable"` ++ ) && + log=$log"\n[User Logs]:\n" && + log=$log`journalctl -b --since=-3m -n 99 _COMM="$base_executable" _UID="$uid"` && + log=`echo -e "$log"` +-- +2.1.0 + diff --git a/0106-ccpp-do-not-unlink-failed-and-big-user-cores.patch b/0106-ccpp-do-not-unlink-failed-and-big-user-cores.patch new file mode 100644 index 0000000..80fc8b8 --- /dev/null +++ b/0106-ccpp-do-not-unlink-failed-and-big-user-cores.patch @@ -0,0 +1,118 @@ +From e79b543156aec759542ca7aa01d727aea548f833 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 20 May 2015 06:07:15 +0200 +Subject: [PATCH] ccpp: do not unlink failed and big user cores + +* We might end up deleting an already existing file. +* Kernel does not delete nor truncate core files. Admittedly, kernel + knows how process's memory is structured, dumps it per logical + segments and checks whether a next segment can be written. +* 'ulimit -c' does not seem to be a hard limit. Kernel wrote 8192 bytes + despite $(ulimit -c) == 6. + +Related: #1212818 + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt-hook-ccpp.c | 45 +++++++++++++++++---------------------------- + 1 file changed, 17 insertions(+), 28 deletions(-) + +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 0448216..59fcfce 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -117,8 +117,8 @@ static off_t copyfd_sparse(int src_fd, int dst_fd1, int dst_fd2, off_t size2) + size2 -= rd; + if (size2 < 0) + dst_fd2 = -1; +-//TODO: truncate to 0 or even delete the second file +-//(currently we delete the file later) ++// truncate to 0 or even delete the second file? ++// No, kernel does not delete nor truncate core files. + } + out: + +@@ -377,13 +377,20 @@ static int open_user_core(uid_t uid, uid_t fsuid, gid_t fsgid, pid_t pid, char * + + user_core_fail: + if (user_core_fd >= 0) +- { + close(user_core_fd); +- unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); +- } + return -1; + } + ++static int close_user_core(int user_core_fd, off_t core_size) ++{ ++ if (user_core_fd >= 0 && (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 || core_size < 0)) ++ { ++ perror_msg("Error writing '%s' at '%s'", core_basename, user_pwd); ++ return -1; ++ } ++ return 0; ++} ++ + /* Like xopen, but on error, unlocks and deletes dd and user core */ + static int create_or_die(const char *filename, int user_core_fd) + { +@@ -398,7 +405,7 @@ static int create_or_die(const char *filename, int user_core_fd) + if (dd) + dd_delete(dd); + if (user_core_fd >= 0) +- unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); ++ close(user_core_fd); + + errno = sv_errno; + perror_msg_and_die("Can't open '%s'", filename); +@@ -431,19 +438,10 @@ static int create_user_core(int user_core_fd, pid_t pid, off_t ulimit_c) + if (user_core_fd >= 0) + { + off_t core_size = copyfd_size(STDIN_FILENO, user_core_fd, ulimit_c, COPYFD_SPARSE); +- if (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 || core_size < 0) +- { +- /* perror first, otherwise unlink may trash errno */ +- perror_msg("Error writing '%s' at '%s'", core_basename, user_pwd); +- unlinkat(dirfd(proc_cwd), core_basename, /*only files*/0); +- goto finito; +- } +- if (ulimit_c == 0 || core_size > ulimit_c) +- { +- unlinkat(dirfd(proc_cwd), core_basename, /*only files*/0); ++ if (close_user_core(user_core_fd, core_size) != 0) + goto finito; +- } +- log_notice("Saved core dump of pid %lu to %s at '%s' (%llu bytes)", (long)pid, core_basename, user_pwd, (long long)core_size); ++ ++ log_notice("Saved core dump of pid %lu to '%s' at '%s' (%llu bytes)", (long)pid, core_basename, user_pwd, (long long)core_size); + } + err = 0; + +@@ -822,6 +820,7 @@ int main(int argc, char** argv) + * ls: cannot access core*: No such file or directory <=== BAD + */ + core_size = copyfd_sparse(STDIN_FILENO, abrt_core_fd, user_core_fd, ulimit_c); ++ close_user_core(user_core_fd, core_size); + if (fsync(abrt_core_fd) != 0 || close(abrt_core_fd) != 0 || core_size < 0) + { + unlink(path); +@@ -832,16 +831,6 @@ int main(int argc, char** argv) + + goto cleanup_and_exit; + } +- if (user_core_fd >= 0 +- /* error writing user coredump? */ +- && (fsync(user_core_fd) != 0 || close(user_core_fd) != 0 +- /* user coredump is too big? */ +- || (ulimit_c == 0 /* paranoia */ || core_size > ulimit_c) +- ) +- ) { +- /* nuke it (silently) */ +- unlinkat(dirfd(proc_cwd), core_basename, /*unlink file*/0); +- } + } + else + { +-- +2.1.0 + diff --git a/0107-hooks-use-root-for-owner-of-all-dump-directories.patch b/0107-hooks-use-root-for-owner-of-all-dump-directories.patch new file mode 100644 index 0000000..fe34abb --- /dev/null +++ b/0107-hooks-use-root-for-owner-of-all-dump-directories.patch @@ -0,0 +1,197 @@ +From f15b6e0f53ceb5ba450e93e406272ffe8a398cbd Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 1 Jun 2015 12:03:55 +0200 +Subject: [PATCH] hooks: use root for owner of all dump directories + +This patch has two goals: +* avoid hard and symbolic link attacks (race conditions) +* keep security sensitive data private (in the near future, a problem + directories will contain elements accessible by privileged + users while the directory itself will be accessible by + non-privileged users (dmesg, journald, /var/log/messages) + +Related: #1212868 (CVE-2015-1870), #1212861 (CVE-2015-1869) + +Signed-off-by: Jakub Filak +--- + src/daemon/abrt-server.c | 2 +- + src/dbus/abrt-dbus.c | 11 ++++------- + src/hooks/abrt-hook-ccpp.c | 18 +++++++++--------- + src/lib/hooklib.c | 2 +- + src/plugins/abrt-dump-xorg.c | 15 ++++----------- + src/plugins/oops-utils.c | 15 ++++----------- + 6 files changed, 23 insertions(+), 40 deletions(-) + +diff --git a/src/daemon/abrt-server.c b/src/daemon/abrt-server.c +index 90339ab..d7556e2 100644 +--- a/src/daemon/abrt-server.c ++++ b/src/daemon/abrt-server.c +@@ -391,7 +391,7 @@ static int create_problem_dir(GHashTable *problem_info, unsigned pid) + /* 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, DEFAULT_DUMP_DIR_MODE); ++ struct dump_dir *dd = dd_create(path, /*fs owner*/0, DEFAULT_DUMP_DIR_MODE); + if (!dd) + { + error_msg_and_die("Error creating problem directory '%s'", path); +diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c +index 62f331b..44778a2 100644 +--- a/src/dbus/abrt-dbus.c ++++ b/src/dbus/abrt-dbus.c +@@ -506,13 +506,10 @@ static void handle_method_call(GDBusConnection *connection, + return; + } + +- if (ddstat & DD_STAT_OWNED_BY_UID) +- { //caller seems to be in group with access to this dir, so no action needed +- log_notice("caller has access to the requested directory %s", problem_dir); +- g_dbus_method_invocation_return_value(invocation, NULL); +- dd_close(dd); +- return; +- } ++ /* It might happen that we will do chowing even if the UID is alreay fs ++ * owner, but DD_STAT_OWNED_BY_UID no longer denotes fs owner and this ++ * method has to ensure file system ownership for the uid. ++ */ + + if ((ddstat & DD_STAT_ACCESSIBLE_BY_UID) == 0 && + polkit_check_authorization_dname(caller, "org.freedesktop.problems.getall") != PolkitYes) +diff --git a/src/hooks/abrt-hook-ccpp.c b/src/hooks/abrt-hook-ccpp.c +index 59fcfce..06dd670 100644 +--- a/src/hooks/abrt-hook-ccpp.c ++++ b/src/hooks/abrt-hook-ccpp.c +@@ -707,14 +707,17 @@ int main(int argc, char** argv) + return create_user_core(user_core_fd, pid, ulimit_c); + } + +- /* use fsuid instead of uid, so we don't expose any sensitive +- * information of suided app in /var/tmp/abrt ++ /* If you don't want to have fs owner as root then: + * +- * dd_create_skeleton() creates a new directory and leaves ownership to +- * the current user, hence, we have to call dd_reset_ownership() after the +- * directory is populated. ++ * - use fsuid instead of uid for fs owner, so we don't expose any ++ * sensitive information of suided app in /var/(tmp|spool)/abrt ++ * ++ * - use dd_create_skeleton() and dd_reset_ownership(), when you finish ++ * creating the new dump directory, to prevent the real owner to write to ++ * the directory until the hook is done (avoid race conditions and defend ++ * hard and symbolic link attacs) + */ +- dd = dd_create_skeleton(path, fsuid, DEFAULT_DUMP_DIR_MODE, /*no flags*/0); ++ dd = dd_create(path, /*fs owner*/0, DEFAULT_DUMP_DIR_MODE); + if (dd) + { + char *rootdir = get_rootdir(pid); +@@ -886,9 +889,6 @@ int main(int argc, char** argv) + if (tid > 0 && setting_CreateCoreBacktrace) + create_core_backtrace(tid, executable, signal_no, dd); + +- /* And finally set the right uid and gid */ +- dd_reset_ownership(dd); +- + /* We close dumpdir before we start catering for crash storm case. + * Otherwise, delete_dump_dir's from other concurrent + * CCpp's won't be able to delete our dump (their delete_dump_dir +diff --git a/src/lib/hooklib.c b/src/lib/hooklib.c +index 0a8d703..39a6ef4 100644 +--- a/src/lib/hooklib.c ++++ b/src/lib/hooklib.c +@@ -410,7 +410,7 @@ char* problem_data_save(problem_data_t *pd) + { + load_abrt_conf(); + +- struct dump_dir *dd = create_dump_dir_from_problem_data(pd, g_settings_dump_location); ++ struct dump_dir *dd = create_dump_dir_from_problem_data_ext(pd, g_settings_dump_location, /*fs owner*/0); + + char *problem_id = NULL; + if (dd) +diff --git a/src/plugins/abrt-dump-xorg.c b/src/plugins/abrt-dump-xorg.c +index 3500629..477ec9c 100644 +--- a/src/plugins/abrt-dump-xorg.c ++++ b/src/plugins/abrt-dump-xorg.c +@@ -73,15 +73,6 @@ static void save_bt_to_dump_dir(const char *bt, const char *exe, const char *rea + { + time_t t = time(NULL); + const char *iso_date = iso_date_string(&t); +- /* dump should be readable by all if we're run with -x */ +- uid_t my_euid = (uid_t)-1L; +- mode_t mode = DEFAULT_DUMP_DIR_MODE | S_IROTH; +- /* and readable only for the owner otherwise */ +- if (!(g_opts & OPT_x)) +- { +- mode = DEFAULT_DUMP_DIR_MODE; +- my_euid = geteuid(); +- } + + pid_t my_pid = getpid(); + +@@ -89,10 +80,10 @@ static void save_bt_to_dump_dir(const char *bt, const char *exe, const char *rea + sprintf(base, "xorg-%s-%lu-%u", iso_date, (long)my_pid, g_bt_count); + char *path = concat_path_file(debug_dumps_dir, base); + +- struct dump_dir *dd = dd_create(path, /*uid:*/ my_euid, mode); ++ struct dump_dir *dd = dd_create(path, /*fs owner*/0, DEFAULT_DUMP_DIR_MODE); + if (dd) + { +- dd_create_basic_files(dd, /*uid:*/ my_euid, NULL); ++ dd_create_basic_files(dd, /*no uid*/(uid_t)-1L, NULL); + dd_save_text(dd, FILENAME_ABRT_VERSION, VERSION); + dd_save_text(dd, FILENAME_ANALYZER, "xorg"); + dd_save_text(dd, FILENAME_TYPE, "xorg"); +@@ -111,6 +102,8 @@ static void save_bt_to_dump_dir(const char *bt, const char *exe, const char *rea + exe = "/usr/bin/Xorg"; + } + dd_save_text(dd, FILENAME_EXECUTABLE, exe); ++ if (!(g_opts & OPT_x)) ++ dd_set_no_owner(dd); + dd_close(dd); + notify_new_path(path); + } +diff --git a/src/plugins/oops-utils.c b/src/plugins/oops-utils.c +index ea6c639..bb6a79c 100644 +--- a/src/plugins/oops-utils.c ++++ b/src/plugins/oops-utils.c +@@ -92,15 +92,6 @@ unsigned abrt_oops_create_dump_dirs(GList *oops_list, const char *dump_location, + + time_t t = time(NULL); + const char *iso_date = iso_date_string(&t); +- /* dump should be readable by all if we're run with -x */ +- uid_t my_euid = (uid_t)-1L; +- mode_t mode = DEFAULT_DUMP_DIR_MODE | S_IROTH; +- /* and readable only for the owner otherwise */ +- if (!(flags & ABRT_OOPS_WORLD_READABLE)) +- { +- mode = DEFAULT_DUMP_DIR_MODE; +- my_euid = geteuid(); +- } + + pid_t my_pid = getpid(); + unsigned idx = 0; +@@ -111,10 +102,10 @@ unsigned abrt_oops_create_dump_dirs(GList *oops_list, const char *dump_location, + sprintf(base, "oops-%s-%lu-%lu", iso_date, (long)my_pid, (long)idx); + char *path = concat_path_file(dump_location, base); + +- struct dump_dir *dd = dd_create(path, /*uid:*/ my_euid, mode); ++ struct dump_dir *dd = dd_create(path, /*fs owner*/0, DEFAULT_DUMP_DIR_MODE); + if (dd) + { +- dd_create_basic_files(dd, /*uid:*/ my_euid, NULL); ++ dd_create_basic_files(dd, /*no uid*/(uid_t)-1L, NULL); + abrt_oops_save_data_in_dump_dir(dd, (char*)g_list_nth_data(oops_list, idx++), proc_modules); + dd_save_text(dd, FILENAME_ABRT_VERSION, VERSION); + dd_save_text(dd, FILENAME_ANALYZER, "Kerneloops"); +@@ -127,6 +118,8 @@ unsigned abrt_oops_create_dump_dirs(GList *oops_list, const char *dump_location, + dd_save_text(dd, "fips_enabled", fips_enabled); + if (suspend_stats) + dd_save_text(dd, "suspend_stats", suspend_stats); ++ if ((flags & ABRT_OOPS_WORLD_READABLE)) ++ dd_set_no_owner(dd); + dd_close(dd); + notify_new_path(path); + } +-- +2.1.0 + diff --git a/0108-cli-chown-before-reporting.patch b/0108-cli-chown-before-reporting.patch new file mode 100644 index 0000000..824a2bf --- /dev/null +++ b/0108-cli-chown-before-reporting.patch @@ -0,0 +1,95 @@ +From b68f8ea31526275127a9f6ae5c8a24fc24b6d664 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Wed, 3 Jun 2015 05:40:41 +0200 +Subject: [PATCH] cli: chown before reporting + +User must have write access to the reported directory to be able to +report it but abrt-dbus allows the user to read data of problems that +belongs to him which may not be accessible in file system. + +The GUI does the same and make sures the user can write to the reported +directory by chowning it before reporting. + +Signed-off-by: Jakub Filak +--- + src/cli/abrt-cli-core.c | 5 +++++ + src/cli/abrt-cli-core.h | 3 +++ + src/cli/report.c | 24 +++++++++++++++--------- + 3 files changed, 23 insertions(+), 9 deletions(-) + +diff --git a/src/cli/abrt-cli-core.c b/src/cli/abrt-cli-core.c +index 77a37f7..46acd01 100644 +--- a/src/cli/abrt-cli-core.c ++++ b/src/cli/abrt-cli-core.c +@@ -107,3 +107,8 @@ char *hash2dirname(const char *hash) + + return found_name; + } ++ ++char *hash2dirname_if_necessary(const char *input) ++{ ++ return isxdigit_str(input) ? hash2dirname(input) : xstrdup(input); ++} +diff --git a/src/cli/abrt-cli-core.h b/src/cli/abrt-cli-core.h +index 33b2ea6..d69d463 100644 +--- a/src/cli/abrt-cli-core.h ++++ b/src/cli/abrt-cli-core.h +@@ -34,6 +34,9 @@ vector_of_problem_data_t *fetch_crash_infos(void); + char *find_problem_by_hash(const char *hash, GList *problems); + /* Returns malloced string, or NULL if not found: */ + char *hash2dirname(const char *hash); ++/* If input looks like a hash, returns malloced string, or NULL if not found. ++ * Otherwise returns a copy of the input. */ ++char *hash2dirname_if_necessary(const char *input); + + + #endif /* ABRT_CLI_CORE_H_ */ +diff --git a/src/cli/report.c b/src/cli/report.c +index 33d8b44..6af9769 100644 +--- a/src/cli/report.c ++++ b/src/cli/report.c +@@ -53,26 +53,32 @@ int cmd_report(int argc, const char **argv) + while (*argv) + { + const char *dir_name = *argv++; ++ char *const real_problem_id = hash2dirname_if_necessary(dir_name); ++ if (real_problem_id == NULL) ++ { ++ error_msg(_("Can't find problem '%s'"), dir_name); ++ continue; ++ } + +- char *free_me = NULL; +- if (access(dir_name, F_OK) != 0 && errno == ENOENT) ++ const int res = chown_dir_over_dbus(real_problem_id); ++ if (res != 0) + { +- free_me = hash2dirname(dir_name); +- if (free_me) +- dir_name = free_me; ++ error_msg(_("Can't take ownership of '%s'"), real_problem_id); ++ free(real_problem_id); ++ continue; + } +- int status = report_problem_in_dir(dir_name, ++ int status = report_problem_in_dir(real_problem_id, + LIBREPORT_WAIT + | LIBREPORT_RUN_CLI); + + /* the problem was successfully reported and option is -d */ + if((opts & OPT_d) && (status == 0 || status == EXIT_STOP_EVENT_RUN)) + { +- log(_("Deleting '%s'"), dir_name); +- delete_dump_dir_possibly_using_abrtd(dir_name); ++ log(_("Deleting '%s'"), real_problem_id); ++ delete_dump_dir_possibly_using_abrtd(real_problem_id); + } + +- free(free_me); ++ free(real_problem_id); + + if (status) + exit(status); +-- +2.1.0 + diff --git a/0110-cli-exit-with-the-number-of-unreported-problems.patch b/0110-cli-exit-with-the-number-of-unreported-problems.patch new file mode 100644 index 0000000..c84a835 --- /dev/null +++ b/0110-cli-exit-with-the-number-of-unreported-problems.patch @@ -0,0 +1,50 @@ +From 69843667275dd3de2381635061eda19eacb07e23 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Thu, 4 Jun 2015 10:22:33 +0200 +Subject: [PATCH] cli: exit with the number of unreported problems + +This patch fixes the broken cli-sanity. + +Signed-off-by: Jakub Filak +--- + src/cli/report.c | 5 ++++- + 1 file changed, 4 insertions(+), 1 deletion(-) + +diff --git a/src/cli/report.c b/src/cli/report.c +index 6af9769..194f7c9 100644 +--- a/src/cli/report.c ++++ b/src/cli/report.c +@@ -50,6 +50,7 @@ int cmd_report(int argc, const char **argv) + load_abrt_conf(); + free_abrt_conf_data(); + ++ int ret = 0; + while (*argv) + { + const char *dir_name = *argv++; +@@ -57,6 +58,7 @@ int cmd_report(int argc, const char **argv) + if (real_problem_id == NULL) + { + error_msg(_("Can't find problem '%s'"), dir_name); ++ ++ret; + continue; + } + +@@ -65,6 +67,7 @@ int cmd_report(int argc, const char **argv) + { + error_msg(_("Can't take ownership of '%s'"), real_problem_id); + free(real_problem_id); ++ ++ret; + continue; + } + int status = report_problem_in_dir(real_problem_id, +@@ -84,5 +87,5 @@ int cmd_report(int argc, const char **argv) + exit(status); + } + +- return 0; ++ return ret; + } +-- +2.1.0 + diff --git a/0111-ccpp-don-t-save-the-system-logs-by-default.patch b/0111-ccpp-don-t-save-the-system-logs-by-default.patch new file mode 100644 index 0000000..1ebe7b4 --- /dev/null +++ b/0111-ccpp-don-t-save-the-system-logs-by-default.patch @@ -0,0 +1,82 @@ +From 65821e8e792a253e5baa1d3633ff115702769b84 Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 8 Jun 2015 12:59:39 +0200 +Subject: [PATCH] ccpp: don't save the system logs by default + +Saving the system logs if uid equals 0 was a bad idea because we are not +sure who really owns the problem directory. It could be reintroduced +when we rewrite those shell script lines in Python/C or better we should +store the system logs in a private element (#989). + +This patch also removes the support for /var/log/messages because it was +making the whole post-create event unnecessarily complex and we can +count on the fact that all currently supported systems use +systemd-journald. + +Related: rhbz#1212868 + +Signed-off-by: Jakub Filak +--- + src/plugins/ccpp_event.conf | 46 ++++++++++++++++++--------------------------- + 1 file changed, 18 insertions(+), 28 deletions(-) + +diff --git a/src/plugins/ccpp_event.conf b/src/plugins/ccpp_event.conf +index 809c3b7..227776d 100644 +--- a/src/plugins/ccpp_event.conf ++++ b/src/plugins/ccpp_event.conf +@@ -29,34 +29,24 @@ EVENT=post-create analyzer=CCpp + # Can't do it as analyzer step, non-root can't read log. + executable=`cat executable` && + base_executable=${executable##*/} && +- # Test if the current version of journalctl has --system switch +- journalctl --system -n1 >/dev/null +- if [ $? -ne 0 ]; +- then +- # Remove the exit below if you don't mind sharing data from the +- # system logs with unprivileged users -> bugzilla.redhat.com/1212868 +- exit 0 +- # It's not an error if /var/log/messages isn't readable: +- test -f /var/log/messages || exit 0 +- test -r /var/log/messages || exit 0 +- log=`grep -F -e "$base_executable" /var/log/messages | tail -99` +- else +- uid=`cat uid` && +- ( +- # Remove the line below if you don't mind sharing data from the +- # system logs with unprivileged users -> bugzilla.redhat.com/1212868 +- [ "$uid" -ne 0 ] && exit 0 +- log="[System Logs]:\n" && +- log=$log`journalctl -b --since=-3m --system -n 99 _COMM="$base_executable"` +- ) && +- log=$log"\n[User Logs]:\n" && +- log=$log`journalctl -b --since=-3m -n 99 _COMM="$base_executable" _UID="$uid"` && +- log=`echo -e "$log"` +- fi +- if test -n "$log"; then +- printf "%s\n" "$log" >var_log_messages +- # echo "Element 'var_log_messages' saved" +- fi ++ uid=`cat $DUMP_DIR/uid` && ++ { ++ user_log=`journalctl -b --since=-3m -n 99 _COMM="$base_executable" _UID="$uid"` && ++ test -n "$user_log" && printf "User logs:\n%s\n" "$user_log" >$DUMP_DIR/var_log_messages ++ # Do not use '&&' here because if $user_log is the empty string ++ # then the script does not continue to get the system logs ++ { ++ # Remove the line below if you don't mind sharing data from the ++ # system logs with unprivileged users -> bugzilla.redhat.com/1212868 ++ false && ++ system_log=$log`journalctl -b --since=-3m --system -n 99 _COMM="$base_executable"` && ++ test -n "$system_log" && printf "System logs:\n%s\n" "$system_log" >>$DUMP_DIR/var_log_messages ++ # Always exit with true here, because the false at ++ # the beginning would cause the post-create hook to remove ++ # the current problem directory. ++ true ++ } ++ } + ) + + EVENT=collect_xsession_errors analyzer=CCpp dso_list~=.*/libX11.* +-- +2.1.0 + diff --git a/0112-vmcore-use-libreport-dd-API-in-the-harvestor.patch b/0112-vmcore-use-libreport-dd-API-in-the-harvestor.patch new file mode 100644 index 0000000..3866d30 --- /dev/null +++ b/0112-vmcore-use-libreport-dd-API-in-the-harvestor.patch @@ -0,0 +1,229 @@ +From fcd6406bdb8e0b0005725ae88c03979e62e740ee Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Mon, 8 Jun 2015 19:39:24 +0200 +Subject: [PATCH] vmcore: use libreport dd API in the harvestor + +The dd API ensure correct permissions and owner. + +Signed-off-by: Jakub Filak +--- + src/hooks/abrt_harvest_vmcore.py.in | 156 ++++++++++++++---------------------- + 1 file changed, 62 insertions(+), 94 deletions(-) + +diff --git a/src/hooks/abrt_harvest_vmcore.py.in b/src/hooks/abrt_harvest_vmcore.py.in +index 256f8f1..f2bddfe 100644 +--- a/src/hooks/abrt_harvest_vmcore.py.in ++++ b/src/hooks/abrt_harvest_vmcore.py.in +@@ -15,6 +15,7 @@ import hashlib + import augeas + + import problem ++import report + + + def get_augeas(module, file_path): +@@ -93,85 +94,39 @@ def parse_kdump(): + return path + + +-def write_to_file(path, content): ++def create_abrtd_info(dest, uuid): + """ +- A function for writing into a file +- +- path - path to the file +- content - content to write into the file +- """ +- +- with open(path, 'w') as wfile: +- wfile.write(content) +- +- +-def change_owner_rec(dest): +- """ +- A simple function to recursively change file mode for a directory. +- +- dest - path to the directory +- """ +- +- os.chown(dest, 0, 0) +- for root, dirs, files in os.walk(dest): +- for i in dirs: +- os.chown(os.path.join(root, i), 0, 0) +- for i in files: +- os.chown(os.path.join(root, i), 0, 0) +- ++ A simple function to write important information for the abrt daemon into ++ the vmcore directory to let abrtd know what kind of problem it is. + +-def change_mode_rec(dest): ++ dest - path to the vmcore directory ++ uuid - unique indentifier of the vmcore + """ +- A simple function to recursively change file mode for a directory. + +- dest - path to the directory +- """ ++ dd = report.dd_create(dest, 0) ++ if dd is None: ++ return None + +- os.chmod(dest, 0700) +- for root, dirs, files in os.walk(dest): +- for i in dirs: +- os.chmod(os.path.join(root, i), 0700) +- for i in files: +- os.chmod(os.path.join(root, i), 0600) ++ dd.create_basic_files(0) ++ dd.save_text('analyzer', 'vmcore') ++ dd.save_text('type', 'vmcore') ++ dd.save_text('component', 'kernel') ++ dd.save_text('uuid', uuid) ++ return dd + + +-def create_abrtd_info(dest): ++def delete_and_close(dd, dd_dirname): + """ +- A simple function to write important information for the abrt daemon into +- the vmcore directory to let abrtd know what kind of problem it is. ++ Deletes the given dump directory and closes it. + +- dest - path to the vmcore directory ++ dd - dump directory object ++ dd_dirname - full path to dump directory + """ ++ if not dd.delete() == 0: ++ sys.stderr.write("Unable to delete '%s'\n" % (dd_dirname)) ++ return + +- write_to_file(os.path.join(dest, 'analyzer'), 'vmcore') +- write_to_file(os.path.join(dest, 'type'), 'vmcore') +- write_to_file(os.path.join(dest, 'component'), 'kernel') +- write_to_file(os.path.join(dest, 'time'), str(time.time()).split('.')[0]) +- shutil.copy(os.path.join(dest, 'time'), +- os.path.join(dest, 'last_occurrence')) +- write_to_file(os.path.join(dest, 'architecture'), os.uname()[4]) +- write_to_file(os.path.join(dest, 'uid'), '0') +- +- # TODO: need to generate *real* UUID, +- # one which has a real chance of catching dups! +- # This one generates different hashes even for similar cores: +- hashobj = hashlib.sha1() +- # Iterate over the file a line at a time in order to not load the whole +- # vmcore file +- with open(os.path.join(dest, 'vmcore'), 'r') as corefile: +- for line in corefile: +- hashobj.update(line) +- write_to_file(os.path.join(dest, 'uuid'), hashobj.hexdigest()) +- +- # Write os info into the vmcore directory +- if os.path.exists('/etc/system-release'): +- shutil.copy('/etc/system-release', os.path.join(dest, 'os_release')) +- elif os.path.exists('/etc/redhat-release'): +- shutil.copy('/etc/redhat-release', os.path.join(dest, 'os_release')) +- elif os.path.exists('/etc/SuSE-release'): +- shutil.copy('/etc/SuSE-release', os.path.join(dest, 'os_release')) +- if os.path.exists('/etc/os-release'): +- shutil.copy('/etc/os-release', os.path.join(dest, 'os_info')) ++ dd.close() + + + def harvest_vmcore(): +@@ -200,8 +155,11 @@ def harvest_vmcore(): + else: + break + ++<<<<<<< HEAD + os.umask(077) + ++======= ++>>>>>>> 71360a6... vmcore: use libreport dd API in the harvestor + # Check abrt config files for copy/move settings and + try: + conf = problem.load_plugin_conf_file("vmcore.conf") +@@ -245,6 +203,8 @@ def harvest_vmcore(): + "VMCore dir '%s' doesn't contain 'vmcore' file.\n" % f_full) + continue + ++ # We use .new suffix - we must make sure abrtd doesn't try ++ # to process partially-copied directory. + destdir = os.path.join(abrtdumpdir, ('vmcore-' + cfile)) + destdirnew = destdir + '.new' + # Did we already copy it last time we booted? +@@ -252,38 +212,46 @@ def harvest_vmcore(): + continue + if os.path.isdir(destdirnew): + continue +- # Copy/move vmcore directory to abrt spool dir. +- # We use .new suffix - we must make sure abrtd doesn't try +- # to process partially-copied directory. +- +- try: +- shutil.copytree(f_full, destdirnew) +- except (OSError, shutil.Error): +- sys.stderr.write("Unable to copy '%s' to '%s'. Skipping\n" +- % (f_full, destdirnew)) + +- # delete .new dir so we don't create mess +- shutil.rmtree(destdirnew) ++ # TODO: need to generate *real* UUID, ++ # one which has a real chance of catching dups! ++ # This one generates different hashes even for similar cores: ++ hashobj = hashlib.sha1() ++ # Iterate over the file a line at a time in order to not load the whole ++ # vmcore file ++ with open(os.path.join(f_full, 'vmcore'), 'r') as corefile: ++ for line in corefile: ++ hashobj.update(line) ++ ++ dd = create_abrtd_info(destdirnew, hashobj.hexdigest()) ++ if dd is None: ++ sys.stderr.write("Unable to create problem directory info") + continue + +- try: +- # Let abrtd know what type of problem it is: +- create_abrtd_info(destdirnew) +- except EnvironmentError as ex: +- sys.stderr.write("Unable to create problem directory info: " + str(ex)) ++ # Copy/move vmcore directory to abrt spool dir. ++ for name in os.listdir(f_full): ++ full_name = os.path.join(f_full, name) ++ ++ # Skip sub-directories, abrt ignores them in its processing anyway ++ if not os.path.isfile(full_name): ++ continue ++ + try: +- shutil.rmtree(destdirnew) +- except Exception as ex: +- sys.stderr.write("Unable to remove incomplete problem directory: " + str(ex)) +- continue ++ if not dd.copy_file(name, full_name) == 0: ++ raise OSError ++ except (OSError, shutil.Error): ++ sys.stderr.write("Unable to copy '%s' to '%s'. Skipping\n" ++ % (full_name, destdirnew)) ++ delete_and_close(dd) ++ continue + +- # chown -R 0:0 +- change_owner_rec(destdirnew) +- # chmod -R u+rwX,go-rwxst +- change_mode_rec(destdirnew) ++ # Get rid of the .new suffix ++ if not dd.rename(destdir) == 0: ++ sys.stderr.write("Unable to rename '%s' to '%s'. Skipping\n" % (destdirnew, destdir)) ++ delete_and_close(dd) ++ continue + +- # Get rid of the .new suffix +- shutil.move(destdirnew, destdir) ++ dd.close() + + if copyvmcore == 'no': + try: +-- +2.1.0 + diff --git a/0113-ccpp-correct-capitalization-of-log-labels.patch b/0113-ccpp-correct-capitalization-of-log-labels.patch new file mode 100644 index 0000000..ebf15f9 --- /dev/null +++ b/0113-ccpp-correct-capitalization-of-log-labels.patch @@ -0,0 +1,37 @@ +From ee80fb98f0c7995e1e4f8b0aae13c23ede012bea Mon Sep 17 00:00:00 2001 +From: Jakub Filak +Date: Tue, 9 Jun 2015 08:10:34 +0200 +Subject: [PATCH] ccpp: correct capitalization of log labels + +I broke it in commit fbea8811e93bed6c8fe665b47feeb9bd1cc7b755 + +Signed-off-by: Jakub Filak +--- + src/plugins/ccpp_event.conf | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/src/plugins/ccpp_event.conf b/src/plugins/ccpp_event.conf +index 227776d..84eecf1 100644 +--- a/src/plugins/ccpp_event.conf ++++ b/src/plugins/ccpp_event.conf +@@ -32,7 +32,7 @@ EVENT=post-create analyzer=CCpp + uid=`cat $DUMP_DIR/uid` && + { + user_log=`journalctl -b --since=-3m -n 99 _COMM="$base_executable" _UID="$uid"` && +- test -n "$user_log" && printf "User logs:\n%s\n" "$user_log" >$DUMP_DIR/var_log_messages ++ test -n "$user_log" && printf "User Logs:\n%s\n" "$user_log" >$DUMP_DIR/var_log_messages + # Do not use '&&' here because if $user_log is the empty string + # then the script does not continue to get the system logs + { +@@ -40,7 +40,7 @@ EVENT=post-create analyzer=CCpp + # system logs with unprivileged users -> bugzilla.redhat.com/1212868 + false && + system_log=$log`journalctl -b --since=-3m --system -n 99 _COMM="$base_executable"` && +- test -n "$system_log" && printf "System logs:\n%s\n" "$system_log" >>$DUMP_DIR/var_log_messages ++ test -n "$system_log" && printf "System Logs:\n%s\n" "$system_log" >>$DUMP_DIR/var_log_messages + # Always exit with true here, because the false at + # the beginning would cause the post-create hook to remove + # the current problem directory. +-- +2.1.0 + diff --git a/abrt.spec b/abrt.spec index 852b0b1..a6bc051 100644 --- a/abrt.spec +++ b/abrt.spec @@ -46,7 +46,7 @@ Summary: Automatic bug detection and reporting tool Name: abrt Version: 2.3.0 -Release: 4%{?dist} +Release: 5%{?dist} License: GPLv2+ Group: Applications/System URL: https://github.com/abrt/abrt/wiki/ABRT-Project @@ -106,6 +106,67 @@ Patch0048: 0048-vmcore-remove-original-vmcore-file-in-the-last-step.patch Patch0050: 0050-console-notifications-add-timeout.patch Patch0051: 0051-Don-t-slurp-unbounded-amounts-of-data-when-invoking-.patch Patch0052: 0052-Rewrite-journalctl-invocations-replace-grep-tail-pip.patch +Patch0053: 0053-abrt-hook-ccpp-minor-refactoring.patch +Patch0054: 0054-Create-core-backtrace-in-unwind-hook.patch +Patch0055: 0055-abrt-install-ccpp-hook-check-configuration.patch +Patch0056: 0056-ccpp-hook-move-proc-pid-utils-to-libreport.patch +Patch0057: 0057-ccpp-hook-move-utility-functions-to-hooklib.patch +Patch0058: 0058-core-use-updated-dump_fd_info.patch +#Patch0059: 0059-spec-Don-t-allow-users-to-list-problems-by-hand.patch +Patch0060: 0060-abrtd-Don-t-allow-users-to-list-problems-by-hand.patch +Patch0061: 0061-retrace-client-stop-failing-on-SSL2.patch +Patch0062: 0062-dbus-add-a-new-method-GetProblemData.patch +Patch0063: 0063-doc-add-documentation-for-GetProblemData.patch +Patch0064: 0064-applet-get-the-list-of-problems-through-D-Bus-servic.patch +Patch0065: 0065-libabrt-add-new-function-fetching-full-problem-data-.patch +Patch0066: 0066-dbus-add-new-method-to-test-existence-of-an-element.patch +Patch0067: 0067-libabrt-add-wrappers-TestElemeExists-and-GetInfo-for.patch +Patch0068: 0068-cli-use-the-DBus-methods-for-getting-problem-informa.patch +Patch0069: 0069-cli-status-don-t-return-0-if-there-is-a-problem-olde.patch +Patch0070: 0070-upload-validate-and-sanitize-uploaded-dump-directori.patch +Patch0071: 0071-applet-switch-to-D-Bus-methods.patch +Patch0072: 0072-cli-do-not-exit-with-segfault-if-dbus-fails.patch +#Patch0073: 0073-spec-add-a-dependency-on-abrt-dbus-to-abrt-cli.patch +Patch0074: 0074-lib-add-new-kernel-taint-flags.patch +Patch0075: 0075-a-a-s-p-d-add-new-known-interpreter-to-conf-file.patch +Patch0076: 0076-doc-update-abrt-cli-man-page.patch +#Patch0077: 0077-spec-remove-analyzer-to-type-conversion.patch +Patch0078: 0078-turn-off-exploring-crashed-process-s-root-directorie.patch +Patch0079: 0079-ccpp-fix-symlink-race-conditions.patch +Patch0080: 0080-ccpp-stop-reading-hs_error.log-from-tmp.patch +Patch0081: 0081-ccpp-postpone-changing-ownership-of-new-dump-directo.patch +Patch0082: 0082-ccpp-create-dump-directory-without-parents.patch +Patch0083: 0083-ccpp-do-not-override-existing-files-by-compat-cores.patch +Patch0084: 0084-ccpp-do-not-use-value-of-proc-PID-cwd-for-chdir.patch +Patch0085: 0085-ccpp-make-saving-of-binary-more-robust.patch +Patch0086: 0086-ccpp-harden-dealing-with-UID-GID.patch +Patch0087: 0087-ccpp-check-for-overflow-in-abrt-coredump-path-creati.patch +Patch0088: 0088-ccpp-emulate-selinux-for-creation-of-compat-cores.patch +#Patch0089: 0089-spec-add-libselinux-devel-to-BRs.patch +Patch0090: 0090-ccpp-avoid-overriding-system-files-by-coredump.patch +Patch0091: 0091-configure-move-the-default-dump-location-to-var-spoo.patch +#Patch0092: 0092-spec-create-vat-spool-abrt.patch +Patch0093: 0093-daemon-use-libreport-s-function-checking-file-name.patch +Patch0094: 0094-lib-add-functions-validating-dump-dir.patch +Patch0095: 0095-dbus-process-only-valid-sub-directories-of-the-dump-.patch +Patch0096: 0096-dbus-avoid-race-conditions-in-tests-for-dum-dir-avai.patch +Patch0097: 0097-dbus-report-invalid-element-names.patch +Patch0098: 0098-a-a-i-d-t-a-cache-sanitize-arguments.patch +Patch0099: 0099-a-a-i-d-t-a-cache-sanitize-umask.patch +Patch0100: 0100-ccpp-revert-the-UID-GID-changes-if-user-core-fails.patch +Patch0101: 0101-daemon-harden-against-race-conditions-in-DELETE.patch +Patch0102: 0102-daemon-allow-only-root-user-to-trigger-the-post-crea.patch +Patch0103: 0103-daemon-dbus-allow-only-root-to-create-CCpp-Koops-vmc.patch +Patch0104: 0104-koops-don-t-save-dmesg-if-kernel.dmesg_restrict-1.patch +Patch0105: 0105-ccpp-include-the-system-logs-only-with-root-s-coredu.patch +Patch0106: 0106-ccpp-do-not-unlink-failed-and-big-user-cores.patch +Patch0107: 0107-hooks-use-root-for-owner-of-all-dump-directories.patch +Patch0108: 0108-cli-chown-before-reporting.patch +#Patch0109: 0109-spec-restart-abrt-dbus-in-posttrans.patch +Patch0110: 0110-cli-exit-with-the-number-of-unreported-problems.patch +Patch0111: 0111-ccpp-don-t-save-the-system-logs-by-default.patch +Patch0112: 0112-vmcore-use-libreport-dd-API-in-the-harvestor.patch +Patch0113: 0113-ccpp-correct-capitalization-of-log-labels.patch # '%%autosetup -S git' -> git @@ -132,6 +193,7 @@ BuildRequires: satyr-devel >= %{satyr_ver} BuildRequires: systemd-python BuildRequires: systemd-python3 BuildRequires: augeas +BuildRequires: libselinux-devel Requires: libreport >= %{libreport_ver} Requires: satyr >= %{satyr_ver} @@ -334,6 +396,7 @@ Group: User Interface/Desktops Requires: %{name} = %{version}-%{release} Requires: libreport-cli >= %{libreport_ver} Requires: abrt-libs = %{version}-%{release} +Requires: abrt-dbus %description tui This package contains a simple command line client for processing abrt reports @@ -491,8 +554,8 @@ to the shell # Default '__scm_apply_git' is 'git apply && git commit' but this workflow # doesn't allow us to create a new file within a patch, so we have to use # 'git am' (see /usr/lib/rpm/macros for more details) -%define __scm_apply_git(qp:m:) %{__git} am -#%%define __scm_apply_git(qp:m:) %%{__git} am --exclude libreport.spec.in --exclude .gitignore +#%%define __scm_apply_git(qp:m:) %%{__git} am +%define __scm_apply_git(qp:m:) %{__git} am --exclude doc/deployment/en-US/Automatic_Bug_Reporting_Tool_ABRT.xml %autosetup -S git %build @@ -529,7 +592,7 @@ find $RPM_BUILD_ROOT -name '*.la' -or -name '*.a' | xargs rm -f mkdir -p ${RPM_BUILD_ROOT}/%{_initrddir} mkdir -p $RPM_BUILD_ROOT/var/cache/abrt-di mkdir -p $RPM_BUILD_ROOT/var/run/abrt -mkdir -p $RPM_BUILD_ROOT/var/tmp/abrt +mkdir -p $RPM_BUILD_ROOT/var/spool/abrt mkdir -p $RPM_BUILD_ROOT/var/spool/abrt-upload mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/lib/abrt @@ -649,7 +712,6 @@ fi %posttrans # update the old problem dirs to contain "type" element -abrtdir=$(grep "DumpLocation" /etc/abrt/abrt.conf | cut -d'=' -f2 | tr -d ' '); cd $abrtdir 2>/dev/null && for i in `find . -name "analyzer" 2>/dev/null`; do len=${#i};cp "$i" "${i:0:$len-9}/type"; done; for i in `find "$abrtdir" -mindepth 1 -maxdepth 1 -type d`; do chown `stat --format=%U:abrt $i` $i/*; done service abrtd condrestart >/dev/null 2>&1 || : %posttrans addon-ccpp @@ -691,6 +753,10 @@ service abrt-upload-watch condrestart >/dev/null 2>&1 || : %posttrans gui gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : +%posttrans dbus +# Force abrt-dbus to restart like we do with the other services +killall abrt-dbus >/dev/null 2>&1 || : + %files -f %{name}.lang %defattr(-,root,root,-) %doc README COPYING @@ -727,7 +793,7 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : %{_mandir}/man5/abrt_event.conf.5.gz %config(noreplace) %{_sysconfdir}/libreport/events.d/smart_event.conf %{_mandir}/man5/smart_event.conf.5.gz -%dir %attr(0755, abrt, abrt) %{_localstatedir}/tmp/%{name} +%dir %attr(0751, abrt, abrt) %{_localstatedir}/spool/%{name} %dir %attr(0700, abrt, abrt) %{_localstatedir}/spool/%{name}-upload # abrtd runs as root %dir %attr(0755, root, root) %{_localstatedir}/run/%{name} @@ -1019,6 +1085,23 @@ gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : %config(noreplace) %{_sysconfdir}/profile.d/abrt-console-notification.sh %changelog +* Tue Jun 16 2015 Matej Habrnal - 2.3.0-5 +- move the default dump location to /var/spool/abrt from /var/tmp/abrt +- hooks: use root for owner of all dump directories +- ccpp: do not unlink failed and big user cores +- ccpp: don't save the system logs by default +- ccpp: stop reading hs_error.log from /tmp +- ccpp: emulate selinux for creation of compat cores +- koops: don't save dmesg if kernel.dmesg_restrict=1 +- dbus: validate passed arguments +- turn off exploring crashed process's root directories +- abrt-python: bug fixes and improvements +- fixes for CVE-2015-3315, CVE-2015-3142, CVE-2015-1869, CVE-2015-1870 +- fixes for CVE-2015-3147, CVE-2015-3151, CVE-2015-3150, CVE-2015-3159 +- spec: add abrt-dbus to Rs of abrt-python and abrt-cli +- spec: restart abrt-dbus in posttrans +- Resolves: #1179752 + * Tue Feb 24 2015 Matej Habrnal - 2.3.0-4 - make gdb aware of the abrt's debuginfo dir - python: load the configuration from correct file