You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gi...@apache.org on 2019/02/12 19:40:31 UTC
[mesos] branch 1.6.x updated (768cfcb -> f260e9d)
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a change to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.
from 768cfcb Added MESOS-9533 to 1.6.2 CHANGELOG.
new 620bf29 Added Linux memfd support.
new aaf5b79 Made the code more robust related to sendfile.
new 447334e Added a test to test memfd file clone.
new b1469bf Updated handleWhitelistFds() to avoid closing FDs with FD_CLOEXEC bit.
new cdf042e Cloned a sealed file of launcher binary.
new 80e270f Cleaned up command executor redundant command string.
new da73d07 Added `--enable-launcher-sealing` compiler flag.
new d4852aa Enabled launcher sealing depending on corresponding compiler flag.
new da82ef9 Fixed `ROOT_PidNamespace` test after enabling launcher sealing.
new f260e9d Secured mesos executor binary using memfd.
The 10 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
3rdparty/libprocess/src/subprocess_posix.hpp | 13 +-
configure.ac | 31 +++++
src/Makefile.am | 11 ++
src/launcher/executor.cpp | 28 ++--
src/linux/memfd.cpp | 147 +++++++++++++++++++++
.../provisioner/utils.hpp => linux/memfd.hpp} | 20 +--
src/slave/constants.hpp | 3 +
src/slave/containerizer/mesos/containerizer.cpp | 57 +++++++-
src/slave/containerizer/mesos/containerizer.hpp | 30 ++++-
src/slave/containerizer/mesos/launch.cpp | 12 +-
src/slave/slave.cpp | 6 -
src/tests/containerizer/isolator_tests.cpp | 2 +-
...ities_test_helper.hpp => linux_memfd_tests.cpp} | 50 +++----
13 files changed, 351 insertions(+), 59 deletions(-)
create mode 100644 src/linux/memfd.cpp
copy src/{slave/containerizer/mesos/provisioner/utils.hpp => linux/memfd.hpp} (71%)
copy src/tests/containerizer/{capabilities_test_helper.hpp => linux_memfd_tests.cpp} (54%)
[mesos] 03/10: Added a test to test memfd file clone.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 447334e673a6fbf7a65f454399c42440bab8b8e6
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Mon Feb 11 12:51:21 2019 -0800
Added a test to test memfd file clone.
This patch added a test to test the memfd::cloneFile.
Review: https://reviews.apache.org/r/69945/
(cherry picked from commit 3139e5137142b884ddae0b07b7bbbd8ad5319c6e)
---
src/Makefile.am | 1 +
src/tests/CMakeLists.txt | 1 +
src/tests/containerizer/linux_memfd_tests.cpp | 61 +++++++++++++++++++++++++++
3 files changed, 63 insertions(+)
diff --git a/src/Makefile.am b/src/Makefile.am
index 1eb6ff0..26d874a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2638,6 +2638,7 @@ mesos_tests_SOURCES += \
tests/containerizer/cni_isolator_tests.cpp \
tests/containerizer/docker_volume_isolator_tests.cpp \
tests/containerizer/linux_filesystem_isolator_tests.cpp \
+ tests/containerizer/linux_memfd_tests.cpp \
tests/containerizer/fs_tests.cpp \
tests/containerizer/memory_pressure_tests.cpp \
tests/containerizer/nested_mesos_containerizer_tests.cpp \
diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index 758bb94..7a1cba5 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -226,6 +226,7 @@ if (LINUX)
containerizer/fs_tests.cpp
containerizer/linux_capabilities_isolator_tests.cpp
containerizer/linux_filesystem_isolator_tests.cpp
+ containerizer/linux_memfd_tests.cpp
containerizer/memory_pressure_tests.cpp
containerizer/nested_mesos_containerizer_tests.cpp
containerizer/ns_tests.cpp
diff --git a/src/tests/containerizer/linux_memfd_tests.cpp b/src/tests/containerizer/linux_memfd_tests.cpp
new file mode 100644
index 0000000..de54ac3
--- /dev/null
+++ b/src/tests/containerizer/linux_memfd_tests.cpp
@@ -0,0 +1,61 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <string>
+
+#include <stout/gtest.hpp>
+#include <stout/path.hpp>
+
+#include <stout/os/close.hpp>
+#include <stout/os/read.hpp>
+#include <stout/os/write.hpp>
+
+#include <stout/tests/utils.hpp>
+
+#include "linux/memfd.hpp"
+
+using std::string;
+
+namespace mesos {
+namespace internal {
+namespace tests {
+
+class MemfdTest : public TemporaryDirectoryTest {};
+
+
+TEST_F(MemfdTest, CloneSealedFile)
+{
+ const string content = "hello world!";
+ const string filePath = path::join(sandbox.get(), "file");
+ ASSERT_SOME(os::write(filePath, content));
+
+ Try<int_fd> fd = memfd::cloneSealedFile(filePath);
+ ASSERT_SOME(fd);
+
+ const string memFdFile = "/proc/self/fd/" + stringify(fd.get());
+
+ Try<string> actual = os::read(memFdFile);
+ EXPECT_SOME_EQ(content, actual);
+
+ // Make sure we cannot write the cloned file.
+ EXPECT_ERROR(os::write(memFdFile, "xxx"));
+
+ os::close(fd.get());
+}
+
+} // namespace tests {
+} // namespace internal {
+} // namespace mesos {
[mesos] 06/10: Cleaned up command executor redundant command string.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 80e270f08f86d672297d60001aa1ae39e8a2713e
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Mon Feb 11 12:51:26 2019 -0800
Cleaned up command executor redundant command string.
Review: https://reviews.apache.org/r/69948/
(cherry picked from commit b9970366a988aef6d735766bf770c32f50aa2bfe)
---
src/launcher/executor.cpp | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index ec12b89..e10b4ab 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -497,15 +497,6 @@ protected:
initPath = "/proc/self/fd/" + stringify(memFd.get());
#endif // __linux__
- // TODO(tillt): Consider using a flag allowing / disallowing the
- // log output of possibly sensitive data. See MESOS-7292.
- string commandString = strings::format(
- "%s %s <POSSIBLY-SENSITIVE-DATA>",
- initPath,
- MesosContainerizerLaunch::NAME).get();
-
- LOG(INFO) << "Running '" << commandString << "'";
-
// Fork the child using launcher.
vector<string> argv(2);
argv[0] = MESOS_CONTAINERIZER;
@@ -539,7 +530,7 @@ protected:
childHooks);
if (s.isError()) {
- ABORT("Failed to launch '" + commandString + "': " + s.error());
+ ABORT("Failed to launch task subprocess: " + s.error());
}
return s->pid();
[mesos] 10/10: Secured mesos executor binary using memfd.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit f260e9dc371823791d3d00479a42b3c7ce2485bc
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Mon Feb 11 12:51:32 2019 -0800
Secured mesos executor binary using memfd.
Review: https://reviews.apache.org/r/69952/
(cherry picked from commit 5c4e839baab8588485c874d05953df21c129af03)
---
src/slave/constants.hpp | 3 +++
src/slave/containerizer/mesos/containerizer.cpp | 29 ++++++++++++++++++++++---
src/slave/containerizer/mesos/containerizer.hpp | 16 ++++++++++++--
src/slave/containerizer/mesos/launch.cpp | 12 +++++++---
src/slave/slave.cpp | 6 -----
5 files changed, 52 insertions(+), 14 deletions(-)
diff --git a/src/slave/constants.hpp b/src/slave/constants.hpp
index cc029e4..10a5dd8 100644
--- a/src/slave/constants.hpp
+++ b/src/slave/constants.hpp
@@ -185,10 +185,13 @@ Duration DEFAULT_MASTER_PING_TIMEOUT();
// Name of the executable for default executor.
#ifdef __WINDOWS__
constexpr char MESOS_DEFAULT_EXECUTOR[] = "mesos-default-executor.exe";
+constexpr char MESOS_EXECUTOR[] = "mesos-executor.exe";
#else
constexpr char MESOS_DEFAULT_EXECUTOR[] = "mesos-default-executor";
+constexpr char MESOS_EXECUTOR[] = "mesos-executor";
#endif // __WINDOWS__
+
// Virtual path on which agent logs are mounted in `/files/` endpoint.
constexpr char AGENT_LOG_VIRTUAL_PATH[] = "/slave/log";
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index d195012..91cc172 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -570,6 +570,7 @@ Try<MesosContainerizer*> MesosContainerizer::create(
Owned<MesosIsolatorProcess>(ioSwitchboard.get()))));
Option<int_fd> initMemFd;
+ Option<int_fd> commandExecutorMemFd;
#ifdef ENABLE_LAUNCHER_SEALING
// Clone the launcher binary in memory for security concerns.
@@ -584,6 +585,19 @@ Try<MesosContainerizer*> MesosContainerizer::create(
}
initMemFd = memFd.get();
+
+ // Clone the command executor binary in memory for security.
+ memFd = memfd::cloneSealedFile(
+ path::join(flags.launcher_dir, MESOS_EXECUTOR));
+
+ if (memFd.isError()) {
+ return Error(
+ "Failed to clone a sealed file '" +
+ path::join(flags.launcher_dir, MESOS_EXECUTOR) + "' in memory: " +
+ memFd.error());
+ }
+
+ commandExecutorMemFd = memFd.get();
#endif // ENABLE_LAUNCHER_SEALING
return new MesosContainerizer(Owned<MesosContainerizerProcess>(
@@ -594,7 +608,8 @@ Try<MesosContainerizer*> MesosContainerizer::create(
launcher,
provisioner,
_isolators,
- initMemFd)));
+ initMemFd,
+ commandExecutorMemFd)));
}
@@ -1901,6 +1916,16 @@ Future<Containerizer::LaunchResult> MesosContainerizerProcess::_launch(
const std::array<int_fd, 2>& pipes = pipes_.get();
+ vector<int_fd> whitelistFds{pipes[0], pipes[1]};
+
+ // Seal the command executor binary if needed.
+ if (container->config->has_task_info() && commandExecutorMemFd.isSome()) {
+ launchInfo.mutable_command()->set_value(
+ "/proc/self/fd/" + stringify(commandExecutorMemFd.get()));
+
+ whitelistFds.push_back(commandExecutorMemFd.get());
+ }
+
// Prepare the flags to pass to the launch process.
MesosContainerizerLaunch::Flags launchFlags;
@@ -1909,8 +1934,6 @@ Future<Containerizer::LaunchResult> MesosContainerizerProcess::_launch(
launchFlags.pipe_read = pipes[0];
launchFlags.pipe_write = pipes[1];
- const vector<int_fd> whitelistFds{pipes[0], pipes[1]};
-
#ifndef __WINDOWS__
// Set the `runtime_directory` launcher flag so that the launch
// helper knows where to checkpoint the status of the container
diff --git a/src/slave/containerizer/mesos/containerizer.hpp b/src/slave/containerizer/mesos/containerizer.hpp
index b0b4aff..d56acac 100644
--- a/src/slave/containerizer/mesos/containerizer.hpp
+++ b/src/slave/containerizer/mesos/containerizer.hpp
@@ -140,7 +140,8 @@ public:
const process::Owned<Launcher>& _launcher,
const process::Shared<Provisioner>& _provisioner,
const std::vector<process::Owned<mesos::slave::Isolator>>& _isolators,
- const Option<int_fd>& _initMemFd)
+ const Option<int_fd>& _initMemFd,
+ const Option<int_fd>& _commandExecutorMemFd)
: ProcessBase(process::ID::generate("mesos-containerizer")),
flags(_flags),
fetcher(_fetcher),
@@ -148,7 +149,8 @@ public:
launcher(_launcher),
provisioner(_provisioner),
isolators(_isolators),
- initMemFd(_initMemFd) {}
+ initMemFd(_initMemFd),
+ commandExecutorMemFd(_commandExecutorMemFd) {}
virtual ~MesosContainerizerProcess()
{
@@ -159,6 +161,15 @@ public:
<< "': " << close.error();
}
}
+
+ if (commandExecutorMemFd.isSome()) {
+ Try<Nothing> close = os::close(commandExecutorMemFd.get());
+ if (close.isError()) {
+ LOG(WARNING) << "Failed to close memfd '"
+ << stringify(commandExecutorMemFd.get())
+ << "': " << close.error();
+ }
+ }
}
virtual process::Future<Nothing> recover(
@@ -316,6 +327,7 @@ private:
const process::Shared<Provisioner> provisioner;
const std::vector<process::Owned<mesos::slave::Isolator>> isolators;
const Option<int_fd> initMemFd;
+ const Option<int_fd> commandExecutorMemFd;
struct Container
{
diff --git a/src/slave/containerizer/mesos/launch.cpp b/src/slave/containerizer/mesos/launch.cpp
index 204d57f..c650756 100644
--- a/src/slave/containerizer/mesos/launch.cpp
+++ b/src/slave/containerizer/mesos/launch.cpp
@@ -1079,9 +1079,15 @@ int MesosContainerizerLaunch::execute()
// Avoid leaking not required file descriptors into the forked process.
foreach (int_fd fd, fds.get()) {
if (fd != STDIN_FILENO && fd != STDOUT_FILENO && fd != STDERR_FILENO) {
- // We use the unwrapped `::close` as opposed to `os::close`
- // since the former is guaranteed to be async signal safe.
- ::close(fd);
+ // NOTE: Set "FD_CLOEXEC" on the fd, instead of closing it
+ // because exec below might exec a memfd.
+ int flags = ::fcntl(fd, F_GETFD);
+ if (flags == -1) {
+ ABORT("Failed to get FD flags");
+ }
+ if (::fcntl(fd, F_SETFD, flags | FD_CLOEXEC) == -1) {
+ ABORT("Failed to set FD_CLOEXEC");
+ }
}
}
}
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 735e165..2a90e96 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -163,12 +163,6 @@ using process::UPID;
using process::http::authentication::Principal;
-#ifdef __WINDOWS__
-constexpr char MESOS_EXECUTOR[] = "mesos-executor.exe";
-#else
-constexpr char MESOS_EXECUTOR[] = "mesos-executor";
-#endif // __WINDOWS__
-
namespace mesos {
namespace internal {
namespace slave {
[mesos] 01/10: Added Linux memfd support.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 620bf29c611caaa633c5f39c71d296e07621d5aa
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Mon Feb 11 12:51:19 2019 -0800
Added Linux memfd support.
This patch adds the memfd support to allow us to copy a regular file
into a sealed memfd. This can be useful when one wants to protect a file
from being overwritten.
Review: https://reviews.apache.org/r/69943/
(cherry picked from commit e4ac39b123c892ec2593381918600c260a75fc32)
---
src/CMakeLists.txt | 1 +
src/Makefile.am | 1 +
src/linux/memfd.cpp | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++++
src/linux/memfd.hpp | 39 ++++++++++++++
4 files changed, 185 insertions(+)
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index de9b5b8..bb7b535 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -276,6 +276,7 @@ set(LINUX_SRC
linux/fs.cpp
linux/ldcache.cpp
linux/ldd.cpp
+ linux/memfd.cpp
linux/ns.cpp
linux/perf.cpp
linux/systemd.cpp
diff --git a/src/Makefile.am b/src/Makefile.am
index 1485229..1eb6ff0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1310,6 +1310,7 @@ MESOS_LINUX_FILES = \
linux/fs.cpp \
linux/ldcache.cpp \
linux/ldd.cpp \
+ linux/memfd.cpp \
linux/ns.cpp \
linux/perf.cpp \
linux/systemd.cpp \
diff --git a/src/linux/memfd.cpp b/src/linux/memfd.cpp
new file mode 100644
index 0000000..706dc0c
--- /dev/null
+++ b/src/linux/memfd.cpp
@@ -0,0 +1,144 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <fcntl.h>
+#include <unistd.h>
+
+#include <sys/sendfile.h>
+#include <sys/syscall.h>
+
+#include <stout/stringify.hpp>
+
+#include <stout/os/close.hpp>
+#include <stout/os/open.hpp>
+#include <stout/os/stat.hpp>
+
+using std::string;
+
+#if !defined(MFD_CLOEXEC)
+#define MFD_CLOEXEC 0x0001U
+#endif
+
+#if !defined(MFD_ALLOW_SEALING)
+#define MFD_ALLOW_SEALING 0x0002U
+#endif
+
+#if !defined(F_ADD_SEALS)
+#define F_ADD_SEALS 1033
+#endif
+
+#if !defined(F_GET_SEALS)
+#define F_GET_SEALS 1034
+#endif
+
+#if !defined(F_SEAL_SEAL)
+#define F_SEAL_SEAL 0x0001
+#endif
+
+#if !defined(F_SEAL_SHRINK)
+#define F_SEAL_SHRINK 0x0002
+#endif
+
+#if !defined(F_SEAL_GROW)
+#define F_SEAL_GROW 0x0004
+#endif
+
+#if !defined(F_SEAL_WRITE)
+#define F_SEAL_WRITE 0x0008
+#endif
+
+namespace mesos {
+namespace internal {
+namespace memfd {
+
+static Try<int_fd> create(const string& name, unsigned int flags)
+{
+#ifdef __NR_memfd_create
+ int_fd fd = ::syscall(__NR_memfd_create, name.c_str(), flags);
+ if (fd == -1) {
+ return ErrnoError("Failed to create memfd");
+ }
+
+ return fd;
+#else
+#error "The memfd syscall is not available."
+#endif
+}
+
+
+Try<int_fd> cloneSealedFile(const std::string& filePath)
+{
+ if (!os::stat::isfile(filePath)) {
+ return Error("The original file '" + filePath + "' is not a regular file");
+ }
+
+ Try<Bytes> size = os::stat::size(filePath);
+ if (size.isError()) {
+ return Error("Failed to get the size of the source file: " + size.error());
+ }
+
+ Try<int_fd> fileFd = os::open(filePath, O_CLOEXEC | O_RDONLY);
+ if (fileFd.isError()) {
+ return Error("Failed to open source file: " + fileFd.error());
+ }
+
+ Try<int_fd> memFd = create(filePath, MFD_CLOEXEC | MFD_ALLOW_SEALING);
+ if (memFd.isError()) {
+ os::close(fileFd.get());
+ return Error("Failed to open memfd file: " + memFd.error());
+ }
+
+ ssize_t written = sendfile(memFd.get(), fileFd.get(), nullptr, size->bytes());
+ if (written == -1) {
+ ErrnoError error("Failed to copy file");
+ os::close(fileFd.get());
+ os::close(memFd.get());
+ return error;
+ } else if (static_cast<uint64_t>(written) != size->bytes()) {
+ os::close(fileFd.get());
+ os::close(memFd.get());
+ return Error(
+ "Expect to write " + stringify(size->bytes()) + " bytes, "
+ "but only " + stringify(written) + " is written");
+ }
+
+ os::close(fileFd.get());
+
+ int ret = fchmod(memFd.get(), S_IRWXU | S_IRWXG | S_IRWXO);
+ if (ret == -1) {
+ ErrnoError error("Failed to chmod");
+ os::close(memFd.get());
+ return error;
+ }
+
+ // Seal the memfd file.
+ ret = fcntl(
+ memFd.get(),
+ F_ADD_SEALS,
+ F_SEAL_SHRINK | F_SEAL_GROW | F_SEAL_WRITE | F_SEAL_SEAL);
+
+ if (ret == -1) {
+ ErrnoError error("Failed to seal the memfd");
+ os::close(memFd.get());
+ return error;
+ }
+
+ return memFd.get();
+}
+
+} // namespace memfd {
+} // namespace internal {
+} // namespace mesos {
diff --git a/src/linux/memfd.hpp b/src/linux/memfd.hpp
new file mode 100644
index 0000000..4cf9906
--- /dev/null
+++ b/src/linux/memfd.hpp
@@ -0,0 +1,39 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef __LINUX_MEMFD_HPP__
+#define __LINUX_MEMFD_HPP__
+
+#include <string>
+
+#include <stout/try.hpp>
+
+#include <stout/os/int_fd.hpp>
+
+namespace mesos {
+namespace internal {
+namespace memfd {
+
+// Clone a file into a sealed private copy such that any attempt to
+// modify it will not modify the original binary. Returns the memfd of
+// the sealed copy.
+Try<int_fd> cloneSealedFile(const std::string& filePath);
+
+} // namespace memfd {
+} // namespace internal {
+} // namespace mesos {
+
+#endif // __LINUX_MEMFD_HPP__
[mesos] 07/10: Added `--enable-launcher-sealing` compiler flag.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit da73d0754c3de7f0df17d08228876bd07f852ebf
Author: Andrei Budnik <ab...@mesosphere.com>
AuthorDate: Mon Feb 11 12:51:27 2019 -0800
Added `--enable-launcher-sealing` compiler flag.
This flag enables sealing of launcher binary during initialization of
Mesos containerizer. It's disabled by default because sealing requires
Linux kernel 3.17+.
Review: https://reviews.apache.org/r/69949/
(cherry picked from commit c97d50e2b0b3bdbcb6dde58395a21c819d56194a)
---
configure.ac | 31 +++++++++++++++++++++++++++++++
src/CMakeLists.txt | 1 -
src/Makefile.am | 13 +++++++++++--
src/tests/CMakeLists.txt | 1 -
4 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/configure.ac b/configure.ac
index fe1bf94..f33b4be 100644
--- a/configure.ac
+++ b/configure.ac
@@ -227,6 +227,17 @@ AC_ARG_ENABLE([last_in_first_out_fixed_size_semaphore],
[enables the optimized LIFO fixed-size semaphore in libprocess]),
[], [enable_last_in_first_out_fixed_size_semaphore=no])
+AC_ARG_ENABLE([launcher_sealing],
+ AS_HELP_STRING([--enable-launcher-sealing],
+ [Enables sealing of launcher binary during
+ initialization of Mesos containerizer.
+ A copy of launcher's binary is cloned into
+ memory and sealed to prevent overwriting of
+ launcher's executable by containers.
+ This is a Linux-specific feature and requires
+ the support of memfd by the kernel.]),
+ [], [enable_launcher_sealing=no])
+
AC_ARG_ENABLE([libevent],
AS_HELP_STRING([--enable-libevent],
[use libevent instead of libev]),
@@ -1413,6 +1424,26 @@ AM_CONDITIONAL([WITH_BUNDLED_LEVELDB],
[test "x$with_bundled_leveldb" = "xyes"])
+# Check if we should use containerizer launcher sealing.
+AC_MSG_CHECKING([whether to enable launcher sealing])
+AC_MSG_RESULT([$enable_launcher_sealing])
+
+AS_IF([test "x$enable_launcher_sealing" = "xyes"], [
+ # We only support launcher sealing on Linux.
+ AS_IF([test "$OS_NAME" = "linux"],
+ [],
+ [AC_MSG_ERROR([no launcher sealing support on $OS_NAME
+-------------------------------------------------------------------
+Launcher sealing is only supported on Linux.
+-------------------------------------------------------------------
+ ])])
+
+ AC_DEFINE([ENABLE_LAUNCHER_SEALING])
+])
+
+AM_CONDITIONAL([ENABLE_LAUNCHER_SEALING], [test "x$enable_launcher_sealing" = "xyes"])
+
+
# Check if libevent prefix path was provided, and if so, add it to
# the CPPFLAGS and LDFLAGS with respective /include and /lib path
# suffixes.
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index bb7b535..de9b5b8 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -276,7 +276,6 @@ set(LINUX_SRC
linux/fs.cpp
linux/ldcache.cpp
linux/ldd.cpp
- linux/memfd.cpp
linux/ns.cpp
linux/perf.cpp
linux/systemd.cpp
diff --git a/src/Makefile.am b/src/Makefile.am
index 26d874a..400e51f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1310,7 +1310,6 @@ MESOS_LINUX_FILES = \
linux/fs.cpp \
linux/ldcache.cpp \
linux/ldd.cpp \
- linux/memfd.cpp \
linux/ns.cpp \
linux/perf.cpp \
linux/systemd.cpp \
@@ -1405,6 +1404,12 @@ MESOS_LINUX_FILES += \
slave/containerizer/mesos/isolators/xfs/disk.hpp
endif
+if ENABLE_LAUNCHER_SEALING
+MESOS_LINUX_FILES += \
+ linux/memfd.cpp \
+ linux/memfd.hpp
+endif
+
MESOS_LINUX_ROUTING_FILES = \
linux/routing/handle.cpp \
linux/routing/handle.hpp \
@@ -2638,7 +2643,6 @@ mesos_tests_SOURCES += \
tests/containerizer/cni_isolator_tests.cpp \
tests/containerizer/docker_volume_isolator_tests.cpp \
tests/containerizer/linux_filesystem_isolator_tests.cpp \
- tests/containerizer/linux_memfd_tests.cpp \
tests/containerizer/fs_tests.cpp \
tests/containerizer/memory_pressure_tests.cpp \
tests/containerizer/nested_mesos_containerizer_tests.cpp \
@@ -2660,6 +2664,11 @@ mesos_tests_SOURCES += \
endif
endif
+if ENABLE_LAUNCHER_SEALING
+mesos_tests_SOURCES += \
+ tests/containerizer/linux_memfd_tests.cpp
+endif
+
if ENABLE_LINUX_ROUTING
mesos_tests_SOURCES += \
tests/containerizer/routing_tests.cpp
diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index 7a1cba5..758bb94 100644
--- a/src/tests/CMakeLists.txt
+++ b/src/tests/CMakeLists.txt
@@ -226,7 +226,6 @@ if (LINUX)
containerizer/fs_tests.cpp
containerizer/linux_capabilities_isolator_tests.cpp
containerizer/linux_filesystem_isolator_tests.cpp
- containerizer/linux_memfd_tests.cpp
containerizer/memory_pressure_tests.cpp
containerizer/nested_mesos_containerizer_tests.cpp
containerizer/ns_tests.cpp
[mesos] 04/10: Updated handleWhitelistFds() to avoid closing FDs
with FD_CLOEXEC bit.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit b1469bf436a43b08ab034a089672a03afca946d3
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Mon Feb 11 12:51:22 2019 -0800
Updated handleWhitelistFds() to avoid closing FDs with FD_CLOEXEC bit.
Since this helper is only called right before exec in child process,
and for those open FDs that is set with FD_CLOEXEC flag, they will
be closed during exec, so that we could skip closing these FDs in
the helper. The motivation of this change is to avoid whitelisting
those FDs that have to survive until exec while we do not want to
expose these FDs to user applications.
Review: https://reviews.apache.org/r/69946/
(cherry picked from commit 03e88611c756b63e398386e11e7866e88d018c6d)
---
3rdparty/libprocess/src/subprocess_posix.hpp | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/3rdparty/libprocess/src/subprocess_posix.hpp b/3rdparty/libprocess/src/subprocess_posix.hpp
index 719bdf3..02d4a3a 100644
--- a/3rdparty/libprocess/src/subprocess_posix.hpp
+++ b/3rdparty/libprocess/src/subprocess_posix.hpp
@@ -130,7 +130,18 @@ static void handleWhitelistFds(const std::vector<int_fd>& whitelist_fds)
}
if (!found) {
- ::close(fd);
+ int flags = ::fcntl(fd, F_GETFD);
+ if (flags == -1) {
+ // TODO(gilbert): clean up the use of `os::strerror` during the
+ // timeframe of fork-exec because it is not signal safe.
+ ABORT(
+ "Failed to get file descriptor flags: " + os::strerror(errno));
+ }
+
+ // Close the FD which does not have the FD_CLOEXEC bit.
+ if ((flags & FD_CLOEXEC) == 0){
+ ::close(fd);
+ }
}
}
}
[mesos] 08/10: Enabled launcher sealing depending on corresponding
compiler flag.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit d4852aa344b45025bf868c3d337b75beaa3795d4
Author: Andrei Budnik <ab...@mesosphere.com>
AuthorDate: Mon Feb 11 12:51:28 2019 -0800
Enabled launcher sealing depending on corresponding compiler flag.
This patch enables containerizer launcher sealing when
`--enable-launcher-sealing` compiler flag is enabled. By default,
this compiler flag is disabled, hence launcher sealing is disabled.
Review: https://reviews.apache.org/r/69950/
(cherry picked from commit d1bff48a677771f77dbcbdd55acfaeae2afa6b9a)
---
src/launcher/executor.cpp | 8 ++++----
src/slave/containerizer/mesos/containerizer.cpp | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index e10b4ab..3ae3a56 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -82,9 +82,9 @@
#include "internal/devolve.hpp"
#include "internal/evolve.hpp"
-#ifdef __linux__
+#ifdef ENABLE_LAUNCHER_SEALING
#include "linux/memfd.hpp"
-#endif // __linux__
+#endif // ENABLE_LAUNCHER_SEALING
#include "logging/logging.hpp"
@@ -485,7 +485,7 @@ protected:
// Determine the mesos containerizer binary depends on whether we
// need to clone and seal it on linux.
string initPath = path::join(launcherDir, MESOS_CONTAINERIZER);
-#ifdef __linux__
+#ifdef ENABLE_LAUNCHER_SEALING
// Clone the launcher binary in memory for security concerns.
Try<int_fd> memFd = memfd::cloneSealedFile(initPath);
if (memFd.isError()) {
@@ -495,7 +495,7 @@ protected:
}
initPath = "/proc/self/fd/" + stringify(memFd.get());
-#endif // __linux__
+#endif // ENABLE_LAUNCHER_SEALING
// Fork the child using launcher.
vector<string> argv(2);
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index 7f00084..d195012 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -50,9 +50,9 @@
#include "hook/manager.hpp"
-#ifdef __linux__
+#ifdef ENABLE_LAUNCHER_SEALING
#include "linux/memfd.hpp"
-#endif // __linux__
+#endif // ENABLE_LAUNCHER_SEALING
#include "module/manager.hpp"
@@ -571,7 +571,7 @@ Try<MesosContainerizer*> MesosContainerizer::create(
Option<int_fd> initMemFd;
-#ifdef __linux__
+#ifdef ENABLE_LAUNCHER_SEALING
// Clone the launcher binary in memory for security concerns.
Try<int_fd> memFd = memfd::cloneSealedFile(
path::join(flags.launcher_dir, MESOS_CONTAINERIZER));
@@ -584,7 +584,7 @@ Try<MesosContainerizer*> MesosContainerizer::create(
}
initMemFd = memFd.get();
-#endif // __linux__
+#endif // ENABLE_LAUNCHER_SEALING
return new MesosContainerizer(Owned<MesosContainerizerProcess>(
new MesosContainerizerProcess(
[mesos] 02/10: Made the code more robust related to sendfile.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit aaf5b791935a6fe5c13f10bde34640f38d8d3a5c
Author: Jie Yu <yu...@gmail.com>
AuthorDate: Mon Feb 11 12:51:20 2019 -0800
Made the code more robust related to sendfile.
It's possible that sendfile returns less bytes written than requested.
Previously, we simply return an error in that case. This patch makes it
more robust by retrying sendfile if less bytes were written than
requested.
Review: https://reviews.apache.org/r/69944/
(cherry picked from commit 4bd0914443888e1434a2c685d2be3c6713b22ec6)
---
src/linux/memfd.cpp | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/src/linux/memfd.cpp b/src/linux/memfd.cpp
index 706dc0c..348d0ce 100644
--- a/src/linux/memfd.cpp
+++ b/src/linux/memfd.cpp
@@ -101,18 +101,21 @@ Try<int_fd> cloneSealedFile(const std::string& filePath)
return Error("Failed to open memfd file: " + memFd.error());
}
- ssize_t written = sendfile(memFd.get(), fileFd.get(), nullptr, size->bytes());
- if (written == -1) {
- ErrnoError error("Failed to copy file");
- os::close(fileFd.get());
- os::close(memFd.get());
- return error;
- } else if (static_cast<uint64_t>(written) != size->bytes()) {
- os::close(fileFd.get());
- os::close(memFd.get());
- return Error(
- "Expect to write " + stringify(size->bytes()) + " bytes, "
- "but only " + stringify(written) + " is written");
+ size_t remaining = size->bytes();
+ while (remaining > 0) {
+ ssize_t written = sendfile(memFd.get(), fileFd.get(), nullptr, remaining);
+ if (written == -1) {
+ ErrnoError error("Failed to copy file");
+ os::close(fileFd.get());
+ os::close(memFd.get());
+ return error;
+ } else if (static_cast<size_t>(written) > remaining) {
+ os::close(fileFd.get());
+ os::close(memFd.get());
+ return Error("More bytes written than requested");
+ } else {
+ remaining -= written;
+ }
}
os::close(fileFd.get());
[mesos] 05/10: Cloned a sealed file of launcher binary.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit cdf042e534caaba9c835f8eb2340ca1acd5fda79
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Mon Feb 11 12:51:24 2019 -0800
Cloned a sealed file of launcher binary.
Cloned this binary during mesos containerizer create and command
executor launch.
This change would copy the mesos-containerizer binary in memory,
which helps avoid the binary being overwritten.
Review: https://reviews.apache.org/r/69947/
(cherry picked from commit f7407d3d5f0fe7d5c9a06774921de514012a54da)
---
src/launcher/executor.cpp | 23 +++++++++++++++++--
src/slave/containerizer/mesos/containerizer.cpp | 30 +++++++++++++++++++++++--
src/slave/containerizer/mesos/containerizer.hpp | 18 ++++++++++++---
3 files changed, 64 insertions(+), 7 deletions(-)
diff --git a/src/launcher/executor.cpp b/src/launcher/executor.cpp
index 3f606e5..ec12b89 100644
--- a/src/launcher/executor.cpp
+++ b/src/launcher/executor.cpp
@@ -82,6 +82,10 @@
#include "internal/devolve.hpp"
#include "internal/evolve.hpp"
+#ifdef __linux__
+#include "linux/memfd.hpp"
+#endif // __linux__
+
#include "logging/logging.hpp"
#include "messages/messages.hpp"
@@ -478,11 +482,26 @@ protected:
launchFlags.launch_info = JSON::protobuf(launchInfo);
+ // Determine the mesos containerizer binary depends on whether we
+ // need to clone and seal it on linux.
+ string initPath = path::join(launcherDir, MESOS_CONTAINERIZER);
+#ifdef __linux__
+ // Clone the launcher binary in memory for security concerns.
+ Try<int_fd> memFd = memfd::cloneSealedFile(initPath);
+ if (memFd.isError()) {
+ ABORT(
+ "Failed to clone a sealed file '" + initPath + "' in memory: " +
+ memFd.error());
+ }
+
+ initPath = "/proc/self/fd/" + stringify(memFd.get());
+#endif // __linux__
+
// TODO(tillt): Consider using a flag allowing / disallowing the
// log output of possibly sensitive data. See MESOS-7292.
string commandString = strings::format(
"%s %s <POSSIBLY-SENSITIVE-DATA>",
- path::join(launcherDir, MESOS_CONTAINERIZER),
+ initPath,
MesosContainerizerLaunch::NAME).get();
LOG(INFO) << "Running '" << commandString << "'";
@@ -508,7 +527,7 @@ protected:
}
Try<Subprocess> s = subprocess(
- path::join(launcherDir, MESOS_CONTAINERIZER),
+ initPath,
argv,
Subprocess::FD(STDIN_FILENO),
Subprocess::FD(STDOUT_FILENO),
diff --git a/src/slave/containerizer/mesos/containerizer.cpp b/src/slave/containerizer/mesos/containerizer.cpp
index c9af06c..7f00084 100644
--- a/src/slave/containerizer/mesos/containerizer.cpp
+++ b/src/slave/containerizer/mesos/containerizer.cpp
@@ -50,6 +50,10 @@
#include "hook/manager.hpp"
+#ifdef __linux__
+#include "linux/memfd.hpp"
+#endif // __linux__
+
#include "module/manager.hpp"
#include "slave/paths.hpp"
@@ -565,6 +569,23 @@ Try<MesosContainerizer*> MesosContainerizer::create(
_isolators.push_back(Owned<Isolator>(new MesosIsolator(
Owned<MesosIsolatorProcess>(ioSwitchboard.get()))));
+ Option<int_fd> initMemFd;
+
+#ifdef __linux__
+ // Clone the launcher binary in memory for security concerns.
+ Try<int_fd> memFd = memfd::cloneSealedFile(
+ path::join(flags.launcher_dir, MESOS_CONTAINERIZER));
+
+ if (memFd.isError()) {
+ return Error(
+ "Failed to clone a sealed file '" +
+ path::join(flags.launcher_dir, MESOS_CONTAINERIZER) + "' in memory: " +
+ memFd.error());
+ }
+
+ initMemFd = memFd.get();
+#endif // __linux__
+
return new MesosContainerizer(Owned<MesosContainerizerProcess>(
new MesosContainerizerProcess(
flags,
@@ -572,7 +593,8 @@ Try<MesosContainerizer*> MesosContainerizer::create(
ioSwitchboard.get(),
launcher,
provisioner,
- _isolators)));
+ _isolators,
+ initMemFd)));
}
@@ -1977,13 +1999,17 @@ Future<Containerizer::LaunchResult> MesosContainerizerProcess::_launch(
launchFlagsEnvironment.end());
// Fork the child using launcher.
+ string initPath = initMemFd.isSome()
+ ? ("/proc/self/fd/" + stringify(initMemFd.get()))
+ : path::join(flags.launcher_dir, MESOS_CONTAINERIZER);
+
vector<string> argv(2);
argv[0] = path::join(flags.launcher_dir, MESOS_CONTAINERIZER);
argv[1] = MesosContainerizerLaunch::NAME;
Try<pid_t> forked = launcher->fork(
containerId,
- argv[0],
+ initPath,
argv,
containerIO.get(),
nullptr,
diff --git a/src/slave/containerizer/mesos/containerizer.hpp b/src/slave/containerizer/mesos/containerizer.hpp
index 22405e6..b0b4aff 100644
--- a/src/slave/containerizer/mesos/containerizer.hpp
+++ b/src/slave/containerizer/mesos/containerizer.hpp
@@ -139,16 +139,27 @@ public:
IOSwitchboard* _ioSwitchboard,
const process::Owned<Launcher>& _launcher,
const process::Shared<Provisioner>& _provisioner,
- const std::vector<process::Owned<mesos::slave::Isolator>>& _isolators)
+ const std::vector<process::Owned<mesos::slave::Isolator>>& _isolators,
+ const Option<int_fd>& _initMemFd)
: ProcessBase(process::ID::generate("mesos-containerizer")),
flags(_flags),
fetcher(_fetcher),
ioSwitchboard(_ioSwitchboard),
launcher(_launcher),
provisioner(_provisioner),
- isolators(_isolators) {}
+ isolators(_isolators),
+ initMemFd(_initMemFd) {}
- virtual ~MesosContainerizerProcess() {}
+ virtual ~MesosContainerizerProcess()
+ {
+ if (initMemFd.isSome()) {
+ Try<Nothing> close = os::close(initMemFd.get());
+ if (close.isError()) {
+ LOG(WARNING) << "Failed to close memfd '" << stringify(initMemFd.get())
+ << "': " << close.error();
+ }
+ }
+ }
virtual process::Future<Nothing> recover(
const Option<state::SlaveState>& state);
@@ -304,6 +315,7 @@ private:
const process::Owned<Launcher> launcher;
const process::Shared<Provisioner> provisioner;
const std::vector<process::Owned<mesos::slave::Isolator>> isolators;
+ const Option<int_fd> initMemFd;
struct Container
{
[mesos] 09/10: Fixed `ROOT_PidNamespace` test after enabling
launcher sealing.
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
gilbert pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit da82ef99dee540b1ea165599b74a3b8bc497f1fe
Author: Andrei Budnik <ab...@mesosphere.com>
AuthorDate: Mon Feb 11 12:51:31 2019 -0800
Fixed `ROOT_PidNamespace` test after enabling launcher sealing.
Previously, this test accessed `comm` file of `init` process to
find out the name of the containerizer launcher binary. After we
introduced launcher sealing, this file does not represent binary
name anymore, so we are accessing `cmdline` file instead.
Review: https://reviews.apache.org/r/69951/
(cherry picked from commit b0b5e3d89542ce2b958953326ab6b94f718d134e)
---
src/tests/containerizer/isolator_tests.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tests/containerizer/isolator_tests.cpp b/src/tests/containerizer/isolator_tests.cpp
index bdc3619..0f82516 100644
--- a/src/tests/containerizer/isolator_tests.cpp
+++ b/src/tests/containerizer/isolator_tests.cpp
@@ -120,7 +120,7 @@ TEST_F(NamespacesIsolatorTest, ROOT_PidNamespace)
// Write the command's pid namespace inode and init name to files.
const string command =
- "stat -Lc %i /proc/self/ns/pid > ns && (cat /proc/1/comm > init)";
+ "stat -Lc %i /proc/self/ns/pid > ns && (cat /proc/1/cmdline > init)";
process::Future<Containerizer::LaunchResult> launch =
containerizer.get()->launch(