[mesos] 03/04: Fixed an authentication request amplification issue in the master.

[bm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bmahler pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 7b44aba601a7ac87aaa7c37dad288a5db47f7906
Author: Benjamin Mahler <bm...@apache.org>
AuthorDate: Fri Aug 10 17:14:05 2018 -0700

    Fixed an authentication request amplification issue in the master.
    
    Per MESOS-9144, re-enqueuing authentication requests leads to an
    amplification effect which can overwhelm the master if requests
    continue to arrive rapidly on a heavily loaded master. This patch
    avoids the re-enqueing and ensures the master immediately processes
    a new authentication request for a client.
    
    Review: https://reviews.apache.org/r/68306
---
 src/master/master.cpp | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/master/master.cpp b/src/master/master.cpp
index 17bd283..ad83be2 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -9563,7 +9563,7 @@ void Master::authenticate(const UPID& from, const UPID& pid)
   //       about this discrepancy via ping messages so that it can
   //       reregister.
 
-  authenticated.erase(pid);
+  bool erased = authenticated.erase(pid) > 0;
 
   if (authenticator.isNone()) {
     // The default authenticator is CRAM-MD5 rather than none.
@@ -9586,30 +9586,28 @@ void Master::authenticate(const UPID& from, const UPID& pid)
     return;
   }
 
+  // If a new authentication is occurring for a client that already
+  // has an authentication in progress, we discard the old one
+  // (since the client is no longer interested in it) and
+  // immediately proceed with the new authentication.
   if (authenticating.contains(pid)) {
-    LOG(INFO) << "Queuing up authentication request from " << pid
-              << " because authentication is still in progress";
-
-    // Try to cancel the in progress authentication by discarding the
-    // future.
-    authenticating[pid].discard();
-
-    // Retry after the current authenticator session finishes.
-    authenticating[pid]
-      .onAny(defer(self(), &Self::authenticate, from, pid));
+    authenticating.at(pid).discard();
+    authenticating.erase(pid);
 
-    return;
+    LOG(INFO) << "Re-authenticating " << pid << ";"
+              << " discarding outstanding authentication";
+  } else {
+    LOG(INFO) << "Authenticating " << pid
+              << (erased ? "; clearing previous authentication" : "");
   }
 
-  LOG(INFO) << "Authenticating " << pid;
-
   // Start authentication.
   const Future<Option<string>> future = authenticator.get()->authenticate(from);
 
   // Save our state.
   authenticating[pid] = future;
 
-  future.onAny(defer(self(), &Self::_authenticate, pid, lambda::_1));
+  future.onAny(defer(self(), &Self::_authenticate, pid, future));
 
   // Don't wait for authentication to complete forever.
   delay(flags.authentication_v0_timeout,
@@ -9623,6 +9621,13 @@ void Master::_authenticate(
     const UPID& pid,
     const Future<Option<string>>& future)
 {
+  // Ignore stale authentication results (if the authentication
+  // future has been overwritten).
+  if (authenticating.get(pid) != future) {
+    LOG(INFO) << "Ignoring stale authentication result of " << pid;
+    return;
+  }
+
   if (future.isReady() && future->isSome()) {
     LOG(INFO) << "Successfully authenticated principal '" << future->get()
               << "' at " << pid;
@@ -9638,7 +9643,6 @@ void Master::_authenticate(
     LOG(INFO) << "Authentication of " << pid << " was discarded";
   }
 
-  CHECK(authenticating.contains(pid));
   authenticating.erase(pid);
 }