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 2014/11/08 01:41:40 UTC

mesos git commit: Fixed Authenticator SASL auxiliary memory access.

Repository: mesos
Updated Branches:
  refs/heads/master 69705dae4 -> ce82e81c1


Fixed Authenticator SASL auxiliary memory access.

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


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

Branch: refs/heads/master
Commit: ce82e81c12d7f91b158c73465c47725331626f32
Parents: 69705da
Author: Till Toenshoff <to...@me.com>
Authored: Sat Nov 8 01:19:50 2014 +0100
Committer: Till Toenshoff <to...@me.com>
Committed: Sat Nov 8 01:19:54 2014 +0100

----------------------------------------------------------------------
 src/authentication/authenticator.hpp          |  6 +-
 src/authentication/cram_md5/authenticator.hpp | 18 +-----
 src/master/master.cpp                         | 10 ++--
 src/tests/cram_md5_authentication_tests.cpp   | 64 +++++++++++-----------
 4 files changed, 43 insertions(+), 55 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/ce82e81c/src/authentication/authenticator.hpp
----------------------------------------------------------------------
diff --git a/src/authentication/authenticator.hpp b/src/authentication/authenticator.hpp
index 2f95db1..460494a 100644
--- a/src/authentication/authenticator.hpp
+++ b/src/authentication/authenticator.hpp
@@ -30,15 +30,15 @@
 namespace mesos {
 namespace internal {
 
+// Note that this interface definition is not hardened yet and will
+// slightly change within the next release. See MESOS-2050.
 class Authenticator
 {
 public:
   Authenticator() {}
   virtual ~Authenticator() {}
 
-  virtual Try<Nothing> initialize(
-      const process::UPID& clientPid,
-      const Option<mesos::Credentials>& credentials) = 0;
+  virtual void initialize(const process::UPID& clientPid) = 0;
 
   // Returns the principal of the Authenticatee if successfully
   // authenticated otherwise None or an error. Note that we

http://git-wip-us.apache.org/repos/asf/mesos/blob/ce82e81c/src/authentication/cram_md5/authenticator.hpp
----------------------------------------------------------------------
diff --git a/src/authentication/cram_md5/authenticator.hpp b/src/authentication/cram_md5/authenticator.hpp
index 601248d..a23bf5d 100644
--- a/src/authentication/cram_md5/authenticator.hpp
+++ b/src/authentication/cram_md5/authenticator.hpp
@@ -56,10 +56,7 @@ public:
   CRAMMD5Authenticator();
   virtual ~CRAMMD5Authenticator();
 
-  virtual Try<Nothing> initialize(
-      const process::UPID& clientPid,
-      const Option<Credentials>& credentials);
-
+  virtual void initialize(const process::UPID& clientPid);
 
   // Returns the principal of the Authenticatee if successfully
   // authenticated otherwise None or an error. Note that we
@@ -486,22 +483,11 @@ CRAMMD5Authenticator::~CRAMMD5Authenticator()
 }
 
 
-Try<Nothing> CRAMMD5Authenticator::initialize(
-    const process::UPID& pid,
-    const Option<Credentials>& credentials)
+void CRAMMD5Authenticator::initialize(const process::UPID& pid)
 {
-  if (credentials.isSome()) {
-    // Load "registration" credentials into SASL based Authenticator.
-    secrets::load(credentials.get());
-  } else {
-    return Error("Authentication requires credentials");
-  }
-
   CHECK(process == NULL) << "Authenticator has already been initialized";
   process = new CRAMMD5AuthenticatorProcess(pid);
   process::spawn(process);
-
-  return Nothing();
 }
 
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/ce82e81c/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index a860496..5712b17 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -402,6 +402,11 @@ void Master::initialize()
     }
     // Store credentials in master to use them in routes.
     credentials = _credentials.get();
+
+    // Give Authenticator access to credentials.
+    // TODO(tillt): Move this into a mechanism (module) specific
+    // Authenticator factory. See MESOS-2050.
+    cram_md5::secrets::load(credentials.get());
   }
 
   if (authorizer.isSome()) {
@@ -3918,10 +3923,7 @@ void Master::authenticate(const UPID& from, const UPID& pid)
   }
   Owned<Authenticator> authenticator_ = Owned<Authenticator>(authenticator);
 
-  Try<Nothing> initialize = authenticator_->initialize(from, credentials);
-  if (initialize.isError()) {
-    EXIT(1) << "Failed to initialize authenticator: " << initialize.error();
-  }
+  authenticator_->initialize(from);
 
   // Start authentication.
   const Future<Option<string>>& future = authenticator_->authenticate()

http://git-wip-us.apache.org/repos/asf/mesos/blob/ce82e81c/src/tests/cram_md5_authentication_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/cram_md5_authentication_tests.cpp b/src/tests/cram_md5_authentication_tests.cpp
index 74ea2ad..1ed2312 100644
--- a/src/tests/cram_md5_authentication_tests.cpp
+++ b/src/tests/cram_md5_authentication_tests.cpp
@@ -54,6 +54,13 @@ TEST(CRAMMD5Authentication, success)
   credential1.set_principal("benh");
   credential1.set_secret("secret");
 
+  Credentials credentials;
+  Credential* credential2 = credentials.add_credentials();
+  credential2->set_principal(credential1.principal());
+  credential2->set_secret(credential1.secret());
+
+  secrets::load(credentials);
+
   Authenticatee authenticatee(credential1, UPID());
 
   Future<Message> message =
@@ -63,13 +70,8 @@ TEST(CRAMMD5Authentication, success)
 
   AWAIT_READY(message);
 
-  Credentials credentials;
-  Credential* credential2 = credentials.add_credentials();
-  credential2->set_principal(credential1.principal());
-  credential2->set_secret(credential1.secret());
-
   CRAMMD5Authenticator authenticator;
-  EXPECT_SOME(authenticator.initialize(message.get().from, credentials));
+  authenticator.initialize(message.get().from);
 
   Future<Option<string>> principal = authenticator.authenticate();
 
@@ -91,6 +93,13 @@ TEST(CRAMMD5Authentication, failed1)
   credential1.set_principal("benh");
   credential1.set_secret("secret");
 
+  Credentials credentials;
+  Credential* credential2 = credentials.add_credentials();
+  credential2->set_principal(credential1.principal());
+  credential2->set_secret("secret2");
+
+  secrets::load(credentials);
+
   Authenticatee authenticatee(credential1, UPID());
 
   Future<Message> message =
@@ -100,13 +109,8 @@ TEST(CRAMMD5Authentication, failed1)
 
   AWAIT_READY(message);
 
-  Credentials credentials;
-  Credential* credential2 = credentials.add_credentials();
-  credential2->set_principal(credential1.principal());
-  credential2->set_secret("secret2");
-
   CRAMMD5Authenticator authenticator;
-  EXPECT_SOME(authenticator.initialize(message.get().from, credentials));
+  authenticator.initialize(message.get().from);
 
   Future<Option<string>> server = authenticator.authenticate();
 
@@ -128,6 +132,13 @@ TEST(CRAMMD5Authentication, failed2)
   credential1.set_principal("benh");
   credential1.set_secret("secret");
 
+  Credentials credentials;
+  Credential* credential2 = credentials.add_credentials();
+  credential2->set_principal("vinod");
+  credential2->set_secret(credential1.secret());
+
+  secrets::load(credentials);
+
   Authenticatee authenticatee(credential1, UPID());
 
   Future<Message> message =
@@ -137,13 +148,8 @@ TEST(CRAMMD5Authentication, failed2)
 
   AWAIT_READY(message);
 
-  Credentials credentials;
-  Credential* credential2 = credentials.add_credentials();
-  credential2->set_principal("vinod");
-  credential2->set_secret(credential1.secret());
-
   CRAMMD5Authenticator authenticator;
-  EXPECT_SOME(authenticator.initialize(message.get().from, credentials));
+  authenticator.initialize(message.get().from);
 
   Future<Option<string>> server = authenticator.authenticate();
 
@@ -167,6 +173,13 @@ TEST(CRAMMD5Authentication, AuthenticatorDestructionRace)
   credential1.set_principal("benh");
   credential1.set_secret("secret");
 
+  Credentials credentials;
+  Credential* credential2 = credentials.add_credentials();
+  credential2->set_principal(credential1.principal());
+  credential2->set_secret(credential1.secret());
+
+  secrets::load(credentials);
+
   Authenticatee authenticatee(credential1, UPID());
 
   Future<Message> message =
@@ -176,13 +189,8 @@ TEST(CRAMMD5Authentication, AuthenticatorDestructionRace)
 
   AWAIT_READY(message);
 
-  Credentials credentials;
-  Credential* credential2 = credentials.add_credentials();
-  credential2->set_principal(credential1.principal());
-  credential2->set_secret(credential1.secret());
-
   CRAMMD5Authenticator* authenticator = new CRAMMD5Authenticator();
-  EXPECT_SOME(authenticator->initialize(message.get().from, credentials));
+  authenticator->initialize(message.get().from);
 
   // Drop the AuthenticationStepMessage from authenticator to keep
   // the authentication from getting completed.
@@ -208,14 +216,6 @@ TEST(CRAMMD5Authentication, AuthenticatorDestructionRace)
   terminate(pid);
 }
 
-
-// Missing credentials should fail the initializing.
-TEST(CRAMMD5Authentication, AuthenticatorCredentialsMissing)
-{
-  CRAMMD5Authenticator authenticator;
-  EXPECT_ERROR(authenticator.initialize(UPID(), None()));
-}
-
 } // namespace cram_md5 {
 } // namespace internal {
 } // namespace mesos {