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 16:04:14 UTC
[1/4] mesos git commit: Updated CHANGELOG with fetcher user changes.
Repository: mesos
Updated Branches:
refs/heads/1.0.x 34ba5f417 -> 6cd20e9f3
Updated CHANGELOG with fetcher user changes.
The fetcher was changed to assume the same user as a task when fetching
files. This patch updates the CHANGELOG accordingly.
Review: https://reviews.apache.org/r/50283/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/044797a6
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/044797a6
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/044797a6
Branch: refs/heads/1.0.x
Commit: 044797a6ad489a00d05dfb646481e19d649a330a
Parents: 37e06d4
Author: Greg Mann <gr...@mesosphere.io>
Authored: Thu Jul 21 09:02:05 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Jul 21 09:03:38 2016 -0700
----------------------------------------------------------------------
CHANGELOG | 1 +
1 file changed, 1 insertion(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/044797a6/CHANGELOG
----------------------------------------------------------------------
diff --git a/CHANGELOG b/CHANGELOG
index e4f91cb..f947aa8 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -411,6 +411,7 @@ All Issues:
* [MESOS-5794] - Agent's /containers endpoint should skip terminated executors.
* [MESOS-5799] - docker::inspect() may get wrong output when a docker container is not in "running" state
* [MESOS-5806] - CNI isolator should prepare network related /etc/* files for containers using host mode but specify container images
+ * [MESOS-5845] - The fetcher can access any local file as root
* [MESOS-5863] - Enabling SSL causes fetcher fail to fetch from HTTPS sites.
** Documentation
[4/4] mesos git commit: Updated upgrades.md with fetcher user changes.
Posted by ji...@apache.org.
Updated upgrades.md with fetcher user changes.
The fetcher was changed to assume the same user as a task when fetching
files. This patch updates upgrades.md accordingly.
Review: https://reviews.apache.org/r/50284/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/6cd20e9f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/6cd20e9f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/6cd20e9f
Branch: refs/heads/1.0.x
Commit: 6cd20e9f32fd27bc7fcdb90c459fec5928d63396
Parents: 044797a
Author: Greg Mann <gr...@mesosphere.io>
Authored: Thu Jul 21 09:02:12 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Jul 21 09:03:39 2016 -0700
----------------------------------------------------------------------
docs/upgrades.md | 5 +++++
1 file changed, 5 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/6cd20e9f/docs/upgrades.md
----------------------------------------------------------------------
diff --git a/docs/upgrades.md b/docs/upgrades.md
index 1c871c4..80aa8f4 100644
--- a/docs/upgrades.md
+++ b/docs/upgrades.md
@@ -54,6 +54,7 @@ We categorize the changes as follows:
<li>R <a href="#1-0-x-executor-environment-variables">Executor environment variables inheritance</a></li>
<li>R <a href="#1-0-x-deprecated-fields-in-container-config">Deprecated fields in ContainerConfig</a></li>
<li>C <a href="#1-0-x-persistent-volume-ownership">Persistent volume ownership</a></li>
+ <li>C <a href="#1-0-x-fetcher-user">Fetcher assumes same user as task</a></li>
</ul>
</td>
<td style="word-wrap: break-word; overflow-wrap: break-word;"><!--Flags-->
@@ -272,6 +273,10 @@ We categorize the changes as follows:
* Namespace and header file of `Allocator` has been moved to be consistent with other packages.
+<a name="1-0-x-fetcher-user"></a>
+
+* When a task is run as a particular user, the fetcher now fetches files as that user also. Note, this means that filesystem permissions for that user will be enforced when fetching local files.
+
<a name="1-0-x-endpoint-authorization"></a>
* Mesos 1.0 introduces authorization support for several HTTP endpoints. Note that some of these endpoints are used by the web UI, and thus using the web UI in a cluster with authorization enabled will require that ACLs be set appropriately. Please refer to the [authorization documentation](authorization.md) for details.
[2/4] mesos git commit: Made the agent fetch files as the task user.
Posted by 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");
[3/4] 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/a93b09bb
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a93b09bb
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a93b09bb
Branch: refs/heads/1.0.x
Commit: a93b09bbfddacb45f46e58aa0f42dc43ba06f49b
Parents: 34ba5f4
Author: Greg Mann <gr...@mesosphere.io>
Authored: Wed Jul 20 17:10:44 2016 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Thu Jul 21 09:03:38 2016 -0700
----------------------------------------------------------------------
src/launcher/fetcher.cpp | 54 ++++++++++++++++++++++++++++++++++++++-----
1 file changed, 48 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/a93b09bb/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()) :