diff options
author | Patrick Freed <patrick.freed@mongodb.com> | 2018-11-30 16:40:02 -0500 |
---|---|---|
committer | Spencer Jackson <spencer.jackson@mongodb.com> | 2019-01-22 12:36:31 -0500 |
commit | 5a176fcf616ca6e461205058d359f3137e486773 (patch) | |
tree | 9af12a699d9f4c7622d2a37eb9a7a7031ca95b01 | |
parent | 20e2aa85690743949ce1a5923feaf535fc72a84a (diff) | |
download | mongo-5a176fcf616ca6e461205058d359f3137e486773.tar.gz |
SERVER-37565 Unlock memory held during SCRAM authentication
This fixes a bug where the server would crash if a large number of parallel connections occurred at once
(cherry picked from commit 916a5553a2db8ae7553fea7c3703ef8fef75b055)
-rw-r--r-- | src/mongo/client/sasl_scram_client_conversation.h | 4 | ||||
-rw-r--r-- | src/mongo/crypto/mechanism_scram.h | 64 | ||||
-rw-r--r-- | src/mongo/db/auth/sasl_plain_server_conversation.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/auth/sasl_scram_server_conversation.cpp | 7 | ||||
-rw-r--r-- | src/mongo/db/auth/sasl_scram_server_conversation.h | 2 |
5 files changed, 57 insertions, 22 deletions
diff --git a/src/mongo/client/sasl_scram_client_conversation.h b/src/mongo/client/sasl_scram_client_conversation.h index ba1631115e3..4329955da0f 100644 --- a/src/mongo/client/sasl_scram_client_conversation.h +++ b/src/mongo/client/sasl_scram_client_conversation.h @@ -120,13 +120,13 @@ public: if (targetHost.isOK()) { _credentials = _clientCache->getCachedSecrets(targetHost.getValue(), presecrets); if (!_credentials) { - _credentials = presecrets; + _credentials = scram::Secrets<HashBlock>(presecrets); _clientCache->setCachedSecrets( std::move(targetHost.getValue()), std::move(presecrets), _credentials); } } else { - _credentials = presecrets; + _credentials = scram::Secrets<HashBlock>(presecrets); } return _credentials.generateClientProof(_authMessage); diff --git a/src/mongo/crypto/mechanism_scram.h b/src/mongo/crypto/mechanism_scram.h index 3ffe382609c..75b4f52f1e5 100644 --- a/src/mongo/crypto/mechanism_scram.h +++ b/src/mongo/crypto/mechanism_scram.h @@ -140,6 +140,47 @@ template <typename T> bool operator!=(const Presecrets<T>& lhs, const Presecrets<T>& rhs) { return !(lhs == rhs); } +template <typename HashBlock> +struct SecretsHolder { + HashBlock clientKey; + HashBlock storedKey; + HashBlock serverKey; +}; + +template <typename HashBlock> +class LockedSecretsPolicy { +public: + LockedSecretsPolicy() = default; + + const SecretsHolder<HashBlock>* operator->() const { + return &(*_holder); + } + SecretsHolder<HashBlock>* operator->() { + return &(*_holder); + } + +private: + using SecureSecrets = SecureAllocatorAuthDomain::SecureHandle<SecretsHolder<HashBlock>>; + + SecureSecrets _holder; +}; + +template <typename HashBlock> +class UnlockedSecretsPolicy { +public: + UnlockedSecretsPolicy() = default; + + const SecretsHolder<HashBlock>* operator->() const { + return &_holder; + } + + SecretsHolder<HashBlock>* operator->() { + return &_holder; + } + +private: + SecretsHolder<HashBlock> _holder; +}; /* Stores all of the keys, generated from a password, needed for a client or server to perform a * SCRAM handshake. @@ -147,21 +188,13 @@ bool operator!=(const Presecrets<T>& lhs, const Presecrets<T>& rhs) { * May be unpopulated. SCRAMSecrets created via the default constructor are unpopulated. * The behavior is undefined if the accessors are called when unpopulated. */ -template <typename HashBlock> +template <typename HashBlock, template <typename> class MemoryPolicy = LockedSecretsPolicy> class Secrets { -private: - struct SecretsHolder { - HashBlock clientKey; - HashBlock storedKey; - HashBlock serverKey; - }; - using SecureSecrets = SecureAllocatorAuthDomain::SecureHandle<SecretsHolder>; - public: Secrets() = default; Secrets(StringData client, StringData stored, StringData server) - : _ptr(std::make_shared<SecureSecrets>()) { + : _ptr(std::make_shared<MemoryPolicy<HashBlock>>()) { if (!client.empty()) { (*_ptr)->clientKey = uassertStatusOK(HashBlock::fromBuffer( reinterpret_cast<const unsigned char*>(client.rawData()), client.size())); @@ -172,13 +205,13 @@ public: reinterpret_cast<const unsigned char*>(server.rawData()), stored.size())); } - Secrets(const HashBlock& saltedPassword) : _ptr(std::make_shared<SecureSecrets>()) { + Secrets(const HashBlock& saltedPassword) : _ptr(std::make_shared<MemoryPolicy<HashBlock>>()) { // ClientKey := HMAC(saltedPassword, "Client Key") - (*_ptr)->clientKey = HashBlock::computeHmac( + (*_ptr)->clientKey = (HashBlock::computeHmac( saltedPassword.data(), saltedPassword.size(), reinterpret_cast<const unsigned char*>(kClientKeyConst.rawData()), - kClientKeyConst.size()); + kClientKeyConst.size())); // StoredKey := H(clientKey) (*_ptr)->storedKey = HashBlock::computeHash(clientKey().data(), clientKey().size()); @@ -250,7 +283,8 @@ public: static BSONObj generateCredentials(std::string password, int iterationCount) { auto salt = Presecrets<HashBlock>::generateSecureRandomSalt(); - Secrets<HashBlock> secrets(Presecrets<HashBlock>(password, salt, iterationCount)); + Secrets<HashBlock, MemoryPolicy> secrets( + Presecrets<HashBlock>(password, salt, iterationCount)); const auto encodedSalt = base64::encode(reinterpret_cast<char*>(salt.data()), salt.size()); return BSON(kIterationCountFieldName << iterationCount << kSaltFieldName << encodedSalt << kStoredKeyFieldName @@ -283,7 +317,7 @@ public: } private: - std::shared_ptr<SecureSecrets> _ptr; + std::shared_ptr<MemoryPolicy<HashBlock>> _ptr; }; } // namespace scram diff --git a/src/mongo/db/auth/sasl_plain_server_conversation.cpp b/src/mongo/db/auth/sasl_plain_server_conversation.cpp index cf782332f4a..cd092238bf7 100644 --- a/src/mongo/db/auth/sasl_plain_server_conversation.cpp +++ b/src/mongo/db/auth/sasl_plain_server_conversation.cpp @@ -55,7 +55,7 @@ StatusWith<bool> trySCRAM(const User::CredentialData& credentials, StringData pw } const auto decodedSalt = base64::decode(scram.salt); - scram::Secrets<HashBlock> secrets(scram::Presecrets<HashBlock>( + scram::Secrets<HashBlock, scram::UnlockedSecretsPolicy> secrets(scram::Presecrets<HashBlock>( pwd.toString(), std::vector<std::uint8_t>(reinterpret_cast<const std::uint8_t*>(decodedSalt.c_str()), reinterpret_cast<const std::uint8_t*>(decodedSalt.c_str()) + diff --git a/src/mongo/db/auth/sasl_scram_server_conversation.cpp b/src/mongo/db/auth/sasl_scram_server_conversation.cpp index 16fd2f86927..353bd37bd40 100644 --- a/src/mongo/db/auth/sasl_scram_server_conversation.cpp +++ b/src/mongo/db/auth/sasl_scram_server_conversation.cpp @@ -222,9 +222,10 @@ StatusWith<std::tuple<bool, std::string>> SaslSCRAMServerMechanism<Policy>::_fir } } - _secrets = scram::Secrets<HashBlock>("", - base64::decode(_scramCredentials.storedKey), - base64::decode(_scramCredentials.serverKey)); + _secrets = scram::Secrets<HashBlock, scram::UnlockedSecretsPolicy>( + "", + base64::decode(_scramCredentials.storedKey), + base64::decode(_scramCredentials.serverKey)); // Generate server-first-message // Create text-based nonce as base64 encoding of a binary blob of length multiple of 3 diff --git a/src/mongo/db/auth/sasl_scram_server_conversation.h b/src/mongo/db/auth/sasl_scram_server_conversation.h index e91a254b54b..1f84b52cc5f 100644 --- a/src/mongo/db/auth/sasl_scram_server_conversation.h +++ b/src/mongo/db/auth/sasl_scram_server_conversation.h @@ -83,7 +83,7 @@ private: int _step{0}; std::string _authMessage; User::SCRAMCredentials<HashBlock> _scramCredentials; - scram::Secrets<HashBlock> _secrets; + scram::Secrets<HashBlock, scram::UnlockedSecretsPolicy> _secrets; // client and server nonce concatenated std::string _nonce; |