SERVER-19940 extended auth checking for find and getMore commands to allow use of term only from internal clients.

The term field was added to the find/getMore commands to support replication protocol version 1 and is meant to be used by replica set nodes to communicate metadata to their sync sources.

9 files changed, 81 insertions, 29 deletions

diff --git a/jstests/auth/lib/commands_lib.js b/jstests/auth/lib/commands_lib.js
index fd56559300e..2ce024ade88 100644
--- a/jstests/auth/lib/commands_lib.js
+++ b/jstests/auth/lib/commands_lib.js
@@ -1140,6 +1140,21 @@ var authCommandsLib = {
             ]
         },
         {
+            testname: "findWithTerm",
+            command: {find: "foo", limit: -1, term: NumberLong(1)},
+            testcases: [
+                {
+                    runOnDb: firstDbName,
+                    roles: {__system: 1},
+                    privileges: [
+                        { resource: {db: firstDbName, collection: "foo"}, actions: ["find"] },
+                        { resource: {cluster: true}, actions: ["internal"] }
+                    ],
+                    expectFail: true // because of invalid limit
+                },
+            ]
+        },
+        {
             testname: "findAndModify",
             command: {findAndModify: "x", query: {_id: "abc"}, update: {$inc: {n: 1}}},
             setup: function (db) {
@@ -1331,6 +1346,21 @@ var authCommandsLib = {
             ]
         },
         {
+            testname: "getMoreWithTerm",
+            command: {getMore: NumberLong("1"), collection: "foo", term: NumberLong(1)},
+            testcases: [
+                {
+                    runOnDb: firstDbName,
+                    roles: {__system: 1},
+                    privileges: [
+                        { resource: {db: firstDbName, collection: "foo"}, actions: ["find"] },
+                        { resource: {cluster: true}, actions: ["internal"] }
+                    ],
+                    expectFail: true
+                }
+            ]
+        },
+        {
             testname: "getnonce",
             command: {getnonce: 1},
             testcases: [
diff --git a/src/mongo/db/auth/authorization_session.cpp b/src/mongo/db/auth/authorization_session.cpp
index a193655b242..b36297ec34c 100644
--- a/src/mongo/db/auth/authorization_session.cpp
+++ b/src/mongo/db/auth/authorization_session.cpp
@@ -177,7 +177,7 @@ PrivilegeVector AuthorizationSession::getDefaultPrivileges() {
     return defaultPrivileges;
 }
 
-Status AuthorizationSession::checkAuthForQuery(const NamespaceString& ns, const BSONObj& query) {
+Status AuthorizationSession::checkAuthForFind(const NamespaceString& ns, bool hasTerm) {
     if (MONGO_unlikely(ns.isCommand())) {
         return Status(ErrorCodes::InternalError,
                       str::stream() << "Checking query auth on command namespace " << ns.ns());
@@ -186,10 +186,23 @@ Status AuthorizationSession::checkAuthForQuery(const NamespaceString& ns, const
         return Status(ErrorCodes::Unauthorized,
                       str::stream() << "not authorized for query on " << ns.ns());
     }
+
+    // Only internal clients (such as other nodes in a replica set) are allowed to use
+    // the 'term' field in a find operation. Use of this field could trigger changes
+    // in the receiving server's replication state and should be protected.
+    if (hasTerm &&
+        !isAuthorizedForActionsOnResource(ResourcePattern::forClusterResource(),
+                                          ActionType::internal)) {
+        return Status(ErrorCodes::Unauthorized,
+                      str::stream() << "not authorized for query with term on " << ns.ns());
+    }
+
     return Status::OK();
 }
 
-Status AuthorizationSession::checkAuthForGetMore(const NamespaceString& ns, long long cursorID) {
+Status AuthorizationSession::checkAuthForGetMore(const NamespaceString& ns,
+                                                 long long cursorID,
+                                                 bool hasTerm) {
     // "ns" can be in one of three formats: "listCollections" format, "listIndexes" format, and
     // normal format.
     if (ns.isListCollectionsCursorNS()) {
@@ -216,6 +229,17 @@ Status AuthorizationSession::checkAuthForGetMore(const NamespaceString& ns, long
                           str::stream() << "not authorized for getMore on " << ns.ns());
         }
     }
+
+    // Only internal clients (such as other nodes in a replica set) are allowed to use
+    // the 'term' field in a getMore operation. Use of this field could trigger changes
+    // in the receiving server's replication state and should be protected.
+    if (hasTerm &&
+        !isAuthorizedForActionsOnResource(ResourcePattern::forClusterResource(),
+                                          ActionType::internal)) {
+        return Status(ErrorCodes::Unauthorized,
+                      str::stream() << "not authorized for getMore with term 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 16c10334011..e46e46cef3a 100644
--- a/src/mongo/db/auth/authorization_session.h
+++ b/src/mongo/db/auth/authorization_session.h
@@ -139,14 +139,14 @@ public:
     // into a state where the first user can be created.
     PrivilegeVector getDefaultPrivileges();
 
-    // Checks if this connection has the privileges necessary to perform the given query on the
-    // given namespace.
-    Status checkAuthForQuery(const NamespaceString& ns, const BSONObj& query);
+    // Checks if this connection has the privileges necessary to perform a find operation
+    // on the supplied namespace identifier.
+    Status checkAuthForFind(const NamespaceString& ns, bool hasTerm);
 
     // 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);
+    Status checkAuthForGetMore(const NamespaceString& ns, long long cursorID, bool hasTerm);
 
     // Checks if this connection has the privileges necessary to perform the given update on the
     // given namespace.
diff --git a/src/mongo/db/commands/find_cmd.cpp b/src/mongo/db/commands/find_cmd.cpp
index 9f35015462e..4505e28fde7 100644
--- a/src/mongo/db/commands/find_cmd.cpp
+++ b/src/mongo/db/commands/find_cmd.cpp
@@ -58,6 +58,12 @@
 
 namespace mongo {
 
+namespace {
+
+const char kTermField[] = "term";
+
+}  // namespace
+
 /**
  * A command for running .find() queries.
  */
@@ -106,14 +112,9 @@ public:
     Status checkAuthForCommand(ClientBasic* client,
                                const std::string& dbname,
                                const BSONObj& cmdObj) override {
-        AuthorizationSession* authzSession = AuthorizationSession::get(client);
-        ResourcePattern pattern = parseResourcePattern(dbname, cmdObj);
-
-        if (authzSession->isAuthorizedForActionsOnResource(pattern, ActionType::find)) {
-            return Status::OK();
-        }
-
-        return Status(ErrorCodes::Unauthorized, "unauthorized");
+        NamespaceString nss(parseNs(dbname, cmdObj));
+        auto hasTerm = cmdObj.hasField(kTermField);
+        return AuthorizationSession::get(client)->checkAuthForFind(nss, hasTerm);
     }
 
     Status explain(OperationContext* txn,
diff --git a/src/mongo/db/commands/getmore_cmd.cpp b/src/mongo/db/commands/getmore_cmd.cpp
index 07f8bfd8813..25889d5c2e2 100644
--- a/src/mongo/db/commands/getmore_cmd.cpp
+++ b/src/mongo/db/commands/getmore_cmd.cpp
@@ -128,7 +128,7 @@ public:
         const GetMoreRequest& request = parseStatus.getValue();
 
         return AuthorizationSession::get(client)
-            ->checkAuthForGetMore(request.nss, request.cursorid);
+            ->checkAuthForGetMore(request.nss, request.cursorid, request.term.is_initialized());
     }
 
     bool run(OperationContext* txn,
diff --git a/src/mongo/db/instance.cpp b/src/mongo/db/instance.cpp
index 14ea21e4dcb..304a7b2876f 100644
--- a/src/mongo/db/instance.cpp
+++ b/src/mongo/db/instance.cpp
@@ -370,7 +370,7 @@ static void receivedQuery(OperationContext* txn,
 
     try {
         Client* client = txn->getClient();
-        Status status = AuthorizationSession::get(client)->checkAuthForQuery(nss, q.query);
+        Status status = AuthorizationSession::get(client)->checkAuthForFind(nss, false);
         audit::logQueryAuthzCheck(client, nss, q.query, status.code());
         uassertStatusOK(status);
 
@@ -863,8 +863,8 @@ bool receivedGetMore(OperationContext* txn, DbResponse& dbresponse, Message& m,
         const NamespaceString nsString(ns);
         uassert(16258, str::stream() << "Invalid ns [" << ns << "]", nsString.isValid());
 
-        Status status =
-            AuthorizationSession::get(txn->getClient())->checkAuthForGetMore(nsString, cursorid);
+        Status status = AuthorizationSession::get(txn->getClient())
+                            ->checkAuthForGetMore(nsString, cursorid, false);
         audit::logGetMoreAuthzCheck(txn->getClient(), nsString, cursorid, status.code());
         uassertStatusOK(status);
 
diff --git a/src/mongo/s/commands/cluster_find_cmd.cpp b/src/mongo/s/commands/cluster_find_cmd.cpp
index 937515d17c7..0b0e60a8a13 100644
--- a/src/mongo/s/commands/cluster_find_cmd.cpp
+++ b/src/mongo/s/commands/cluster_find_cmd.cpp
@@ -44,6 +44,8 @@ using std::unique_ptr;
 using std::string;
 using std::vector;
 
+const char kTermField[] = "term";
+
 /**
  * Implements the find command on mongos.
  */
@@ -88,14 +90,9 @@ public:
     Status checkAuthForCommand(ClientBasic* client,
                                const std::string& dbname,
                                const BSONObj& cmdObj) final {
-        AuthorizationSession* authzSession = AuthorizationSession::get(client);
-        ResourcePattern pattern = parseResourcePattern(dbname, cmdObj);
-
-        if (authzSession->isAuthorizedForActionsOnResource(pattern, ActionType::find)) {
-            return Status::OK();
-        }
-
-        return Status(ErrorCodes::Unauthorized, "unauthorized");
+        NamespaceString nss(parseNs(dbname, cmdObj));
+        auto hasTerm = cmdObj.hasField(kTermField);
+        return AuthorizationSession::get(client)->checkAuthForFind(nss, hasTerm);
     }
 
     Status explain(OperationContext* txn,
diff --git a/src/mongo/s/commands/cluster_getmore_cmd.cpp b/src/mongo/s/commands/cluster_getmore_cmd.cpp
index 85b36701049..b630d4c81d4 100644
--- a/src/mongo/s/commands/cluster_getmore_cmd.cpp
+++ b/src/mongo/s/commands/cluster_getmore_cmd.cpp
@@ -91,7 +91,7 @@ public:
         const GetMoreRequest& request = parseStatus.getValue();
 
         return AuthorizationSession::get(client)
-            ->checkAuthForGetMore(request.nss, request.cursorid);
+            ->checkAuthForGetMore(request.nss, request.cursorid, request.term.is_initialized());
     }
 
     bool run(OperationContext* txn,
diff --git a/src/mongo/s/strategy.cpp b/src/mongo/s/strategy.cpp
index 357cd8843c0..95d9504b95f 100644
--- a/src/mongo/s/strategy.cpp
+++ b/src/mongo/s/strategy.cpp
@@ -158,7 +158,7 @@ void Strategy::queryOp(OperationContext* txn, Request& request) {
     NamespaceString ns(q.ns);
     ClientBasic* client = txn->getClient();
     AuthorizationSession* authSession = AuthorizationSession::get(client);
-    Status status = authSession->checkAuthForQuery(ns, q.query);
+    Status status = authSession->checkAuthForFind(ns, false);
     audit::logQueryAuthzCheck(client, ns, q.query, status.code());
     uassertStatusOK(status);
 
@@ -625,7 +625,7 @@ void Strategy::getMore(OperationContext* txn, Request& request) {
     ClientBasic* client = ClientBasic::getCurrent();
     NamespaceString nsString(ns);
     AuthorizationSession* authSession = AuthorizationSession::get(client);
-    Status status = authSession->checkAuthForGetMore(nsString, id);
+    Status status = authSession->checkAuthForGetMore(nsString, id, false);
     audit::logGetMoreAuthzCheck(client, nsString, id, status.code());
     uassertStatusOK(status);