You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by me...@apache.org on 2014/11/12 21:53:27 UTC

[2/3] mesos git commit: Integrated CRAM-MD5 Test Authenticatee module into slave.

Integrated CRAM-MD5 Test Authenticatee module into slave.

Also fixes messages.proto to use a raw bytestream instead of a string
for AuthenticationStartMessage as non CRAM-MD5 authentication may
transmit binary data.
Note that the change of AuthenticationStartMessage does basically have
no impact on C++ based proto code other than the prevention of a
warning due to non-UTF8 characters being encoded. That does in fact
occur when using non CRAM-MD5 based SASL authentication mechanisms.

Note that this patch covers modularized slave authentication only.
Framework authentication is currently covered by the default (built-in)
implementation. There will be a subsequent patch for modularized
framework authentication.

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


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

Branch: refs/heads/master
Commit: 6451af3ee26cba317fa9b65ef0d6e381378e9099
Parents: da3f41a
Author: Till Toenshoff <to...@me.com>
Authored: Wed Nov 12 12:49:19 2014 -0800
Committer: Adam B <ad...@mesosphere.io>
Committed: Wed Nov 12 12:49:19 2014 -0800

----------------------------------------------------------------------
 src/messages/messages.proto |  2 +-
 src/sched/sched.cpp         | 10 ++++++----
 src/scheduler/scheduler.cpp | 10 ++++++----
 src/slave/constants.cpp     |  1 +
 src/slave/constants.hpp     |  3 +++
 src/slave/flags.hpp         |  8 ++++++++
 src/slave/slave.cpp         | 29 ++++++++++++++++++++++++-----
 src/slave/slave.hpp         |  7 ++++---
 8 files changed, 53 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/6451af3e/src/messages/messages.proto
----------------------------------------------------------------------
diff --git a/src/messages/messages.proto b/src/messages/messages.proto
index de0e2a2..566ce53 100644
--- a/src/messages/messages.proto
+++ b/src/messages/messages.proto
@@ -372,7 +372,7 @@ message AuthenticationMechanismsMessage {
 
 message AuthenticationStartMessage {
   required string mechanism = 1;
-  optional string data = 2;
+  optional bytes data = 2;
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/6451af3e/src/sched/sched.cpp
----------------------------------------------------------------------
diff --git a/src/sched/sched.cpp b/src/sched/sched.cpp
index 8ca0526..4981dfb 100644
--- a/src/sched/sched.cpp
+++ b/src/sched/sched.cpp
@@ -59,6 +59,7 @@
 #include <stout/utils.hpp>
 #include <stout/uuid.hpp>
 
+#include "authentication/authenticatee.hpp"
 #include "authentication/cram_md5/authenticatee.hpp"
 
 #include "common/lock.hpp"
@@ -297,7 +298,7 @@ protected:
     CHECK_SOME(credential);
 
     CHECK(authenticatee == NULL);
-    authenticatee = new cram_md5::Authenticatee(credential.get(), self());
+    authenticatee = new cram_md5::CRAMMD5Authenticatee();
 
     // NOTE: We do not pass 'Owned<Authenticatee>' here because doing
     // so could make 'AuthenticateeProcess' responsible for deleting
@@ -312,8 +313,9 @@ protected:
     //     'Authenticatee'.
     // --> '~Authenticatee()' is invoked by 'AuthenticateeProcess'.
     // TODO(vinod): Consider using 'Shared' to 'Owned' upgrade.
-    authenticating = authenticatee->authenticate(master.get())
-      .onAny(defer(self(), &Self::_authenticate));
+    authenticating =
+      authenticatee->authenticate(master.get(), self(), credential.get())
+        .onAny(defer(self(), &Self::_authenticate));
 
     delay(Seconds(5),
           self(),
@@ -1062,7 +1064,7 @@ private:
 
   const Option<Credential> credential;
 
-  cram_md5::Authenticatee* authenticatee;
+  Authenticatee* authenticatee;
 
   // Indicates if an authentication attempt is in progress.
   Option<Future<bool> > authenticating;

http://git-wip-us.apache.org/repos/asf/mesos/blob/6451af3e/src/scheduler/scheduler.cpp
----------------------------------------------------------------------
diff --git a/src/scheduler/scheduler.cpp b/src/scheduler/scheduler.cpp
index c74187c..cbb982a 100644
--- a/src/scheduler/scheduler.cpp
+++ b/src/scheduler/scheduler.cpp
@@ -53,6 +53,7 @@
 #include <stout/os.hpp>
 #include <stout/uuid.hpp>
 
+#include "authentication/authenticatee.hpp"
 #include "authentication/cram_md5/authenticatee.hpp"
 
 #include "common/type_utils.hpp"
@@ -465,7 +466,7 @@ protected:
     CHECK_SOME(credential);
 
     CHECK(authenticatee == NULL);
-    authenticatee = new cram_md5::Authenticatee(credential.get(), self());
+    authenticatee = new cram_md5::CRAMMD5Authenticatee();
 
     // NOTE: We do not pass 'Owned<Authenticatee>' here because doing
     // so could make 'AuthenticateeProcess' responsible for deleting
@@ -480,8 +481,9 @@ protected:
     //     'Authenticatee'.
     // --> '~Authenticatee()' is invoked by 'AuthenticateeProcess'.
     // TODO(vinod): Consider using 'Shared' to 'Owned' upgrade.
-    authenticating = authenticatee->authenticate(master.get())
-      .onAny(defer(self(), &Self::_authenticate));
+    authenticating =
+      authenticatee->authenticate(master.get(), self(), credential.get())
+        .onAny(defer(self(), &Self::_authenticate));
 
     delay(Seconds(5),
           self(),
@@ -808,7 +810,7 @@ private:
 
   Option<UPID> master;
 
-  cram_md5::Authenticatee* authenticatee;
+  Authenticatee* authenticatee;
 
   // Indicates if an authentication attempt is in progress.
   Option<Future<bool> > authenticating;

http://git-wip-us.apache.org/repos/asf/mesos/blob/6451af3e/src/slave/constants.cpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.cpp b/src/slave/constants.cpp
index d6ad78c..2a99b11 100644
--- a/src/slave/constants.cpp
+++ b/src/slave/constants.cpp
@@ -50,6 +50,7 @@ const std::string DEFAULT_PORTS = "[31000-32000]";
 const uint16_t DEFAULT_EPHEMERAL_PORTS_PER_CONTAINER = 1024;
 #endif
 const Duration DOCKER_REMOVE_DELAY = Hours(6);
+const std::string DEFAULT_AUTHENTICATEE = "crammd5";
 
 Duration MASTER_PING_TIMEOUT()
 {

http://git-wip-us.apache.org/repos/asf/mesos/blob/6451af3e/src/slave/constants.hpp
----------------------------------------------------------------------
diff --git a/src/slave/constants.hpp b/src/slave/constants.hpp
index 701dd89..fd1c1ab 100644
--- a/src/slave/constants.hpp
+++ b/src/slave/constants.hpp
@@ -97,6 +97,9 @@ extern const uint16_t DEFAULT_EPHEMERAL_PORTS_PER_CONTAINER;
 // Default duration that docker containers will be removed after exit.
 extern const Duration DOCKER_REMOVE_DELAY;
 
+// Name of the default, CRAM-MD5 authenticatee.
+extern const std::string DEFAULT_AUTHENTICATEE;
+
 // If no pings received within this timeout, then the slave will
 // trigger a re-detection of the master to cause a re-registration.
 Duration MASTER_PING_TIMEOUT();

http://git-wip-us.apache.org/repos/asf/mesos/blob/6451af3e/src/slave/flags.hpp
----------------------------------------------------------------------
diff --git a/src/slave/flags.hpp b/src/slave/flags.hpp
index efbd35d..4ec5954 100644
--- a/src/slave/flags.hpp
+++ b/src/slave/flags.hpp
@@ -404,6 +404,13 @@ public:
         "    }\n"
         "  ]\n"
         "}");
+
+    add(&Flags::authenticatee,
+        "authenticatee",
+        "Authenticatee implementation to use when authenticating against the\n"
+        "master. Use the default '" + DEFAULT_AUTHENTICATEE + "', or\n"
+        "load an alternate authenticatee module using --modules.",
+        DEFAULT_AUTHENTICATEE);
   }
 
   bool version;
@@ -454,6 +461,7 @@ public:
   Option<Bytes> egress_rate_limit_per_container;
 #endif
   Option<Modules> modules;
+  std::string authenticatee;
 };
 
 } // namespace slave {

http://git-wip-us.apache.org/repos/asf/mesos/blob/6451af3e/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index ba00bf5..99fd055 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -73,6 +73,9 @@
 
 #include "logging/logging.hpp"
 
+#include "module/authenticatee.hpp"
+#include "module/manager.hpp"
+
 #include "slave/constants.hpp"
 #include "slave/flags.hpp"
 #include "slave/paths.hpp"
@@ -261,6 +264,8 @@ void Slave::initialize()
             << "Must be less than " << REGISTER_RETRY_INTERVAL_MAX;
   }
 
+  authenticateeName = flags.authenticatee;
+
   if (flags.credential.isSome()) {
     const string& path =
       strings::remove(flags.credential.get(), "file://", strings::PREFIX);
@@ -664,13 +669,27 @@ void Slave::authenticate()
 
   LOG(INFO) << "Authenticating with master " << master.get();
 
-  CHECK_SOME(credential);
-
   CHECK(authenticatee == NULL);
-  authenticatee = new cram_md5::Authenticatee(credential.get(), self());
 
-  authenticating = authenticatee->authenticate(master.get())
-    .onAny(defer(self(), &Self::_authenticate));
+  if (authenticateeName == DEFAULT_AUTHENTICATEE) {
+    LOG(INFO) << "Using default CRAM-MD5 authenticatee";
+    authenticatee = new cram_md5::CRAMMD5Authenticatee();
+  } else {
+    Try<Authenticatee*> module =
+      modules::ModuleManager::create<Authenticatee>(authenticateeName);
+    if (module.isError()) {
+      EXIT(1) << "Could not create authenticatee module '"
+              << authenticateeName << "': " << module.error();
+    }
+    LOG(INFO) << "Using '" << authenticateeName << "' authenticatee";
+    authenticatee = module.get();
+  }
+
+  CHECK_SOME(credential);
+
+  authenticating =
+    authenticatee->authenticate(master.get(), self(), credential.get())
+      .onAny(defer(self(), &Self::_authenticate));
 
   delay(Seconds(5),
         self(),

http://git-wip-us.apache.org/repos/asf/mesos/blob/6451af3e/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index d9f9d2e..29bea65 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -70,9 +70,7 @@ namespace internal {
 
 class MasterDetector; // Forward declaration.
 
-namespace cram_md5 {
 class Authenticatee;
-} // namespace cram_md5 {
 
 namespace slave {
 
@@ -496,7 +494,10 @@ private:
 
   Option<Credential> credential;
 
-  cram_md5::Authenticatee* authenticatee;
+  // Authenticatee name as supplied via flags.
+  std::string authenticateeName;
+
+  Authenticatee* authenticatee;
 
   // Indicates if an authentication attempt is in progress.
   Option<Future<bool> > authenticating;