call abrt-action-trim-files from abrt-action-install-debuginfo

This allows us to not setuid abrt-action-trim-files

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>

4 files changed, 123 insertions, 50 deletions

diff --git a/abrt.spec.in b/abrt.spec.in
index 691e3923..b23aaa6e 100644
--- a/abrt.spec.in
+++ b/abrt.spec.in
@@ -529,7 +529,7 @@ fi
 %{_initrddir}/abrt-ccpp
 %{_libexecdir}/abrt-hook-ccpp
 %{_bindir}/abrt-action-analyze-c
-%attr(4755, abrt, abrt) %{_bindir}/abrt-action-trim-files
+%{_bindir}/abrt-action-trim-files
 %attr(4755, abrt, abrt) %{_bindir}/abrt-action-install-debuginfo
 %{_bindir}/abrt-action-analyze-core.py*
 %{_bindir}/abrt-action-install-debuginfo.py*
diff --git a/src/plugins/abrt-action-install-debuginfo.c b/src/plugins/abrt-action-install-debuginfo.c
index 357762ee..112620b4 100644
--- a/src/plugins/abrt-action-install-debuginfo.c
+++ b/src/plugins/abrt-action-install-debuginfo.c
@@ -21,8 +21,7 @@
 #include <stdlib.h>
 #include <string.h>
 
-// TODO: honor configure --prefix here:
-#define EXECUTABLE "/usr/bin/abrt-action-install-debuginfo.py"
+#define EXECUTABLE "abrt-action-install-debuginfo.py"
 
 static void error_msg_and_die(const char *msg, const char *arg)
 {
@@ -30,7 +29,7 @@ static void error_msg_and_die(const char *msg, const char *arg)
     if (arg)
     {
         write(2, " '", 2);
-        write(2, msg, strlen(msg));
+        write(2, arg, strlen(arg));
         write(2, "'", 1);
     }
     write(2, "\n", 1);
@@ -47,7 +46,7 @@ int main(int argc, char **argv)
 {
     /*
      * We disallow passing of arguments which point to writable dirs.
-     * This way, the script will always use default arguments.
+     * This way, the script will always use default values for these arguments.
      */
     char **pp = argv;
     char *arg;
@@ -57,7 +56,7 @@ int main(int argc, char **argv)
             error_msg_and_die("bad option", arg);
         if (strncmp(arg, "--tmpdir", 8) == 0)
             error_msg_and_die("bad option", arg);
-        if (strncmp(arg, "-i", 2) == 0)
+        if (strncmp(arg, "--ids", 5) == 0)
             error_msg_and_die("bad option", arg);
     }
 
@@ -70,12 +69,35 @@ int main(int argc, char **argv)
         setregid(g, g);
     uid_t u = geteuid();
     if (u != getuid())
+    {
         setreuid(u, u);
+        /* We are suid'ed! */
+        /* Prevent malicious user from messing up with suid'ed process: */
+        /* Set safe PATH */
+// TODO: honor configure --prefix here by adding it to PATH
+// (otherwise abrt-action-install-debuginfo.py would fail to spawn abrt-action-trim-files):
+        if (u == 0)
+            putenv((char*) "PATH=/usr/sbin:/sbin:/usr/bin:/bin");
+        else
+            putenv((char*) "PATH=/usr/bin:/bin");
+        /* Clear dangerous stuff from env */
+        static const char forbid[] =
+            "LD_LIBRARY_PATH" "\0"
+            "LD_PRELOAD" "\0"
+            "LD_TRACE_LOADED_OBJECTS" "\0"
+            "LD_BIND_NOW" "\0"
+            "LD_AOUT_LIBRARY_PATH" "\0"
+            "LD_AOUT_PRELOAD" "\0"
+            "LD_NOWARN" "\0"
+            "LD_KEEPDIR" "\0"
+        ;
+        const char *p = forbid;
+        do {
+            unsetenv(p);
+            p += strlen(p) + 1;
+        } while (*p);
+    }
 
-    /* We use full path, and execv instead of execvp in order to
-     * disallow user to execute his own abrt-action-install-debuginfo.py
-     * in his dir by setting up corresponding malicious $PATH.
-     */
-    execv(EXECUTABLE, argv);
+    execvp(EXECUTABLE, argv);
     error_msg_and_die("Can't execute", EXECUTABLE);
 }
diff --git a/src/plugins/abrt-action-install-debuginfo.py b/src/plugins/abrt-action-install-debuginfo.py
index ab131f7c..23ce017f 100755
--- a/src/plugins/abrt-action-install-debuginfo.py
+++ b/src/plugins/abrt-action-install-debuginfo.py
@@ -1,8 +1,10 @@
 #! /usr/bin/python -u
 # -*- coding: utf-8 -*-
-
 # WARNING: python -u means unbuffered I/O without it the messages are
-# passed to the parent asynchronously which looks bad in clients..
+# passed to the parent asynchronously which looks bad in clients.
+
+PROGNAME = "abrt-action-install-debuginfo.py"
+
 from subprocess import Popen, PIPE
 import sys
 import os
@@ -35,8 +37,25 @@ def init_gettext():
     gettext.textdomain(GETTEXT_PROGNAME)
 
 
-def error_msg_and_die(s):
-    sys.stderr.write("%s\n" % s)
+verbose = 0
+def log(fmt, *args):
+    sys.stderr.write("%s\n" % (fmt % args))
+
+def log1(fmt, *args):
+    """ prints log message if verbosity >= 1 """
+    if verbose >= 1:
+        sys.stderr.write("%s\n" % (fmt % args))
+
+def log2(fmt, *args):
+    """ prints log message if verbosity >= 2 """
+    if verbose >= 2:
+        sys.stderr.write("%s\n" % (fmt % args))
+
+def error_msg(fmt, *args):
+    sys.stderr.write("%s\n" % (fmt % args))
+
+def error_msg_and_die(fmt, *args):
+    sys.stderr.write("%s\n" % (fmt % args))
     os.exit(1)
 
 
@@ -76,7 +95,7 @@ def ask_yes_no(prompt, retries=4):
 def unpack_rpm(package_nevra, files, tmp_dir, destdir, keeprpm):
     package_name = package_nevra + ".rpm"
     package_full_path = tmp_dir + "/" + package_name
-    log1("Extracting %s to %s" % (package_full_path, destdir))
+    log1("Extracting %s to %s", package_full_path, destdir)
     log2(files)
     print _("Extracting cpio from %s") % (package_full_path)
     unpacked_cpio_path = tmp_dir + "/unpacked.cpio"
@@ -92,7 +111,7 @@ def unpack_rpm(package_nevra, files, tmp_dir, destdir, keeprpm):
     if retcode == 0:
         log1("cpio written OK")
         if not keeprpm:
-            log1("keeprpms = False, removing %s" % package_full_path)
+            log1("keeprpms = False, removing %s", package_full_path)
             #print _("Removing temporary rpm file")
             os.unlink(package_full_path)
     else:
@@ -106,7 +125,7 @@ def unpack_rpm(package_nevra, files, tmp_dir, destdir, keeprpm):
     unpacked_cpio = open(unpacked_cpio_path, 'rb')
 
     print _("Caching files from %s made from %s") % ("unpacked.cpio", package_name)
-    cpio = Popen(["cpio","-i", "-d", "--quiet"],
+    cpio = Popen(["cpio", "-idu", "--quiet"],
                   stdin=unpacked_cpio, cwd=destdir, bufsize=-1)
     retcode = cpio.wait()
 
@@ -208,7 +227,7 @@ class DebugInfoDownload(YumBase):
             #print repo
             repo.enable()
             rid = self.repos.enableRepo(repo.id)
-            log1("enabled repo %s" % rid)
+            log1("enabled repo %s", rid)
             setattr(repo, "skip_if_unavailable", True)
 
         self.repos.doSetup()
@@ -241,7 +260,7 @@ class DebugInfoDownload(YumBase):
         not_found = []
         package_files_dict = {}
         for debuginfo_path in files:
-            log2("yum whatprovides %s" % debuginfo_path)
+            log2("yum whatprovides %s", debuginfo_path)
             pkg = self.pkgSack.searchFiles(debuginfo_path)
             # sometimes one file is provided by more rpms, we can use either of
             # them, so let's use the first match
@@ -254,9 +273,9 @@ class DebugInfoDownload(YumBase):
                     installed_size += float(pkg[0].installedsize)
                     total_pkgs += 1
 
-                log2("found pkg for %s: %s" % (debuginfo_path, pkg[0]))
+                log2("found pkg for %s: %s", debuginfo_path, pkg[0])
             else:
-                log2("not found pkg for %s" % debuginfo_path)
+                log2("not found pkg for %s", debuginfo_path)
                 not_found.append(debuginfo_path)
 
         # connect our progress update callback
@@ -317,17 +336,6 @@ class DebugInfoDownload(YumBase):
             except OSError:
                 print _("Can't remove %s, probably contains an error log") % self.tmpdir
 
-verbose = 0
-def log1(message):
-    """ prints log message if verbosity > 0 """
-    if verbose > 0:
-        print "LOG1:", message
-
-def log2(message):
-    """ prints log message if verbosity > 1 """
-    if verbose > 1:
-        print "LOG2:", message
-
 def build_ids_to_path(build_ids):
     """
     build_id1=${build_id:0:2}
@@ -344,14 +352,14 @@ def filter_installed_debuginfos(build_ids, cache_dir):
     files = build_ids_to_path(build_ids)
     for debuginfo_path in files:
         cache_debuginfo_path = cache_dir + debuginfo_path
-        log2("checking path: %s" % debuginfo_path)
+        log2("checking path: %s", debuginfo_path)
         if os.path.exists(debuginfo_path):
-            log2("found: %s" % debuginfo_path)
+            log2("found: %s", debuginfo_path)
             continue
         if os.path.exists(cache_debuginfo_path):
-            log2("found: %s" % cache_debuginfo_path)
+            log2("found: %s", cache_debuginfo_path)
             continue
-        log2("not found: %s" % (cache_debuginfo_path))
+        log2("not found: %s", cache_debuginfo_path)
         missing_di.append(debuginfo_path)
     return missing_di
 
@@ -380,6 +388,7 @@ if __name__ == "__main__":
     signal.signal(signal.SIGINT, sigint_handler)
     fbuild_ids = "build_ids"
     cachedir = None
+    size_mb = 4096
     tmpdir = None
     keeprpms = False
     noninteractive = False
@@ -395,12 +404,29 @@ if __name__ == "__main__":
         except:
             pass
 
-    progname = os.path.basename(sys.argv[0])
-    help_text = _("Usage: %s [-i <build_ids_file>] [--tmpdir=TMPDIR] "
-                            "[--cache=CACHEDIR] [--keeprpms]") % progname
+    PROGNAME = os.path.basename(sys.argv[0])
+    # ____________________________________________________________________________________ 7
+    # ______01234567890123456789012345678901234567890123456789012345678901234567890123456789
+    help_text = _(
+            "Usage: %s [-vy] [--ids=BUILD_IDS_FILE]\n"
+            "       [--tmpdir=TMPDIR] [--cache=CACHEDIR] [--size_mb=SIZE]\n"
+            "\n"
+            "Installs debuginfos for all build-ids listed in BUILD_IDS_FILE\n"
+            "to CACHEDIR, using TMPDIR as temporary staging area.\n"
+            "Old files in CACHEDIR are deleted until it is smaller than SIZE.\n"
+            "\n"
+            "    -v          Be verbose\n"
+            "    -y          Noninteractive, assume 'Yes' to all questions\n"
+            "    --ids       Default: build_ids\n"
+            "    --tmpdir    Default: /tmp/abrt-tmp-debuginfo-RANDOM_SUFFIX\n"
+            "    --cache     Default: /var/cache/abrt-di\n"
+            "    --size_mb   Default: 4096\n"
+            # --keeprpms is not documented yet because it's a NOP so far
+    ) % PROGNAME
+
     try:
-        opts, args = getopt.getopt(sys.argv[1:], "vyhi:", ["help", "cache=",
-                                                           "tmpdir=","keeprpms"])
+        opts, args = getopt.getopt(sys.argv[1:], "vyh",
+                ["help", "ids=", "cache=", "size_mb=", "tmpdir=", "keeprpms"])
     except getopt.GetoptError, err:
         print str(err) # prints something like "option -a not recognized"
         exit(RETURN_FAILURE)
@@ -413,13 +439,18 @@ if __name__ == "__main__":
             verbose += 1
         elif opt == "-y":
             noninteractive = True
-        elif opt == "-i":
+        elif opt == "--ids":
             fbuild_ids = arg
-        elif opt in ("--cache"):
+        elif opt == "--cache":
             cachedir = arg
-        elif opt in ("--tmpdir"):
+        elif opt == "--size_mb":
+            try:
+                size_mb = int(arg)
+            except:
+                pass
+        elif opt == "--tmpdir":
             tmpdir = arg
-        elif opt in ("--keeprpms"):
+        elif opt == "--keeprpms":
             keeprpms = True
 
     if not cachedir:
@@ -440,15 +471,37 @@ if __name__ == "__main__":
     if not b_ids:
         exit(RETURN_FAILURE)
 
+    # Delete oldest/biggest files from cachedir.
+    # (Note that we need to do it before we check for missing debuginfos)
+    #
+    # We can do it as a separate step in abrt_event.conf, but this
+    # would require setuid'ing abrt-action-trim-files to abrt:abrt.
+    # Since we (abrt-action-install-debuginfo) are already running setuid,
+    # it makes sense to NOT setuid abrt-action-trim-files too,
+    # but instead run it as our child:
+    sys.stdout.flush()
+    try:
+        pid = os.fork()
+        if pid == 0:
+            log1("abrt-action-trim-files -f %um:%s", size_mb, cachedir);
+            os.execlp("abrt-action-trim-files", "abrt-action-trim-files", "-f", "%um:%s" % (size_mb, cachedir));
+        if pid > 0:
+            os.waitpid(pid, 0);
+    except Exception, e:
+        error_msg("Can't execute abrt-action-trim-files: %s", e);
+
     missing = filter_installed_debuginfos(b_ids, cachedir)
     if missing:
         log2(missing)
         print _("Coredump references %u debuginfo files, %u of them are not installed") % (len(b_ids), len(missing))
+
+        # TODO: should we pass keep_rpms=keeprpms to DebugInfoDownload here??
         downloader = DebugInfoDownload(cache=cachedir, tmp=tmpdir)
         try:
             result = downloader.download(missing)
         except Exception, ex:
             error_msg_and_die("Can't download debuginfos: %s", ex)
+
         missing = filter_installed_debuginfos(b_ids, cachedir)
         for bid in missing:
             print _("Missing debuginfo file: %s") % bid
diff --git a/src/plugins/ccpp_events.conf b/src/plugins/ccpp_events.conf
index 1e111f04..bff2eae5 100644
--- a/src/plugins/ccpp_events.conf
+++ b/src/plugins/ccpp_events.conf
@@ -6,9 +6,8 @@ EVENT=post-create analyzer=CCpp
 # TODO: can we still specify additional directories to search for debuginfos,
 # or was this ability lost with move to python installer?
 EVENT=analyze_LocalGDB analyzer=CCpp backtrace=
-                                abrt-action-trim-files -f 4096m:/var/cache/abrt-di &&
                                 abrt-action-analyze-core.py --core=coredump -o build_ids &&
-                                abrt-action-install-debuginfo &&
+                                abrt-action-install-debuginfo --size_mb=4096 &&
                                 abrt-action-generate-backtrace &&
                                 abrt-action-analyze-backtrace
 
@@ -19,9 +18,8 @@ EVENT=analyze_RetraceServer analyzer=CCpp backtrace=
 # Same as "analyze", but executed when user requests "refresh" in GUI
 # It doesn't check that backtrace is empty:
 EVENT=reanalyze_LocalGDB analyzer=CCpp
-                                abrt-action-trim-files -f 4096m:/var/cache/abrt-di &&
                                 abrt-action-analyze-core.py --core=coredump -o build_ids &&
-                                abrt-action-install-debuginfo &&
+                                abrt-action-install-debuginfo --size_mb=4096 &&
                                 abrt-action-generate-backtrace &&
                                 abrt-action-analyze-backtrace