diff options
Diffstat (limited to 'src/mongo/db')
-rw-r--r-- | src/mongo/db/auth/SConscript | 1 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_session.cpp | 76 | ||||
-rw-r--r-- | src/mongo/db/auth/authorization_session.h | 10 | ||||
-rw-r--r-- | src/mongo/db/catalog/cursor_manager.cpp | 60 | ||||
-rw-r--r-- | src/mongo/db/commands/list_collections.cpp | 2 | ||||
-rw-r--r-- | src/mongo/db/commands/list_indexes.cpp | 6 | ||||
-rw-r--r-- | src/mongo/db/instance.cpp | 10 | ||||
-rw-r--r-- | src/mongo/db/namespace_string.cpp | 18 | ||||
-rw-r--r-- | src/mongo/db/namespace_string.h | 12 | ||||
-rw-r--r-- | src/mongo/db/namespace_string_test.cpp | 35 | ||||
-rw-r--r-- | src/mongo/db/query/find.cpp | 10 |
11 files changed, 184 insertions, 56 deletions
diff --git a/src/mongo/db/auth/SConscript b/src/mongo/db/auth/SConscript index 1cda5da1ee5..2f15a110423 100644 --- a/src/mongo/db/auth/SConscript +++ b/src/mongo/db/auth/SConscript @@ -34,6 +34,7 @@ env.Library('authcore', ['action_set.cpp', '$BUILD_DIR/mongo/db/common', '$BUILD_DIR/mongo/db/ops/update_driver', '$BUILD_DIR/mongo/md5', + '$BUILD_DIR/mongo/namespace_string', '$BUILD_DIR/mongo/signal_handlers_synchronous', '$BUILD_DIR/mongo/stringutils']) diff --git a/src/mongo/db/auth/authorization_session.cpp b/src/mongo/db/auth/authorization_session.cpp index 7173ca17a75..5b7a7c5e459 100644 --- a/src/mongo/db/auth/authorization_session.cpp +++ b/src/mongo/db/auth/authorization_session.cpp @@ -176,21 +176,46 @@ namespace { Status AuthorizationSession::checkAuthForQuery(const NamespaceString& ns, const BSONObj& query) { if (MONGO_unlikely(ns.isCommand())) { - return Status(ErrorCodes::InternalError, mongoutils::str::stream() << + return Status(ErrorCodes::InternalError, str::stream() << "Checking query auth on command namespace " << ns.ns()); } if (!isAuthorizedForActionsOnNamespace(ns, ActionType::find)) { return Status(ErrorCodes::Unauthorized, - mongoutils::str::stream() << "not authorized for query on " << ns.ns()); + str::stream() << "not authorized for query on " << ns.ns()); } return Status::OK(); } Status AuthorizationSession::checkAuthForGetMore(const NamespaceString& ns, long long cursorID) { - if (!isAuthorizedForActionsOnNamespace(ns, ActionType::find)) { - return Status(ErrorCodes::Unauthorized, - mongoutils::str::stream() << "not authorized for getmore on " << ns.ns()); + // "ns" can be in one of three formats: "listCollections" format, "listIndexes" format, and + // normal format. + if (ns.isListCollectionsGetMore()) { + // "ns" is of the form "<db>.$cmd.listCollections". Check if we can perform the + // listCollections action on the database resource for "<db>". + if (!isAuthorizedForActionsOnResource(ResourcePattern::forDatabaseName(ns.db()), + ActionType::listCollections)) { + return Status(ErrorCodes::Unauthorized, + str::stream() << "not authorized for listCollections getMore on " + << ns.ns()); + } + } + else if (ns.isListIndexesGetMore()) { + // "ns" is of the form "<db>.$cmd.listIndexes.<coll>". Check if we can perform the + // listIndexes action on the "<db>.<coll>" namespace. + NamespaceString targetNS = ns.getTargetNSForListIndexesGetMore(); + if (!isAuthorizedForActionsOnNamespace(targetNS, ActionType::listIndexes)) { + return Status(ErrorCodes::Unauthorized, + str::stream() << "not authorized for listIndexes getMore on " + << ns.ns()); + } + } + else { + // "ns" is a regular namespace string. Check if we can perform the find action on it. + if (!isAuthorizedForActionsOnNamespace(ns, ActionType::find)) { + return Status(ErrorCodes::Unauthorized, + str::stream() << "not authorized for getMore on " << ns.ns()); + } } return Status::OK(); } @@ -206,14 +231,13 @@ namespace { NamespaceString indexNS(nsElement.str()); if (!isAuthorizedForActionsOnNamespace(indexNS, ActionType::createIndex)) { return Status(ErrorCodes::Unauthorized, - mongoutils::str::stream() << "not authorized to create index on " << + str::stream() << "not authorized to create index on " << indexNS.ns()); } } else { if (!isAuthorizedForActionsOnNamespace(ns, ActionType::insert)) { return Status(ErrorCodes::Unauthorized, - mongoutils::str::stream() << "not authorized for insert on " << - ns.ns()); + str::stream() << "not authorized for insert on " << ns.ns()); } } @@ -227,8 +251,7 @@ namespace { if (!upsert) { if (!isAuthorizedForActionsOnNamespace(ns, ActionType::update)) { return Status(ErrorCodes::Unauthorized, - mongoutils::str::stream() << "not authorized for update on " << - ns.ns()); + str::stream() << "not authorized for update on " << ns.ns()); } } else { @@ -237,8 +260,7 @@ namespace { required.addAction(ActionType::insert); if (!isAuthorizedForActionsOnNamespace(ns, required)) { return Status(ErrorCodes::Unauthorized, - mongoutils::str::stream() << "not authorized for upsert on " << - ns.ns()); + str::stream() << "not authorized for upsert on " << ns.ns()); } } return Status::OK(); @@ -248,7 +270,35 @@ namespace { const BSONObj& query) { if (!isAuthorizedForActionsOnNamespace(ns, ActionType::remove)) { return Status(ErrorCodes::Unauthorized, - mongoutils::str::stream() << "not authorized to remove from " << ns.ns()); + str::stream() << "not authorized to remove from " << ns.ns()); + } + return Status::OK(); + } + + Status AuthorizationSession::checkAuthForKillCursors(const NamespaceString& ns, + long long cursorID) { + // See implementation comments in checkAuthForGetMore(). This method looks very similar. + if (ns.isListCollectionsGetMore()) { + if (!isAuthorizedForActionsOnResource(ResourcePattern::forDatabaseName(ns.db()), + ActionType::killCursors)) { + return Status(ErrorCodes::Unauthorized, + str::stream() << "not authorized to kill listCollections cursor on " + << ns.ns()); + } + } + else if (ns.isListIndexesGetMore()) { + NamespaceString targetNS = ns.getTargetNSForListIndexesGetMore(); + if (!isAuthorizedForActionsOnNamespace(targetNS, ActionType::killCursors)) { + return Status(ErrorCodes::Unauthorized, + str::stream() << "not authorized to kill listIndexes cursor on " + << ns.ns()); + } + } + else { + if (!isAuthorizedForActionsOnNamespace(ns, ActionType::killCursors)) { + return Status(ErrorCodes::Unauthorized, + str::stream() << "not authorized to kill cursor on " << ns.ns()); + } } return Status::OK(); } diff --git a/src/mongo/db/auth/authorization_session.h b/src/mongo/db/auth/authorization_session.h index 3a184e4be7d..190b854312d 100644 --- a/src/mongo/db/auth/authorization_session.h +++ b/src/mongo/db/auth/authorization_session.h @@ -115,8 +115,9 @@ namespace mongo { // given namespace. Status checkAuthForQuery(const NamespaceString& ns, const BSONObj& query); - // Checks if this connection has the privileges necessary to perform a getMore on the given - // cursor in the given namespace. + // Checks if this connection has the privileges necessary to perform a getMore operation on + // the identified cursor, supposing that cursor is associated with the supplied namespace + // identifier. Status checkAuthForGetMore(const NamespaceString& ns, long long cursorID); // Checks if this connection has the privileges necessary to perform the given update on the @@ -135,6 +136,11 @@ namespace mongo { // namespace. Status checkAuthForDelete(const NamespaceString& ns, const BSONObj& query); + // Checks if this connection has the privileges necessary to perform a killCursor on + // the identified cursor, supposing that cursor is associated with the supplied namespace + // identifier. + Status checkAuthForKillCursors(const NamespaceString& ns, long long cursorID); + // Checks if this connection has the privileges necessary to grant the given privilege // to a role. Status checkAuthorizedToGrantPrivilege(const Privilege& privilege); diff --git a/src/mongo/db/catalog/cursor_manager.cpp b/src/mongo/db/catalog/cursor_manager.cpp index aea0310fe90..1e8d2480eee 100644 --- a/src/mongo/db/catalog/cursor_manager.cpp +++ b/src/mongo/db/catalog/cursor_manager.cpp @@ -179,28 +179,40 @@ namespace mongo { } bool GlobalCursorIdCache::eraseCursor(OperationContext* txn, CursorId id, bool checkAuth) { - string ns; - { - SimpleMutex::scoped_lock lk( _mutex ); - unsigned nsid = idFromCursorId( id ); - Map::const_iterator it = _idToNS.find( nsid ); - if ( it == _idToNS.end() ) { + // Figure out what the namespace of this cursor is. + std::string ns; + if (globalCursorManager->ownsCursorId(id)) { + ClientCursorPin pin(globalCursorManager.get(), id); + if (!pin.c()) { + // No such cursor. TODO: Consider writing to audit log here (even though we don't + // have a namespace). + return false; + } + ns = pin.c()->ns(); + } + else { + SimpleMutex::scoped_lock lk(_mutex); + unsigned nsid = idFromCursorId(id); + Map::const_iterator it = _idToNS.find(nsid); + if (it == _idToNS.end()) { + // No namespace corresponding to this cursor id prefix. TODO: Consider writing to + // audit log here (even though we don't have a namespace). return false; } ns = it->second; } + const NamespaceString nss(ns); + invariant(nss.isValid()); - const NamespaceString nss( ns ); - - if ( checkAuth ) { + // Check if we are authorized to erase this cursor. + if (checkAuth) { AuthorizationSession* as = txn->getClient()->getAuthorizationSession(); - bool isAuthorized = as->isAuthorizedForActionsOnNamespace( - nss, ActionType::killCursors); - if ( !isAuthorized ) { - audit::logKillCursorsAuthzCheck( txn->getClient(), - nss, - id, - ErrorCodes::Unauthorized ); + Status authorizationStatus = as->checkAuthForKillCursors(nss, id); + if (!authorizationStatus.isOK()) { + audit::logKillCursorsAuthzCheck(txn->getClient(), + nss, + id, + ErrorCodes::Unauthorized); return false; } } @@ -213,17 +225,13 @@ namespace mongo { // If not, then the cursor must be owned by a collection. Erase the cursor under the // collection lock (to prevent the collection from going away during the erase). AutoGetCollectionForRead ctx(txn, nss); - if (!ctx.getDb()) { - return false; - } - Collection* collection = ctx.getCollection(); - if ( !collection ) { - if ( checkAuth ) - audit::logKillCursorsAuthzCheck( txn->getClient(), - nss, - id, - ErrorCodes::CursorNotFound ); + if (!collection) { + if (checkAuth) + audit::logKillCursorsAuthzCheck(txn->getClient(), + nss, + id, + ErrorCodes::CursorNotFound); return false; } return collection->cursorManager()->eraseCursor(txn, id, checkAuth); diff --git a/src/mongo/db/commands/list_collections.cpp b/src/mongo/db/commands/list_collections.cpp index c9938ae7c9e..73202ada937 100644 --- a/src/mongo/db/commands/list_collections.cpp +++ b/src/mongo/db/commands/list_collections.cpp @@ -133,6 +133,8 @@ namespace mongo { } std::string cursorNamespace = str::stream() << dbname << ".$cmd." << name; + dassert(NamespaceString(cursorNamespace).isValid()); + dassert(NamespaceString(cursorNamespace).isListCollectionsGetMore()); PlanExecutor* rawExec; Status makeStatus = PlanExecutor::make(txn, diff --git a/src/mongo/db/commands/list_indexes.cpp b/src/mongo/db/commands/list_indexes.cpp index 8cbc97da8f2..859d37116fc 100644 --- a/src/mongo/db/commands/list_indexes.cpp +++ b/src/mongo/db/commands/list_indexes.cpp @@ -133,7 +133,11 @@ namespace mongo { root->pushBack(*member); } - std::string cursorNamespace = str::stream() << dbname << ".$cmd." << name; + std::string cursorNamespace = str::stream() << dbname << ".$cmd." << name << "." + << ns.coll(); + dassert(NamespaceString(cursorNamespace).isValid()); + dassert(NamespaceString(cursorNamespace).isListIndexesGetMore()); + dassert(ns == NamespaceString(cursorNamespace).getTargetNSForListIndexesGetMore()); PlanExecutor* rawExec; Status makeStatus = PlanExecutor::make(txn, diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp index c23c20c2eb3..6b5df1654f2 100644 --- a/src/mongo/db/instance.cpp +++ b/src/mongo/db/instance.cpp @@ -749,14 +749,8 @@ namespace mongo { const NamespaceString nsString( ns ); uassert( 16258, str::stream() << "Invalid ns [" << ns << "]", nsString.isValid() ); - Status status = Status::OK(); - if (CursorManager::getGlobalCursorManager()->ownsCursorId(cursorid)) { - // TODO Implement auth check for global cursors. SERVER-16657. - } - else { - status = txn->getClient()->getAuthorizationSession()->checkAuthForGetMore( - nsString, cursorid); - } + Status status = txn->getClient()->getAuthorizationSession()->checkAuthForGetMore( + nsString, cursorid); audit::logGetMoreAuthzCheck(txn->getClient(), nsString, cursorid, status.code()); uassertStatusOK(status); diff --git a/src/mongo/db/namespace_string.cpp b/src/mongo/db/namespace_string.cpp index e875920c905..afdb6f70c62 100644 --- a/src/mongo/db/namespace_string.cpp +++ b/src/mongo/db/namespace_string.cpp @@ -48,4 +48,22 @@ namespace mongo { return false; } + bool NamespaceString::isListCollectionsGetMore() const { + return coll() == StringData("$cmd.listCollections", StringData::LiteralTag()); + } + + namespace { + const StringData listIndexesGetMoreNSPrefix("$cmd.listIndexes.", StringData::LiteralTag()); + } // namespace + + bool NamespaceString::isListIndexesGetMore() const { + return coll().size() > listIndexesGetMoreNSPrefix.size() && + coll().startsWith(listIndexesGetMoreNSPrefix); + } + + NamespaceString NamespaceString::getTargetNSForListIndexesGetMore() const { + dassert(isListIndexesGetMore()); + return NamespaceString(db(), coll().substr(listIndexesGetMoreNSPrefix.size())); + } + } diff --git a/src/mongo/db/namespace_string.h b/src/mongo/db/namespace_string.h index 2d4ef9f32e7..5fba9dc530b 100644 --- a/src/mongo/db/namespace_string.h +++ b/src/mongo/db/namespace_string.h @@ -97,6 +97,10 @@ namespace mongo { size_t size() const { return _ns.size(); } + // + // The following methods assume isValid() is true for this NamespaceString. + // + bool isSystem() const { return coll().startsWith( "system." ); } bool isSystemDotIndexes() const { return coll() == "system.indexes"; } bool isConfigDB() const { return db() == "config"; } @@ -105,6 +109,14 @@ namespace mongo { bool isSpecialCommand() const { return coll().startsWith("$cmd.sys"); } bool isSpecial() const { return special( _ns ); } bool isNormal() const { return normal( _ns ); } + bool isListCollectionsGetMore() const; + bool isListIndexesGetMore() const; + + /** + * Given a NamespaceString for which isListIndexesGetMore() returns true, returns the + * NamespaceString for the collection that the "listIndexesGetMore" targets. + */ + NamespaceString getTargetNSForListIndexesGetMore() const; /** * @return true if the namespace is valid. Special namespaces for internal use are considered as valid. diff --git a/src/mongo/db/namespace_string_test.cpp b/src/mongo/db/namespace_string_test.cpp index 45e4ae00177..0a062e28922 100644 --- a/src/mongo/db/namespace_string_test.cpp +++ b/src/mongo/db/namespace_string_test.cpp @@ -86,6 +86,41 @@ namespace mongo { ASSERT( NamespaceString::normal( "local.oplog.$main" ) ); } + TEST(NamespaceStringTest, ListCollectionsGetMore) { + ASSERT(NamespaceString("test.$cmd.listCollections").isListCollectionsGetMore()); + + ASSERT(!NamespaceString("test.foo").isListCollectionsGetMore()); + ASSERT(!NamespaceString("test.foo.$cmd.listCollections").isListCollectionsGetMore()); + ASSERT(!NamespaceString("test.$cmd.").isListCollectionsGetMore()); + ASSERT(!NamespaceString("test.$cmd.foo.").isListCollectionsGetMore()); + ASSERT(!NamespaceString("test.$cmd.listCollections.").isListCollectionsGetMore()); + ASSERT(!NamespaceString("test.$cmd.listIndexes").isListCollectionsGetMore()); + ASSERT(!NamespaceString("test.$cmd.listIndexes.foo").isListCollectionsGetMore()); + } + + TEST(NamespaceStringTest, ListIndexesGetMore) { + NamespaceString ns1("test.$cmd.listIndexes.f"); + ASSERT(ns1.isListIndexesGetMore()); + ASSERT("test.f" == ns1.getTargetNSForListIndexesGetMore().ns()); + + NamespaceString ns2("test.$cmd.listIndexes.foo"); + ASSERT(ns2.isListIndexesGetMore()); + ASSERT("test.foo" == ns2.getTargetNSForListIndexesGetMore().ns()); + + NamespaceString ns3("test.$cmd.listIndexes.foo.bar"); + ASSERT(ns3.isListIndexesGetMore()); + ASSERT("test.foo.bar" == ns3.getTargetNSForListIndexesGetMore().ns()); + + ASSERT(!NamespaceString("test.foo").isListIndexesGetMore()); + ASSERT(!NamespaceString("test.foo.$cmd.listIndexes").isListIndexesGetMore()); + ASSERT(!NamespaceString("test.$cmd.").isListIndexesGetMore()); + ASSERT(!NamespaceString("test.$cmd.foo.").isListIndexesGetMore()); + ASSERT(!NamespaceString("test.$cmd.listIndexes").isListIndexesGetMore()); + ASSERT(!NamespaceString("test.$cmd.listIndexes.").isListIndexesGetMore()); + ASSERT(!NamespaceString("test.$cmd.listCollections").isListIndexesGetMore()); + ASSERT(!NamespaceString("test.$cmd.listCollections.foo").isListIndexesGetMore()); + } + TEST( NamespaceStringTest, CollectionComponentValidNames ) { ASSERT( NamespaceString::validCollectionComponent( "a.b" ) ); ASSERT( NamespaceString::validCollectionComponent( "a.b" ) ); diff --git a/src/mongo/db/query/find.cpp b/src/mongo/db/query/find.cpp index c3d6575ae2d..4aeb13d1b16 100644 --- a/src/mongo/db/query/find.cpp +++ b/src/mongo/db/query/find.cpp @@ -256,12 +256,10 @@ namespace mongo { else { // Check for spoofing of the ns such that it does not match the one originally // there for the cursor. - if (globalCursorManager->ownsCursorId(cursorid)) { - // TODO Implement auth check for global cursors. SERVER-16657. - } - else { - uassert(17011, "auth error", str::equals(ns, cc->ns().c_str())); - } + uassert(ErrorCodes::Unauthorized, + str::stream() << "Requested getMore on namespace " << ns << ", but cursor " + << cursorid << " belongs to namespace " << cc->ns(), + ns == cc->ns()); *isCursorAuthorized = true; // Restore the RecoveryUnit if we need to. |