SERVER-32835 Refactor SaslSCRAMSHA1ClientConversation to be block independent

5 files changed, 107 insertions, 96 deletions

diff --git a/src/mongo/client/SConscript b/src/mongo/client/SConscript
index 35cd948c207..84c18eadc6a 100644
--- a/src/mongo/client/SConscript
+++ b/src/mongo/client/SConscript
@@ -84,7 +84,7 @@ saslClientSource = [
     'sasl_client_conversation.cpp',
     'sasl_client_session.cpp',
     'sasl_plain_client_conversation.cpp',
-    'sasl_scramsha1_client_conversation.cpp',
+    'sasl_scram_client_conversation.cpp',
 ]
 
 # Add in actual sasl dependencies if sasl is enabled, otherwise
diff --git a/src/mongo/client/native_sasl_client_session.cpp b/src/mongo/client/native_sasl_client_session.cpp
index e87997c9af1..bce0e220bc8 100644
--- a/src/mongo/client/native_sasl_client_session.cpp
+++ b/src/mongo/client/native_sasl_client_session.cpp
@@ -32,7 +32,7 @@
 #include "mongo/base/init.h"
 #include "mongo/client/sasl_client_conversation.h"
 #include "mongo/client/sasl_plain_client_conversation.h"
-#include "mongo/client/sasl_scramsha1_client_conversation.h"
+#include "mongo/client/sasl_scram_client_conversation.h"
 #include "mongo/client/scram_client_cache.h"
 #include "mongo/util/mongoutils/str.h"
 
diff --git a/src/mongo/client/sasl_scramsha1_client_conversation.cpp b/src/mongo/client/sasl_scram_client_conversation.cpp
index 9f7e0da6121..0a95f5f9cf8 100644
--- a/src/mongo/client/sasl_scramsha1_client_conversation.cpp
+++ b/src/mongo/client/sasl_scram_client_conversation.cpp
@@ -28,12 +28,11 @@
 
 #include "mongo/platform/basic.h"
 
-#include "mongo/client/sasl_scramsha1_client_conversation.h"
+#include "mongo/client/sasl_scram_client_conversation.h"
 
 #include <boost/algorithm/string/replace.hpp>
 
 #include "mongo/base/parse_number.h"
-#include "mongo/client/sasl_client_session.h"
 #include "mongo/client/scram_client_cache.h"
 #include "mongo/platform/random.h"
 #include "mongo/util/base64.h"
@@ -46,16 +45,7 @@ namespace mongo {
 using std::unique_ptr;
 using std::string;
 
-SaslSCRAMSHA1ClientConversation::SaslSCRAMSHA1ClientConversation(
-    SaslClientSession* saslClientSession, SCRAMSHA1ClientCache* clientCache)
-    : SaslClientConversation(saslClientSession),
-      _step(0),
-      _authMessage(),
-      _clientCache(clientCache),
-      _clientNonce() {}
-
-StatusWith<bool> SaslSCRAMSHA1ClientConversation::step(StringData inputData,
-                                                       std::string* outputData) {
+StatusWith<bool> SaslSCRAMClientConversation::step(StringData inputData, std::string* outputData) {
     std::vector<std::string> input = StringSplitter::split(inputData.toString(), ",");
     _step++;
 
@@ -71,7 +61,7 @@ StatusWith<bool> SaslSCRAMSHA1ClientConversation::step(StringData inputData,
         default:
             return StatusWith<bool>(
                 ErrorCodes::AuthenticationFailed,
-                mongoutils::str::stream() << "Invalid SCRAM-SHA-1 authentication step: " << _step);
+                mongoutils::str::stream() << "Invalid SCRAM authentication step: " << _step);
     }
 }
 
@@ -88,7 +78,7 @@ static void encodeSCRAMUsername(std::string& user) {
  * Generate client-first-message of the form:
  * n,a=authzid,n=encoded-username,r=client-nonce
  */
-StatusWith<bool> SaslSCRAMSHA1ClientConversation::_firstStep(std::string* outputData) {
+StatusWith<bool> SaslSCRAMClientConversation::_firstStep(std::string* outputData) {
     if (_saslClientSession->getParameter(SaslClientSession::parameterPassword).empty()) {
         return StatusWith<bool>(ErrorCodes::BadValue,
                                 mongoutils::str::stream() << "Empty client password provided");
@@ -127,34 +117,33 @@ StatusWith<bool> SaslSCRAMSHA1ClientConversation::_firstStep(std::string* output
  * c=channel-binding(base64),r=client-nonce|server-nonce,p=ClientProof
  *
  **/
-StatusWith<bool> SaslSCRAMSHA1ClientConversation::_secondStep(const std::vector<string>& input,
-                                                              std::string* outputData) {
+StatusWith<bool> SaslSCRAMClientConversation::_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 first SCRAM-SHA-1 server message, got "
+                << "Incorrect number of arguments for first SCRAM server message, got "
                 << input.size()
                 << " expected 3");
     } else if (!str::startsWith(input[0], "r=") || input[0].size() < 2) {
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream() << "Incorrect SCRAM-SHA-1 client|server nonce: " << input[0]);
+        return StatusWith<bool>(ErrorCodes::BadValue,
+                                mongoutils::str::stream() << "Incorrect SCRAM client|server nonce: "
+                                                          << input[0]);
     } else if (!str::startsWith(input[1], "s=") || input[1].size() < 6) {
         return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream() << "Incorrect SCRAM-SHA-1 salt: "
-                                                          << input[1]);
+                                mongoutils::str::stream() << "Incorrect SCRAM salt: " << input[1]);
     } else if (!str::startsWith(input[2], "i=") || input[2].size() < 3) {
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream() << "Incorrect SCRAM-SHA-1 iteration count: " << input[2]);
+        return StatusWith<bool>(ErrorCodes::BadValue,
+                                mongoutils::str::stream() << "Incorrect SCRAM iteration count: "
+                                                          << input[2]);
     }
 
     std::string nonce = input[0].substr(2);
     if (!str::startsWith(nonce, _clientNonce)) {
         return StatusWith<bool>(ErrorCodes::BadValue,
                                 mongoutils::str::stream()
-                                    << "Server SCRAM-SHA-1 nonce does not match client nonce: "
+                                    << "Server SCRAM nonce does not match client nonce: "
                                     << input[0]);
     }
 
@@ -163,10 +152,9 @@ StatusWith<bool> SaslSCRAMSHA1ClientConversation::_secondStep(const std::vector<
 
     Status status = parseNumberFromStringWithBase(input[2].substr(2), 10, &iterationCount);
     if (status != Status::OK()) {
-        return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream()
-                                    << "Failed to parse SCRAM-SHA-1 iteration count: "
-                                    << input[2]);
+        return StatusWith<bool>(
+            ErrorCodes::BadValue,
+            mongoutils::str::stream() << "Failed to parse SCRAM iteration count: " << input[2]);
     }
 
     // Append client-final-message-without-proof to _authMessage
@@ -179,28 +167,8 @@ StatusWith<bool> SaslSCRAMSHA1ClientConversation::_secondStep(const std::vector<
         return StatusWith<bool>(ex.toStatus());
     }
 
-    scram::SHA1Presecrets presecrets(
-        _saslClientSession->getParameter(SaslClientSession::parameterPassword).toString(),
-        std::vector<std::uint8_t>(decodedSalt.begin(), decodedSalt.end()),
-        iterationCount);
-
-    StatusWith<HostAndPort> targetHost = HostAndPort::parse(
-        _saslClientSession->getParameter(SaslClientSession::parameterServiceHostAndPort));
-
-    if (targetHost.isOK()) {
-        _credentials = _clientCache->getCachedSecrets(targetHost.getValue(), presecrets);
-
-        if (!_credentials) {
-            _credentials = presecrets;
-
-            _clientCache->setCachedSecrets(
-                std::move(targetHost.getValue()), std::move(presecrets), _credentials);
-        }
-    } else {
-        _credentials = presecrets;
-    }
-
-    std::string clientProof = _credentials.generateClientProof(_authMessage);
+    auto clientProof = generateClientProof(
+        std::vector<std::uint8_t>(decodedSalt.begin(), decodedSalt.end()), iterationCount);
 
     StringBuilder sb;
     sb << "c=biws,r=" << nonce << ",p=" << clientProof;
@@ -216,40 +184,35 @@ StatusWith<bool> SaslSCRAMSHA1ClientConversation::_secondStep(const std::vector<
  * or failed authentication server-final-message on the form:
  * e=message
  **/
-StatusWith<bool> SaslSCRAMSHA1ClientConversation::_thirdStep(const std::vector<string>& input,
-                                                             std::string* outputData) {
+StatusWith<bool> SaslSCRAMClientConversation::_thirdStep(const std::vector<string>& input,
+                                                         std::string* outputData) {
     if (input.size() != 1) {
         return StatusWith<bool>(
             ErrorCodes::BadValue,
             mongoutils::str::stream()
-                << "Incorrect number of arguments for final SCRAM-SHA-1 server message, got "
+                << "Incorrect number of arguments for final SCRAM server message, got "
                 << input.size()
                 << " expected 1");
     } else if (input[0].size() < 3) {
-        return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream()
-                                    << "Incorrect SCRAM-SHA-1 server message length: "
-                                    << input[0]);
+        return StatusWith<bool>(
+            ErrorCodes::BadValue,
+            mongoutils::str::stream() << "Incorrect SCRAM server message length: " << input[0]);
     } else if (str::startsWith(input[0], "e=")) {
         return StatusWith<bool>(ErrorCodes::AuthenticationFailed,
-                                mongoutils::str::stream() << "SCRAM-SHA-1 authentication failure: "
+                                mongoutils::str::stream() << "SCRAM authentication failure: "
                                                           << input[0].substr(2));
     } else if (!str::startsWith(input[0], "v=")) {
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream() << "Incorrect SCRAM-SHA-1 ServerSignature: " << input[0]);
+        return StatusWith<bool>(ErrorCodes::BadValue,
+                                mongoutils::str::stream() << "Incorrect SCRAM ServerSignature: "
+                                                          << input[0]);
     }
 
-    bool validServerSignature =
-        _credentials.verifyServerSignature(_authMessage, base64::decode(input[0].substr(2)));
-
-    if (!validServerSignature) {
+    if (!verifyServerSignature(base64::decode(input[0].substr(2)))) {
         *outputData = "e=Invalid server signature";
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream()
-                << "Client failed to verify SCRAM-SHA-1 ServerSignature, received "
-                << input[0].substr(2));
+        return StatusWith<bool>(ErrorCodes::BadValue,
+                                mongoutils::str::stream()
+                                    << "Client failed to verify SCRAM ServerSignature, received "
+                                    << input[0].substr(2));
     }
 
     *outputData = "";
diff --git a/src/mongo/client/sasl_scramsha1_client_conversation.h b/src/mongo/client/sasl_scram_client_conversation.h
index 1d262dc9f10..f714e3145d7 100644
--- a/src/mongo/client/sasl_scramsha1_client_conversation.h
+++ b/src/mongo/client/sasl_scram_client_conversation.h
@@ -35,6 +35,7 @@
 #include "mongo/base/status.h"
 #include "mongo/base/string_data.h"
 #include "mongo/client/sasl_client_conversation.h"
+#include "mongo/client/sasl_client_session.h"
 #include "mongo/client/scram_client_cache.h"
 #include "mongo/crypto/mechanism_scram.h"
 
@@ -43,24 +44,31 @@ namespace mongo {
 /**
  *  Client side authentication session for SASL PLAIN.
  */
-class SaslSCRAMSHA1ClientConversation : public SaslClientConversation {
-    MONGO_DISALLOW_COPYING(SaslSCRAMSHA1ClientConversation);
+class SaslSCRAMClientConversation : public SaslClientConversation {
+    MONGO_DISALLOW_COPYING(SaslSCRAMClientConversation);
 
 public:
-    /**
-     * Implements the client side of a SASL PLAIN mechanism session.
-     **/
-    SaslSCRAMSHA1ClientConversation(SaslClientSession* saslClientSession,
-                                    SCRAMSHA1ClientCache* clientCache);
+    using SaslClientConversation::SaslClientConversation;
 
     /**
-     * Takes one step in a SCRAM-SHA-1 conversation.
+     * Takes one step in a SCRAM conversation.
      *
      * @return !Status::OK() for failure. The boolean part indicates if the
      * authentication conversation is finished or not.
      *
      **/
-    virtual StatusWith<bool> step(StringData inputData, std::string* outputData);
+    StatusWith<bool> step(StringData inputData, std::string* outputData) override;
+
+    /**
+     * Initialize the Presecrets/Secrets and return signed client proof.
+     */
+    virtual std::string generateClientProof(const std::vector<std::uint8_t>& salt,
+                                            size_t iterationCount) = 0;
+
+    /**
+     * Verify the server's signature.
+     */
+    virtual bool verifyServerSignature(StringData sig) const = 0;
 
 private:
     /**
@@ -78,15 +86,55 @@ private:
      **/
     StatusWith<bool> _thirdStep(const std::vector<std::string>& input, std::string* outputData);
 
-    int _step;
+protected:
+    int _step{0};
     std::string _authMessage;
 
-    // Secrets and secrets cache
-    scram::SHA1Secrets _credentials;
-    SCRAMSHA1ClientCache* const _clientCache;
-
     // client and server nonce concatenated
     std::string _clientNonce;
 };
 
+template <typename HashBlock>
+class SaslSCRAMClientConversationImpl : public SaslSCRAMClientConversation {
+public:
+    SaslSCRAMClientConversationImpl(SaslClientSession* saslClientSession,
+                                    SCRAMClientCache<HashBlock>* clientCache)
+        : SaslSCRAMClientConversation(saslClientSession), _clientCache(clientCache) {}
+
+    std::string generateClientProof(const std::vector<std::uint8_t>& salt,
+                                    size_t iterationCount) final {
+        scram::Presecrets<HashBlock> presecrets(
+            _saslClientSession->getParameter(SaslClientSession::parameterPassword).toString(),
+            salt,
+            iterationCount);
+
+        auto targetHost = HostAndPort::parse(
+            _saslClientSession->getParameter(SaslClientSession::parameterServiceHostAndPort));
+        if (targetHost.isOK()) {
+            _credentials = _clientCache->getCachedSecrets(targetHost.getValue(), presecrets);
+            if (!_credentials) {
+                _credentials = presecrets;
+
+                _clientCache->setCachedSecrets(
+                    std::move(targetHost.getValue()), std::move(presecrets), _credentials);
+            }
+        } else {
+            _credentials = presecrets;
+        }
+
+        return _credentials.generateClientProof(_authMessage);
+    }
+
+    bool verifyServerSignature(StringData sig) const final {
+        return _credentials.verifyServerSignature(_authMessage, sig);
+    }
+
+private:
+    // Secrets and secrets cache
+    scram::Secrets<HashBlock> _credentials;
+    SCRAMClientCache<HashBlock>* const _clientCache;
+};
+
+using SaslSCRAMSHA1ClientConversation = SaslSCRAMClientConversationImpl<SHA1Block>;
+
 }  // namespace mongo
diff --git a/src/mongo/db/auth/sasl_scramsha1_test.cpp b/src/mongo/db/auth/sasl_scramsha1_test.cpp
index d2c972798bf..d6c7661a0c7 100644
--- a/src/mongo/db/auth/sasl_scramsha1_test.cpp
+++ b/src/mongo/db/auth/sasl_scramsha1_test.cpp
@@ -257,7 +257,7 @@ TEST_F(SCRAMSHA1Fixture, testServerStep1DoesNotIncludeNonceFromClientStep1) {
     });
     ASSERT_EQ(SCRAMStepsResult(SaslTestState(SaslTestState::kClient, 2),
                                Status(ErrorCodes::BadValue,
-                                      "Server SCRAM-SHA-1 nonce does not match client nonce: r=")),
+                                      "Server SCRAM nonce does not match client nonce: r=")),
               runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
 }
 
@@ -309,6 +309,7 @@ TEST_F(SCRAMSHA1Fixture, testClientStep2GivesBadProof) {
     ASSERT_EQ(SCRAMStepsResult(SaslTestState(SaslTestState::kServer, 2),
                                Status(ErrorCodes::AuthenticationFailed,
                                       "SCRAM-SHA-1 authentication failed, storedKey mismatch")),
+
               runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
 }
 
@@ -339,13 +340,12 @@ TEST_F(SCRAMSHA1Fixture, testServerStep2GivesBadVerifier) {
 
     auto result = runSteps(saslServerSession.get(), saslClientSession.get(), mutator);
 
-    ASSERT_EQ(
-        SCRAMStepsResult(
-            SaslTestState(SaslTestState::kClient, 3),
-            Status(ErrorCodes::BadValue,
-                   str::stream() << "Client failed to verify SCRAM-SHA-1 ServerSignature, received "
-                                 << encodedVerifier)),
-        result);
+    ASSERT_EQ(SCRAMStepsResult(
+                  SaslTestState(SaslTestState::kClient, 3),
+                  Status(ErrorCodes::BadValue,
+                         str::stream() << "Client failed to verify SCRAM ServerSignature, received "
+                                       << encodedVerifier)),
+              result);
 }