mesos git commit: Fixed a bug in authentication validation.

[bm...@apache.org]

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 <be...@gmail.com>
Authored: Thu Jul 30 15:13:23 2015 -0700
Committer: Benjamin Mahler <be...@gmail.com>
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.