SERVER-32834 Refactor SaslSCRAMSHA1ServerConversation to be block independent

5 files changed, 85 insertions, 63 deletions

diff --git a/src/mongo/db/auth/SConscript b/src/mongo/db/auth/SConscript
index 7c29c354026..e1bfbbabeac 100644
--- a/src/mongo/db/auth/SConscript
+++ b/src/mongo/db/auth/SConscript
@@ -172,7 +172,7 @@ env.Library('saslauth',
              ['native_sasl_authentication_session.cpp',
              'sasl_authentication_session.cpp',
              'sasl_plain_server_conversation.cpp',
-             'sasl_scramsha1_server_conversation.cpp',
+             'sasl_scram_server_conversation.cpp',
              'sasl_server_conversation.cpp'],
              LIBDEPS=[
                 'authcore',
diff --git a/src/mongo/db/auth/native_sasl_authentication_session.cpp b/src/mongo/db/auth/native_sasl_authentication_session.cpp
index e6316452baf..bfddca7fc5f 100644
--- a/src/mongo/db/auth/native_sasl_authentication_session.cpp
+++ b/src/mongo/db/auth/native_sasl_authentication_session.cpp
@@ -44,7 +44,7 @@
 #include "mongo/db/auth/authz_session_external_state_mock.h"
 #include "mongo/db/auth/sasl_options.h"
 #include "mongo/db/auth/sasl_plain_server_conversation.h"
-#include "mongo/db/auth/sasl_scramsha1_server_conversation.h"
+#include "mongo/db/auth/sasl_scram_server_conversation.h"
 #include "mongo/db/commands.h"
 #include "mongo/stdx/memory.h"
 #include "mongo/util/assert_util.h"
diff --git a/src/mongo/db/auth/sasl_scramsha1_server_conversation.cpp b/src/mongo/db/auth/sasl_scram_server_conversation.cpp
index 084907333a1..f332d7e200d 100644
--- a/src/mongo/db/auth/sasl_scramsha1_server_conversation.cpp
+++ b/src/mongo/db/auth/sasl_scram_server_conversation.cpp
@@ -30,12 +30,11 @@
 
 #include "mongo/platform/basic.h"
 
-#include "mongo/db/auth/sasl_scramsha1_server_conversation.h"
+#include "mongo/db/auth/sasl_scram_server_conversation.h"
 
 #include <boost/algorithm/string/join.hpp>
 #include <boost/algorithm/string/replace.hpp>
 
-#include "mongo/crypto/mechanism_scram.h"
 #include "mongo/crypto/sha1_block.h"
 #include "mongo/db/auth/sasl_options.h"
 #include "mongo/platform/random.h"
@@ -50,19 +49,14 @@ namespace mongo {
 using std::unique_ptr;
 using std::string;
 
-SaslSCRAMSHA1ServerConversation::SaslSCRAMSHA1ServerConversation(
-    SaslAuthenticationSession* saslAuthSession)
-    : SaslServerConversation(saslAuthSession), _step(0), _authMessage(""), _nonce("") {}
-
-StatusWith<bool> SaslSCRAMSHA1ServerConversation::step(StringData inputData,
-                                                       std::string* outputData) {
+StatusWith<bool> SaslSCRAMServerConversation::step(StringData inputData, std::string* outputData) {
     std::vector<std::string> input = StringSplitter::split(inputData.toString(), ",");
     _step++;
 
     if (_step > 3 || _step <= 0) {
-        return StatusWith<bool>(
-            ErrorCodes::AuthenticationFailed,
-            mongoutils::str::stream() << "Invalid SCRAM-SHA-1 authentication step: " << _step);
+        return StatusWith<bool>(ErrorCodes::AuthenticationFailed,
+                                mongoutils::str::stream() << "Invalid SCRAM authentication step: "
+                                                          << _step);
     }
     if (_step == 1) {
         return _firstStep(input, outputData);
@@ -94,8 +88,8 @@ static void decodeSCRAMUsername(std::string& user) {
  *
  * NOTE: we are ignoring the authorization ID part of the message
  */
-StatusWith<bool> SaslSCRAMSHA1ServerConversation::_firstStep(std::vector<string>& input,
-                                                             std::string* outputData) {
+StatusWith<bool> SaslSCRAMServerConversation::_firstStep(std::vector<string>& input,
+                                                         std::string* outputData) {
     std::string authzId = "";
 
     if (input.size() == 4) {
@@ -108,7 +102,7 @@ StatusWith<bool> SaslSCRAMSHA1ServerConversation::_firstStep(std::vector<string>
          */
         if (!str::startsWith(input[1], "a=") || input[1].size() < 3) {
             return StatusWith<bool>(ErrorCodes::BadValue,
-                                    mongoutils::str::stream() << "Incorrect SCRAM-SHA-1 authzid: "
+                                    mongoutils::str::stream() << "Incorrect SCRAM authzid: "
                                                               << input[1]);
         }
         authzId = input[1].substr(2);
@@ -119,7 +113,7 @@ StatusWith<bool> SaslSCRAMSHA1ServerConversation::_firstStep(std::vector<string>
         return StatusWith<bool>(
             ErrorCodes::BadValue,
             mongoutils::str::stream()
-                << "Incorrect number of arguments for first SCRAM-SHA-1 client message, got "
+                << "Incorrect number of arguments for first SCRAM client message, got "
                 << input.size()
                 << " expected 4");
     } else if (str::startsWith(input[0], "p=")) {
@@ -127,24 +121,23 @@ StatusWith<bool> SaslSCRAMSHA1ServerConversation::_firstStep(std::vector<string>
                                 mongoutils::str::stream()
                                     << "Server does not support channel binding");
     } else if (input[0] != "n" && input[0] != "y") {
-        return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream()
-                                    << "Incorrect SCRAM-SHA-1 client message prefix: "
-                                    << input[0]);
+        return StatusWith<bool>(
+            ErrorCodes::BadValue,
+            mongoutils::str::stream() << "Incorrect SCRAM client message prefix: " << input[0]);
     } else if (!str::startsWith(input[1], "n=") || input[1].size() < 3) {
         return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream() << "Incorrect SCRAM-SHA-1 user name: "
+                                mongoutils::str::stream() << "Incorrect SCRAM user name: "
                                                           << input[1]);
     } else if (!str::startsWith(input[2], "r=") || input[2].size() < 6) {
         return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream() << "Incorrect SCRAM-SHA-1 client nonce: "
+                                mongoutils::str::stream() << "Incorrect SCRAM client nonce: "
                                                           << input[2]);
     }
 
     _user = input[1].substr(2);
     if (!authzId.empty() && _user != authzId) {
         return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream() << "SCRAM-SHA-1 user name " << _user
+                                mongoutils::str::stream() << "SCRAM user name " << _user
                                                           << " does not match authzid "
                                                           << authzId);
     }
@@ -190,7 +183,7 @@ StatusWith<bool> SaslSCRAMSHA1ServerConversation::_firstStep(std::vector<string>
 
     if (!_creds.scram.isValid()) {
         return Status(ErrorCodes::AuthenticationFailed,
-                      "Unable to perform SCRAM-SHA-1 authentication for a user with missing "
+                      "Unable to perform SCRAM authentication for a user with missing "
                       "or invalid SCRAM credentials");
     }
 
@@ -229,26 +222,26 @@ StatusWith<bool> SaslSCRAMSHA1ServerConversation::_firstStep(std::vector<string>
  *
  * NOTE: we are ignoring the channel binding part of the message
 **/
-StatusWith<bool> SaslSCRAMSHA1ServerConversation::_secondStep(const std::vector<string>& input,
-                                                              std::string* outputData) {
+StatusWith<bool> SaslSCRAMServerConversation::_secondStep(const std::vector<string>& input,
+                                                          std::string* outputData) {
     if (input.size() != 3) {
         return StatusWith<bool>(
             ErrorCodes::BadValue,
             mongoutils::str::stream()
-                << "Incorrect number of arguments for second SCRAM-SHA-1 client message, got "
+                << "Incorrect number of arguments for second SCRAM client message, got "
                 << input.size()
                 << " expected 3");
     } else if (!str::startsWith(input[0], "c=") || input[0].size() < 3) {
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream() << "Incorrect SCRAM-SHA-1 channel binding: " << input[0]);
+        return StatusWith<bool>(ErrorCodes::BadValue,
+                                mongoutils::str::stream() << "Incorrect SCRAM channel binding: "
+                                                          << input[0]);
     } else if (!str::startsWith(input[1], "r=") || input[1].size() < 6) {
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream() << "Incorrect SCRAM-SHA-1 client|server nonce: " << input[1]);
+        return StatusWith<bool>(ErrorCodes::BadValue,
+                                mongoutils::str::stream() << "Incorrect SCRAM client|server nonce: "
+                                                          << input[1]);
     } else if (!str::startsWith(input[2], "p=") || input[2].size() < 3) {
         return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream() << "Incorrect SCRAM-SHA-1 ClientProof: "
+                                mongoutils::str::stream() << "Incorrect SCRAM ClientProof: "
                                                           << input[2]);
     }
 
@@ -261,7 +254,7 @@ StatusWith<bool> SaslSCRAMSHA1ServerConversation::_secondStep(const std::vector<
         return StatusWith<bool>(
             ErrorCodes::BadValue,
             mongoutils::str::stream()
-                << "Unmatched SCRAM-SHA-1 nonce received from client in second step, expected "
+                << "Unmatched SCRAM nonce received from client in second step, expected "
                 << _nonce
                 << " but received "
                 << nonce);
@@ -277,17 +270,15 @@ StatusWith<bool> SaslSCRAMSHA1ServerConversation::_secondStep(const std::vector<
     // ClientKey := ClientSignature XOR ClientProof
     // ServerSignature := HMAC(ServerKey, AuthMessage)
     invariant(_creds.scram.isValid());
-    scram::SHA1Secrets secrets(
-        "", base64::decode(_creds.scram.storedKey), base64::decode(_creds.scram.serverKey));
 
-    if (!secrets.verifyClientProof(_authMessage, base64::decode(clientProof))) {
+    if (!verifyClientProof(base64::decode(clientProof))) {
         return StatusWith<bool>(ErrorCodes::AuthenticationFailed,
                                 mongoutils::str::stream()
-                                    << "SCRAM-SHA-1 authentication failed, storedKey mismatch");
+                                    << "SCRAM authentication failed, storedKey mismatch");
     }
 
     // ServerSignature := HMAC(ServerKey, AuthMessage)
-    const auto serverSignature = secrets.generateServerSignature(_authMessage);
+    const auto serverSignature = generateServerSignature();
 
     StringBuilder sb;
     sb << "v=" << serverSignature;
diff --git a/src/mongo/db/auth/sasl_scramsha1_server_conversation.h b/src/mongo/db/auth/sasl_scram_server_conversation.h
index 68b3e226168..d131433f494 100644
--- a/src/mongo/db/auth/sasl_scramsha1_server_conversation.h
+++ b/src/mongo/db/auth/sasl_scram_server_conversation.h
@@ -34,22 +34,20 @@
 #include "mongo/base/disallow_copying.h"
 #include "mongo/base/status.h"
 #include "mongo/base/string_data.h"
+#include "mongo/crypto/mechanism_scram.h"
 #include "mongo/db/auth/sasl_server_conversation.h"
 
 namespace mongo {
 /**
  *  Server side authentication session for SASL SCRAM-SHA-1.
  */
-class SaslSCRAMSHA1ServerConversation : public SaslServerConversation {
-    MONGO_DISALLOW_COPYING(SaslSCRAMSHA1ServerConversation);
+class SaslSCRAMServerConversation : public SaslServerConversation {
+    MONGO_DISALLOW_COPYING(SaslSCRAMServerConversation);
 
 public:
-    /**
-     * Implements the server side of a SASL SCRAM-SHA-1 mechanism session.
-     **/
-    explicit SaslSCRAMSHA1ServerConversation(SaslAuthenticationSession* saslAuthSession);
-
-    virtual ~SaslSCRAMSHA1ServerConversation(){};
+    explicit SaslSCRAMServerConversation(SaslAuthenticationSession* session)
+        : SaslServerConversation(session) {}
+    ~SaslSCRAMServerConversation() override = default;
 
     /**
      * Take one step in a SCRAM-SHA-1 conversation.
@@ -58,7 +56,17 @@ public:
      * authentication conversation is finished or not.
      *
      **/
-    virtual StatusWith<bool> step(StringData inputData, std::string* outputData);
+    StatusWith<bool> step(StringData inputData, std::string* outputData) override;
+
+    /**
+     * Verify proof submitted by authenticating client.
+     */
+    virtual bool verifyClientProof(StringData) = 0;
+
+    /**
+     * Generate a signature to prove ourselves.
+     */
+    virtual std::string generateServerSignature() const = 0;
 
 private:
     /**
@@ -71,7 +79,8 @@ private:
      **/
     StatusWith<bool> _secondStep(const std::vector<std::string>& input, std::string* outputData);
 
-    int _step;
+protected:
+    int _step{0};
     std::string _authMessage;
     User::CredentialData _creds;
 
@@ -79,4 +88,27 @@ private:
     std::string _nonce;
 };
 
+template <typename HashBlock>
+class SaslSCRAMServerConversationImpl : public SaslSCRAMServerConversation {
+public:
+    explicit SaslSCRAMServerConversationImpl(SaslAuthenticationSession* session)
+        : SaslSCRAMServerConversation(session) {}
+    ~SaslSCRAMServerConversationImpl() override = default;
+
+    bool verifyClientProof(StringData clientProof) final {
+        _credentials = scram::Secrets<HashBlock>(
+            "", base64::decode(_creds.scram.storedKey), base64::decode(_creds.scram.serverKey));
+        return _credentials.verifyClientProof(_authMessage, clientProof);
+    }
+
+    std::string generateServerSignature() const final {
+        return _credentials.generateServerSignature(_authMessage);
+    }
+
+private:
+    scram::Secrets<HashBlock> _credentials;
+};
+
+using SaslSCRAMSHA1ServerConversation = SaslSCRAMServerConversationImpl<SHA1Block>;
+
 }  // namespace mongo
diff --git a/src/mongo/db/auth/sasl_scramsha1_test.cpp b/src/mongo/db/auth/sasl_scramsha1_test.cpp
index d6c7661a0c7..c1fd5d6cd82 100644
--- a/src/mongo/db/auth/sasl_scramsha1_test.cpp
+++ b/src/mongo/db/auth/sasl_scramsha1_test.cpp
@@ -35,7 +35,7 @@
 #include "mongo/db/auth/authz_manager_external_state_mock.h"
 #include "mongo/db/auth/authz_session_external_state_mock.h"
 #include "mongo/db/auth/native_sasl_authentication_session.h"
-#include "mongo/db/auth/sasl_scramsha1_server_conversation.h"
+#include "mongo/db/auth/sasl_scram_server_conversation.h"
 #include "mongo/db/service_context_noop.h"
 #include "mongo/stdx/memory.h"
 #include "mongo/unittest/unittest.h"
@@ -279,10 +279,10 @@ TEST_F(SCRAMSHA1Fixture, testClientStep2DoesNotIncludeNonceFromServerStep1) {
         std::string::iterator nonceEnd = std::find(nonceBegin, clientMessage.end(), ',');
         clientMessage = clientMessage.replace(nonceBegin, nonceEnd, "r=");
     });
-    ASSERT_EQ(SCRAMStepsResult(
-                  SaslTestState(SaslTestState::kServer, 2),
-                  Status(ErrorCodes::BadValue, "Incorrect SCRAM-SHA-1 client|server nonce: r=")),
-              runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
+    ASSERT_EQ(
+        SCRAMStepsResult(SaslTestState(SaslTestState::kServer, 2),
+                         Status(ErrorCodes::BadValue, "Incorrect SCRAM client|server nonce: r=")),
+        runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
 }
 
 TEST_F(SCRAMSHA1Fixture, testClientStep2GivesBadProof) {
@@ -308,7 +308,7 @@ TEST_F(SCRAMSHA1Fixture, testClientStep2GivesBadProof) {
 
     ASSERT_EQ(SCRAMStepsResult(SaslTestState(SaslTestState::kServer, 2),
                                Status(ErrorCodes::AuthenticationFailed,
-                                      "SCRAM-SHA-1 authentication failed, storedKey mismatch")),
+                                      "SCRAM authentication failed, storedKey mismatch")),
 
               runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
 }
@@ -424,11 +424,10 @@ TEST_F(SCRAMSHA1Fixture, testSCRAMWithInvalidChannelBinding) {
         clientMessage.replace(clientMessage.begin(), clientMessage.begin() + 1, "v=illegalGarbage");
     });
 
-    ASSERT_EQ(
-        SCRAMStepsResult(SaslTestState(SaslTestState::kServer, 1),
-                         Status(ErrorCodes::BadValue,
-                                "Incorrect SCRAM-SHA-1 client message prefix: v=illegalGarbage")),
-        runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
+    ASSERT_EQ(SCRAMStepsResult(SaslTestState(SaslTestState::kServer, 1),
+                               Status(ErrorCodes::BadValue,
+                                      "Incorrect SCRAM client message prefix: v=illegalGarbage")),
+              runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
 }
 
 TEST_F(SCRAMSHA1Fixture, testNULLInPassword) {
@@ -488,7 +487,7 @@ TEST_F(SCRAMSHA1Fixture, testIncorrectPassword) {
 
     ASSERT_EQ(SCRAMStepsResult(SaslTestState(SaslTestState::kServer, 2),
                                Status(ErrorCodes::AuthenticationFailed,
-                                      "SCRAM-SHA-1 authentication failed, storedKey mismatch")),
+                                      "SCRAM authentication failed, storedKey mismatch")),
               runSteps(saslServerSession.get(), saslClientSession.get()));
 }