rpms/abrt
3
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Several bug fixes

Browse source 
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 <mhabrnal@redhat.com>
This commit is contained in:
Matej Habrnal 2015-06-17 16:27:48 +02:00
commit fa1950198b
56 changed files with 6592 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

211
0053-abrt-hook-ccpp-minor-refactoring.patch Normal file
View file

@ -0,0 +1,211 @@
From 7a10fa7c19e876ea7e5109d3d71dc9bbffc70214 Mon Sep 17 00:00:00 2001
From: Martin Milata <mmilata@redhat.com>
Date: Mon, 1 Dec 2014 11:47:55 +0100
Subject: [PATCH] abrt-hook-ccpp: minor refactoring
Related to #829.
Signed-off-by: Martin Milata <mmilata@redhat.com>
---
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

356
0054-Create-core-backtrace-in-unwind-hook.patch Normal file
View file

@ -0,0 +1,356 @@
From 0f669af4d7180b12142a41117ae2459d6960dd31 Mon Sep 17 00:00:00 2001
From: Martin Milata <mmilata@redhat.com>
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 <mmilata@redhat.com>
---
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 ]</screen>
<para>
Please note that installing <application>ABRT</application> packages overwrites the <filename>/proc/sys/kernel/core_pattern</filename> file which can contain a template used to name core dump files. The content of this file will be overwritten to:
</para>
- <programlisting language="Bash">|/usr/libexec/abrt-hook-ccpp %s %c %p %u %g %t e</programlisting>
+ <programlisting language="Bash">|/usr/libexec/abrt-hook-ccpp %s %c %p %u %g %t e %i</programlisting>
</warning>
<para>
Finally, if you run ABRT in a graphical desktop environment, you can verify that the <systemitem class="service">ABRT notification applet</systemitem> 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 <sys/utsname.h>
#include "libabrt.h"
+#ifdef ENABLE_DUMP_TIME_UNWIND
+#include <satyr/abrt.h>
+#include <satyr/utils.h>
+#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<signal.h>\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<signal.h>\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/<pid> of crashing
--
2.1.0

124
0055-abrt-install-ccpp-hook-check-configuration.patch Normal file
View file

@ -0,0 +1,124 @@
From 8f6a49db8997b1296eafa639920e039f87fb1989 Mon Sep 17 00:00:00 2001
From: Martin Milata <mmilata@redhat.com>
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 <mmilata@redhat.com>
---
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

309
0056-ccpp-hook-move-proc-pid-utils-to-libreport.patch Normal file
View file

@ -0,0 +1,309 @@
From 105ff10f12dce019ca2a455001239967c2e0e856 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

166
0057-ccpp-hook-move-utility-functions-to-hooklib.patch Normal file
View file

@ -0,0 +1,166 @@
From 8e8d2edd5c5eac60b33cb36cc6012d076ddb9b13 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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 <satyr/utils.h>
#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

29
0058-core-use-updated-dump_fd_info.patch Normal file
View file

@ -0,0 +1,29 @@
From 0d9f9bba421c8271e52b67320b161c60bcf0151d Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

28
0060-abrtd-Don-t-allow-users-to-list-problems-by-hand.patch Normal file
View file

@ -0,0 +1,28 @@
From 2624beb1e91b4de9286c6e4220c63cc2157d868b Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

37
0061-retrace-client-stop-failing-on-SSL2.patch Normal file
View file

@ -0,0 +1,37 @@
From 9c1e3a75c81aa498231e59b1997a2408c580b879 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

115
0062-dbus-add-a-new-method-GetProblemData.patch Normal file
View file

@ -0,0 +1,115 @@
From 96a53a3d949142e1a401a0eaac1d09b30476e78c Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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[] =
" <arg type='s' name='problem_dir' direction='in'/>"
" <arg type='s' name='name' direction='in'/>"
" </method>"
+ " <method name='GetProblemData'>"
+ " <arg type='s' name='problem_dir' direction='in'/>"
+ " <arg type='a{s(its)}' name='problem_data' direction='out'/>"
+ " </method>"
" <method name='ChownProblemDir'>"
" <arg type='s' name='problem_dir' direction='in'/>"
" </method>"
@@ -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

38
0063-doc-add-documentation-for-GetProblemData.patch Normal file
View file

@ -0,0 +1,38 @@
From 7af709b3054bafc118d216c59b2d2c9ed49ce31c Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Mon, 16 Mar 2015 14:13:14 +0100
Subject: [PATCH] doc: add documentation for GetProblemData
For cockpit-abrt.
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
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():
</arg>
</method>
+ <method name='GetProblemData'>"
+ <tp:docstring>Returns problem's data.</tp:docstring>
+
+ <arg type='s' name='problem_dir' direction='in'>
+ <tp:docstring>An identifier of problem.</tp:docstring>
+ </arg>
+
+ <arg type='a{sits}' name='problem_data' direction='out'>
+ <tp:docstring>A dictionary where keys are element names and values are triplets (element libreport flags, element size, element contents).</tp:docstring>
+ </arg>
+ </method>
+
<method name='ChownProblemDir'>
<tp:docstring>Assures ownership of a specified problem for the caller.</tp:docstring>
--
2.1.0

98
0064-applet-get-the-list-of-problems-through-D-Bus-servic.patch Normal file
View file

@ -0,0 +1,98 @@
From b6ed011ee153bb698d176019e9c2fbedeacad5fa Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

88
0065-libabrt-add-new-function-fetching-full-problem-data-.patch Normal file
View file

@ -0,0 +1,88 @@
From 6ba08b9f28343da5c6102833d1a5062475f09468 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

110
0066-dbus-add-new-method-to-test-existence-of-an-element.patch Normal file
View file

@ -0,0 +1,110 @@
From 502491485b3c1c69120c1413b18f7fd55f6a94c7 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
.../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():
</arg>
</method>
+ <method name='TestElementExists'>
+ <tp:docstring>Checks whether the element exists.</tp:docstring>
+
+ <arg type='s' name='problem_dir' direction='in'>
+ <tp:docstring>An identifier of problem.</tp:docstring>
+ </arg>
+
+ <arg type='s' name='name' direction='in'>
+ <tp:docstring>A name of checked element.</tp:docstring>
+ </arg>
+
+ <arg type='b' name='response' direction='out'>
+ <tp:docstring>True if the element exists; otherwise false.</tp:docstring>
+ </arg>
+ </method>
+
<method name='GetProblemData'>"
<tp:docstring>Returns problem's data.</tp:docstring>
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[] =
" <arg type='s' name='problem_dir' direction='in'/>"
" <arg type='s' name='name' direction='in'/>"
" </method>"
+ " <method name='TestElementExists'>"
+ " <arg type='s' name='problem_dir' direction='in'/>"
+ " <arg type='s' name='name' direction='in'/>"
+ " <arg type='b' name='response' direction='out'/>"
+ " </method>"
" <method name='GetProblemData'>"
" <arg type='s' name='problem_dir' direction='in'/>"
" <arg type='a{s(its)}' name='problem_data' direction='out'/>"
@@ -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

129
0067-libabrt-add-wrappers-TestElemeExists-and-GetInfo-for.patch Normal file
View file

@ -0,0 +1,129 @@
From 96ff7e247ca0f767b0f24f109d9248101ee6baa2 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

347
0068-cli-use-the-DBus-methods-for-getting-problem-informa.patch Normal file
View file

@ -0,0 +1,347 @@
From 739b52102b162209d3add0988e829f4409971a3c Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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, &param);
- 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 <sys/types.h>
#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

49
0069-cli-status-don-t-return-0-if-there-is-a-problem-olde.patch Normal file
View file

@ -0,0 +1,49 @@
From 491668f3d83f763a11111151a3fc1f3d78cec3ab Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

144
0070-upload-validate-and-sanitize-uploaded-dump-directori.patch Normal file
View file

@ -0,0 +1,144 @@
From a8ee7db7bdd391154918a9fef814d315f3a092b6 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

362
0071-applet-switch-to-D-Bus-methods.patch Normal file
View file

@ -0,0 +1,362 @@
From b1d43d73cce12980017dc5fa773d88ef72ec432b Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

30
0072-cli-do-not-exit-with-segfault-if-dbus-fails.patch Normal file
View file

@ -0,0 +1,30 @@
From 3d46d2a1326c5cb8a443db4e3a1b744482ddca94 Mon Sep 17 00:00:00 2001
From: Matej Habrnal <mhabrnal@redhat.com>
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 <mhabrnal@redhat.com>
---
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

60
0074-lib-add-new-kernel-taint-flags.patch Normal file
View file

@ -0,0 +1,60 @@
From 5484c326298f5cfab97553154398e805059a1756 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Tue, 5 May 2015 10:46:06 +0200
Subject: [PATCH] lib: add new kernel taint flags
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
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

29
0075-a-a-s-p-d-add-new-known-interpreter-to-conf-file.patch Normal file
View file

@ -0,0 +1,29 @@
From 4fca9db63a9674f5d64a519a7594eb0cd5649574 Mon Sep 17 00:00:00 2001
From: Matej Habrnal <mhabrnal@redhat.com>
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 <mhabrnal@redhat.com>
---
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

71
0076-doc-update-abrt-cli-man-page.patch Normal file
View file

@ -0,0 +1,71 @@
From 75d3d3bbc5cf7cddbac6e8c4b7c880dd76bb8d5c Mon Sep 17 00:00:00 2001
From: Matej Habrnal <mhabrnal@redhat.com>
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 <mhabrnal@redhat.com>
---
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

130
0078-turn-off-exploring-crashed-process-s-root-directorie.patch Normal file
View file

@ -0,0 +1,130 @@
From 2b1fa2caf4ef3cb253e931f53b43fc9499661da4 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

93
0079-ccpp-fix-symlink-race-conditions.patch Normal file
View file

@ -0,0 +1,93 @@
From 506b867ad956fe28efef0b80b53a26ed0258abef Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

39
0080-ccpp-stop-reading-hs_error.log-from-tmp.patch Normal file
View file

@ -0,0 +1,39 @@
From 1d09c7271eae9aee2fae62367c9a83b30a685c69 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

52
0081-ccpp-postpone-changing-ownership-of-new-dump-directo.patch Normal file
View file

@ -0,0 +1,52 @@
From ccbab90e154f7917178cc1d56d8990b01ea45023 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Wed, 15 Apr 2015 15:27:09 +0200
Subject: [PATCH] ccpp: postpone changing ownership of new dump directories
Florian Weimer <fweimer@redhat.com>:
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 <jfilak@redhat.com>
---
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

31
0082-ccpp-create-dump-directory-without-parents.patch Normal file
View file

@ -0,0 +1,31 @@
From a691dd91f561bb32367ecab510930767871137c6 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

82
0083-ccpp-do-not-override-existing-files-by-compat-cores.patch Normal file
View file

@ -0,0 +1,82 @@
From 0516cbbb356ca0bfd3faf7accb36c60fc5da89d7 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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 <satyr/utils.h>
#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) <fail>, 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

269
0084-ccpp-do-not-use-value-of-proc-PID-cwd-for-chdir.patch Normal file
View file

@ -0,0 +1,269 @@
From 49279f4030225cee166362d7222e97496022331f Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

59
0085-ccpp-make-saving-of-binary-more-robust.patch Normal file
View file

@ -0,0 +1,59 @@
From ce7d0e05de76ae5383bcdfdba3ffa8816ffb63bd Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

61
0086-ccpp-harden-dealing-with-UID-GID.patch Normal file
View file

@ -0,0 +1,61 @@
From e38774dea8d0a23b952a423b4e7a946f0f570149 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

30
0087-ccpp-check-for-overflow-in-abrt-coredump-path-creati.patch Normal file
View file

@ -0,0 +1,30 @@
From 3d9e235072f6d219181a12b003112d5315544649 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

183
0088-ccpp-emulate-selinux-for-creation-of-compat-cores.patch Normal file
View file

@ -0,0 +1,183 @@
From cfc1a5da0a0f2273e0ce39da0c2fa81053ed0eaa Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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 <sys/utsname.h>
#include "libabrt.h"
+#include <selinux/selinux.h>
#ifdef ENABLE_DUMP_TIME_UNWIND
#include <satyr/abrt.h>
@@ -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

28
0090-ccpp-avoid-overriding-system-files-by-coredump.patch Normal file
View file

@ -0,0 +1,28 @@
From de3b8b654d4962e1fa2d7b068644beeed7b0826d Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

47
0091-configure-move-the-default-dump-location-to-var-spoo.patch Normal file
View file

@ -0,0 +1,47 @@
From b0f5fad5d92a8df57c9cf1e92e06d9ed60e49537 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

54
0093-daemon-use-libreport-s-function-checking-file-name.patch Normal file
View file

@ -0,0 +1,54 @@
From 8ff7f7f65cf871b889c3a9a53cd1a432c63d2180 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

312
0094-lib-add-functions-validating-dump-dir.patch Normal file
View file

@ -0,0 +1,312 @@
From 427b1c2b9b03c1fe52edb8a129e821a9ea2e0bea Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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 <assert.h>
+
+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

127
0095-dbus-process-only-valid-sub-directories-of-the-dump-.patch Normal file
View file

@ -0,0 +1,127 @@
From c5cd9d25473e2fea29f7fa609c81ae821e50f118 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

390
0096-dbus-avoid-race-conditions-in-tests-for-dum-dir-avai.patch Normal file
View file

@ -0,0 +1,390 @@
From 0bfb03ea68eb745172feccfc0f01b2ad13ff33bb Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Fri, 24 Apr 2015 13:48:32 +0200
Subject: [PATCH] dbus: avoid race-conditions in tests for dum dir availability
Florian Weimer <fweimer@redhat.com>
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 <jfilak@redhat.com>
---
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

92
0097-dbus-report-invalid-element-names.patch Normal file
View file

@ -0,0 +1,92 @@
From 23c800077fb6e821d54080ccc5d1258f37fcd8d4 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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", &timestamp_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

212
0098-a-a-i-d-t-a-cache-sanitize-arguments.patch Normal file
View file

@ -0,0 +1,212 @@
From c8ab547f17910391d0cd66a9c6f1917c295480db Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

31
0099-a-a-i-d-t-a-cache-sanitize-umask.patch Normal file
View file

@ -0,0 +1,31 @@
From 35fe31aceb8221fd8bd8ea8d48d1bb4f0fbdf837 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

102
0100-ccpp-revert-the-UID-GID-changes-if-user-core-fails.patch Normal file
View file

@ -0,0 +1,102 @@
From f51412944254a5abcbd1458e0b4bfa9d876e2fa9 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
Date: Mon, 4 May 2015 13:23:43 +0200
Subject: [PATCH] ccpp: revert the UID/GID changes if user core fails
Thanks Florian Weimer <fweimer@redhat.com>
Signed-off-by: Jakub Filak <jfilak@redhat.com>
---
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

58
0101-daemon-harden-against-race-conditions-in-DELETE.patch Normal file
View file

@ -0,0 +1,58 @@
From f1188d8857f2c9773156890d0037296f5361b0bf Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

65
0102-daemon-allow-only-root-user-to-trigger-the-post-crea.patch Normal file
View file

@ -0,0 +1,65 @@
From ee698cf90dc5be666a115db6db245de6812e6ee2 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

127
0103-daemon-dbus-allow-only-root-to-create-CCpp-Koops-vmc.patch Normal file
View file

@ -0,0 +1,127 @@
From 4e85328fd73b0d61fb82b535a7d2d8b642b3f95f Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <fweimer@redhat.com>:
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 <jfilak@redhat.com>
---
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

44
0104-koops-don-t-save-dmesg-if-kernel.dmesg_restrict-1.patch Normal file
View file

@ -0,0 +1,44 @@
From 31e54ec8ea43d246e745a3a58fc5f07bfce4dfa0 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

47
0105-ccpp-include-the-system-logs-only-with-root-s-coredu.patch Normal file
View file

@ -0,0 +1,47 @@
From 52b7072c2c821fcf7d132967a03a2086d4621069 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

118
0106-ccpp-do-not-unlink-failed-and-big-user-cores.patch Normal file
View file

@ -0,0 +1,118 @@
From e79b543156aec759542ca7aa01d727aea548f833 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

197
0107-hooks-use-root-for-owner-of-all-dump-directories.patch Normal file
View file

@ -0,0 +1,197 @@
From f15b6e0f53ceb5ba450e93e406272ffe8a398cbd Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

95
0108-cli-chown-before-reporting.patch Normal file
View file

@ -0,0 +1,95 @@
From b68f8ea31526275127a9f6ae5c8a24fc24b6d664 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

50
0110-cli-exit-with-the-number-of-unreported-problems.patch Normal file
View file

@ -0,0 +1,50 @@
From 69843667275dd3de2381635061eda19eacb07e23 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

82
0111-ccpp-don-t-save-the-system-logs-by-default.patch Normal file
View file

@ -0,0 +1,82 @@
From 65821e8e792a253e5baa1d3633ff115702769b84 Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

229
0112-vmcore-use-libreport-dd-API-in-the-harvestor.patch Normal file
View file

@ -0,0 +1,229 @@
From fcd6406bdb8e0b0005725ae88c03979e62e740ee Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

37
0113-ccpp-correct-capitalization-of-log-labels.patch Normal file
View file

@ -0,0 +1,37 @@
From ee80fb98f0c7995e1e4f8b0aae13c23ede012bea Mon Sep 17 00:00:00 2001
From: Jakub Filak <jfilak@redhat.com>
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 <jfilak@redhat.com>
---
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

95
abrt.spec
View file

@ -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 <mhabrnal@redhat.com> - 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 <mhabrnal@redhat.com> - 2.3.0-4
- make gdb aware of the abrt's debuginfo dir
- python: load the configuration from correct file