Repository: mesos Updated Branches: refs/heads/master ba16f492b -> d144fc7ba
Fixed a bug in authentication validation. When --flags.authenticate_frameworks is false, we still should be checking that the authenticated principal matches the framework's principal. Review: https://reviews.apache.org/r/36958 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d144fc7b Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d144fc7b Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d144fc7b Branch: refs/heads/master Commit: d144fc7ba8c576687addae590afa3d83bcdf105e Parents: ba16f49 Author: Benjamin Mahler <[email protected]> Authored: Thu Jul 30 15:13:23 2015 -0700 Committer: Benjamin Mahler <[email protected]> Committed: Thu Jul 30 17:55:08 2015 -0700 ---------------------------------------------------------------------- src/master/master.cpp | 31 +++++++++++++------------ src/tests/authentication_tests.cpp | 40 ++++++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/d144fc7b/src/master/master.cpp ---------------------------------------------------------------------- diff --git a/src/master/master.cpp b/src/master/master.cpp index ce27dbe..351a3c2 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -1555,22 +1555,21 @@ Option<Error> Master::validateFrameworkAuthentication( return Error("Re-authentication in progress"); } - if (flags.authenticate_frameworks) { - if (!authenticated.contains(from)) { - // This could happen if another authentication request came - // through before we are here or if a framework tried to - // (re-)register without authentication. - return Error("Framework at " + stringify(from) + " is not authenticated"); - } - - // TODO(bmahler): Currently the scheduler driver does not - // set 'principal', so we allow framweworks to omit it. - if (frameworkInfo.has_principal() && - frameworkInfo.principal() != authenticated[from]) { - return Error("Framework principal '" + frameworkInfo.principal() + "'" - " does not match authenticated principal" - " '" + authenticated[from] + "'"); - } + if (flags.authenticate_frameworks && !authenticated.contains(from)) { + // This could happen if another authentication request came + // through before we are here or if a framework tried to + // (re-)register without authentication. + return Error("Framework at " + stringify(from) + " is not authenticated"); + } + + // TODO(bmahler): Currently the scheduler driver does not + // set 'principal', so we allow frameworks to omit it. + if (frameworkInfo.has_principal() && + authenticated.contains(from) && + frameworkInfo.principal() != authenticated[from]) { + return Error("Framework principal '" + frameworkInfo.principal() + "'" + " does not match authenticated principal" + " '" + authenticated[from] + "'"); } return None(); http://git-wip-us.apache.org/repos/asf/mesos/blob/d144fc7b/src/tests/authentication_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/authentication_tests.cpp b/src/tests/authentication_tests.cpp index d5f5574..d80dce0 100644 --- a/src/tests/authentication_tests.cpp +++ b/src/tests/authentication_tests.cpp @@ -161,7 +161,7 @@ TEST_F(AuthenticationTest, DisableSlaveAuthentication) // This test verifies that an authenticated framework is denied // registration by the master if it uses a different -// FrameworkInfo::principal. +// FrameworkInfo.principal than Credential.principal. TEST_F(AuthenticationTest, MismatchedFrameworkInfoPrincipal) { Try<PID<Master> > master = StartMaster(); @@ -193,6 +193,44 @@ TEST_F(AuthenticationTest, MismatchedFrameworkInfoPrincipal) } +// This test verifies that an authenticated framework is denied +// registration by the master if it uses a different +// FrameworkInfo::principal than Credential.principal, even +// when authentication is not required. +TEST_F(AuthenticationTest, DisabledFrameworkAuthenticationPrincipalMismatch) +{ + master::Flags flags = CreateMasterFlags(); + flags.authenticate_frameworks = false; // Authentication not required. + + Try<PID<Master> > master = StartMaster(flags); + ASSERT_SOME(master); + + MockScheduler sched; + FrameworkInfo frameworkInfo = DEFAULT_FRAMEWORK_INFO; + frameworkInfo.set_principal("mismatched-principal"); + + MesosSchedulerDriver driver( + &sched, + frameworkInfo, + master.get(), + DEFAULT_CREDENTIAL); + + Future<Nothing> error; + EXPECT_CALL(sched, error(&driver, _)) + .WillOnce(FutureSatisfy(&error)); + + driver.start(); + + // Scheduler should get error message from the master. + AWAIT_READY(error); + + driver.stop(); + driver.join(); + + Shutdown(); +} + + // This test verifies that if a Framework successfully authenticates // but does not set FrameworkInfo::principal, it is allowed to // register.
