SERVER-28997: Limit SCRAM-SHA-1 cache's use of Secure Memory

10 files changed, 116 insertions, 65 deletions

diff --git a/src/mongo/client/sasl_scramsha1_client_conversation.cpp b/src/mongo/client/sasl_scramsha1_client_conversation.cpp
index 2b52f1a28a2..1c04d395708 100644
--- a/src/mongo/client/sasl_scramsha1_client_conversation.cpp
+++ b/src/mongo/client/sasl_scramsha1_client_conversation.cpp
@@ -188,11 +188,9 @@ StatusWith<bool> SaslSCRAMSHA1ClientConversation::_secondStep(const std::vector<
         _saslClientSession->getParameter(SaslClientSession::parameterServiceHostAndPort));
 
     if (targetHost.isOK()) {
-        auto cachedSecrets = _clientCache->getCachedSecrets(targetHost.getValue(), presecrets);
+        _credentials = _clientCache->getCachedSecrets(targetHost.getValue(), presecrets);
 
-        if (cachedSecrets) {
-            _credentials = *cachedSecrets;
-        } else {
+        if (!_credentials) {
             _credentials = scram::generateSecrets(presecrets);
 
             _clientCache->setCachedSecrets(
diff --git a/src/mongo/client/scram_sha1_client_cache.cpp b/src/mongo/client/scram_sha1_client_cache.cpp
index a087f27ca0a..ca3bf9eca14 100644
--- a/src/mongo/client/scram_sha1_client_cache.cpp
+++ b/src/mongo/client/scram_sha1_client_cache.cpp
@@ -32,7 +32,7 @@
 
 namespace mongo {
 
-boost::optional<scram::SCRAMSecrets> SCRAMSHA1ClientCache::getCachedSecrets(
+scram::SCRAMSecrets SCRAMSHA1ClientCache::getCachedSecrets(
     const HostAndPort& target, const scram::SCRAMPresecrets& presecrets) const {
     const stdx::lock_guard<stdx::mutex> lock(_hostToSecretsMutex);
 
diff --git a/src/mongo/client/scram_sha1_client_cache.h b/src/mongo/client/scram_sha1_client_cache.h
index 4d5fdb3b323..ac6a139af2c 100644
--- a/src/mongo/client/scram_sha1_client_cache.h
+++ b/src/mongo/client/scram_sha1_client_cache.h
@@ -68,8 +68,8 @@ public:
      * match those recorded for the hostname. Otherwise, no secrets
      * are returned.
      */
-    boost::optional<scram::SCRAMSecrets> getCachedSecrets(
-        const HostAndPort& target, const scram::SCRAMPresecrets& presecrets) const;
+    scram::SCRAMSecrets getCachedSecrets(const HostAndPort& target,
+                                         const scram::SCRAMPresecrets& presecrets) const;
 
     /**
      * Records a set of precomputed SCRAMSecrets for the specified
diff --git a/src/mongo/crypto/mechanism_scram.cpp b/src/mongo/crypto/mechanism_scram.cpp
index b426dda6cbd..df290cb6fa5 100644
--- a/src/mongo/crypto/mechanism_scram.cpp
+++ b/src/mongo/crypto/mechanism_scram.cpp
@@ -95,27 +95,27 @@ SCRAMSecrets generateSecrets(const SCRAMPresecrets& presecrets) {
 }
 
 SCRAMSecrets generateSecrets(const SHA1Block& saltedPassword) {
-    SCRAMSecrets credentials;
-
-    // ClientKey := HMAC(saltedPassword, "Client Key")
-    credentials.clientKey =
-        SHA1Block::computeHmac(saltedPassword.data(),
-                               saltedPassword.size(),
-                               reinterpret_cast<const unsigned char*>(clientKeyConst.data()),
-                               clientKeyConst.size());
-
-    // StoredKey := H(clientKey)
-    credentials.storedKey =
-        SHA1Block::computeHash(credentials.clientKey->data(), credentials.clientKey->size());
-
-    // ServerKey       := HMAC(SaltedPassword, "Server Key")
-    credentials.serverKey =
-        SHA1Block::computeHmac(saltedPassword.data(),
-                               saltedPassword.size(),
-                               reinterpret_cast<const unsigned char*>(serverKeyConst.data()),
-                               serverKeyConst.size());
-
-    return credentials;
+    auto generateAndStoreSecrets = [&saltedPassword](
+        SHA1Block& clientKey, SHA1Block& storedKey, SHA1Block& serverKey) {
+
+        // ClientKey := HMAC(saltedPassword, "Client Key")
+        clientKey =
+            SHA1Block::computeHmac(saltedPassword.data(),
+                                   saltedPassword.size(),
+                                   reinterpret_cast<const unsigned char*>(clientKeyConst.data()),
+                                   clientKeyConst.size());
+
+        // StoredKey := H(clientKey)
+        storedKey = SHA1Block::computeHash(clientKey.data(), clientKey.size());
+
+        // ServerKey       := HMAC(SaltedPassword, "Server Key")
+        serverKey =
+            SHA1Block::computeHmac(saltedPassword.data(),
+                                   saltedPassword.size(),
+                                   reinterpret_cast<const unsigned char*>(serverKeyConst.data()),
+                                   serverKeyConst.size());
+    };
+    return SCRAMSecrets(std::move(generateAndStoreSecrets));
 }
 
 
@@ -140,8 +140,8 @@ BSONObj generateCredentials(const std::string& hashedPassword, int iterationCoun
                                                       saltLenQWords * sizeof(uint64_t)),
                         iterationCount));
 
-    std::string encodedStoredKey = secrets.storedKey->toString();
-    std::string encodedServerKey = secrets.serverKey->toString();
+    std::string encodedStoredKey = secrets->storedKey.toString();
+    std::string encodedServerKey = secrets->serverKey.toString();
 
     return BSON(iterationCountFieldName << iterationCount << saltFieldName << encodedUserSalt
                                         << storedKeyFieldName
@@ -154,12 +154,12 @@ std::string generateClientProof(const SCRAMSecrets& clientCredentials,
                                 const std::string& authMessage) {
     // ClientSignature := HMAC(StoredKey, AuthMessage)
     SHA1Block clientSignature =
-        SHA1Block::computeHmac(clientCredentials.storedKey->data(),
-                               clientCredentials.storedKey->size(),
+        SHA1Block::computeHmac(clientCredentials->storedKey.data(),
+                               clientCredentials->storedKey.size(),
                                reinterpret_cast<const unsigned char*>(authMessage.c_str()),
                                authMessage.size());
 
-    clientSignature.xorInline(*clientCredentials.clientKey);
+    clientSignature.xorInline(clientCredentials->clientKey);
     return clientSignature.toString();
 }
 
@@ -168,8 +168,8 @@ bool verifyServerSignature(const SCRAMSecrets& clientCredentials,
                            const std::string& receivedServerSignature) {
     // ServerSignature := HMAC(ServerKey, AuthMessage)
     SHA1Block serverSignature =
-        SHA1Block::computeHmac(clientCredentials.serverKey->data(),
-                               clientCredentials.serverKey->size(),
+        SHA1Block::computeHmac(clientCredentials->serverKey.data(),
+                               clientCredentials->serverKey.size(),
                                reinterpret_cast<const unsigned char*>(authMessage.c_str()),
                                authMessage.size());
 
diff --git a/src/mongo/crypto/mechanism_scram.h b/src/mongo/crypto/mechanism_scram.h
index fb070e8162c..b12cebbf2e1 100644
--- a/src/mongo/crypto/mechanism_scram.h
+++ b/src/mongo/crypto/mechanism_scram.h
@@ -76,12 +76,50 @@ SHA1Block generateSaltedPassword(const SCRAMPresecrets& presecrets);
 
 /*
  * Stores all of the keys, generated from a password, needed for a client or server to perform a
- * SCRAM handshake. This structure will secureZeroMemory itself on destruction.
+ * SCRAM handshake.
+ * These keys are reference counted, and allocated using the SecureAllocator.
+ * May be unpopulated. SCRAMSecrets created via the default constructor are unpopulated.
+ * The behavior is undefined if the accessors are called when unpopulated.
  */
-struct SCRAMSecrets {
-    SecureHandle<SHA1Block> clientKey;
-    SecureHandle<SHA1Block> storedKey;
-    SecureHandle<SHA1Block> serverKey;
+class SCRAMSecrets {
+private:
+    struct SCRAMSecretsHolder {
+        SHA1Block clientKey;
+        SHA1Block storedKey;
+        SHA1Block serverKey;
+    };
+
+public:
+    // Creates an unpopulated SCRAMSecrets object.
+    SCRAMSecrets() = default;
+
+    // Creates a populated SCRAMSecrets object. First, allocates secure storage, then provides it
+    // to a callback, which fills the memory.
+    template <typename T>
+    explicit SCRAMSecrets(T initializationFun)
+        : _ptr(std::make_shared<SecureHandle<SCRAMSecretsHolder>>()) {
+        initializationFun((*this)->clientKey, (*this)->storedKey, (*this)->serverKey);
+    }
+
+    // Returns true if the underlying shared_pointer is populated.
+    explicit operator bool() const {
+        return static_cast<bool>(_ptr);
+    }
+
+    const SecureHandle<SCRAMSecretsHolder>& operator*() const& {
+        invariant(_ptr);
+        return *_ptr;
+    }
+    void operator*() && = delete;
+
+    const SecureHandle<SCRAMSecretsHolder>& operator->() const& {
+        invariant(_ptr);
+        return *_ptr;
+    }
+    void operator->() && = delete;
+
+private:
+    std::shared_ptr<SecureHandle<SCRAMSecretsHolder>> _ptr;
 };
 
 /*
diff --git a/src/mongo/crypto/sha1_block.h b/src/mongo/crypto/sha1_block.h
index 3cf92c9e187..3cead63d6f9 100644
--- a/src/mongo/crypto/sha1_block.h
+++ b/src/mongo/crypto/sha1_block.h
@@ -63,12 +63,28 @@ public:
     static SHA1Block computeHmac(const uint8_t* key,
                                  size_t keyLen,
                                  const uint8_t* input,
-                                 size_t inputLen);
+                                 size_t inputLen) {
+        SHA1Block output;
+        SHA1Block::computeHmac(key, keyLen, input, inputLen, &output);
+        return output;
+    }
 
-    const uint8_t* data() const {
+    /**
+     * Computes a HMAC SHA-1 keyed hash of 'input' using the key 'key'. Writes the results into
+     * a pre-allocated SHA1Block. This lets us allocate SHA1Blocks with the SecureAllocator.
+     */
+    static void computeHmac(const uint8_t* key,
+                            size_t keyLen,
+                            const uint8_t* input,
+                            size_t inputLen,
+                            SHA1Block* const output);
+
+    const uint8_t* data() const& {
         return _hash.data();
     }
 
+    uint8_t* data() const&& = delete;
+
     size_t size() const {
         return _hash.size();
     }
diff --git a/src/mongo/crypto/sha1_block_openssl.cpp b/src/mongo/crypto/sha1_block_openssl.cpp
index a3c4b8c4049..75bb53e3960 100644
--- a/src/mongo/crypto/sha1_block_openssl.cpp
+++ b/src/mongo/crypto/sha1_block_openssl.cpp
@@ -80,15 +80,15 @@ SHA1Block SHA1Block::computeHash(const uint8_t* input, size_t inputLen) {
 }
 
 /*
- * Computes a HMAC SHA-1 keyed hash of 'input' using the key 'key'
+ * Computes a HMAC SHA-1 keyed hash of 'input' using the key 'key', writes output into 'output'.
  */
-SHA1Block SHA1Block::computeHmac(const uint8_t* key,
-                                 size_t keyLen,
-                                 const uint8_t* input,
-                                 size_t inputLen) {
-    HashType output;
-    fassert(40380, HMAC(EVP_sha1(), key, keyLen, input, inputLen, output.data(), NULL) != NULL);
-    return SHA1Block(output);
+void SHA1Block::computeHmac(const uint8_t* key,
+                            size_t keyLen,
+                            const uint8_t* input,
+                            size_t inputLen,
+                            SHA1Block* const output) {
+    fassert(40380,
+            HMAC(EVP_sha1(), key, keyLen, input, inputLen, output->_hash.data(), NULL) != NULL);
 }
 
 }  // namespace mongo
diff --git a/src/mongo/crypto/sha1_block_tom.cpp b/src/mongo/crypto/sha1_block_tom.cpp
index d1078e60458..7d32dbc2f53 100644
--- a/src/mongo/crypto/sha1_block_tom.cpp
+++ b/src/mongo/crypto/sha1_block_tom.cpp
@@ -58,12 +58,12 @@ SHA1Block SHA1Block::computeHash(const uint8_t* input, size_t inputLen) {
 /*
  * Computes a HMAC SHA-1 keyed hash of 'input' using the key 'key'
  */
-SHA1Block SHA1Block::computeHmac(const uint8_t* key,
-                                 size_t keyLen,
-                                 const uint8_t* input,
-                                 size_t inputLen) {
+void SHA1Block::computeHmac(const uint8_t* key,
+                            size_t keyLen,
+                            const uint8_t* input,
+                            size_t inputLen,
+                            SHA1Block* const output) {
     invariant(key && input);
-    HashType output;
 
     static int hashId = -1;
     if (hashId == -1) {
@@ -73,9 +73,8 @@ SHA1Block SHA1Block::computeHmac(const uint8_t* key,
 
     unsigned long sha1HashLen = 20;
     fassert(40382,
-            hmac_memory(hashId, key, keyLen, input, inputLen, output.data(), &sha1HashLen) ==
+            hmac_memory(hashId, key, keyLen, input, inputLen, output->_hash.data(), &sha1HashLen) ==
                 CRYPT_OK);
-    return SHA1Block(output);
 }
 
 }  // namespace mongo
diff --git a/src/mongo/db/auth/sasl_plain_server_conversation.cpp b/src/mongo/db/auth/sasl_plain_server_conversation.cpp
index 24c5d94181f..67ad4d52272 100644
--- a/src/mongo/db/auth/sasl_plain_server_conversation.cpp
+++ b/src/mongo/db/auth/sasl_plain_server_conversation.cpp
@@ -123,8 +123,8 @@ StatusWith<bool> SaslPLAINServerConversation::step(StringData inputData, std::st
                                           16),
             creds.scram.iterationCount));
         if (creds.scram.storedKey !=
-            base64::encode(reinterpret_cast<const char*>(secrets.storedKey->data()),
-                           secrets.storedKey->size())) {
+            base64::encode(reinterpret_cast<const char*>(secrets->storedKey.data()),
+                           secrets->storedKey.size())) {
             return StatusWith<bool>(ErrorCodes::AuthenticationFailed,
                                     mongoutils::str::stream() << "Incorrect user name or password");
         }
diff --git a/src/mongo/db/auth/sasl_scramsha1_test.cpp b/src/mongo/db/auth/sasl_scramsha1_test.cpp
index 99794ffdf29..c575d94ddc0 100644
--- a/src/mongo/db/auth/sasl_scramsha1_test.cpp
+++ b/src/mongo/db/auth/sasl_scramsha1_test.cpp
@@ -460,9 +460,9 @@ TEST(SCRAMSHA1Cache, testSetAndGet) {
     cache.setCachedSecrets(host, scram::SCRAMPresecrets("aaa", salt, 10000), secret);
     auto cachedSecret = cache.getCachedSecrets(host, scram::SCRAMPresecrets("aaa", salt, 10000));
     ASSERT_TRUE(cachedSecret);
-    ASSERT_TRUE(*secret.clientKey == *cachedSecret->clientKey);
-    ASSERT_TRUE(*secret.serverKey == *cachedSecret->serverKey);
-    ASSERT_TRUE(*secret.storedKey == *cachedSecret->storedKey);
+    ASSERT_TRUE(secret->clientKey == cachedSecret->clientKey);
+    ASSERT_TRUE(secret->serverKey == cachedSecret->serverKey);
+    ASSERT_TRUE(secret->storedKey == cachedSecret->storedKey);
 }
 
 
@@ -499,9 +499,9 @@ TEST(SCRAMSHA1Cache, testSetAndReset) {
     ASSERT_FALSE(cache.getCachedSecrets(host, scram::SCRAMPresecrets("aaa", salt, 10000)));
     auto cachedSecret = cache.getCachedSecrets(host, scram::SCRAMPresecrets("aab", salt, 10000));
     ASSERT_TRUE(cachedSecret);
-    ASSERT_TRUE(*newSecret.clientKey == *cachedSecret->clientKey);
-    ASSERT_TRUE(*newSecret.serverKey == *cachedSecret->serverKey);
-    ASSERT_TRUE(*newSecret.storedKey == *cachedSecret->storedKey);
+    ASSERT_TRUE(newSecret->clientKey == cachedSecret->clientKey);
+    ASSERT_TRUE(newSecret->serverKey == cachedSecret->serverKey);
+    ASSERT_TRUE(newSecret->storedKey == cachedSecret->storedKey);
 }
 
 }  // namespace mongo