summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--jstests/auth/sasl_mechanism_discovery.js43
-rw-r--r--jstests/auth/user_management_commands_mechanisms.js10
-rw-r--r--src/mongo/client/sasl_scram_client_conversation.cpp8
-rw-r--r--src/mongo/db/auth/sasl_mechanism_registry.cpp71
-rw-r--r--src/mongo/db/auth/sasl_mechanism_registry_test.cpp52
-rw-r--r--src/mongo/db/auth/sasl_scram_server_conversation.cpp6
-rw-r--r--src/mongo/db/commands/user_management_commands.cpp7
7 files changed, 39 insertions, 158 deletions
diff --git a/jstests/auth/sasl_mechanism_discovery.js b/jstests/auth/sasl_mechanism_discovery.js
index 7ea4c75f5b4..2d5404178fd 100644
--- a/jstests/auth/sasl_mechanism_discovery.js
+++ b/jstests/auth/sasl_mechanism_discovery.js
@@ -4,30 +4,36 @@
"use strict";
function runTest(conn) {
+ function checkMechs(userid, mechs) {
+ const res =
+ assert.commandWorked(db.runCommand({isMaster: 1, saslSupportedMechs: userid}));
+ assert.eq(mechs.sort(), res.saslSupportedMechs.sort(), tojson(res));
+ }
+
var db = conn.getDB("admin");
var externalDB = conn.getDB("$external");
- // Check that unknown or users produce the correct errors.
+ // Check that unknown or invalid users produce the correct errors.
assert.commandFailedWithCode(db.runCommand({isMaster: 1, saslSupportedMechs: "test.bogus"}),
ErrorCodes.UserNotFound);
assert.commandFailedWithCode(db.runCommand({isMaster: 1, saslSupportedMechs: "bogus"}),
ErrorCodes.BadValue);
+ // Create a legacy user.
+ assert.commandWorked(db.adminCommand({setFeatureCompatibilityVersion: "3.6"}));
+ assert.commandWorked(db.runCommand({createUser: "legacyUser", pwd: "pwd", roles: []}));
+ checkMechs("admin.legacyUser", ["SCRAM-SHA-1"]);
+
// Enable SCRAM-SHA-256.
assert.commandWorked(db.adminCommand({setFeatureCompatibilityVersion: "4.0"}));
- function checkMechs(userid, mechs) {
- const res =
- assert.commandWorked(db.runCommand({isMaster: 1, saslSupportedMechs: userid}));
- assert.eq(mechs.sort(), res.saslSupportedMechs.sort(), tojson(res));
- }
+ // Legacy users continue expressing SCRAM-SHA-1
+ checkMechs("admin.legacyUser", ["SCRAM-SHA-1"]);
// Make users.
assert.commandWorked(db.runCommand({createUser: "user", pwd: "pwd", roles: []}));
assert.commandWorked(externalDB.runCommand({createUser: "user", roles: []}));
- assert.commandWorked(db.runCommand(
- {createUser: "IX", pwd: "pwd", roles: [], mechanisms: ["SCRAM-SHA-256"]}));
// Internal users should support scram methods.
checkMechs("admin.user", ["SCRAM-SHA-256", "SCRAM-SHA-1"]);
@@ -39,16 +45,19 @@
checkMechs("$external.user", []);
}
- // Check non-normalized name finds normalized user.
- const IXchar = "\u2168";
- const IXuserid = "admin." + IXchar;
- checkMechs(IXuserid, ["SCRAM-SHA-256"]);
-
- // Check that names with compatibility equivalence collide.
+ // Users with explicit mechs should only support those mechanisms
assert.commandWorked(db.runCommand(
- {createUser: IXchar, pwd: "pwd", roles: [], mechanisms: ["SCRAM-SHA-1"]}));
- assert.commandFailedWithCode(db.runCommand({isMaster: 1, saslSupportedMechs: IXuserid}),
- ErrorCodes.BadValue);
+ {createUser: "256Only", pwd: "pwd", roles: [], mechanisms: ["SCRAM-SHA-256"]}));
+ checkMechs("admin.256Only", ["SCRAM-SHA-256"]);
+ assert.commandWorked(db.runCommand(
+ {createUser: "1Only", pwd: "pwd", roles: [], mechanisms: ["SCRAM-SHA-1"]}));
+ checkMechs("admin.1Only", ["SCRAM-SHA-1"]);
+
+ // Users with normalized and unnormalized names do not conflict
+ assert.commandWorked(db.runCommand({createUser: "IX", pwd: "pwd", roles: []}));
+ checkMechs("admin.IX", ["SCRAM-SHA-1", "SCRAM-SHA-256"]);
+ assert.commandWorked(db.runCommand({createUser: "\u2168", pwd: "pwd", roles: []}));
+ checkMechs("admin.\u2168", ["SCRAM-SHA-1", "SCRAM-SHA-256"]);
}
// Test standalone.
diff --git a/jstests/auth/user_management_commands_mechanisms.js b/jstests/auth/user_management_commands_mechanisms.js
index 76c778d6c28..8acd695920a 100644
--- a/jstests/auth/user_management_commands_mechanisms.js
+++ b/jstests/auth/user_management_commands_mechanisms.js
@@ -183,21 +183,11 @@
test.updateUser('user', {pwd: 'passEmpty', mechanisms: []});
});
- // Fail when passing a non-normalized username and producing SHA-256 creds.
- assert.throws(function() {
- test.createUser({user: "\u2168", pwd: 'pass', roles: jsTest.basicUserRoles});
- });
-
// Succeed if we're using SHA-1 only.
test.createUser(
{user: "\u2168", pwd: 'pass', roles: jsTest.basicUserRoles, mechanisms: ['SCRAM-SHA-1']});
checkUser("\u2168", 'pass', true, false);
- // Then fail again trying to promote that user to SHA-256.
- assert.throws(function() {
- test.updateUser("\u2168", {pwd: 'pass'});
- });
-
// Demonstrate that usersInfo returns all users with mechanisms lists
const allUsersInfo = assert.commandWorked(test.runCommand({usersInfo: 1}));
allUsersInfo.users.forEach(function(userObj) {
diff --git a/src/mongo/client/sasl_scram_client_conversation.cpp b/src/mongo/client/sasl_scram_client_conversation.cpp
index 227a793486e..6c2fa950e22 100644
--- a/src/mongo/client/sasl_scram_client_conversation.cpp
+++ b/src/mongo/client/sasl_scram_client_conversation.cpp
@@ -90,12 +90,8 @@ StatusWith<bool> SaslSCRAMClientConversation::_firstStep(std::string* outputData
binaryNonce[1] = sr->nextInt64();
binaryNonce[2] = sr->nextInt64();
- auto swUser =
- saslPrep(_saslClientSession->getParameter(SaslClientSession::parameterUser).toString());
- if (!swUser.isOK()) {
- return swUser.getStatus();
- }
- auto user = swUser.getValue();
+ std::string user =
+ _saslClientSession->getParameter(SaslClientSession::parameterUser).toString();
encodeSCRAMUsername(user);
_clientNonce = base64::encode(reinterpret_cast<char*>(binaryNonce), sizeof(binaryNonce));
diff --git a/src/mongo/db/auth/sasl_mechanism_registry.cpp b/src/mongo/db/auth/sasl_mechanism_registry.cpp
index 1572acb2794..69c5f495e25 100644
--- a/src/mongo/db/auth/sasl_mechanism_registry.cpp
+++ b/src/mongo/db/auth/sasl_mechanism_registry.cpp
@@ -43,60 +43,6 @@ namespace mongo {
namespace {
const auto getSASLServerMechanismRegistry =
ServiceContext::declareDecoration<std::unique_ptr<SASLServerMechanismRegistry>>();
-
-/**
- * Fetch credentials for the named user either using the unnormalized form provided,
- * or the form returned from saslPrep().
- * If both forms exist as different user records, produce an error.
- */
-User* getUserPtr(OperationContext* opCtx, const UserName& userName) {
- AuthorizationManager* authManager = AuthorizationManager::get(opCtx->getServiceContext());
-
- User* rawObj = nullptr;
- const auto rawStatus = authManager->acquireUser(opCtx, userName, &rawObj);
- auto rawGuard = MakeGuard([authManager, &rawObj] {
- if (rawObj) {
- authManager->releaseUser(rawObj);
- }
- });
-
- // Attempt to normalize the provided username (excluding DB portion).
- // If saslPrep() fails, then there can't possibly be another user with
- // compatibility equivalence, so fall-through.
- const auto swPrepUser = saslPrep(userName.getUser());
- if (swPrepUser.isOK()) {
- UserName prepUserName(swPrepUser.getValue(), userName.getDB());
- if (prepUserName != userName) {
- // User has a SASLPREPable name which differs from the raw presentation.
- // Double check that we don't have a different user by that new name.
- User* prepObj = nullptr;
- const auto prepStatus = authManager->acquireUser(opCtx, prepUserName, &prepObj);
-
- auto prepGuard = MakeGuard([authManager, &prepObj] {
- if (prepObj) {
- authManager->releaseUser(prepObj);
- }
- });
-
- if (prepStatus.isOK()) {
- // If both statuses are OK, then we have two distinct users with "different" names.
- uassert(ErrorCodes::BadValue,
- "Two users exist with names exhibiting compatibility equivalence",
- !rawStatus.isOK());
- // Otherwise, only the normalized version exists.
- User* returnObj = nullptr;
- std::swap(returnObj, prepObj);
- return returnObj;
- }
- }
- }
-
- uassertStatusOK(rawStatus);
- User* returnObj = nullptr;
- std::swap(returnObj, rawObj);
- return returnObj;
-}
-
} // namespace
SASLServerMechanismRegistry& SASLServerMechanismRegistry::get(ServiceContext* serviceContext) {
@@ -133,12 +79,17 @@ void SASLServerMechanismRegistry::advertiseMechanismNamesForUser(OperationContex
if (saslSupportedMechs.type() == BSONType::String) {
const auto userName = uassertStatusOK(UserName::parse(saslSupportedMechs.String()));
- const auto userPtr = getUserPtr(opCtx, userName);
- auto guard = MakeGuard([&opCtx, &userPtr] {
- AuthorizationManager* authManager =
- AuthorizationManager::get(opCtx->getServiceContext());
- authManager->releaseUser(userPtr);
+
+ AuthorizationManager* authManager = AuthorizationManager::get(opCtx->getServiceContext());
+
+ User* user = nullptr;
+ const auto status = authManager->acquireUser(opCtx, userName, &user);
+ auto guard = MakeGuard([authManager, &user] {
+ if (user) {
+ authManager->releaseUser(user);
+ }
});
+ uassertStatusOK(status);
BSONArrayBuilder mechanismsBuilder;
auto& map = _getMapRef(userName.getDB());
@@ -151,7 +102,7 @@ void SASLServerMechanismRegistry::advertiseMechanismNamesForUser(OperationContex
continue;
}
- if (factoryIt.second->canMakeMechanismForUser(userPtr)) {
+ if (factoryIt.second->canMakeMechanismForUser(user)) {
mechanismsBuilder << factoryIt.first;
}
}
diff --git a/src/mongo/db/auth/sasl_mechanism_registry_test.cpp b/src/mongo/db/auth/sasl_mechanism_registry_test.cpp
index 8c8515adb43..1fdea125852 100644
--- a/src/mongo/db/auth/sasl_mechanism_registry_test.cpp
+++ b/src/mongo/db/auth/sasl_mechanism_registry_test.cpp
@@ -179,40 +179,6 @@ public:
<< "roles"
<< BSONArray()),
BSONObj()));
-
-
- ASSERT_OK(authManagerExternalState->insert(
- opCtx.get(),
- NamespaceString("admin.system.users"),
- BSON("_id"
- << "test.collision"
- << "user"
- << "collision"
- << "db"
- << "test"
- << "credentials"
- << BSON("SCRAM-SHA-256"
- << scram::Secrets<SHA256Block>::generateCredentials("collision", 15000))
- << "roles"
- << BSONArray()),
- BSONObj()));
-
- // A user whose name does not equal "test.collision"'s, but ends in a Zero Width Joiner.
- ASSERT_OK(authManagerExternalState->insert(
- opCtx.get(),
- NamespaceString("admin.system.users"),
- BSON("_id"
- << "test.collision‍" // This string ends in a ZWJ
- << "user"
- << "collision‍" // This string ends in a ZWJ
- << "db"
- << "test"
- << "credentials"
- << BSON("SCRAM-SHA-256"
- << scram::Secrets<SHA256Block>::generateCredentials("collision", 15000))
- << "roles"
- << BSONArray()),
- BSONObj()));
}
ServiceContextNoop serviceContext;
@@ -271,24 +237,6 @@ TEST_F(MechanismRegistryTest, invalidUserCantAdvertiseMechs) {
AssertionException);
}
-TEST_F(MechanismRegistryTest, collisionsPreventAdvertisement) {
- registry.registerFactory<FooMechanismFactory<true>>(
- SASLServerMechanismRegistry::kNoValidateGlobalMechanisms);
-
- BSONObjBuilder builder;
-
- registry.advertiseMechanismNamesForUser(opCtx.get(),
- BSON("isMaster" << 1 << "saslSupportedMechs"
- << "test.collision"),
- &builder);
- ASSERT_THROWS(
- registry.advertiseMechanismNamesForUser(opCtx.get(),
- BSON("isMaster" << 1 << "saslSupportedMechs"
- << "test.collision‍"),
- &builder),
- AssertionException);
-}
-
TEST_F(MechanismRegistryTest, strongMechCanAdvertise) {
registry.registerFactory<BarMechanismFactory<true>>(
SASLServerMechanismRegistry::kNoValidateGlobalMechanisms);
diff --git a/src/mongo/db/auth/sasl_scram_server_conversation.cpp b/src/mongo/db/auth/sasl_scram_server_conversation.cpp
index 25cc6659247..c82bb5965c6 100644
--- a/src/mongo/db/auth/sasl_scram_server_conversation.cpp
+++ b/src/mongo/db/auth/sasl_scram_server_conversation.cpp
@@ -165,12 +165,6 @@ StatusWith<std::tuple<bool, std::string>> SaslSCRAMServerMechanism<Policy>::_fir
ServerMechanismBase::_principalName = input[0].substr(2);
decodeSCRAMUsername(ServerMechanismBase::_principalName);
- auto swUser = saslPrep(ServerMechanismBase::ServerMechanismBase::_principalName);
- if (!swUser.isOK()) {
- return swUser.getStatus();
- }
- ServerMechanismBase::ServerMechanismBase::_principalName = std::move(swUser.getValue());
-
if (!authzId.empty() && ServerMechanismBase::_principalName != authzId) {
return Status(ErrorCodes::BadValue,
str::stream() << "SCRAM user name " << ServerMechanismBase::_principalName
diff --git a/src/mongo/db/commands/user_management_commands.cpp b/src/mongo/db/commands/user_management_commands.cpp
index aa126b229da..fd3519d4871 100644
--- a/src/mongo/db/commands/user_management_commands.cpp
+++ b/src/mongo/db/commands/user_management_commands.cpp
@@ -666,13 +666,6 @@ Status buildCredentials(BSONObjBuilder* builder, const auth::CreateOrUpdateUserA
}
if (buildSCRAMSHA256) {
- const auto swPreppedName = saslPrep(args.userName.getUser());
- if (!swPreppedName.isOK() || (swPreppedName.getValue() != args.userName.getUser())) {
- return {
- ErrorCodes::BadValue,
- "Username must be normalized according to SASLPREP rules when using SCRAM-SHA-256"};
- }
-
// FCV check is deferred till this point so that the suitability checks can be performed
// regardless.
const auto fcv = serverGlobalParams.featureCompatibility.getVersion();
View Lorry Controller Status