From 57a9fd688c95ec634900d0470f0e87987c3955d2 Mon Sep 17 00:00:00 2001 From: Spencer Jackson Date: Mon, 26 Sep 2016 12:54:12 -0400 Subject: SERVER-25994: Make applyOps work without universal privileges --- jstests/auth/lib/commands_lib.js | 232 ++++++++++++++++++++++++- jstests/core/apply_ops1.js | 140 ++++++++++++++- src/mongo/db/commands/apply_ops.cpp | 2 + src/mongo/db/commands/apply_ops_cmd_common.cpp | 76 +++++++- src/mongo/db/commands/apply_ops_cmd_common.h | 9 + 5 files changed, 442 insertions(+), 17 deletions(-) diff --git a/jstests/auth/lib/commands_lib.js b/jstests/auth/lib/commands_lib.js index 2cc54f30eff..98a89b9a78a 100644 --- a/jstests/auth/lib/commands_lib.js +++ b/jstests/auth/lib/commands_lib.js @@ -232,24 +232,242 @@ var authCommandsLib = { roles: Object.extend({readWriteAnyDatabase: 1}, roles_clusterManager) }] }, + { - testname: "applyOps", - command: {applyOps: "x"}, + testname: "applyOps_empty", + command: {applyOps: []}, + skipSharded: true, testcases: [ { + roles: {__system: 1}, runOnDb: adminDbName, + }, + { roles: {__system: 1}, - privileges: [{resource: {anyResource: true}, actions: ["anyAction"]}], - expectFail: true + runOnDb: firstDbName, + } + ] + }, + { + testname: "applyOps_precondition", + command: { + applyOps: [{ + "ts": Timestamp(1473353037, 1), + "h": NumberLong(0), + "v": 2, + "op": "n", + "ns": "", + "o": {} + }], + preCondition: [{ns: firstDbName + ".x", q: {x: 5}, res: []}] + }, + skipSharded: true, + setup: function(db) { + db.getSisterDB(firstDbName).x.save({}); + }, + teardown: function(db) { + db.getSisterDB(firstDbName).x.drop(); + }, + testcases: [ + { + runOnDb: adminDbName, + privileges: [ + {resource: {db: firstDbName, collection: "x"}, actions: ["find"]}, + { + resource: {cluster: true}, + actions: ["appendOplogNote"], + removeWhenTestingAuthzFailure: false + }, + ], + }, + ] + }, + { + testname: "applyOps_noop", + command: { + applyOps: [{ + "ts": Timestamp(1473353037, 1), + "h": NumberLong(0), + "v": 2, + "op": "n", + "ns": "", + "o": {} + }] + }, + skipSharded: true, + testcases: [ + { + runOnDb: adminDbName, + privileges: [{resource: {cluster: true}, actions: ["appendOplogNote"]}, ], }, { runOnDb: firstDbName, - roles: {__system: 1}, - privileges: [{resource: {anyResource: true}, actions: ["anyAction"]}], - expectFail: true + privileges: [{resource: {cluster: true}, actions: ["appendOplogNote"]}, ], + expectFailure: true } ] }, + { + testname: "applyOps_c_renameCollection_twoDbs", + command: { + applyOps: [{ + "ts": Timestamp(1474051004, 1), + "h": NumberLong(0), + "v": 2, + "op": "c", + "ns": "test.$cmd", + "o": { + "renameCollection": firstDbName + ".x", + "to": secondDbName + ".y", + "stayTemp": false, + "dropTarget": false + } + }] + }, + skipSharded: true, + setup: function(db) { + db.getSisterDB(firstDbName).x.save({}); + db.getSisterDB(adminDbName).runCommand({movePrimary: firstDbName, to: shard0name}); + db.getSisterDB(adminDbName).runCommand({movePrimary: secondDbName, to: shard0name}); + }, + teardown: function(db) { + db.getSisterDB(firstDbName).x.drop(); + db.getSisterDB(secondDbName).y.drop(); + }, + testcases: [ + { + runOnDb: adminDbName, + roles: {readWriteAnyDatabase: 1, root: 1, __system: 1}, + privileges: [ + { + resource: {db: firstDbName, collection: "x"}, + actions: ["find", "dropCollection"] + }, + { + resource: {db: secondDbName, collection: "y"}, + actions: ["insert", "createIndex"] + } + ] + }, + ] + }, + { + testname: "applyOps_insert", + command: { + applyOps: [{ + "ts": Timestamp(1474051453, 1), + "h": NumberLong(0), + "v": 2, + "op": "i", + "ns": firstDbName + ".x", + "o": {"_id": ObjectId("57dc3d7da4fce4358afa85b8"), "data": 5} + }] + }, + skipSharded: true, + setup: function(db) { + db.getSisterDB(firstDbName).x.save({}); + }, + teardown: function(db) { + db.getSisterDB(firstDbName).x.drop(); + }, + testcases: [ + { + runOnDb: adminDbName, + roles: roles_write, + privileges: + [{resource: {db: firstDbName, collection: "x"}, actions: ["insert"]}, ], + }, + ] + }, + { + testname: "applyOps_upsert", + command: { + applyOps: [{ + "ts": Timestamp(1474053682, 1), + "h": NumberLong(0), + "v": 2, + "op": "u", + "ns": firstDbName + ".x", + "o2": {"_id": 1}, + "o": {"_id": 1, "data": 8} + }] + }, + skipSharded: true, + setup: function(db) { + db.getSisterDB(firstDbName).x.save({_id: 1, data: 1}); + }, + teardown: function(db) { + db.getSisterDB(firstDbName).x.drop(); + }, + testcases: [ + { + runOnDb: adminDbName, + roles: Object.merge(roles_write, {restore: 0}, true), + privileges: [ + {resource: {db: firstDbName, collection: "x"}, actions: ["update", "insert"]}, + ], + }, + ] + }, + { + testname: "applyOps_update", + command: { + applyOps: [{ + "ts": Timestamp(1474053682, 1), + "h": NumberLong(0), + "v": 2, + "op": "u", + "ns": firstDbName + ".x", + "o2": {"_id": 1}, + "o": {"_id": 1, "data": 8} + }], + alwaysUpsert: false + }, + skipSharded: true, + setup: function(db) { + db.getSisterDB(firstDbName).x.save({_id: 1, data: 1}); + }, + teardown: function(db) { + db.getSisterDB(firstDbName).x.drop(); + }, + testcases: [ + { + runOnDb: adminDbName, + roles: Object.merge(roles_write, {restore: 0}, true), + privileges: + [{resource: {db: firstDbName, collection: "x"}, actions: ["update"]}, ], + }, + ] + }, + { + testname: "applyOps_delete", + command: { + applyOps: [{ + "ts": Timestamp(1474056194, 1), + "h": NumberLong(0), + "v": 2, + "op": "d", + "ns": firstDbName + ".x", + "o": {"_id": 1} + }] + }, + skipSharded: true, + setup: function(db) { + db.getSisterDB(firstDbName).x.save({_id: 1, data: 1}); + }, + teardown: function(db) { + db.getSisterDB(firstDbName).x.drop(); + }, + testcases: [ + { + runOnDb: adminDbName, + roles: Object.merge(roles_write, {restore: 0}, true), + privileges: + [{resource: {db: firstDbName, collection: "x"}, actions: ["remove"]}, ], + }, + ] + }, + { testname: "aggregate_readonly", command: {aggregate: "foo", pipeline: []}, diff --git a/jstests/core/apply_ops1.js b/jstests/core/apply_ops1.js index 8a19caa9f23..1770baef860 100644 --- a/jstests/core/apply_ops1.js +++ b/jstests/core/apply_ops1.js @@ -60,6 +60,139 @@ db.adminCommand({applyOps: [{op: 'c', ns: ''}]}), 'applyOps should fail on non-"n" operation type with empty "ns" field value'); + // Excessively nested applyOps commands gracefully fail. + assert.commandFailed(db.adminCommand({ + "applyOps": [{ + "ts": {"$timestamp": {"t": 1, "i": 100}}, + "h": 0, + "v": 2, + "op": "c", + "ns": "test.$cmd", + "o": { + "applyOps": [{ + "ts": {"$timestamp": {"t": 1, "i": 100}}, + "h": 0, + "v": 2, + "op": "c", + "ns": "test.$cmd", + "o": { + "applyOps": [{ + "ts": {"$timestamp": {"t": 1, "i": 100}}, + "h": 0, + "v": 2, + "op": "c", + "ns": "test.$cmd", + "o": { + "applyOps": [{ + "ts": {"$timestamp": {"t": 1, "i": 100}}, + "h": 0, + "v": 2, + "op": "c", + "ns": "test.$cmd", + "o": { + "applyOps": [{ + "ts": {"$timestamp": {"t": 1, "i": 100}}, + "h": 0, + "v": 2, + "op": "c", + "ns": "test.$cmd", + "o": { + "applyOps": [{ + "ts": {"$timestamp": {"t": 1, "i": 100}}, + "h": 0, + "v": 2, + "op": "c", + "ns": "test.$cmd", + "o": { + "applyOps": [{ + "ts": + {"$timestamp": {"t": 1, "i": 100}}, + "h": 0, + "v": 2, + "op": "c", + "ns": "test.$cmd", + "o": { + "applyOps": [{ + "ts": { + "$timestamp": + {"t": 1, "i": 100} + }, + "h": 0, + "v": 2, + "op": "c", + "ns": "test.$cmd", + "o": { + "applyOps": [{ + "ts": { + "$timestamp": { + "t": 1, + "i": 100 + } + }, + "h": 0, + "v": 2, + "op": "c", + "ns": "test.$cmd", + "o": { + "applyOps": [{ + "ts": { + "$timestamp": + { + "t": + 1, + "i": + 100 + } + }, + "h": 0, + "v": 2, + "op": "c", + "ns": + "test.$cmd", + "o": { + "applyOps": [{ + "ts": { + "$timestamp": { + "t": + 1, + "i": + 100 + } + }, + "h": 0, + "v": 2, + "op": + "c", + "ns": + "test.$cmd", + "o": { + "applyOps": + [] + } + }] + } + }] + } + }] + } + }] + } + }] + } + }] + } + }] + } + }] + } + }] + } + }] + } + }] + }), + "Excessively nested applyOps should be rejected"); + // Missing 'o' field value in an operation of type 'i' on 'system.indexes' collection. assert.commandFailedWithCode( db.adminCommand({applyOps: [{op: 'i', ns: db.getName() + '.system.indexes'}]}), @@ -147,7 +280,8 @@ "Applying an insert operation on a non-existent collection should fail"); assert.commandWorked(db.createCollection(t.getName())); - var a = db.adminCommand({applyOps: [{"op": "i", "ns": t.getFullName(), "o": {_id: 5, x: 17}}]}); + var a = assert.commandWorked( + db.adminCommand({applyOps: [{"op": "i", "ns": t.getFullName(), "o": {_id: 5, x: 17}}]})); assert.eq(1, t.find().count(), "Valid insert failed"); assert.eq(true, a.results[0], "Bad result value for valid insert"); @@ -162,12 +296,12 @@ }; assert.eq(o, t.findOne(), "Mismatching document inserted."); - var res = db.runCommand({ + var res = assert.commandWorked(db.runCommand({ applyOps: [ {op: "u", ns: t.getFullName(), o2: {_id: 5}, o: {$inc: {x: 1}}}, {op: "u", ns: t.getFullName(), o2: {_id: 5}, o: {$inc: {x: 1}}} ] - }); + })); o.x++; o.x++; diff --git a/src/mongo/db/commands/apply_ops.cpp b/src/mongo/db/commands/apply_ops.cpp index 04e9117b548..f4f0b930a8c 100644 --- a/src/mongo/db/commands/apply_ops.cpp +++ b/src/mongo/db/commands/apply_ops.cpp @@ -90,6 +90,8 @@ public: int, string& errmsg, BSONObjBuilder& result) { + validateApplyOpsCommand(cmdObj); + boost::optional maybeDisableValidation; if (shouldBypassDocumentValidationForCommand(cmdObj)) maybeDisableValidation.emplace(txn); diff --git a/src/mongo/db/commands/apply_ops_cmd_common.cpp b/src/mongo/db/commands/apply_ops_cmd_common.cpp index 045570abf8a..91809b77d20 100644 --- a/src/mongo/db/commands/apply_ops_cmd_common.cpp +++ b/src/mongo/db/commands/apply_ops_cmd_common.cpp @@ -28,6 +28,8 @@ #include "mongo/db/commands/apply_ops_cmd_common.h" +#include + #include "mongo/base/status.h" #include "mongo/base/string_data.h" #include "mongo/bson/bsonobj.h" @@ -111,22 +113,82 @@ Status checkOperationAuthorization(OperationContext* txn, return Status(ErrorCodes::FailedToParse, "Unrecognized opType"); } - } // namespace +ApplyOpsValidity validateApplyOpsCommand(const BSONObj& cmdObj) { + const size_t maxApplyOpsDepth = 10; + std::stack> toCheck; + + auto operationContainsApplyOps = [](const BSONObj& opObj) { + BSONElement opTypeElem = opObj["op"]; + checkBSONType(BSONType::String, opTypeElem); + const StringData opType = opTypeElem.checkAndGetStringData(); + + if (opType == "c") { + BSONElement oElem = opObj["o"]; + checkBSONType(BSONType::Object, oElem); + BSONObj o = oElem.Obj(); + + if (o.firstElement().fieldNameStringData() == "applyOps") { + return true; + } + } + return false; + }; + + // Insert the top level applyOps command into the stack. + toCheck.emplace(std::make_pair(0, cmdObj)); + + while (!toCheck.empty()) { + std::pair item = toCheck.top(); + toCheck.pop(); + + checkBSONType(BSONType::Array, item.second.firstElement()); + // Check if the applyOps command is empty. This is probably not something that should + // happen, so require a superuser to do this. + if (item.second.firstElement().Array().empty()) { + return ApplyOpsValidity::kNeedsSuperuser; + } + + // For each applyOps command, iterate the ops. + for (BSONElement element : item.second.firstElement().Array()) { + checkBSONType(BSONType::Object, element); + BSONObj elementObj = element.Obj(); + + // If the op itself contains an applyOps... + if (operationContainsApplyOps(elementObj)) { + // And we've recursed too far, then bail out. + uassert(ErrorCodes::FailedToParse, + "Too many nested applyOps", + item.first < maxApplyOpsDepth); + + // Otherwise, if the op contains an applyOps, but we haven't recursed too far: + // extract the applyOps command, and insert it into the stack. + checkBSONType(BSONType::Object, elementObj["o"]); + BSONObj oObj = elementObj["o"].Obj(); + toCheck.emplace(std::make_pair(item.first + 1, std::move(oObj))); + } + } + } + + return ApplyOpsValidity::kOk; +} Status checkAuthForApplyOpsCommand(OperationContext* txn, const std::string& dbname, const BSONObj& cmdObj) { AuthorizationSession* authSession = AuthorizationSession::get(txn->getClient()); - - std::vector universalPrivileges; - RoleGraph::generateUniversalPrivileges(&universalPrivileges); - if (!authSession->isAuthorizedForPrivileges(universalPrivileges)) { - return Status(ErrorCodes::Unauthorized, "Unauthorized"); + ApplyOpsValidity validity = validateApplyOpsCommand(cmdObj); + if (validity == ApplyOpsValidity::kNeedsSuperuser) { + std::vector universalPrivileges; + RoleGraph::generateUniversalPrivileges(&universalPrivileges); + if (!authSession->isAuthorizedForPrivileges(universalPrivileges)) { + return Status(ErrorCodes::Unauthorized, "Unauthorized"); + } + return Status::OK(); } - + fassert(40314, validity == ApplyOpsValidity::kOk); boost::optional maybeDisableValidation; if (shouldBypassDocumentValidationForCommand(cmdObj)) diff --git a/src/mongo/db/commands/apply_ops_cmd_common.h b/src/mongo/db/commands/apply_ops_cmd_common.h index 2b4856f3f74..443e862b798 100644 --- a/src/mongo/db/commands/apply_ops_cmd_common.h +++ b/src/mongo/db/commands/apply_ops_cmd_common.h @@ -43,4 +43,13 @@ Status checkAuthForApplyOpsCommand(OperationContext* txn, const std::string& dbname, const BSONObj& cmdObj); +enum class ApplyOpsValidity { kOk, kNeedsSuperuser }; + +/** + * Returns either kNeedsSuperuser, if the provided applyOps command contains an empty applyOps + * command, or kOk if no other handlable conditions detected. May throw exceptions if the input + * is malformed. + */ +ApplyOpsValidity validateApplyOpsCommand(const BSONObj& cmdObj); + } // namespace mongo -- cgit v1.2.1