You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ti...@apache.org on 2017/02/23 02:38:01 UTC
[1/2] mesos git commit: Fixed fetcher to not pick up environment
variables it should not see.
Repository: mesos
Updated Branches:
refs/heads/master 020b37ee9 -> 209f8e7fd
Fixed fetcher to not pick up environment variables it should not see.
Review: https://reviews.apache.org/r/56711/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/916a43e2
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/916a43e2
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/916a43e2
Branch: refs/heads/master
Commit: 916a43e2cb2cb1c26889fa3ce86633718789d274
Parents: 020b37e
Author: Till Toenshoff <to...@me.com>
Authored: Thu Feb 23 01:53:42 2017 +0100
Committer: Till Toenshoff <to...@me.com>
Committed: Thu Feb 23 01:53:42 2017 +0100
----------------------------------------------------------------------
src/slave/containerizer/fetcher.cpp | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/916a43e2/src/slave/containerizer/fetcher.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/fetcher.cpp b/src/slave/containerizer/fetcher.cpp
index 9ec38dc..34a5fbb 100644
--- a/src/slave/containerizer/fetcher.cpp
+++ b/src/slave/containerizer/fetcher.cpp
@@ -22,8 +22,10 @@
#include <process/dispatch.hpp>
#include <process/owned.hpp>
+#include <stout/hashset.hpp>
#include <stout/net.hpp>
#include <stout/path.hpp>
+#include <stout/strings.hpp>
#ifdef __WINDOWS__
#include <stout/windows.hpp>
#endif // __WINDOWS__
@@ -45,6 +47,8 @@ using std::string;
using std::transform;
using std::vector;
+using strings::startsWith;
+
using mesos::fetcher::FetcherInfo;
using process::async;
@@ -822,12 +826,27 @@ Future<Nothing> FetcherProcess::run(
// We pass arguments to the fetcher program by means of an
// environment variable.
- map<string, string> environment = os::environment();
-
- // The libprocess port is explicitly removed because this will conflict
- // with the already-running agent.
- environment.erase("LIBPROCESS_PORT");
- environment.erase("LIBPROCESS_ADVERTISE_PORT");
+ // For assuring that we pass on variables that may be consumed by
+ // the mesos-fetcher, we whitelist them before masking out any
+ // unwanted agent->fetcher environment spillover.
+ // TODO(tillt): Consider using the `mesos::internal::logging::Flags`
+ // to determine the whitelist.
+ const hashset<string> whitelist = {
+ "MESOS_EXTERNAL_LOG_FILE",
+ "MESOS_INITIALIZE_DRIVER_LOGGING",
+ "MESOS_LOG_DIR",
+ "MESOS_LOGBUFSECS",
+ "MESOS_LOGGING_LEVEL",
+ "MESOS_QUIET"
+ };
+
+ map<string, string> environment;
+ foreachpair (const string& key, const string& value, os::environment()) {
+ if (whitelist.contains(strings::upper(key)) ||
+ (!startsWith(key, "LIBPROCESS_") && !startsWith(key, "MESOS_"))) {
+ environment.emplace(key, value);
+ }
+ }
environment["MESOS_FETCHER_INFO"] = stringify(JSON::protobuf(info));
[2/2] mesos git commit: Added regression test against fetcher SSL
spillover.
Posted by ti...@apache.org.
Added regression test against fetcher SSL spillover.
Review: https://reviews.apache.org/r/56771/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/209f8e7f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/209f8e7f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/209f8e7f
Branch: refs/heads/master
Commit: 209f8e7fd1c9da1140976748f85d05a426848e0e
Parents: 916a43e
Author: Till Toenshoff <to...@me.com>
Authored: Thu Feb 23 01:53:50 2017 +0100
Committer: Till Toenshoff <to...@me.com>
Committed: Thu Feb 23 01:53:50 2017 +0100
----------------------------------------------------------------------
src/tests/fetcher_tests.cpp | 61 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/209f8e7f/src/tests/fetcher_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/fetcher_tests.cpp b/src/tests/fetcher_tests.cpp
index 9c7e8b9..c4854b9 100644
--- a/src/tests/fetcher_tests.cpp
+++ b/src/tests/fetcher_tests.cpp
@@ -1088,6 +1088,67 @@ TEST_F(FetcherTest, HdfsURI)
}
#endif // __WINDOWS__
+
+// Regression test against unwanted environment inheritance from the
+// agent towards the fetcher. By supplying an invalid SSL setup, we
+// force the fetcher to fail if the parent process does not filter
+// them out.
+TEST_F_TEMP_DISABLED_ON_WINDOWS(FetcherTest, SSLEnvironmentSpillover)
+{
+ // Patch some critical libprocess environment variables into the
+ // parent process of the mesos-fetcher. We expect this test to fail
+ // when the code path triggered does not filter them.
+ char* enabled = getenv("LIBPROCESS_SSL_ENABLED");
+ char* key = getenv("LIBPROCESS_SSL_KEY_FILE");
+
+ os::setenv("LIBPROCESS_SSL_ENABLED", "true");
+ os::unsetenv("LIBPROCESS_SSL_KEY_FILE");
+
+ // First construct a temporary file that can be fetched and archived with
+ // gzip.
+ Try<string> dir = os::mkdtemp(path::join(os::getcwd(), "XXXXXX"));
+ ASSERT_SOME(dir);
+
+ Try<string> path = os::mktemp(path::join(dir.get(), "XXXXXX"));
+ ASSERT_SOME(path);
+
+ ASSERT_SOME(os::write(path.get(), "hello world"));
+ ASSERT_SOME(os::shell("gzip " + path.get()));
+
+ ContainerID containerId;
+ containerId.set_value(UUID::random().toString());
+
+ CommandInfo commandInfo;
+ CommandInfo::URI* uri = commandInfo.add_uris();
+ uri->set_value(path.get() + ".gz");
+ uri->set_extract(true);
+
+ slave::Flags flags;
+ flags.launcher_dir = getLauncherDir();
+
+ Fetcher fetcher;
+ SlaveID slaveId;
+
+ Future<Nothing> fetch = fetcher.fetch(
+ containerId, commandInfo, os::getcwd(), None(), slaveId, flags);
+
+ // The mesos-fetcher runnable will fail initializing libprocess if
+ // the SSL environment spilled over. Such failure would cause it to
+ // abort and exit and that in turn would fail the `fetch` returned
+ // future.
+ AWAIT_READY(fetch);
+
+ if (enabled != nullptr) {
+ os::setenv("LIBPROCESS_SSL_ENABLED", enabled);
+ } else {
+ os::unsetenv("LIBPROCESS_SSL_ENABLED");
+ }
+
+ if (key != nullptr) {
+ os::setenv("LIBPROCESS_SSL_KEY_FILE", key);
+ }
+}
+
} // namespace tests {
} // namespace internal {
} // namespace mesos {