SERVER-37833 Retry internal auth with alternate key during keyfile rollover

8 files changed, 230 insertions, 99 deletions

diff --git a/src/mongo/db/auth/SConscript b/src/mongo/db/auth/SConscript
index 48111b6be5e..b5ed9fc9f87 100644
--- a/src/mongo/db/auth/SConscript
+++ b/src/mongo/db/auth/SConscript
@@ -185,8 +185,6 @@ env.Library(
     ],
     LIBDEPS=[
         '$BUILD_DIR/mongo/base',
-        '$BUILD_DIR/mongo/bson/mutable/mutable_bson',
-        '$BUILD_DIR/mongo/bson/util/bson_extract',
     ],
 )
 
@@ -222,6 +220,19 @@ env.Library(
     ],
 )
 
+env.CppUnitTest(
+    target='security_key_test',
+    source=[
+        'security_key_test.cpp',
+    ],
+    LIBDEPS=[
+        'auth',
+        'security_file',
+        'security_key',
+        'user',
+    ],
+)
+
 env.Library(
     target='authservercommon',
     source=[
@@ -259,16 +270,6 @@ yamlEnv.Library(
     ],
 )
 
-env.CppUnitTest(
-    target='security_file_test',
-    source=[
-        'security_file_test.cpp',
-    ],
-    LIBDEPS=[
-        'security_file',
-    ],
-)
-
 env.Library(
     target='sasl_options',
     source=[
diff --git a/src/mongo/db/auth/authorization_manager.h b/src/mongo/db/auth/authorization_manager.h
index 05cc9236ba7..1e1c29dc1b7 100644
--- a/src/mongo/db/auth/authorization_manager.h
+++ b/src/mongo/db/auth/authorization_manager.h
@@ -33,6 +33,8 @@
 #include <memory>
 #include <string>
 
+#include <boost/optional.hpp>
+
 #include "mongo/base/disallow_copying.h"
 #include "mongo/base/secure_allocator.h"
 #include "mongo/base/shim.h"
@@ -67,6 +69,9 @@ class UserDocumentParser;
  */
 struct AuthInfo {
     UserHandle user;
+
+    // Used during keyfile rollover to store the alternate key used to authenticate
+    boost::optional<User::CredentialData> alternateCredentials;
 };
 extern AuthInfo internalSecurity;  // set at startup and not changed after initialization.
 
diff --git a/src/mongo/db/auth/internal_user_auth.cpp b/src/mongo/db/auth/internal_user_auth.cpp
index 47e97a24355..4c2598072dc 100644
--- a/src/mongo/db/auth/internal_user_auth.cpp
+++ b/src/mongo/db/auth/internal_user_auth.cpp
@@ -34,12 +34,11 @@
 
 #include "mongo/db/auth/internal_user_auth.h"
 
-#include "mongo/bson/mutable/document.h"
-#include "mongo/bson/mutable/element.h"
+#include "mongo/bson/bsonobj.h"
 #include "mongo/util/log.h"
 
 namespace mongo {
-namespace mmb = mongo::mutablebson;
+namespace {
 
 // not guarded by the authParams mutex never changed in
 // multi-threaded operation
@@ -47,30 +46,50 @@ static bool authParamsSet = false;
 
 // Store default authentication parameters for internal authentication to cluster members,
 // guarded by the authParams mutex
-static BSONObj authParams;
+static std::vector<BSONObj> authParams;
 
 static stdx::mutex authParamMutex;
 
-bool isInternalAuthSet() {
-    return authParamsSet;
-}
-
-void setInternalUserAuthParams(const BSONObj& authParamsIn) {
+template <typename Container>
+void setInternalUserAuthParamsFromContainer(Container&& params) {
     if (!isInternalAuthSet()) {
         authParamsSet = true;
     }
+
     stdx::lock_guard<stdx::mutex> lk(authParamMutex);
+    authParams.clear();
+    std::transform(params.begin(),
+                   params.end(),
+                   std::back_inserter(authParams),
+                   [](const BSONObj& obj) { return obj.getOwned(); });
+}
+
+}  // namespace
+
+bool isInternalAuthSet() {
+    return authParamsSet;
+}
+
+void setInternalUserAuthParams(BSONObj authParamsIn) {
+    setInternalUserAuthParamsFromContainer(std::initializer_list<BSONObj>{authParamsIn});
+}
+
+void setInternalUserAuthParams(const std::vector<BSONObj>& keys) {
+    setInternalUserAuthParamsFromContainer(keys);
+}
 
-    authParams = authParamsIn.copy();
+size_t getInternalUserAuthParamsCount() {
+    stdx::lock_guard<stdx::mutex> lk(authParamMutex);
+    return authParams.size();
 }
 
-BSONObj getInternalUserAuthParams() {
+BSONObj getInternalUserAuthParams(size_t idx) {
     if (!authParamsSet) {
         return BSONObj();
     }
 
     stdx::lock_guard<stdx::mutex> lk(authParamMutex);
-    return authParams.copy();
+    return (idx < authParams.size()) ? authParams.at(idx) : BSONObj();
 }
 
 }  // namespace mongo
diff --git a/src/mongo/db/auth/internal_user_auth.h b/src/mongo/db/auth/internal_user_auth.h
index 775f883d592..9b8106f9333 100644
--- a/src/mongo/db/auth/internal_user_auth.h
+++ b/src/mongo/db/auth/internal_user_auth.h
@@ -30,6 +30,9 @@
 
 #pragma once
 
+#include <string>
+#include <vector>
+
 namespace mongo {
 class BSONObj;
 
@@ -41,19 +44,29 @@ class BSONObj;
 bool isInternalAuthSet();
 
 /**
+ * This method initializes the authParams from a list of keys. It defaults to generating
+ * SCRAM-SHA-1 credentials only.
+ */
+void setInternalUserAuthParams(const std::vector<BSONObj>& keys);
+
+/**
  * This method initializes the authParams object with authentication
  * credentials to be used by authenticateInternalUser.
  */
-void setInternalUserAuthParams(const BSONObj& authParamsIn);
+void setInternalUserAuthParams(BSONObj obj);
 
 /**
- * Returns a copy of the authParams object to be used by authenticateInternalUser
+ * Returns a BSON object containing user info to be used by authenticateInternalUser
  *
  * The format of the return object is { authparams, fallbackParams:params}
  *
  * If SCRAM-SHA-1 is the internal auth mechanism the fallbackParams sub document is
  * for MONGODB-CR auth is included. For MONGODB-XC509 no fallbackParams document is
  * returned.
+ *
+ * If no internal auth information has been set then this returns an empty BSONObj.
  **/
-BSONObj getInternalUserAuthParams();
+
+BSONObj getInternalUserAuthParams(size_t idx = 0);
+
 }  // namespace mongo
diff --git a/src/mongo/db/auth/sasl_scram_server_conversation.cpp b/src/mongo/db/auth/sasl_scram_server_conversation.cpp
index 8919f51dd96..0e11a6238bf 100644
--- a/src/mongo/db/auth/sasl_scram_server_conversation.cpp
+++ b/src/mongo/db/auth/sasl_scram_server_conversation.cpp
@@ -204,9 +204,9 @@ StatusWith<std::tuple<bool, std::string>> SaslSCRAMServerMechanism<Policy>::_fir
     User::CredentialData credentials = userObj->getCredentials();
     UserName userName = userObj->getName();
 
-    _scramCredentials = credentials.scram<HashBlock>();
+    auto scramCredentials = credentials.scram<HashBlock>();
 
-    if (!_scramCredentials.isValid()) {
+    if (!scramCredentials.isValid()) {
         // Check for authentication attempts of the __system user on
         // systems started without a keyfile.
         if (userName == internalSecurity.user->getName()) {
@@ -220,9 +220,16 @@ StatusWith<std::tuple<bool, std::string>> SaslSCRAMServerMechanism<Policy>::_fir
         }
     }
 
-    _secrets = scram::Secrets<HashBlock>("",
-                                         base64::decode(_scramCredentials.storedKey),
-                                         base64::decode(_scramCredentials.serverKey));
+    _secrets.push_back(scram::Secrets<HashBlock>("",
+                                                 base64::decode(scramCredentials.storedKey),
+                                                 base64::decode(scramCredentials.serverKey)));
+
+    if (userName == internalSecurity.user->getName() && internalSecurity.alternateCredentials) {
+        auto altCredentials = internalSecurity.alternateCredentials->scram<HashBlock>();
+        _secrets.push_back(scram::Secrets<HashBlock>("",
+                                                     base64::decode(altCredentials.storedKey),
+                                                     base64::decode(altCredentials.serverKey)));
+    }
 
     // Generate server-first-message
     // Create text-based nonce as base64 encoding of a binary blob of length multiple of 3
@@ -238,8 +245,8 @@ StatusWith<std::tuple<bool, std::string>> SaslSCRAMServerMechanism<Policy>::_fir
     _nonce =
         clientNonce + base64::encode(reinterpret_cast<char*>(binaryNonce), sizeof(binaryNonce));
     StringBuilder sb;
-    sb << "r=" << _nonce << ",s=" << _scramCredentials.salt
-       << ",i=" << _scramCredentials.iterationCount;
+    sb << "r=" << _nonce << ",s=" << scramCredentials.salt
+       << ",i=" << scramCredentials.iterationCount;
     std::string outputData = sb.str();
 
     // add client-first-message-bare and server-first-message to _authMessage
@@ -327,14 +334,25 @@ StatusWith<std::tuple<bool, std::string>> SaslSCRAMServerMechanism<Policy>::_sec
     // ClientKey := ClientSignature XOR ClientProof
     // ServerSignature := HMAC(ServerKey, AuthMessage)
 
-    if (!_secrets.verifyClientProof(_authMessage, base64::decode(proof.toString()))) {
+    const auto decodedProof = base64::decode(proof.toString());
+    std::string serverSignature;
+    const auto checkSecret = [&](const scram::Secrets<HashBlock>& secret) {
+        if (!secret.verifyClientProof(_authMessage, decodedProof))
+            return false;
+
+        serverSignature = secret.generateServerSignature(_authMessage);
+        return true;
+    };
+
+    if (!std::any_of(_secrets.begin(), _secrets.end(), checkSecret)) {
         return Status(ErrorCodes::AuthenticationFailed,
                       "SCRAM authentication failed, storedKey mismatch");
     }
 
+    invariant(!serverSignature.empty());
     StringBuilder sb;
     // ServerSignature := HMAC(ServerKey, AuthMessage)
-    sb << "v=" << _secrets.generateServerSignature(_authMessage);
+    sb << "v=" << serverSignature;
 
     return std::make_tuple(false, sb.str());
 }
diff --git a/src/mongo/db/auth/sasl_scram_server_conversation.h b/src/mongo/db/auth/sasl_scram_server_conversation.h
index e91a254b54b..5a398f242ea 100644
--- a/src/mongo/db/auth/sasl_scram_server_conversation.h
+++ b/src/mongo/db/auth/sasl_scram_server_conversation.h
@@ -82,8 +82,10 @@ private:
 
     int _step{0};
     std::string _authMessage;
-    User::SCRAMCredentials<HashBlock> _scramCredentials;
-    scram::Secrets<HashBlock> _secrets;
+
+    // The secrets to check the client proof against during the second step
+    // Usually only contains one element, except during key rollover.
+    std::vector<scram::Secrets<HashBlock>> _secrets;
 
     // client and server nonce concatenated
     std::string _nonce;
diff --git a/src/mongo/db/auth/security_key.cpp b/src/mongo/db/auth/security_key.cpp
index 5bcca77d563..beec7b11c3b 100644
--- a/src/mongo/db/auth/security_key.cpp
+++ b/src/mongo/db/auth/security_key.cpp
@@ -59,71 +59,123 @@
 
 namespace mongo {
 namespace {
-template <typename CredsTarget, typename CredsSource>
-void copyCredentials(CredsTarget&& target, const CredsSource&& source) {
-    target.iterationCount = source[scram::kIterationCountFieldName].Int();
-    target.salt = source[scram::kSaltFieldName].String();
-    target.storedKey = source[scram::kStoredKeyFieldName].String();
-    target.serverKey = source[scram::kServerKeyFieldName].String();
-}
-
 constexpr size_t kMinKeyLength = 6;
 constexpr size_t kMaxKeyLength = 1024;
 
+class CredentialsGenerator {
+public:
+    explicit CredentialsGenerator(StringData filename)
+        : _salt1(scram::Presecrets<SHA1Block>::generateSecureRandomSalt()),
+          _salt256(scram::Presecrets<SHA256Block>::generateSecureRandomSalt()),
+          _filename(filename) {}
+
+    boost::optional<std::pair<User::CredentialData, BSONObj>> generate(
+        const std::string& password) {
+        if (password.size() < kMinKeyLength || password.size() > kMaxKeyLength) {
+            error() << " security key in " << _filename << " has length " << password.size()
+                    << ", must be between 6 and 1024 chars";
+            return boost::none;
+        }
+
+        auto swSaslPassword = saslPrep(password);
+        if (!swSaslPassword.isOK()) {
+            error() << "Could not prep security key file for SCRAM-SHA-256: "
+                    << swSaslPassword.getStatus();
+            return boost::none;
+        }
+        const auto passwordDigest = mongo::createPasswordDigest(
+            internalSecurity.user->getName().getUser().toString(), password);
+
+        User::CredentialData credentials;
+        if (!_copyCredentials(
+                credentials.scram_sha1,
+                scram::Secrets<SHA1Block>::generateCredentials(
+                    _salt1, passwordDigest, saslGlobalParams.scramSHA1IterationCount.load())))
+            return boost::none;
+
+        if (!_copyCredentials(credentials.scram_sha256,
+                              scram::Secrets<SHA256Block>::generateCredentials(
+                                  _salt256,
+                                  swSaslPassword.getValue(),
+                                  saslGlobalParams.scramSHA256IterationCount.load())))
+            return boost::none;
+
+        auto internalAuthParams =
+            BSON(saslCommandMechanismFieldName << "SCRAM-SHA-1" << saslCommandUserDBFieldName
+                                               << internalSecurity.user->getName().getDB()
+                                               << saslCommandUserFieldName
+                                               << internalSecurity.user->getName().getUser()
+                                               << saslCommandPasswordFieldName
+                                               << passwordDigest
+                                               << saslCommandDigestPasswordFieldName
+                                               << false);
+
+        return std::make_pair(std::move(credentials), std::move(internalAuthParams));
+    }
+
+private:
+    template <typename CredsTarget, typename CredsSource>
+    bool _copyCredentials(CredsTarget&& target, const CredsSource&& source) {
+        target.iterationCount = source[scram::kIterationCountFieldName].Int();
+        target.salt = source[scram::kSaltFieldName].String();
+        target.storedKey = source[scram::kStoredKeyFieldName].String();
+        target.serverKey = source[scram::kServerKeyFieldName].String();
+        if (!target.isValid()) {
+            error() << "Could not generate valid credentials from key in " << _filename;
+            return false;
+        }
+
+        return true;
+    }
+
+    const std::vector<uint8_t> _salt1;
+    const std::vector<uint8_t> _salt256;
+    const StringData _filename;
+};
+
 }  // namespace
 
 using std::string;
 
 bool setUpSecurityKey(const string& filename) {
-    auto keyStrings = mongo::readSecurityFile(filename);
-    if (!keyStrings.isOK()) {
-        log() << keyStrings.getStatus().reason();
+    auto swKeyStrings = mongo::readSecurityFile(filename);
+    if (!swKeyStrings.isOK()) {
+        log() << swKeyStrings.getStatus().reason();
         return false;
     }
 
+    auto keyStrings = std::move(swKeyStrings.getValue());
 
-    const auto& password = keyStrings.getValue().front();
-    if (password.size() < kMinKeyLength || password.size() > kMaxKeyLength) {
-        error() << " security key in " << filename << " has length " << password.size()
-                << ", must be between 6 and 1024 chars";
+    if (keyStrings.size() > 2) {
+        error() << "Only two keys are supported in the security key file, " << keyStrings.size()
+                << " are specified in " << filename;
+        return false;
     }
 
-    auto swSaslPassword = saslPrep(password);
-    if (!swSaslPassword.isOK()) {
-        error() << "Could not prep security key file for SCRAM-SHA-256: "
-                << swSaslPassword.getStatus();
+    std::vector<BSONObj> internalAuthParams;
+    CredentialsGenerator generator(filename);
+    auto credentials = generator.generate(keyStrings.front());
+    if (!credentials) {
         return false;
     }
 
-    // Generate SCRAM-SHA-1/SCRAM-SHA-256 credentials for the internal user based on
-    // the keyfile.
-    User::CredentialData credentials;
-    const auto passwordDigest = mongo::createPasswordDigest(
-        internalSecurity.user->getName().getUser().toString(), password);
+    internalSecurity.user->setCredentials(std::move(credentials->first));
+    internalAuthParams.push_back(std::move(credentials->second));
 
-    copyCredentials(credentials.scram_sha1,
-                    scram::Secrets<SHA1Block>::generateCredentials(
-                        passwordDigest, saslGlobalParams.scramSHA1IterationCount.load()));
+    if (keyStrings.size() == 2) {
+        credentials = generator.generate(keyStrings[1]);
+        if (!credentials) {
+            return false;
+        }
 
-    copyCredentials(
-        credentials.scram_sha256,
-        scram::Secrets<SHA256Block>::generateCredentials(
-            swSaslPassword.getValue(), saslGlobalParams.scramSHA256IterationCount.load()));
-
-    internalSecurity.user->setCredentials(credentials);
+        internalSecurity.alternateCredentials = std::move(credentials->first);
+        internalAuthParams.push_back(std::move(credentials->second));
+    }
 
     int clusterAuthMode = serverGlobalParams.clusterAuthMode.load();
     if (clusterAuthMode == ServerGlobalParams::ClusterAuthMode_keyFile ||
         clusterAuthMode == ServerGlobalParams::ClusterAuthMode_sendKeyFile) {
-        setInternalUserAuthParams(
-            BSON(saslCommandMechanismFieldName << "SCRAM-SHA-1" << saslCommandUserDBFieldName
-                                               << internalSecurity.user->getName().getDB()
-                                               << saslCommandUserFieldName
-                                               << internalSecurity.user->getName().getUser()
-                                               << saslCommandPasswordFieldName
-                                               << passwordDigest
-                                               << saslCommandDigestPasswordFieldName
-                                               << false));
+        setInternalUserAuthParams(internalAuthParams);
     }
 
     return true;
diff --git a/src/mongo/db/auth/security_file_test.cpp b/src/mongo/db/auth/security_key_test.cpp
index eebf77a6ac4..851b5a46fd2 100644
--- a/src/mongo/db/auth/security_file_test.cpp
+++ b/src/mongo/db/auth/security_key_test.cpp
@@ -29,11 +29,12 @@
 
 #include "mongo/platform/basic.h"
 
-#include "mongo/db/auth/security_file.h"
-
 #include "boost/filesystem.hpp"
 
 #include "mongo/base/string_data.h"
+#include "mongo/db/auth/authorization_manager.h"
+#include "mongo/db/auth/security_file.h"
+#include "mongo/db/auth/security_key.h"
 #include "mongo/unittest/unittest.h"
 
 namespace mongo {
@@ -68,22 +69,27 @@ private:
 };
 
 struct TestCase {
-    enum class FailureMode { Success, Permissions, Parsing };
+    enum class FailureMode { Success, Permissions, Parsing, SecurityKeyConstraint };
 
     TestCase(StringData contents_,
              std::initializer_list<std::string> expected_,
-             FailureMode mode = FailureMode::Success)
-        : fileContents(contents_.toString()),
-          expected(expected_),
-          expectGoodParse(mode == FailureMode::Success),
-          expectGoodPerms(mode == FailureMode::Success || mode == FailureMode::Parsing) {}
+             FailureMode mode_ = FailureMode::Success)
+        : fileContents(contents_.toString()), expected(expected_), mode(mode_) {}
 
     std::string fileContents;
     std::vector<std::string> expected;
-    bool expectGoodParse = true;
-    bool expectGoodPerms = true;
+    FailureMode mode = FailureMode::Success;
 };
 
+StringData longKeyMaker() {
+    static const auto longKey = [] {
+        std::array<char, 1026> ret;
+        ret.fill('a');
+        return ret;
+    }();
+    return StringData(longKey.data(), longKey.size());
+}
+
 std::initializer_list<TestCase> testCases = {
     // Our good ole insecure key
     {"foop de doop", {"foopdedoop"}},
@@ -107,7 +113,7 @@ std::initializer_list<TestCase> testCases = {
      {"foopdedoop", "G92sqe/Y9Nn92fU1M8Q=cIKI"}},
 
     // An array of keys with the JSON-like YAML array format
-    {"[ \"foop de doop\", \"key 2\" ]", {"foopdedoop", "key2"}},
+    {"[ \"foop de doop\", \"other key\" ]", {"foopdedoop", "otherkey"}},
 
     // An empty file doesn't parse correctly
     {"", {}, TestCase::FailureMode::Parsing},
@@ -123,14 +129,19 @@ std::initializer_list<TestCase> testCases = {
 
     // A file with bad permissions doesn't parse correctly
     {"", {}, TestCase::FailureMode::Permissions},
-};
+
+    // These two keys should pass the security file parsing, but fail loading them as
+    // security keys because they are too short or two long
+    {"abc", {"abc"}, TestCase::FailureMode::SecurityKeyConstraint},
+    {longKeyMaker(), {longKeyMaker().toString()}, TestCase::FailureMode::SecurityKeyConstraint}};
 
 TEST(SecurityFile, Test) {
     for (const auto& testCase : testCases) {
-        TestFile file(testCase.fileContents, testCase.expectGoodPerms);
+        TestFile file(testCase.fileContents, testCase.mode != TestCase::FailureMode::Permissions);
 
         auto swKeys = readSecurityFile(file.path().string());
-        if (testCase.expectGoodParse && testCase.expectGoodPerms) {
+        if (testCase.mode == TestCase::FailureMode::Success ||
+            testCase.mode == TestCase::FailureMode::SecurityKeyConstraint) {
             ASSERT_OK(swKeys.getStatus());
         } else {
             ASSERT_NOT_OK(swKeys.getStatus());
@@ -139,12 +150,22 @@ TEST(SecurityFile, Test) {
 
         auto keys = std::move(swKeys.getValue());
         ASSERT_EQ(keys.size(), testCase.expected.size());
-        if (testCase.expected.size() == 1) {
-            ASSERT_EQ(keys.front(), testCase.expected.front());
+        for (size_t i = 0; i < keys.size(); i++) {
+            ASSERT_EQ(keys.at(i), testCase.expected.at(i));
+        }
+    }
+}
+
+TEST(SecurityKey, Test) {
+    internalSecurity.user = std::make_shared<User>(UserName("__system", "local"));
+
+    for (const auto& testCase : testCases) {
+        TestFile file(testCase.fileContents, testCase.mode != TestCase::FailureMode::Permissions);
+
+        if (testCase.mode == TestCase::FailureMode::Success) {
+            ASSERT_TRUE(setUpSecurityKey(file.path().string()));
         } else {
-            for (size_t i = 0; i < keys.size(); i++) {
-                ASSERT_EQ(keys.at(i), testCase.expected.at(i));
-            }
+            ASSERT_FALSE(setUpSecurityKey(file.path().string()));
         }
     }
 }