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/07/21 00:14:29 UTC
[1/2] mesos git commit: Made the agent fetch files as the task user.
Repository: mesos
Updated Branches:
refs/heads/master 79de97c4a -> 0fde47699
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/0fde4769
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0fde4769
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0fde4769
Branch: refs/heads/master
Commit: 0fde476992036488bf5c373aba1297dfaa37be0e
Parents: 73d9bd2
Author: Greg Mann <gr...@mesosphere.io>
Authored: Wed Jul 20 17:13:58 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Jul 20 17:14:25 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/0fde4769/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/0fde4769/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");
[2/2] mesos git commit: Refactored fetcher cache directory creation.
Posted by ji...@apache.org.
Refactored fetcher cache directory creation.
The fetcher's launcher creates the fetcher cache directory for each user
immediately before an artifact is fetched. In order to allow this
directory to be created by a different user than the user doing the
fetching, this patch factors out this directory creation and places it
before all fetches occur.
Review: https://reviews.apache.org/r/50199/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/73d9bd22
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/73d9bd22
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/73d9bd22
Branch: refs/heads/master
Commit: 73d9bd22df0d448eead124bb7e3272b6c1150eaf
Parents: 79de97c
Author: Greg Mann <gr...@mesosphere.io>
Authored: Wed Jul 20 17:10:44 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Wed Jul 20 17:14:25 2016 -0700
----------------------------------------------------------------------
src/launcher/fetcher.cpp | 54 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 48 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/73d9bd22/src/launcher/fetcher.cpp
----------------------------------------------------------------------
diff --git a/src/launcher/fetcher.cpp b/src/launcher/fetcher.cpp
index 0539b01..94b522d 100644
--- a/src/launcher/fetcher.cpp
+++ b/src/launcher/fetcher.cpp
@@ -378,15 +378,12 @@ static Try<string> fetchThroughCache(
CHECK_NE(FetcherInfo::Item::BYPASS_CACHE, item.action())
<< "Unexpected fetcher action selector";
+ CHECK(os::exists(cacheDirectory.get()))
+ << "Fetcher cache directory was expected to exist but was not found";
+
if (item.action() == FetcherInfo::Item::DOWNLOAD_AND_CACHE) {
LOG(INFO) << "Downloading into cache";
- Try<Nothing> mkdir = os::mkdir(cacheDirectory.get());
- if (mkdir.isError()) {
- return Error("Failed to create fetcher cache directory '" +
- cacheDirectory.get() + "': " + mkdir.error());
- }
-
Try<string> downloaded = download(
item.uri().value(),
path::join(cacheDirectory.get(), item.cache_filename()),
@@ -426,6 +423,45 @@ static Try<string> fetch(
}
+// Checks to see if it's necessary to create a fetcher cache directory for this
+// user, and creates it if so.
+static Try<Nothing> createCacheDirectory(const FetcherInfo& fetcherInfo)
+{
+ if (!fetcherInfo.has_cache_directory()) {
+ return Nothing();
+ }
+
+ foreach (const FetcherInfo::Item& item, fetcherInfo.items()) {
+ if (item.action() != FetcherInfo::Item::BYPASS_CACHE) {
+ // If this user has fetched anything into the cache before, their cache
+ // directory will already exist. Set `recursive = true` when calling
+ // `os::mkdir` to ensure no error is returned in this case.
+ Try<Nothing> mkdir = os::mkdir(fetcherInfo.cache_directory(), true);
+ if (mkdir.isError()) {
+ return mkdir;
+ }
+
+ if (fetcherInfo.has_user()) {
+ // Fetching is performed as the task's user,
+ // so chown the cache directory.
+ Try<Nothing> chown = os::chown(
+ fetcherInfo.user(),
+ fetcherInfo.cache_directory(),
+ false);
+
+ if (chown.isError()) {
+ return chown;
+ }
+ }
+
+ break;
+ }
+ }
+
+ return Nothing();
+}
+
+
// This "fetcher program" is invoked by the slave's fetcher actor
// (Fetcher, FetcherProcess) to "fetch" URIs into the sandbox directory
// of a given task. Its parameters are provided in the form of the env
@@ -473,6 +509,12 @@ int main(int argc, char* argv[])
const string sandboxDirectory = fetcherInfo.get().sandbox_directory();
+ Try<Nothing> result = createCacheDirectory(fetcherInfo.get());
+ if (result.isError()) {
+ EXIT(EXIT_FAILURE)
+ << "Could not create the fetcher cache directory: " << result.error();
+ }
+
const Option<string> cacheDirectory =
fetcherInfo.get().has_cache_directory() ?
Option<string>::some(fetcherInfo.get().cache_directory()) :