SERVER-16657 Auth check for ops on cursors owned by global cursor mgr

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.