You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2016/08/01 18:26:29 UTC

mesos git commit: Supported non-shell command in MesosLaunch to avoid arbitrary commands.

Repository: mesos
Updated Branches:
  refs/heads/master 9182f1c39 -> 25626fcf8


Supported non-shell command in MesosLaunch to avoid arbitrary commands.

Currently all pre_exec_commands are executed as shell commands in Mesos
Launch. It is not safe because arbitrary shell command may be included
in some user facing api (e.g., container_path).  We should execute those
command as a subprocess to prevent arbitrary shell command injection.

Review: https://reviews.apache.org/r/50214/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/25626fcf
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/25626fcf
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/25626fcf

Branch: refs/heads/master
Commit: 25626fcf8f63875ed0ccfe2ddb67a9998e5ba934
Parents: 9182f1c
Author: Gilbert Song <so...@gmail.com>
Authored: Mon Aug 1 09:50:13 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Mon Aug 1 11:26:19 2016 -0700

----------------------------------------------------------------------
 src/slave/containerizer/mesos/launch.cpp | 48 ++++++++++++++++++++-------
 1 file changed, 36 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/25626fcf/src/slave/containerizer/mesos/launch.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 51f0c11..f854a9a 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -22,6 +22,8 @@
 
 #include <iostream>
 
+#include <process/subprocess.hpp>
+
 #include <stout/foreach.hpp>
 #include <stout/os.hpp>
 #include <stout/protobuf.hpp>
@@ -42,6 +44,8 @@ using std::endl;
 using std::string;
 using std::vector;
 
+using process::Subprocess;
+
 namespace mesos {
 namespace internal {
 namespace slave {
@@ -208,23 +212,43 @@ int MesosContainerizerLaunch::execute()
         return 1;
       }
 
-      // TODO(jieyu): Currently, we only accept shell commands for the
-      // preparation commands.
-      if (!parse.get().shell()) {
-        cerr << "Preparation commands need to be shell commands" << endl;
-        return 1;
-      }
-
       if (!parse.get().has_value()) {
         cerr << "The 'value' of a preparation command is not specified" << endl;
         return 1;
       }
 
-      // Block until the command completes.
-      int status = os::system(parse.get().value());
-      if (!WIFEXITED(status) || (WEXITSTATUS(status) != 0)) {
-        cerr << "Failed to execute a preparation shell command" << endl;
-        return 1;
+      Try<Subprocess> s = Error("Not launched");
+
+      if (parse->shell()) {
+        s = subprocess(parse->value(), Subprocess::PATH("/dev/null"));
+      } else {
+        // Launch non-shell command as a subprocess to avoid injecting
+        // arbitrary shell commands.
+        vector<string> args;
+        foreach (const string& arg, parse->arguments()) {
+          args.push_back(arg);
+        }
+
+        s = subprocess(parse->value(), args, Subprocess::PATH("/dev/null"));
+      }
+
+      if (s.isError()) {
+        cerr << "Failed to create the pre-exec subprocess: "
+             << s.error() << endl;
+        return EXIT_FAILURE;
+      }
+
+      s->status().await();
+
+      Option<int> status = s->status().get();
+      if (status.isNone()) {
+        cerr << "Failed to reap the pre-exec subprocess "
+             << "'" << value << "'" << endl;
+        return EXIT_FAILURE;
+      } else if (status.get() != 0) {
+        cerr << "The pre-exec subprocess '" << value << "' "
+             << "failed" << endl;
+        return EXIT_FAILURE;
       }
     }
   }