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 <[email protected]> Authored: Sat Nov 8 01:19:50 2014 +0100 Committer: Till Toenshoff <[email protected]> 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 {
