add support for guarding pipestream with NO_NEW_PRIVS
authorPhilipp Gesang <philipp.gesang@intra2net.com>
Tue, 26 Jun 2018 07:38:56 +0000 (09:38 +0200)
committerPhilipp Gesang <philipp.gesang@intra2net.com>
Tue, 14 Aug 2018 15:12:25 +0000 (17:12 +0200)
Add an option to the pipestream and related APIs to drop the
right to obtain further privileges before exec()ing the binary
(off by default). This may be used as an additional measure to
guard invocations of untrusted binaries or trusted ones that
operate on untrusted inputs.

Target audience: arnied scheduler, everywhere file(1) or
imagemagick tools are called.

Defects: it will be tricky to properly unit test this.

[0] https://www.kernel.org/doc/Documentation/prctl/no_new_privs.txt

src/pipestream.cpp
src/pipestream.hxx
test/test_pipestream.cpp

index d3f1d3b..a5e4a00 100644 (file)
@@ -312,6 +312,18 @@ inpipebuf::init_without_shell (const char *const *argv,
              */
             fcntl (pipefd [1], F_SETFD, 0);
 
+            /*
+             * Prevent the child from receiving more privileges than the
+             * parent. This concerns mainly suid binaries.
+             */
+            errno = 0;
+            if (   flags & capture_flag::no_new_privs
+                && prctl (PR_SET_NO_NEW_PRIVS,  1, 0, 0, 0) == -1)
+            {
+                (void)write (errfd [1], (char *)&errno, sizeof(errno));
+                exit (EXIT_FAILURE);
+            }
+
             int save_errno = 0;
 
             /*
index 0aed8a4..e22479b 100644 (file)
@@ -27,6 +27,7 @@ on this file might be covered by the GNU General Public License.
 #ifndef _PIPESTREAM
 #define _PIPESTREAM
 
+#include <sys/prctl.h>
 #include <stdio.h>
 
 #include <cstring>
@@ -75,6 +76,7 @@ namespace capture_flag {
     static const int collect_err    = 1 << 1;
     static const int search_path    = 1 << 2;
     static const int env_passthru   = 1 << 3;
+    static const int no_new_privs   = 1 << 4;
 
     static const int dflt           = collect_out;
     static const int collect_any    = collect_out | collect_err;
index 974f1a4..16e7635 100644 (file)
@@ -393,5 +393,32 @@ BOOST_AUTO_TEST_CASE(env_nil)
 
 BOOST_AUTO_TEST_SUITE_END() /* [pipestream->env] */
 
+BOOST_AUTO_TEST_SUITE(privs)
+
+#define I2N_EXPECT_OUTPUT "the caged whale knows nothing of the mighty deeps"
+const char *const echo_argv [] = { "echo", I2N_EXPECT_OUTPUT, NULL };
+
+/*
+ * this is not as such a functionality test, in the sense that
+ * we can’t easily (let alone portably) test the behavior of suid
+ * binaries. thus we only check that the option is indeed accepted
+ * in a trivial case.
+ */
+BOOST_AUTO_TEST_CASE(no_new_privs)
+{
+    ExecResult exres = ExecResult ();
+
+    const std::string result = capture_exec (echo_argv, exres,
+                                               capture_flag::collect_out
+                                             | capture_flag::search_path
+                                             | capture_flag::no_new_privs);
+
+    BOOST_CHECK(exres.normal_exit);
+    BOOST_CHECK_EQUAL(exres.return_code, EXIT_SUCCESS);
+    BOOST_CHECK_EQUAL(result, (std::string)I2N_EXPECT_OUTPUT + "\n");
+}
+
+BOOST_AUTO_TEST_SUITE_END() /* [pipestream->privs] */
+
 BOOST_AUTO_TEST_SUITE_END() /* [pipestream] */