Skip to content

Commit

Permalink
SERVER-26952: Cache SCRAM-SHA-1 ClientKey
Browse files Browse the repository at this point in the history
(cherry picked from commit 47da0b5)
  • Loading branch information
spencerjackson committed Jul 11, 2017
1 parent 7880057 commit 16e8333
Show file tree
Hide file tree
Showing 29 changed files with 570 additions and 332 deletions.
37 changes: 37 additions & 0 deletions buildscripts/resmokeconfig/suites/multiversion_auth.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# Section that is ignored by resmoke.py.
config_variables:
- &keyFile jstests/libs/authTestsKey
- &keyFileData Thiskeyisonlyforrunningthesuitewithauthenticationdontuseitinanytestsdirectly

selector:
js_test:
roots:
- jstests/multiVersion/*.js
exclude_files:
# Needs extra work to support MONGODB-CR
- jstests/multiVersion/upgrade_cluster_v5_to_v6.js
# Uses threads which don't propagate jsTest() information
- jstests/multiVersion/mixed_storage_version_replication.js
# Uses ToolTest, which doesn't start servers with keyFile
- jstests/multiVersion/transitioning_to_and_from_WT.js
# TODO: SERVER-21578
- jstests/multiVersion/balancer_multiVersion_detect.js

# Multiversion tests start their own mongod's.
executor:
js_test:
config:
shell_options:
global_vars:
TestData:
auth: true
authMechanism: SCRAM-SHA-1
keyFile: *keyFile
keyFileData: *keyFileData
authenticationDatabase: local
authenticationMechanism: SCRAM-SHA-1
password: *keyFileData
username: __system
nodb: ''
readMode: legacy
writeMode: legacy
14 changes: 14 additions & 0 deletions etc/evergreen.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1362,6 +1362,17 @@ tasks:
resmoke_args: --suites=multiversion
run_multiple_jobs: true

- <<: *task_template
name: multiversion_auth
commands:
- func: "do setup"
- func: "do multiversion setup"
- func: "run tests"
vars:
path_prefix: PATH=$PATH:/data/multiversion
resmoke_args: --suites=multiversion_auth
run_multiple_jobs: true

- <<: *task_template
name: noPassthrough
commands:
Expand Down Expand Up @@ -3523,6 +3534,7 @@ buildvariants:
- name: mmap
- name: mongosTest
- name: multiversion
- name: multiversion_auth
- name: noPassthrough
- name: noPassthrough_WT
- name: noPassthroughWithMongod
Expand Down Expand Up @@ -4306,6 +4318,7 @@ buildvariants:
- name: mmap
- name: mongosTest
- name: multiversion
- name: multiversion_auth
- name: noPassthrough
- name: noPassthroughWithMongod
- name: noPassthroughWithMongod_WT
Expand Down Expand Up @@ -6114,6 +6127,7 @@ buildvariants:
- name: mmap
- name: mongosTest
- name: multiversion
- name: multiversion_auth
- name: noPassthrough
- name: noPassthroughWithMongod
- name: noPassthroughWithMongod_WT
Expand Down
4 changes: 4 additions & 0 deletions jstests/libs/test_background_ops.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ var getResult = function(mongo, name) {
* Overrides the parallel shell code in mongo
*/
function startParallelShell(jsCode, port) {
if (TestData) {
jsCode = "TestData = " + tojson(TestData) + ";" + jsCode;
}

var x;
if (port) {
x = startMongoProgramNoConnect("mongo", "--port", port, "--eval", jsCode);
Expand Down
1 change: 1 addition & 0 deletions jstests/multiVersion/libs/multi_rs.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ ReplSetTest.prototype.upgradeNode = function(node, opts, user, pwd) {
if (user != undefined) {
assert.eq(1, node.getDB("admin").auth(user, pwd));
}
jsTest.authenticate(node);

var isMaster = node.getDB('admin').runCommand({isMaster: 1});

Expand Down
1 change: 1 addition & 0 deletions src/mongo/client/SConscript
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ saslClientSource = [
'sasl_client_session.cpp',
'sasl_plain_client_conversation.cpp',
'sasl_scramsha1_client_conversation.cpp',
'scram_sha1_client_cache.cpp',
]

# Add in actual sasl dependencies if sasl is enabled, otherwise
Expand Down
6 changes: 3 additions & 3 deletions src/mongo/client/authenticate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ void authX509(RunCommandHook runCommand,

void auth(RunCommandHook runCommand,
const BSONObj& params,
StringData hostname,
const HostAndPort& hostname,
StringData clientName,
AuthCompletionHandler handler) {
std::string mechanism;
Expand Down Expand Up @@ -273,7 +273,7 @@ bool needsFallback(const AuthResponse& response) {

void asyncAuth(RunCommandHook runCommand,
const BSONObj& params,
StringData hostname,
const HostAndPort& hostname,
StringData clientName,
AuthCompletionHandler handler) {
auth(runCommand,
Expand All @@ -297,7 +297,7 @@ void asyncAuth(RunCommandHook runCommand,
} // namespace

void authenticateClient(const BSONObj& params,
StringData hostname,
const HostAndPort& hostname,
StringData clientName,
RunCommandHook runCommand,
AuthCompletionHandler handler) {
Expand Down
2 changes: 1 addition & 1 deletion src/mongo/client/authenticate.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ extern const char* const kMechanismScramSha1;
* tantamount to authentication failure, but may also indicate more serious problems.
*/
void authenticateClient(const BSONObj& params,
StringData hostname,
const HostAndPort& hostname,
StringData clientSubjectName,
RunCommandHook runCommand,
AuthCompletionHandler handler = AuthCompletionHandler());
Expand Down
8 changes: 4 additions & 4 deletions src/mongo/client/authenticate_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,13 @@ class AuthClientTest : public mongo::unittest::Test {

TEST_F(AuthClientTest, MongoCR) {
auto params = loadMongoCRConversation();
auth::authenticateClient(std::move(params), "", "", _runCommandCallback);
auth::authenticateClient(std::move(params), HostAndPort(), "", _runCommandCallback);
}

TEST_F(AuthClientTest, asyncMongoCR) {
auto params = loadMongoCRConversation();
auth::authenticateClient(std::move(params),
"",
HostAndPort(),
"",
_runCommandCallback,
[this](auth::AuthResponse response) { ASSERT(response.isOK()); });
Expand All @@ -188,13 +188,13 @@ TEST_F(AuthClientTest, asyncMongoCR) {
#ifdef MONGO_CONFIG_SSL
TEST_F(AuthClientTest, X509) {
auto params = loadX509Conversation();
auth::authenticateClient(std::move(params), "", _username, _runCommandCallback);
auth::authenticateClient(std::move(params), HostAndPort(), _username, _runCommandCallback);
}

TEST_F(AuthClientTest, asyncX509) {
auto params = loadX509Conversation();
auth::authenticateClient(std::move(params),
"",
HostAndPort(),
_username,
_runCommandCallback,
[this](auth::AuthResponse response) { ASSERT(response.isOK()); });
Expand Down
2 changes: 1 addition & 1 deletion src/mongo/client/dbclient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,7 @@ void DBClientWithCommands::_auth(const BSONObj& params) {

auth::authenticateClient(
params,
HostAndPort(getServerAddress()).host(),
HostAndPort(getServerAddress()),
clientName,
[this](RemoteCommandRequest request, auth::AuthCompletionHandler handler) {
BSONObj info;
Expand Down
6 changes: 5 additions & 1 deletion src/mongo/client/native_sasl_client_session.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "mongo/client/sasl_client_conversation.h"
#include "mongo/client/sasl_plain_client_conversation.h"
#include "mongo/client/sasl_scramsha1_client_conversation.h"
#include "mongo/client/scram_sha1_client_cache.h"
#include "mongo/util/mongoutils/str.h"

namespace mongo {
Expand All @@ -47,6 +48,9 @@ MONGO_INITIALIZER(NativeSaslClientContext)(InitializerContext* context) {
return Status::OK();
}

// Global cache for SCRAM-SHA-1 credentials
SCRAMSHA1ClientCache* scramsha1ClientCache = new SCRAMSHA1ClientCache;

} // namespace

NativeSaslClientSession::NativeSaslClientSession()
Expand All @@ -63,7 +67,7 @@ Status NativeSaslClientSession::initialize() {
if (mechanism == "PLAIN") {
_saslConversation.reset(new SaslPLAINClientConversation(this));
} else if (mechanism == "SCRAM-SHA-1") {
_saslConversation.reset(new SaslSCRAMSHA1ClientConversation(this));
_saslConversation.reset(new SaslSCRAMSHA1ClientConversation(this, scramsha1ClientCache));
} else {
return Status(ErrorCodes::BadValue,
mongoutils::str::stream() << "SASL mechanism " << mechanism
Expand Down
2 changes: 1 addition & 1 deletion src/mongo/client/sasl_client_authenticate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ namespace mongo {
using namespace mongoutils;

void (*saslClientAuthenticate)(auth::RunCommandHook runCommand,
StringData hostname,
const HostAndPort& hostname,
const BSONObj& saslParameters,
auth::AuthCompletionHandler handler) = nullptr;

Expand Down
2 changes: 1 addition & 1 deletion src/mongo/client/sasl_client_authenticate.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class BSONObj;
* returned.
*/
extern void (*saslClientAuthenticate)(auth::RunCommandHook runCommand,
StringData hostname,
const HostAndPort& hostname,
const BSONObj& saslParameters,
auth::AuthCompletionHandler handler);

Expand Down
7 changes: 4 additions & 3 deletions src/mongo/client/sasl_client_authenticate_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ Status extractPassword(const BSONObj& saslParameters,
* Returns Status::OK() on success.
*/
Status configureSession(SaslClientSession* session,
StringData hostname,
const HostAndPort& hostname,
StringData targetDatabase,
const BSONObj& saslParameters) {
std::string mechanism;
Expand All @@ -134,10 +134,11 @@ Status configureSession(SaslClientSession* session,
session->setParameter(SaslClientSession::parameterServiceName, value);

status = bsonExtractStringFieldWithDefault(
saslParameters, saslCommandServiceHostnameFieldName, hostname, &value);
saslParameters, saslCommandServiceHostnameFieldName, hostname.host(), &value);
if (!status.isOK())
return status;
session->setParameter(SaslClientSession::parameterServiceHostname, value);
session->setParameter(SaslClientSession::parameterServiceHostAndPort, hostname.toString());

status = bsonExtractStringField(saslParameters, saslCommandUserFieldName, &value);
if (!status.isOK())
Expand Down Expand Up @@ -247,7 +248,7 @@ void asyncSaslConversation(auth::RunCommandHook runCommand,
* "client".
*/
void saslClientAuthenticateImpl(auth::RunCommandHook runCommand,
StringData hostname,
const HostAndPort& hostname,
const BSONObj& saslParameters,
auth::AuthCompletionHandler handler) {
int saslLogLevel = getSaslClientLogLevel(saslParameters);
Expand Down
1 change: 1 addition & 0 deletions src/mongo/client/sasl_client_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class SaslClientSession {
enum Parameter {
parameterServiceName = 0,
parameterServiceHostname,
parameterServiceHostAndPort,
parameterMechanism,
parameterUser,
parameterPassword,
Expand Down
48 changes: 32 additions & 16 deletions src/mongo/client/sasl_scramsha1_client_conversation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

#include "mongo/base/parse_number.h"
#include "mongo/client/sasl_client_session.h"
#include "mongo/client/scram_sha1_client_cache.h"
#include "mongo/platform/random.h"
#include "mongo/util/base64.h"
#include "mongo/util/mongoutils/str.h"
Expand All @@ -46,13 +47,12 @@ using std::unique_ptr;
using std::string;

SaslSCRAMSHA1ClientConversation::SaslSCRAMSHA1ClientConversation(
SaslClientSession* saslClientSession)
: SaslClientConversation(saslClientSession), _step(0), _authMessage(""), _clientNonce("") {}

SaslSCRAMSHA1ClientConversation::~SaslSCRAMSHA1ClientConversation() {
// clear the _saltedPassword memory
memset(_saltedPassword, 0, scram::hashSize);
}
SaslClientSession* saslClientSession, SCRAMSHA1ClientCache* clientCache)
: SaslClientConversation(saslClientSession),
_step(0),
_authMessage(),
_clientCache(clientCache),
_clientNonce() {}

StatusWith<bool> SaslSCRAMSHA1ClientConversation::step(StringData inputData,
std::string* outputData) {
Expand Down Expand Up @@ -159,7 +159,7 @@ StatusWith<bool> SaslSCRAMSHA1ClientConversation::_secondStep(const std::vector<
}

std::string salt = input[1].substr(2);
int iterationCount;
size_t iterationCount;

Status status = parseNumberFromStringWithBase(input[2].substr(2), 10, &iterationCount);
if (status != Status::OK()) {
Expand All @@ -178,14 +178,30 @@ StatusWith<bool> SaslSCRAMSHA1ClientConversation::_secondStep(const std::vector<
return StatusWith<bool>(ex.toStatus());
}

scram::generateSaltedPassword(
_saslClientSession->getParameter(SaslClientSession::parameterPassword),
reinterpret_cast<const unsigned char*>(decodedSalt.c_str()),
decodedSalt.size(),
iterationCount,
_saltedPassword);
scram::SCRAMPresecrets presecrets(
_saslClientSession->getParameter(SaslClientSession::parameterPassword).toString(),
std::vector<std::uint8_t>(decodedSalt.begin(), decodedSalt.end()),
iterationCount);

StatusWith<HostAndPort> targetHost = HostAndPort::parse(
_saslClientSession->getParameter(SaslClientSession::parameterServiceHostAndPort));

if (targetHost.isOK()) {
auto cachedSecrets = _clientCache->getCachedSecrets(targetHost.getValue(), presecrets);

if (cachedSecrets) {
_credentials = *cachedSecrets;
} else {
_credentials = scram::generateSecrets(presecrets);

_clientCache->setCachedSecrets(
std::move(targetHost.getValue()), std::move(presecrets), _credentials);
}
} else {
_credentials = scram::generateSecrets(presecrets);
}

std::string clientProof = scram::generateClientProof(_saltedPassword, _authMessage);
std::string clientProof = scram::generateClientProof(_credentials, _authMessage);

StringBuilder sb;
sb << "c=biws,r=" << nonce << ",p=" << clientProof;
Expand Down Expand Up @@ -224,7 +240,7 @@ StatusWith<bool> SaslSCRAMSHA1ClientConversation::_thirdStep(const std::vector<s
}

bool validServerSignature =
scram::verifyServerSignature(_saltedPassword, _authMessage, input[0].substr(2));
scram::verifyServerSignature(_credentials, _authMessage, input[0].substr(2));

if (!validServerSignature) {
*outputData = "e=Invalid server signature";
Expand Down
12 changes: 8 additions & 4 deletions src/mongo/client/sasl_scramsha1_client_conversation.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
#include "mongo/crypto/mechanism_scram.h"

namespace mongo {

class SCRAMSHA1ClientCache;
/**
* Client side authentication session for SASL PLAIN.
*/
Expand All @@ -48,9 +50,8 @@ class SaslSCRAMSHA1ClientConversation : public SaslClientConversation {
/**
* Implements the client side of a SASL PLAIN mechanism session.
**/
explicit SaslSCRAMSHA1ClientConversation(SaslClientSession* saslClientSession);

virtual ~SaslSCRAMSHA1ClientConversation();
SaslSCRAMSHA1ClientConversation(SaslClientSession* saslClientSession,
SCRAMSHA1ClientCache* clientCache);

/**
* Takes one step in a SCRAM-SHA-1 conversation.
Expand Down Expand Up @@ -79,7 +80,10 @@ class SaslSCRAMSHA1ClientConversation : public SaslClientConversation {

int _step;
std::string _authMessage;
unsigned char _saltedPassword[scram::hashSize];

// Secrets and secrets cache
scram::SCRAMSecrets _credentials;
SCRAMSHA1ClientCache* const _clientCache;

// client and server nonce concatenated
std::string _clientNonce;
Expand Down
Loading

0 comments on commit 16e8333

Please sign in to comment.