You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by vi...@apache.org on 2017/04/13 22:44:40 UTC

[1/2] mesos git commit: Changed '--executor_secret_key' agent flag to accept a path.

Repository: mesos
Updated Branches:
  refs/heads/master f73f29bd3 -> 8bbe70041


Changed '--executor_secret_key' agent flag to accept a path.

This patch changes the agent flag '--executor_secret_key' to accept
a path, so that the secret will not be leaked in logs.

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


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

Branch: refs/heads/master
Commit: 0b7a40102a33ab46c2794963d9a0133b9e76b880
Parents: f73f29b
Author: Greg Mann <gr...@mesosphere.io>
Authored: Thu Apr 13 15:44:19 2017 -0700
Committer: Vinod Kone <vi...@gmail.com>
Committed: Thu Apr 13 15:44:19 2017 -0700

----------------------------------------------------------------------
 docs/configuration.md |  5 +++--
 src/slave/flags.cpp   |  5 +++--
 src/slave/flags.hpp   |  2 +-
 src/slave/slave.cpp   | 24 +++++++++++++++++++++++-
 4 files changed, 30 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/0b7a4010/docs/configuration.md
----------------------------------------------------------------------
diff --git a/docs/configuration.md b/docs/configuration.md
index 452478e..159f946 100644
--- a/docs/configuration.md
+++ b/docs/configuration.md
@@ -1418,8 +1418,9 @@ in memory. (default: 150)
     --executor_secret_key=VALUE
   </td>
   <td>
-The key used when generating executor secrets. This flag is only
-available when Mesos is built with SSL support.
+Path to a file containing the key used when generating executor
+secrets. This flag is only available when Mesos is built with SSL
+support.
   </td>
 </tr>
 <tr>

http://git-wip-us.apache.org/repos/asf/mesos/blob/0b7a4010/src/slave/flags.cpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.cpp b/src/slave/flags.cpp
index 9365da2..c50e43c 100644
--- a/src/slave/flags.cpp
+++ b/src/slave/flags.cpp
@@ -345,8 +345,9 @@ mesos::internal::slave::Flags::Flags()
 #ifdef USE_SSL_SOCKET
   add(&Flags::executor_secret_key,
       "executor_secret_key",
-      "The key used when generating executor secrets. This flag is only\n"
-      "available when Mesos is built with SSL support.");
+      "Path to a file containing the key used when generating executor\n"
+      "secrets. This flag is only available when Mesos is built with SSL\n"
+      "support.");
 #endif // USE_SSL_SOCKET
 
   add(&Flags::gc_delay,

http://git-wip-us.apache.org/repos/asf/mesos/blob/0b7a4010/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index 171f67e..c7a4604 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -79,7 +79,7 @@ public:
   Duration executor_registration_timeout;
   Duration executor_shutdown_grace_period;
 #ifdef USE_SSL_SOCKET
-  Option<std::string> executor_secret_key;
+  Option<Path> executor_secret_key;
 #endif // USE_SSL_SOCKET
   Duration gc_delay;
   double gc_disk_headroom;

http://git-wip-us.apache.org/repos/asf/mesos/blob/0b7a4010/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index f013e9c..3ad4ce4 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -291,7 +291,29 @@ void Slave::initialize()
   Option<string> secretKey;
 #ifdef USE_SSL_SOCKET
   if (flags.executor_secret_key.isSome()) {
-    secretKey = flags.executor_secret_key.get();
+    Try<string> secretKey_ = os::read(flags.executor_secret_key.get());
+
+    if (secretKey_.isError()) {
+      EXIT(EXIT_FAILURE) << "Failed to read the file specified by "
+                         << "--executor_secret_key";
+    }
+
+    // TODO(greggomann): Factor the following code out into a common helper,
+    // since we also do this when loading credentials.
+    Try<os::Permissions> permissions =
+      os::permissions(flags.executor_secret_key.get());
+    if (permissions.isError()) {
+      LOG(WARNING) << "Failed to stat executor secret key file '"
+                   << flags.executor_secret_key.get()
+                   << "': " << permissions.error();
+    } else if (permissions.get().others.rwx) {
+      LOG(WARNING) << "Permissions on executor secret key file '"
+                   << flags.executor_secret_key.get()
+                   << "' are too open; it is recommended that your"
+                   << " key file is NOT accessible by others";
+    }
+
+    secretKey = secretKey_.get();
     secretGenerator = new JWTSecretGenerator(secretKey.get());
   }
 


[2/2] mesos git commit: Updated tests to set '--executor_secret_key' as a path.

Posted by vi...@apache.org.
Updated tests to set '--executor_secret_key' as a path.

This patch updates the test code to generate a secret key file
and set the agent '--executor_secret_key' flag with its path.

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


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

Branch: refs/heads/master
Commit: 8bbe700411298b7b17219f9521820b9dcc7ef2c8
Parents: 0b7a401
Author: Greg Mann <gr...@mesosphere.io>
Authored: Thu Apr 13 15:44:28 2017 -0700
Committer: Vinod Kone <vi...@gmail.com>
Committed: Thu Apr 13 15:44:28 2017 -0700

----------------------------------------------------------------------
 src/tests/mesos.cpp | 20 +++++++++++++++++++-
 src/tests/mesos.hpp |  5 +++++
 2 files changed, 24 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/8bbe7004/src/tests/mesos.cpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.cpp b/src/tests/mesos.cpp
index 099ec37..27cfcab 100644
--- a/src/tests/mesos.cpp
+++ b/src/tests/mesos.cpp
@@ -212,7 +212,25 @@ slave::Flags MesosTest::CreateSlaveFlags()
   // Executor authentication currently has SSL as a dependency, so we
   // cannot enable it if Mesos was not built with SSL support.
   flags.authenticate_http_executors = true;
-  flags.executor_secret_key = "secret_key";
+
+  {
+    // Create a secret key for executor authentication.
+    const string path = path::join(directory.get(), "executor_secret_key");
+
+    Try<int_fd> fd = os::open(
+        path,
+        O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC,
+        S_IRUSR | S_IWUSR | S_IRGRP);
+
+    CHECK_SOME(fd);
+
+    CHECK_SOME(os::write(fd.get(), DEFAULT_EXECUTOR_SECRET_KEY))
+      << "Failed to write executor secret key to '" << path << "'";
+
+    CHECK_SOME(os::close(fd.get()));
+
+    flags.executor_secret_key = path;
+  }
 #endif // USE_SSL_SOCKET
 
   {

http://git-wip-us.apache.org/repos/asf/mesos/blob/8bbe7004/src/tests/mesos.hpp
----------------------------------------------------------------------
diff --git a/src/tests/mesos.hpp b/src/tests/mesos.hpp
index fe897c1..3cff0e7 100644
--- a/src/tests/mesos.hpp
+++ b/src/tests/mesos.hpp
@@ -99,6 +99,11 @@ namespace tests {
 constexpr char READONLY_HTTP_AUTHENTICATION_REALM[] = "test-readonly-realm";
 constexpr char READWRITE_HTTP_AUTHENTICATION_REALM[] = "test-readwrite-realm";
 constexpr char DEFAULT_TEST_ROLE[] = "default-role";
+constexpr char DEFAULT_EXECUTOR_SECRET_KEY[] =
+  "72kUKUFtghAjNbIOvLzfF2RxNBfeM64Bri8g9WhpyaunwqRB/yozHAqSnyHbddAV"
+  "PcWRQlrJAt871oWgSH+n52vMZ3aVI+AFMzXSo8+sUfMk83IGp0WJefhzeQsjDlGH"
+  "GYQgCAuGim0BE2X5U+lEue8s697uQpAO8L/FFRuDH2s";
+
 
 // Forward declarations.
 class MockExecutor;