diff options
author | Ben Caimano <ben.caimano@10gen.com> | 2021-03-22 15:32:15 +0000 |
---|---|---|
committer | Evergreen Agent <no-reply@evergreen.mongodb.com> | 2021-03-22 16:59:29 +0000 |
commit | bab0267533bdcd0f6c82d43c52b43b5e9cdf0f3f (patch) | |
tree | 2f5b1eee2950c7263ce9f1ee979512ff35b666b0 /src/mongo/db/auth | |
parent | c45819ca1324f65a2eb0040eff1754289383711a (diff) | |
download | mongo-bab0267533bdcd0f6c82d43c52b43b5e9cdf0f3f.tar.gz |
SERVER-55172 Add and use a fail point to wait for user cache invalidation
Diffstat (limited to 'src/mongo/db/auth')
-rw-r--r-- | src/mongo/db/auth/SConscript | 16 | ||||
-rw-r--r-- | src/mongo/db/auth/auth_types.idl | 6 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_manager.h | 5 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_manager_impl.cpp | 49 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_manager_impl.h | 4 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_session_impl.cpp | 99 | ||||
-rw-r--r-- | src/mongo/db/auth/authz_manager_external_state_local.cpp | 6 |
7 files changed, 126 insertions, 59 deletions
diff --git a/src/mongo/db/auth/SConscript b/src/mongo/db/auth/SConscript index a068a11714b..63b8b4fa121 100644 --- a/src/mongo/db/auth/SConscript +++ b/src/mongo/db/auth/SConscript @@ -87,7 +87,6 @@ env.Library( env.Library( target='auth_impl_internal_local', source=[ - 'auth_types.idl', 'authz_manager_external_state_local.cpp', ], LIBDEPS=[ @@ -97,6 +96,7 @@ env.Library( '$BUILD_DIR/mongo/db/catalog/collection_catalog', '$BUILD_DIR/mongo/db/concurrency/lock_manager', '$BUILD_DIR/mongo/db/db_raii', + 'auth_types', ] ) @@ -165,6 +165,7 @@ env.Library( '$BUILD_DIR/mongo/util/net/ssl_types', 'address_restriction', 'auth', + 'auth_types', 'auth_umc', 'authorization_manager_global', 'authprivilege', @@ -230,6 +231,19 @@ env.Library( ) env.Library( + target='auth_types', + source=[ + 'auth_types.idl', + ], + LIBDEPS=[ + '$BUILD_DIR/mongo/base', + '$BUILD_DIR/mongo/idl/idl_parser', + 'auth', + 'authprivilege', + ], +) + +env.Library( target='auth_umc', source=[ 'impersonation_session.cpp', diff --git a/src/mongo/db/auth/auth_types.idl b/src/mongo/db/auth/auth_types.idl index a653d0152ab..3b55541a4da 100644 --- a/src/mongo/db/auth/auth_types.idl +++ b/src/mongo/db/auth/auth_types.idl @@ -72,5 +72,9 @@ structs: fields: resolveRolesDelayMS: type: int - default: 0 validator: { gte: 0 } + waitForUserCacheInvalidationFailPoint: + description: Data for waitForUserCacheInvalidation fail point + fields: + userName: + type: UserName diff --git a/src/mongo/db/auth/authorization_manager.h b/src/mongo/db/auth/authorization_manager.h index 1dcd8208b0f..0c92156d70f 100644 --- a/src/mongo/db/auth/authorization_manager.h +++ b/src/mongo/db/auth/authorization_manager.h @@ -306,9 +306,8 @@ public: /** * Validate the ID associated with a known user while refreshing session cache. */ - virtual StatusWith<UserHandle> acquireUserForSessionRefresh(OperationContext* opCtx, - const UserName& userName, - const User::UserId& uid) = 0; + virtual StatusWith<UserHandle> reacquireUser(OperationContext* opCtx, + const UserHandle& user) = 0; /** * Marks the given user as invalid and removes it from the user cache. diff --git a/src/mongo/db/auth/authorization_manager_impl.cpp b/src/mongo/db/auth/authorization_manager_impl.cpp index 729df9178a7..807c17ef80b 100644 --- a/src/mongo/db/auth/authorization_manager_impl.cpp +++ b/src/mongo/db/auth/authorization_manager_impl.cpp @@ -44,6 +44,7 @@ #include "mongo/config.h" #include "mongo/crypto/mechanism_scram.h" #include "mongo/db/auth/address_restriction.h" +#include "mongo/db/auth/auth_types_gen.h" #include "mongo/db/auth/authorization_manager_global_parameters_gen.h" #include "mongo/db/auth/authorization_manager_impl_parameters_gen.h" #include "mongo/db/auth/authorization_session_impl.h" @@ -283,6 +284,40 @@ std::unique_ptr<AuthorizationManager> authorizationManagerCreateImpl( auto authorizationManagerCreateRegistration = MONGO_WEAK_FUNCTION_REGISTRATION(AuthorizationManager::create, authorizationManagerCreateImpl); +MONGO_FAIL_POINT_DEFINE(waitForUserCacheInvalidation); +void handleWaitForUserCacheInvalidation(OperationContext* opCtx, const UserHandle& user) { + auto fp = waitForUserCacheInvalidation.scopedIf([&](const auto& bsonData) { + IDLParserErrorContext ctx("waitForUserCacheInvalidation"); + auto data = WaitForUserCacheInvalidationFailPoint::parse(ctx, bsonData); + + const auto& blockedUserName = data.getUserName(); + return blockedUserName == user->getName(); + }); + + if (!fp.isActive()) { + return; + } + + // Since we do not have notifications from both the user cache and the fail point itself, + // loop until our condition is satisfied. To avoid this loop, we would need a way to union + // notifications from both. This may be possible with a CancellationToken and a condition + // variable or with some novel Notifiable/Waitable-like type that synthesizes multiple + // notifications. + constexpr auto kCheckPeriod = Milliseconds{1}; + auto m = MONGO_MAKE_LATCH(); + auto cv = stdx::condition_variable{}; + auto pred = [&] { return !fp.isStillEnabled() || !user.isValid(); }; + auto waitOneCycle = [&] { + auto lk = stdx::unique_lock(m); + return !opCtx->waitForConditionOrInterruptFor(cv, lk, kCheckPeriod, pred); + }; + + while (waitOneCycle()) { + // Not yet finished. + } +} + + } // namespace int authorizationManagerCacheSize; @@ -465,15 +500,23 @@ StatusWith<UserHandle> AuthorizationManagerImpl::acquireUser(OperationContext* o return ex.toStatus(); } -StatusWith<UserHandle> AuthorizationManagerImpl::acquireUserForSessionRefresh( - OperationContext* opCtx, const UserName& userName, const User::UserId& uid) { +StatusWith<UserHandle> AuthorizationManagerImpl::reacquireUser(OperationContext* opCtx, + const UserHandle& user) { + const UserName& userName = user->getName(); + handleWaitForUserCacheInvalidation(opCtx, user); + if (user.isValid()) { + return user; + } + + // Make a good faith effort to acquire an up-to-date user object, since the one + // we've cached is marked "out-of-date." auto swUserHandle = acquireUser(opCtx, userName); if (!swUserHandle.isOK()) { return swUserHandle.getStatus(); } auto ret = std::move(swUserHandle.getValue()); - if (uid != ret->getID()) { + if (user->getID() != ret->getID()) { return {ErrorCodes::UserNotFound, str::stream() << "User id from privilege document '" << userName.toString() << "' does not match user id in session."}; diff --git a/src/mongo/db/auth/authorization_manager_impl.h b/src/mongo/db/auth/authorization_manager_impl.h index c197f100dbd..6b6d059d790 100644 --- a/src/mongo/db/auth/authorization_manager_impl.h +++ b/src/mongo/db/auth/authorization_manager_impl.h @@ -98,9 +98,7 @@ public: std::vector<BSONObj>* result) override; StatusWith<UserHandle> acquireUser(OperationContext* opCtx, const UserName& userName) override; - StatusWith<UserHandle> acquireUserForSessionRefresh(OperationContext* opCtx, - const UserName& userName, - const User::UserId& uid) override; + StatusWith<UserHandle> reacquireUser(OperationContext* opCtx, const UserHandle& user) override; /** * Invalidate a user, and repin it if necessary. diff --git a/src/mongo/db/auth/authorization_session_impl.cpp b/src/mongo/db/auth/authorization_session_impl.cpp index 0686c974bcb..be0346d38cb 100644 --- a/src/mongo/db/auth/authorization_session_impl.cpp +++ b/src/mongo/db/auth/authorization_session_impl.cpp @@ -502,59 +502,33 @@ bool AuthorizationSessionImpl::isAuthenticated() { void AuthorizationSessionImpl::_refreshUserInfoAsNeeded(OperationContext* opCtx) { AuthorizationManager& authMan = getAuthorizationManager(); UserSet::iterator it = _authenticatedUsers.begin(); + auto removeUser = [&](const auto& it) { + // Take out a lock on the client here to ensure that no one reads while + // _authenticatedUsers is being modified. + stdx::lock_guard<Client> lk(*opCtx->getClient()); - while (it != _authenticatedUsers.end()) { - auto& user = *it; - if (!user.isValid()) { - // Make a good faith effort to acquire an up-to-date user object, since the one - // we've cached is marked "out-of-date." - UserName name = user->getName(); - UserHandle updatedUser; - - auto swUser = authMan.acquireUserForSessionRefresh(opCtx, name, user->getID()); - auto& status = swUser.getStatus(); - - // Take out a lock on the client here to ensure that no one reads while - // _authenticatedUsers is being modified. - stdx::lock_guard<Client> lk(*opCtx->getClient()); + // The user is invalid, so make sure that we erase it from _authenticateUsers. + _authenticatedUsers.removeAt(it); + }; - // The user is invalid, so make sure that we erase it from _authenticateUsers at the - // end of this block. - auto removeGuard = makeGuard([&] { _authenticatedUsers.removeAt(it++); }); + auto replaceUser = [&](const auto& it, UserHandle updatedUser) { + // Take out a lock on the client here to ensure that no one reads while + // _authenticatedUsers is being modified. + stdx::lock_guard<Client> lk(*opCtx->getClient()); + _authenticatedUsers.replaceAt(it, std::move(updatedUser)); + }; + while (it != _authenticatedUsers.end()) { + // Anchor the UserHandle on the stack so we can refer to it throughout this iteration. + const auto currentUser = *it; + const auto& name = currentUser->getName(); + auto swUser = authMan.reacquireUser(opCtx, currentUser); + if (!swUser.isOK()) { + auto& status = swUser.getStatus(); switch (status.code()) { - case ErrorCodes::OK: { - updatedUser = std::move(swUser.getValue()); - try { - auto restrictionStatus = updatedUser->validateRestrictions(opCtx); - if (!restrictionStatus.isOK()) { - LOGV2(20242, - "Removed user with unmet authentication restrictions from " - "session cache of user information. Restriction failed", - "user"_attr = name, - "reason"_attr = restrictionStatus.reason()); - // If we remove from the UserSet, we cannot increment the iterator. - continue; - } - } catch (...) { - LOGV2(20243, - "Evaluating authentication restrictions for user resulted in an " - "unknown exception. Removing user from the session cache", - "user"_attr = name); - continue; - } - - // Success! Replace the old User object with the updated one. - removeGuard.dismiss(); - _authenticatedUsers.replaceAt(it, std::move(updatedUser)); - LOGV2_DEBUG(20244, - 1, - "Updated session cache of user information for user", - "user"_attr = name); - break; - } case ErrorCodes::UserNotFound: { // User does not exist anymore; remove it from _authenticatedUsers. + removeUser(it++); LOGV2(20245, "Removed deleted user from session cache of user information", "user"_attr = name); @@ -562,6 +536,7 @@ void AuthorizationSessionImpl::_refreshUserInfoAsNeeded(OperationContext* opCtx) } case ErrorCodes::UnsupportedFormat: { // An auth subsystem has explicitly indicated a failure. + removeUser(it++); LOGV2(20246, "Removed user from session cache of user information because of " "refresh failure", @@ -579,10 +554,38 @@ void AuthorizationSessionImpl::_refreshUserInfoAsNeeded(OperationContext* opCtx) "to use old information", "user"_attr = name, "error"_attr = redact(status)); - removeGuard.dismiss(); break; } + } else if (!currentUser.isValid()) { + // Our user handle has changed, update the our list of users. + auto updatedUser = std::move(swUser.getValue()); + try { + uassertStatusOK(updatedUser->validateRestrictions(opCtx)); + } catch (const DBException& ex) { + removeUser(it++); + + LOGV2(20242, + "Removed user with unmet authentication restrictions from " + "session cache of user information. Restriction failed", + "user"_attr = name, + "reason"_attr = ex.reason()); + continue; // No need to advance "it" in this case. + } catch (...) { + removeUser(it++); + + LOGV2(20243, + "Evaluating authentication restrictions for user resulted in an " + "unknown exception. Removing user from the session cache", + "user"_attr = name); + continue; // No need to advance "it" in this case. + } + + replaceUser(it, std::move(updatedUser)); + + LOGV2_DEBUG( + 20244, 1, "Updated session cache of user information for user", "user"_attr = name); } + ++it; } _buildAuthenticatedRolesVector(); diff --git a/src/mongo/db/auth/authz_manager_external_state_local.cpp b/src/mongo/db/auth/authz_manager_external_state_local.cpp index 953e3ea1ec4..f5067d6aa33 100644 --- a/src/mongo/db/auth/authz_manager_external_state_local.cpp +++ b/src/mongo/db/auth/authz_manager_external_state_local.cpp @@ -318,6 +318,12 @@ StatusWith<User> AuthzManagerExternalStateLocal::getUserObject(OperationContext* user.addPrivileges(data.privileges.get()); user.setIndirectRestrictions(data.restrictions.get()); + LOGV2_DEBUG(5517200, + 3, + "Acquired new user object", + "userName"_attr = userName, + "directRoles"_attr = directRoles); + return std::move(user); } catch (const AssertionException& ex) { return ex.toStatus(); |