SERVER-32968 Make SCRAM-SHA-1 mechanism ignore optional extensions

5 files changed, 267 insertions, 182 deletions

diff --git a/src/mongo/client/sasl_scram_client_conversation.cpp b/src/mongo/client/sasl_scram_client_conversation.cpp
index 5cca962271f..227a793486e 100644
--- a/src/mongo/client/sasl_scram_client_conversation.cpp
+++ b/src/mongo/client/sasl_scram_client_conversation.cpp
@@ -46,18 +46,15 @@ using std::unique_ptr;
 using std::string;
 
 StatusWith<bool> SaslSCRAMClientConversation::step(StringData inputData, std::string* outputData) {
-    std::vector<std::string> input = StringSplitter::split(inputData.toString(), ",");
     _step++;
 
     switch (_step) {
         case 1:
             return _firstStep(outputData);
         case 2:
-            // Append server-first-message to _authMessage
-            _authMessage += inputData.toString() + ",";
-            return _secondStep(input, outputData);
+            return _secondStep(inputData, outputData);
         case 3:
-            return _thirdStep(input, outputData);
+            return _thirdStep(inputData, outputData);
         default:
             return StatusWith<bool>(
                 ErrorCodes::AuthenticationFailed,
@@ -80,8 +77,7 @@ static void encodeSCRAMUsername(std::string& user) {
  */
 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");
+        return Status(ErrorCodes::BadValue, "Empty client password provided");
     }
 
     // Create text-based nonce as base64 encoding of a binary blob of length multiple of 3
@@ -105,80 +101,82 @@ StatusWith<bool> SaslSCRAMClientConversation::_firstStep(std::string* outputData
     _clientNonce = base64::encode(reinterpret_cast<char*>(binaryNonce), sizeof(binaryNonce));
 
     // Append client-first-message-bare to authMessage
-    _authMessage = "n=" + user + ",r=" + _clientNonce + ",";
+    _authMessage = "n=" + user + ",r=" + _clientNonce;
 
     StringBuilder sb;
-    sb << "n,,n=" << user << ",r=" << _clientNonce;
+    sb << "n,," << _authMessage;
     *outputData = sb.str();
 
-    return StatusWith<bool>(false);
+    return false;
 }
 
 /**
  * Parse server-first-message on the form:
- * r=client-nonce|server-nonce,s=user-salt,i=iteration-count
+ * [reserved-mext ',']r=client-nonce|server-nonce,s=user-salt,i=iteration-count[,extensions]
  *
  * Generate client-final-message of the form:
  * c=channel-binding(base64),r=client-nonce|server-nonce,p=ClientProof
  *
  **/
-StatusWith<bool> SaslSCRAMClientConversation::_secondStep(const std::vector<string>& input,
+StatusWith<bool> SaslSCRAMClientConversation::_secondStep(StringData inputData,
                                                           std::string* outputData) {
-    if (input.size() != 3) {
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream()
-                << "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 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 salt: " << input[1]);
-    } else if (!str::startsWith(input[2], "i=") || input[2].size() < 3) {
-        return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream() << "Incorrect SCRAM iteration count: "
-                                                          << input[2]);
-    }
-
-    std::string nonce = input[0].substr(2);
+    if (inputData.startsWith("m=")) {
+        return Status(ErrorCodes::BadValue, "SCRAM required extensions not supported");
+    }
+    const auto input = StringSplitter::split(inputData.toString(), ",");
+
+    if (input.size() < 3) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream()
+                          << "Incorrect number of arguments for first SCRAM server message, got "
+                          << input.size()
+                          << " expected at least 3");
+    }
+
+    if (!str::startsWith(input[0], "r=") || input[0].size() < 3) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Incorrect SCRAM client|server nonce: " << input[0]);
+    }
+
+    const auto nonce = input[0].substr(2);
     if (!str::startsWith(nonce, _clientNonce)) {
-        return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream()
-                                    << "Server SCRAM nonce does not match client nonce: "
-                                    << input[0]);
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Server SCRAM nonce does not match client nonce: " << nonce);
     }
 
-    std::string salt = input[1].substr(2);
-    size_t iterationCount;
+    if (!str::startsWith(input[1], "s=") || input[1].size() < 6) {
+        return Status(ErrorCodes::BadValue, str::stream() << "Incorrect SCRAM salt: " << input[1]);
+    }
+    const auto salt64 = input[1].substr(2);
 
+    if (!str::startsWith(input[2], "i=") || input[2].size() < 3) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Incorrect SCRAM iteration count: " << input[2]);
+    }
+    size_t iterationCount;
     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 iteration count: " << input[2]);
+    if (!status.isOK()) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Failed to parse SCRAM iteration count: " << input[2]);
     }
 
-    // Append client-final-message-without-proof to _authMessage
-    _authMessage += "c=biws,r=" + nonce;
+    // Append server-first-message and client-final-message-without-proof.
+    _authMessage += "," + inputData.toString() + ",c=biws,r=" + nonce;
 
     std::string decodedSalt, clientProof;
     try {
-        decodedSalt = base64::decode(salt);
+        decodedSalt = base64::decode(salt64);
         clientProof = generateClientProof(
             std::vector<std::uint8_t>(decodedSalt.begin(), decodedSalt.end()), iterationCount);
     } catch (const DBException& ex) {
-        return StatusWith<bool>(ex.toStatus());
+        return ex.toStatus();
     }
 
     StringBuilder sb;
     sb << "c=biws,r=" << nonce << ",p=" << clientProof;
     *outputData = sb.str();
 
-    return StatusWith<bool>(false);
+    return false;
 }
 
 /**
@@ -188,39 +186,40 @@ StatusWith<bool> SaslSCRAMClientConversation::_secondStep(const std::vector<stri
  * or failed authentication server-final-message on the form:
  * e=message
  **/
-StatusWith<bool> SaslSCRAMClientConversation::_thirdStep(const std::vector<string>& input,
+StatusWith<bool> SaslSCRAMClientConversation::_thirdStep(StringData inputData,
                                                          std::string* outputData) {
-    if (input.size() != 1) {
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream()
-                << "Incorrect number of arguments for final SCRAM server message, got "
-                << input.size()
-                << " expected 1");
-    } else if (input[0].size() < 3) {
-        return StatusWith<bool>(
+    const auto input = StringSplitter::split(inputData.toString(), ",");
+
+    if (input.empty()) {
+        return Status(
             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 authentication failure: "
-                                                          << input[0].substr(2));
-    } else if (!str::startsWith(input[0], "v=")) {
-        return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream() << "Incorrect SCRAM ServerSignature: "
-                                                          << input[0]);
+            "Incorrect number of arguments for final SCRAM server message, got 0 expected 1");
+    }
+
+    if (input[0].size() < 3) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Incorrect SCRAM server message length: " << input[0]);
+    }
+
+    if (str::startsWith(input[0], "e=")) {
+        return Status(ErrorCodes::AuthenticationFailed,
+                      str::stream() << "SCRAM authentication failure: " << input[0].substr(2));
+    }
+
+    if (!str::startsWith(input[0], "v=")) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Incorrect SCRAM ServerSignature: " << input[0]);
     }
 
     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 ServerSignature, received "
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Client failed to verify SCRAM ServerSignature, received "
                                     << input[0].substr(2));
     }
 
     *outputData = "";
 
-    return StatusWith<bool>(true);
+    return true;
 }
 }  // namespace mongo
diff --git a/src/mongo/client/sasl_scram_client_conversation.h b/src/mongo/client/sasl_scram_client_conversation.h
index 63ffca61db6..0800816e6a5 100644
--- a/src/mongo/client/sasl_scram_client_conversation.h
+++ b/src/mongo/client/sasl_scram_client_conversation.h
@@ -85,12 +85,12 @@ private:
     /**
      * Parses server-first-message and generate client-final-message.
      **/
-    StatusWith<bool> _secondStep(const std::vector<std::string>& input, std::string* outputData);
+    StatusWith<bool> _secondStep(StringData input, std::string* outputData);
 
     /**
      * Generates client-first-message.
      **/
-    StatusWith<bool> _thirdStep(const std::vector<std::string>& input, std::string* outputData);
+    StatusWith<bool> _thirdStep(StringData input, std::string* outputData);
 
 protected:
     int _step{0};
diff --git a/src/mongo/db/auth/sasl_scram_server_conversation.cpp b/src/mongo/db/auth/sasl_scram_server_conversation.cpp
index 7ae3373e614..3e5fc0676b6 100644
--- a/src/mongo/db/auth/sasl_scram_server_conversation.cpp
+++ b/src/mongo/db/auth/sasl_scram_server_conversation.cpp
@@ -50,24 +50,22 @@ using std::unique_ptr;
 using std::string;
 
 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 authentication step: "
-                                                          << _step);
+        return Status(ErrorCodes::AuthenticationFailed,
+                      str::stream() << "Invalid SCRAM authentication step: " << _step);
     }
     if (_step == 1) {
-        return _firstStep(input, outputData);
+        return _firstStep(inputData, outputData);
     }
     if (_step == 2) {
-        return _secondStep(input, outputData);
+        return _secondStep(inputData, outputData);
     }
 
     *outputData = "";
 
-    return StatusWith<bool>(true);
+    return true;
 }
 
 /*
@@ -88,60 +86,76 @@ static void decodeSCRAMUsername(std::string& user) {
  *
  * NOTE: we are ignoring the authorization ID part of the message
  */
-StatusWith<bool> SaslSCRAMServerConversation::_firstStep(std::vector<string>& input,
+StatusWith<bool> SaslSCRAMServerConversation::_firstStep(StringData inputData,
                                                          std::string* outputData) {
-    std::string authzId = "";
-
-    if (input.size() == 4) {
-        /* The second entry a=authzid is optional. If provided it will be
-         * validated against the encoded username.
-         *
-         * The two allowed input forms are:
-         * n,,n=encoded-username,r=client-nonce
-         * n,a=authzid,n=encoded-username,r=client-nonce
-         */
-        if (!str::startsWith(input[1], "a=") || input[1].size() < 3) {
-            return StatusWith<bool>(ErrorCodes::BadValue,
-                                    mongoutils::str::stream() << "Incorrect SCRAM authzid: "
-                                                              << input[1]);
+    const auto badCount = [](int got) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream()
+                          << "Incorrect number of arguments for first SCRAM client message, got "
+                          << got
+                          << " expected at least 3");
+    };
+
+    /**
+     * gs2-cbind-flag := ("p=" cb-name) / 'y' / 'n'
+     * gs2-header := gs2-cbind-flag ',' [ authzid ] ','
+     * reserved-mext := "m=" 1*(value-char)
+     * client-first-message-bare := [reserved-mext  ','] username ',' nonce [',' extensions]
+     * client-first-message := gs2-header client-first-message-bare
+     */
+    const auto gs2_cbind_comma = inputData.find(',');
+    if (gs2_cbind_comma == string::npos) {
+        return badCount(1);
+    }
+    const auto gs2_cbind_flag = inputData.substr(0, gs2_cbind_comma);
+    if (gs2_cbind_flag.startsWith("p=")) {
+        return Status(ErrorCodes::BadValue, "Server does not support channel binding");
+    }
+
+    if ((gs2_cbind_flag != "y") && (gs2_cbind_flag != "n")) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Incorrect SCRAM client message prefix: " << gs2_cbind_flag);
+    }
+
+    const auto gs2_header_comma = inputData.find(',', gs2_cbind_comma + 1);
+    if (gs2_header_comma == string::npos) {
+        return badCount(2);
+    }
+    auto authzId = inputData.substr(gs2_cbind_comma + 1, gs2_header_comma - (gs2_cbind_comma + 1));
+    if (authzId.size()) {
+        if (authzId.startsWith("a=")) {
+            authzId = authzId.substr(2);
+        } else {
+            return Status(ErrorCodes::BadValue,
+                          str::stream() << "Incorrect SCRAM authzid: " << authzId);
         }
-        authzId = input[1].substr(2);
-        input.erase(input.begin() + 1);
     }
 
-    if (input.size() != 3) {
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream()
-                << "Incorrect number of arguments for first SCRAM client message, got "
-                << input.size()
-                << " expected 4");
-    } else if (str::startsWith(input[0], "p=")) {
-        return StatusWith<bool>(ErrorCodes::BadValue,
-                                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 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 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 client nonce: "
-                                                          << input[2]);
+    const auto client_first_message_bare = inputData.substr(gs2_header_comma + 1);
+    if (client_first_message_bare.startsWith("m=")) {
+        return Status(ErrorCodes::BadValue, "SCRAM mandatory extensions are not supported");
     }
 
-    _user = input[1].substr(2);
-    if (!authzId.empty() && _user != authzId) {
-        return StatusWith<bool>(ErrorCodes::BadValue,
-                                mongoutils::str::stream() << "SCRAM user name " << _user
-                                                          << " does not match authzid "
-                                                          << authzId);
+    /* StringSplitter::split() will ignore consecutive delimiters.
+     * e.g. "foo,,bar" => {"foo","bar"}
+     * This makes our implementation of SCRAM *slightly* more generous
+     * in what it will accept than the standard calls for.
+     *
+     * This does not impact _authMessage, as it's composed from the raw
+     * string input, rather than the output of the split operation.
+     */
+    const auto input = StringSplitter::split(client_first_message_bare.toString(), ",");
+
+    if (input.size() < 2) {
+        // gs2-header is not included in this count, so add it back in.
+        return badCount(input.size() + 2);
     }
 
+    if (!str::startsWith(input[0], "n=") || input[0].size() < 3) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Invalid SCRAM user name: " << input[0]);
+    }
+    _user = input[0].substr(2);
     decodeSCRAMUsername(_user);
 
     auto swUser = saslPrep(_user);
@@ -150,20 +164,28 @@ StatusWith<bool> SaslSCRAMServerConversation::_firstStep(std::vector<string>& in
     }
     _user = std::move(swUser.getValue());
 
+    if (!authzId.empty() && _user != authzId) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "SCRAM user name " << _user << " does not match authzid "
+                                    << authzId);
+    }
+
+    if (!str::startsWith(input[1], "r=") || input[1].size() < 6) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Invalid SCRAM client nonce: " << input[1]);
+    }
+    const auto clientNonce = input[1].substr(2);
+
+
     // SERVER-16534, SCRAM-SHA-1 must be enabled for authenticating the internal user, so that
     // cluster members may communicate with each other. Hence ignore disabled auth mechanism
     // for the internal user.
     UserName user(_user, _saslAuthSession->getAuthenticationDatabase());
     if (!sequenceContains(saslGlobalParams.authenticationMechanisms, "SCRAM-SHA-1") &&
         user != internalSecurity.user->getName()) {
-        return StatusWith<bool>(ErrorCodes::BadValue, "SCRAM-SHA-1 authentication is disabled");
+        return Status(ErrorCodes::BadValue, "SCRAM-SHA-1 authentication is disabled");
     }
 
-    // add client-first-message-bare to _authMessage
-    _authMessage += input[1] + "," + input[2] + ",";
-
-    std::string clientNonce = input[2].substr(2);
-
     // The authentication database is also the source database for the user.
     User* userObj;
     Status status =
@@ -171,7 +193,7 @@ StatusWith<bool> SaslSCRAMServerConversation::_firstStep(std::vector<string>& in
             _saslAuthSession->getOpCtxt(), user, &userObj);
 
     if (!status.isOK()) {
-        return StatusWith<bool>(status);
+        return status;
     }
 
     _creds = userObj->getCredentials();
@@ -210,10 +232,10 @@ StatusWith<bool> SaslSCRAMServerConversation::_firstStep(std::vector<string>& in
     sb << "r=" << _nonce << ",s=" << getSalt() << ",i=" << getIterationCount();
     *outputData = sb.str();
 
-    // add server-first-message to authMessage
-    _authMessage += *outputData + ",";
+    // add client-first-message-bare and server-first-message to _authMessage
+    _authMessage = client_first_message_bare.toString() + "," + *outputData;
 
-    return StatusWith<bool>(false);
+    return false;
 }
 
 /**
@@ -228,46 +250,64 @@ StatusWith<bool> SaslSCRAMServerConversation::_firstStep(std::vector<string>& in
  *
  * NOTE: we are ignoring the channel binding part of the message
 **/
-StatusWith<bool> SaslSCRAMServerConversation::_secondStep(const std::vector<string>& input,
+StatusWith<bool> SaslSCRAMServerConversation::_secondStep(StringData inputData,
                                                           std::string* outputData) {
-    if (input.size() != 3) {
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream()
-                << "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 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 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 ClientProof: "
-                                                          << input[2]);
+    const auto badCount = [](int got) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream()
+                          << "Incorrect number of arguments for second SCRAM client message, got "
+                          << got
+                          << " expected at least 3");
+    };
+
+    /**
+     * client-final-message-without-proof := cbind ',' nonce ',' [ ',' extensions ]
+     * client-final-message := client-final-message-without-proof ',' proof
+     */
+    const auto last_comma = inputData.rfind(',');
+    if (last_comma == string::npos) {
+        return badCount(1);
     }
 
     // add client-final-message-without-proof to authMessage
-    _authMessage += input[0] + "," + input[1];
+    const auto client_final_message_without_proof = inputData.substr(0, last_comma);
+    _authMessage += "," + client_final_message_without_proof.toString();
+
+    const auto last_field = inputData.substr(last_comma + 1);
+    if ((last_field.size() < 3) || !last_field.startsWith("p=")) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Incorrect SCRAM ClientProof: " << last_field);
+    }
+    const auto proof = last_field.substr(2);
+
+    const auto input = StringSplitter::split(client_final_message_without_proof.toString(), ",");
+    if (input.size() < 2) {
+        // Add count for proof back on.
+        return badCount(input.size() + 1);
+    }
+
+    if (!str::startsWith(input[0], "c=") || input[0].size() < 3) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Incorrect SCRAM channel binding: " << input[0]);
+    }
+    const auto cbind = input[0].substr(2);
+
+    if (!str::startsWith(input[1], "r=") || input[1].size() < 6) {
+        return Status(ErrorCodes::BadValue,
+                      str::stream() << "Incorrect SCRAM client|server nonce: " << input[1]);
+    }
+    const auto nonce = input[1].substr(2);
 
     // Concatenated nonce sent by client should equal the one in server-first-message
-    std::string nonce = input[1].substr(2);
     if (nonce != _nonce) {
-        return StatusWith<bool>(
-            ErrorCodes::BadValue,
-            mongoutils::str::stream()
-                << "Unmatched SCRAM nonce received from client in second step, expected "
-                << _nonce
-                << " but received "
-                << nonce);
+        return Status(ErrorCodes::BadValue,
+                      str::stream()
+                          << "Unmatched SCRAM nonce received from client in second step, expected "
+                          << _nonce
+                          << " but received "
+                          << nonce);
     }
 
-    std::string clientProof = input[2].substr(2);
-
     // Do server side computations, compare storedKeys and generate client-final-message
     // AuthMessage     := client-first-message-bare + "," +
     //                    server-first-message + "," +
@@ -277,19 +317,16 @@ StatusWith<bool> SaslSCRAMServerConversation::_secondStep(const std::vector<stri
     // ServerSignature := HMAC(ServerKey, AuthMessage)
     invariant(initAndValidateCredentials());
 
-    if (!verifyClientProof(base64::decode(clientProof))) {
-        return StatusWith<bool>(ErrorCodes::AuthenticationFailed,
-                                mongoutils::str::stream()
-                                    << "SCRAM authentication failed, storedKey mismatch");
+    if (!verifyClientProof(base64::decode(proof.toString()))) {
+        return Status(ErrorCodes::AuthenticationFailed,
+                      "SCRAM authentication failed, storedKey mismatch");
     }
 
-    // ServerSignature := HMAC(ServerKey, AuthMessage)
-    const auto serverSignature = generateServerSignature();
-
     StringBuilder sb;
-    sb << "v=" << serverSignature;
+    // ServerSignature := HMAC(ServerKey, AuthMessage)
+    sb << "v=" << generateServerSignature();
     *outputData = sb.str();
 
-    return StatusWith<bool>(false);
+    return false;
 }
 }  // namespace mongo
diff --git a/src/mongo/db/auth/sasl_scram_server_conversation.h b/src/mongo/db/auth/sasl_scram_server_conversation.h
index d2166bb4377..718e168feea 100644
--- a/src/mongo/db/auth/sasl_scram_server_conversation.h
+++ b/src/mongo/db/auth/sasl_scram_server_conversation.h
@@ -94,12 +94,12 @@ private:
     /**
      * Parse client-first-message and generate server-first-message
      **/
-    StatusWith<bool> _firstStep(std::vector<std::string>& input, std::string* outputData);
+    StatusWith<bool> _firstStep(StringData input, std::string* outputData);
 
     /**
      * Parse client-final-message and generate server-final-message
      **/
-    StatusWith<bool> _secondStep(const std::vector<std::string>& input, std::string* outputData);
+    StatusWith<bool> _secondStep(StringData input, std::string* outputData);
 
 protected:
     int _step{0};
diff --git a/src/mongo/db/auth/sasl_scramsha1_test.cpp b/src/mongo/db/auth/sasl_scramsha1_test.cpp
index c1fd5d6cd82..d0c326ea570 100644
--- a/src/mongo/db/auth/sasl_scramsha1_test.cpp
+++ b/src/mongo/db/auth/sasl_scramsha1_test.cpp
@@ -255,10 +255,10 @@ TEST_F(SCRAMSHA1Fixture, testServerStep1DoesNotIncludeNonceFromClientStep1) {
         serverMessage = serverMessage.replace(nonceBegin, nonceEnd, "r=");
 
     });
-    ASSERT_EQ(SCRAMStepsResult(SaslTestState(SaslTestState::kClient, 2),
-                               Status(ErrorCodes::BadValue,
-                                      "Server SCRAM nonce does not match client nonce: r=")),
-              runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
+    ASSERT_EQ(
+        SCRAMStepsResult(SaslTestState(SaslTestState::kClient, 2),
+                         Status(ErrorCodes::BadValue, "Incorrect SCRAM client|server nonce: r=")),
+        runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
 }
 
 TEST_F(SCRAMSHA1Fixture, testClientStep2DoesNotIncludeNonceFromServerStep1) {
@@ -491,6 +491,55 @@ TEST_F(SCRAMSHA1Fixture, testIncorrectPassword) {
               runSteps(saslServerSession.get(), saslClientSession.get()));
 }
 
+TEST_F(SCRAMSHA1Fixture, testOptionalClientExtensions) {
+    // Verify server ignores unknown/optional extensions sent by client.
+    ASSERT_OK(authzManagerExternalState->insertPrivilegeDocument(
+        opCtx.get(), generateSCRAMUserDocument("sajack", "sajack"), BSONObj()));
+
+    saslClientSession->setParameter(NativeSaslClientSession::parameterUser, "sajack");
+    saslClientSession->setParameter(NativeSaslClientSession::parameterPassword,
+                                    createPasswordDigest("sajack", "sajack"));
+
+    ASSERT_OK(saslClientSession->initialize());
+
+    SCRAMMutators mutator;
+    mutator.setMutator(SaslTestState(SaslTestState::kClient, 1), [](std::string& clientMessage) {
+        clientMessage += ",x=unsupported-extension";
+    });
+
+    // Optional client extension is successfully ignored, or we'd have failed in step 1.
+    // We still fail at step 2, because client was unaware of the injected extension.
+    ASSERT_EQ(SCRAMStepsResult(SaslTestState(SaslTestState::kServer, 2),
+                               Status(ErrorCodes::AuthenticationFailed,
+                                      "SCRAM authentication failed, storedKey mismatch")),
+              runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
+}
+
+TEST_F(SCRAMSHA1Fixture, testOptionalServerExtensions) {
+    // Verify client errors on unknown/optional extensions sent by server.
+    ASSERT_OK(authzManagerExternalState->insertPrivilegeDocument(
+        opCtx.get(), generateSCRAMUserDocument("sajack", "sajack"), BSONObj()));
+
+    saslClientSession->setParameter(NativeSaslClientSession::parameterUser, "sajack");
+    saslClientSession->setParameter(NativeSaslClientSession::parameterPassword,
+                                    createPasswordDigest("sajack", "sajack"));
+
+    ASSERT_OK(saslClientSession->initialize());
+
+    SCRAMMutators mutator;
+    mutator.setMutator(SaslTestState(SaslTestState::kServer, 1), [](std::string& serverMessage) {
+        serverMessage += ",x=unsupported-extension";
+    });
+
+    // As with testOptionalClientExtensions, we can be confident that the optionality
+    // is respected because we would have failed at client step 2.
+    // We do still fail at server step 2 because server was unaware of injected extension.
+    ASSERT_EQ(SCRAMStepsResult(SaslTestState(SaslTestState::kServer, 2),
+                               Status(ErrorCodes::AuthenticationFailed,
+                                      "SCRAM authentication failed, storedKey mismatch")),
+              runSteps(saslServerSession.get(), saslClientSession.get(), mutator));
+}
+
 TEST(SCRAMSHA1Cache, testGetFromEmptyCache) {
     SCRAMSHA1ClientCache cache;
     std::string saltStr("saltsaltsaltsalt");