[2/4] mesos git commit: Made the agent fetch files as the task user.

[ji...@apache.org]

Made the agent fetch files as the task user.

To ensure that a task cannot fetch root-protected files from the local
filesystem when running as a non-root user, this patch changes the
fetcher to fetch files as the task user.

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


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

Branch: refs/heads/1.0.x
Commit: 37e06d402267e2a9044dbb577c8887ad1bd9185f
Parents: a93b09b
Author: Greg Mann <gr...@mesosphere.io>
Authored: Wed Jul 20 17:13:58 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Jul 21 09:03:38 2016 -0700

----------------------------------------------------------------------
 src/launcher/fetcher.cpp    | 24 ++++++++++++------------
 src/tests/fetcher_tests.cpp | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/37e06d40/src/launcher/fetcher.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/fetcher.cpp b/src/launcher/fetcher.cpp
index 94b522d..4456c28 100644
--- a/src/launcher/fetcher.cpp
+++ b/src/launcher/fetcher.cpp
@@ -515,6 +515,17 @@ int main(int argc, char* argv[])
       << "Could not create the fetcher cache directory: " << result.error();
   }
 
+  // If the `FetcherInfo` specifies a user, use `os::su()` to fetch files as the
+  // task's user to ensure that filesystem permissions are enforced.
+  if (fetcherInfo.get().has_user()) {
+    result = os::su(fetcherInfo.get().user());
+    if (result.isError()) {
+      EXIT(EXIT_FAILURE)
+        << "Fetcher could not execute `os::su()` for user '"
+        << fetcherInfo.get().user() << "'";
+    }
+  }
+
   const Option<string> cacheDirectory =
     fetcherInfo.get().has_cache_directory() ?
       Option<string>::some(fetcherInfo.get().cache_directory()) :
@@ -525,7 +536,7 @@ int main(int argc, char* argv[])
       Option<string>::some(fetcherInfo.get().frameworks_home()) :
         Option<string>::none();
 
-  // Fetch each URI to a local file, chmod, then chown if a user is provided.
+  // Fetch each URI to a local file and chmod if necessary.
   foreach (const FetcherInfo::Item& item, fetcherInfo.get().items()) {
     Try<string> fetched =
       fetch(item, cacheDirectory, sandboxDirectory, frameworksHome);
@@ -538,16 +549,5 @@ int main(int argc, char* argv[])
     }
   }
 
-  // Recursively chown the sandbox directory if a user is provided.
-  if (fetcherInfo.get().has_user()) {
-    Try<Nothing> chowned = os::chown(
-        fetcherInfo.get().user(),
-        sandboxDirectory);
-    if (chowned.isError()) {
-      EXIT(EXIT_FAILURE)
-        << "Failed to chown " << sandboxDirectory << ": " << chowned.error();
-    }
-  }
-
   return 0;
 }

http://git-wip-us.apache.org/repos/asf/mesos/blob/37e06d40/src/tests/fetcher_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/fetcher_tests.cpp b/src/tests/fetcher_tests.cpp
index d38ce6e..aea0900 100644
--- a/src/tests/fetcher_tests.cpp
+++ b/src/tests/fetcher_tests.cpp
@@ -100,6 +100,44 @@ TEST_F(FetcherTest, FileURI)
 }
 
 
+// Tests that non-root users are unable to fetch root-protected files on the
+// local filesystem.
+TEST_F(FetcherTest, ROOT_RootProtectedFileURI)
+{
+  const string user = "nobody";
+  ASSERT_SOME(os::getuid(user));
+
+  string fromDir = path::join(os::getcwd(), "from");
+  ASSERT_SOME(os::mkdir(fromDir));
+  string testFile = path::join(fromDir, "test");
+  EXPECT_SOME(os::write(testFile, "data"));
+  EXPECT_SOME(os::chmod(testFile, 600));
+
+  slave::Flags flags;
+  flags.launcher_dir = getLauncherDir();
+
+  ContainerID containerId;
+  containerId.set_value(UUID::random().toString());
+
+  CommandInfo commandInfo;
+  commandInfo.set_user(user);
+
+  CommandInfo::URI* uri = commandInfo.add_uris();
+  uri->set_value("file://" + testFile);
+
+  Fetcher fetcher;
+  SlaveID slaveId;
+
+  AWAIT_FAILED(fetcher.fetch(
+      containerId,
+      commandInfo,
+      os::getcwd(),
+      None(),
+      slaveId,
+      flags));
+}
+
+
 TEST_F(FetcherTest, CustomOutputFileSubdirectory)
 {
   string testFile = path::join(os::getcwd(), "test");