SERVER-32410 Validate User::CredentialData during auth

(cherry picked from commit fb8046d813af032d6d51327affbab9b6199fe654)

base64::validate() checks removed as they're a 3.6 API.
This doesn't materially hurt the fix as the later decodes
will fail in a predictable and correct way.

clang-format reapplied to match v3.2 formatting.

6 files changed, 80 insertions, 5 deletions

diff --git a/jstests/auth/scram-credentials-invalid.js b/jstests/auth/scram-credentials-invalid.js
new file mode 100644
index 00000000000..f26e70fae43
--- /dev/null
+++ b/jstests/auth/scram-credentials-invalid.js
@@ -0,0 +1,44 @@
+// Ensure that attempting to use SCRAM-SHA-1 auth on a
+// user with invalid SCRAM-SHA-1 credentials fails gracefully.
+
+(function() {
+    'use strict';
+
+    function runTest(mongod) {
+        assert(mongod);
+        const admin = mongod.getDB('admin');
+        const test = mongod.getDB('test');
+
+        admin.createUser({user: 'admin', pwd: 'pass', roles: jsTest.adminUserRoles});
+        assert(admin.auth('admin', 'pass'));
+
+        test.createUser({user: 'user', pwd: 'pass', roles: jsTest.basicUserRoles});
+
+        // Give the test user an invalid set of SCRAM-SHA-1 credentials.
+        assert.eq(admin.system.users.update({_id: "test.user"},
+                                            {
+                                              $set: {
+                                                  "credentials.SCRAM-SHA-1": {
+                                                      salt: "AAAA",
+                                                      storedKey: "AAAA",
+                                                      serverKey: "AAAA",
+                                                      iterationCount: 10000
+                                                  }
+                                              }
+                                            }).nModified,
+                  1,
+                  "Should have updated one document for user@test");
+        admin.logout();
+
+        assert(!test.auth({user: 'user', pwd: 'pass'}));
+
+        assert.soon(function() {
+            const log = cat(mongod.fullOptions.logFile);
+            return /Unable to perform SCRAM-SHA-1 auth.* invalid SCRAM credentials/.test(log);
+        }, "No warning issued for invalid SCRAM-SHA-1 credendials doc", 30 * 1000, 5 * 1000);
+    }
+
+    const mongod = MongoRunner.runMongod({auth: "", useLogFiles: true});
+    runTest(mongod);
+    MongoRunner.stopMongod(mongod);
+})();
diff --git a/src/mongo/crypto/mechanism_scram.cpp b/src/mongo/crypto/mechanism_scram.cpp
index c18bc39cc51..4dcc2ffae1d 100644
--- a/src/mongo/crypto/mechanism_scram.cpp
+++ b/src/mongo/crypto/mechanism_scram.cpp
@@ -224,6 +224,10 @@ bool verifyClientProof(StringData clientProof, StringData storedKey, StringData
     SHA1Block computedStoredKey =
         SHA1Block::computeHash(clientSignature.data(), clientSignature.size());
 
+    if (storedKey.size() != computedStoredKey.size()) {
+        return false;
+    }
+
     return consttimeMemEqual(reinterpret_cast<const unsigned char*>(storedKey.rawData()),
                              computedStoredKey.data(),
                              computedStoredKey.size());
diff --git a/src/mongo/crypto/mechanism_scram.h b/src/mongo/crypto/mechanism_scram.h
index 92b79efffb9..fe34b259fe8 100644
--- a/src/mongo/crypto/mechanism_scram.h
+++ b/src/mongo/crypto/mechanism_scram.h
@@ -53,15 +53,22 @@ const std::string serverKeyFieldName = "serverKey";
  */
 struct SCRAMPresecrets {
     SCRAMPresecrets(std::string hashedPassword,
-                    std::vector<std::uint8_t> salt,
-                    size_t iterationCount)
+                    std::vector<std::uint8_t> salt_,
+                    size_t iterationCount_)
         : hashedPassword(std::move(hashedPassword)),
-          salt(std::move(salt)),
-          iterationCount(iterationCount) {}
+          salt(std::move(salt_)),
+          iterationCount(iterationCount_) {
+        uassert(ErrorCodes::BadValue,
+                "Invalid salt for SCRAM mechanism",
+                salt.size() >= kSaltLengthMin);
+    }
 
     std::string hashedPassword;
     std::vector<std::uint8_t> salt;
     size_t iterationCount;
+
+private:
+    static const size_t kSaltLengthMin = 16;
 };
 
 inline bool operator==(const SCRAMPresecrets& lhs, const SCRAMPresecrets& rhs) {
diff --git a/src/mongo/db/auth/sasl_scramsha1_server_conversation.cpp b/src/mongo/db/auth/sasl_scramsha1_server_conversation.cpp
index 5809306456d..855a4a24f5d 100644
--- a/src/mongo/db/auth/sasl_scramsha1_server_conversation.cpp
+++ b/src/mongo/db/auth/sasl_scramsha1_server_conversation.cpp
@@ -195,6 +195,12 @@ StatusWith<bool> SaslSCRAMSHA1ServerConversation::_firstStep(std::vector<string>
         _creds.scram.serverKey = scramCreds[scram::serverKeyFieldName].String();
     }
 
+    if (!_creds.scram.isValid()) {
+        return Status(ErrorCodes::AuthenticationFailed,
+                      "Unable to perform SCRAM-SHA-1 authentication for a user with missing "
+                      "or invalid SCRAM credentials");
+    }
+
     // Generate server-first-message
     // Create text-based nonce as base64 encoding of a binary blob of length multiple of 3
     const int nonceLenQWords = 3;
@@ -274,6 +280,7 @@ StatusWith<bool> SaslSCRAMSHA1ServerConversation::_secondStep(const std::vector<
     // ClientSignature := HMAC(StoredKey, AuthMessage)
     // ClientKey := ClientSignature XOR ClientProof
     // ServerSignature := HMAC(ServerKey, AuthMessage)
+    invariant(_creds.scram.isValid());
 
     if (!scram::verifyClientProof(
             base64::decode(clientProof), base64::decode(_creds.scram.storedKey), _authMessage)) {
diff --git a/src/mongo/db/auth/user.h b/src/mongo/db/auth/user.h
index d4aea7e442b..cdef6e52ddd 100644
--- a/src/mongo/db/auth/user.h
+++ b/src/mongo/db/auth/user.h
@@ -66,6 +66,17 @@ public:
         std::string salt;
         std::string serverKey;
         std::string storedKey;
+
+        bool isValid() const {
+            // 160bit -> 20octets -> * 4/3 -> 26.667 -> padded to 28
+            const size_t kEncodedSHA1Length = 28;
+            // 128bit -> 16octets -> * 4/3 -> 21.333 -> padded to 24
+            const size_t kEncodedSaltLength = 24;
+
+            return (salt.size() == kEncodedSaltLength) &&
+                (serverKey.size() == kEncodedSHA1Length) &&
+                (storedKey.size() == kEncodedSHA1Length);
+        }
     };
     struct CredentialData {
         CredentialData() : password(""), scram(), isExternal(false) {}
diff --git a/src/mongo/db/auth/user_document_parser_test.cpp b/src/mongo/db/auth/user_document_parser_test.cpp
index ae6c566d109..85ccbb63871 100644
--- a/src/mongo/db/auth/user_document_parser_test.cpp
+++ b/src/mongo/db/auth/user_document_parser_test.cpp
@@ -30,11 +30,11 @@
  */
 
 
+#include "mongo/db/auth/user_document_parser.h"
 #include "mongo/base/status.h"
 #include "mongo/db/auth/action_set.h"
 #include "mongo/db/auth/action_type.h"
 #include "mongo/db/auth/authorization_manager.h"
-#include "mongo/db/auth/user_document_parser.h"
 #include "mongo/db/jsobj.h"
 #include "mongo/unittest/unittest.h"
 
@@ -401,6 +401,7 @@ TEST_F(V2UserDocumentParsing, V2CredentialExtraction) {
                                                                       << BSON("MONGODB-CR"
                                                                               << "a"))));
     ASSERT(user->getCredentials().password == "a");
+    ASSERT(!user->getCredentials().scram.isValid());
     ASSERT(!user->getCredentials().isExternal);
 
     // Credentials are {external:true if users's db is $external
@@ -413,6 +414,7 @@ TEST_F(V2UserDocumentParsing, V2CredentialExtraction) {
                                                                 << "credentials"
                                                                 << BSON("external" << true))));
     ASSERT(user->getCredentials().password.empty());
+    ASSERT(!user->getCredentials().scram.isValid());
     ASSERT(user->getCredentials().isExternal);
 }